External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2025-02-22 11:22:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

qemu: Fix deadlock across fork() in QEMU driver

Browse Source 
The functions in virCommand() after fork() must be careful with regard
to accessing any mutexes that may have been locked by other threads in
the parent process. It is possible that another thread in the parent
process holds the lock for the virQEMUDriver while fork() is called.
This leads to a deadlock in the child process when
'virQEMUDriverGetConfig(driver)' is called and therefore the handshake
never completes between the child and the parent process. Ultimately
the virDomainObjectPtr will never be unlocked.

It gets much worse if the other thread of the parent process, that
holds the lock for the virQEMUDriver, tries to lock the already locked
virDomainObject. This leads to a completely unresponsive libvirtd.

It's possible to reproduce this case with calling 'virsh start XXX'
and 'virsh managedsave XXX' in a tight loop for multiple domains.

This commit fixes the deadlock in the same way as it is described in
commit 61b52d2e3813cc8c9ff3ab67f232bd0c65f7318d.

Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
This commit is contained in:
Marc Hartmayer 2017-02-21 13:11:00 +01:00 committed by Michal Privoznik
parent 6304277534
commit e22de286b1
3 changed files with 37 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

73
src/qemu/qemu_domain.c
View File

@ -7045,13 +7045,12 @@ qemuDomainGetHostdevPath(virDomainDefPtr def,
* Returns 0 on success, -1 otherwise (with error reported) * Returns 0 on success, -1 otherwise (with error reported)
*/ */
static int static int
qemuDomainGetPreservedMounts(virQEMUDriverPtr driver, qemuDomainGetPreservedMounts(virQEMUDriverConfigPtr cfg,
virDomainObjPtr vm, virDomainObjPtr vm,
char ***devPath, char ***devPath,
char ***devSavePath, char ***devSavePath,
size_t *ndevPath) size_t *ndevPath)
{ {
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
char **paths = NULL, **mounts = NULL; char **paths = NULL, **mounts = NULL;
size_t i, nmounts; size_t i, nmounts;
@ -7092,13 +7091,11 @@ qemuDomainGetPreservedMounts(virQEMUDriverPtr driver,
if (ndevPath) if (ndevPath)
*ndevPath = nmounts; *ndevPath = nmounts;
virObjectUnref(cfg);
return 0; return 0;
error: error:
virStringListFreeCount(mounts, nmounts); virStringListFreeCount(mounts, nmounts);
virStringListFreeCount(paths, nmounts); virStringListFreeCount(paths, nmounts);
virObjectUnref(cfg);
return -1; return -1;
} }
@ -7310,11 +7307,10 @@ qemuDomainCreateDevice(const char *device,
static int static int
qemuDomainPopulateDevices(virQEMUDriverPtr driver, qemuDomainPopulateDevices(virQEMUDriverConfigPtr cfg,
virDomainObjPtr vm ATTRIBUTE_UNUSED, virDomainObjPtr vm ATTRIBUTE_UNUSED,
const char *path) const char *path)
{ {
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
const char *const *devices = (const char *const *) cfg->cgroupDeviceACL; const char *const *devices = (const char *const *) cfg->cgroupDeviceACL;
size_t i; size_t i;
int ret = -1; int ret = -1;
@ -7329,13 +7325,13 @@ qemuDomainPopulateDevices(virQEMUDriverPtr driver,
ret = 0; ret = 0;
cleanup: cleanup:
virObjectUnref(cfg);
return ret; return ret;
} }
static int static int
qemuDomainSetupDev(virQEMUDriverPtr driver, qemuDomainSetupDev(virQEMUDriverConfigPtr cfg,
virSecurityManagerPtr mgr,
virDomainObjPtr vm, virDomainObjPtr vm,
const char *path) const char *path)
{ {
@ -7345,7 +7341,7 @@ qemuDomainSetupDev(virQEMUDriverPtr driver,
VIR_DEBUG("Setting up /dev/ for domain %s", vm->def->name); VIR_DEBUG("Setting up /dev/ for domain %s", vm->def->name);
mount_options = virSecurityManagerGetMountOptions(driver->securityManager, mount_options = virSecurityManagerGetMountOptions(mgr,
vm->def); vm->def);
if (!mount_options && if (!mount_options &&
@ -7363,7 +7359,7 @@ qemuDomainSetupDev(virQEMUDriverPtr driver,
if (virFileSetupDev(path, opts) < 0) if (virFileSetupDev(path, opts) < 0)
goto cleanup; goto cleanup;
if (qemuDomainPopulateDevices(driver, vm, path) < 0) if (qemuDomainPopulateDevices(cfg, vm, path) < 0)
goto cleanup; goto cleanup;
ret = 0; ret = 0;
@ -7375,7 +7371,7 @@ qemuDomainSetupDev(virQEMUDriverPtr driver,
static int static int
qemuDomainSetupDisk(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, qemuDomainSetupDisk(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED,
virDomainDiskDefPtr disk, virDomainDiskDefPtr disk,
const char *devPath) const char *devPath)
{ {
@ -7401,7 +7397,7 @@ qemuDomainSetupDisk(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
static int static int
qemuDomainSetupAllDisks(virQEMUDriverPtr driver, qemuDomainSetupAllDisks(virQEMUDriverConfigPtr cfg,
virDomainObjPtr vm, virDomainObjPtr vm,
const char *devPath) const char *devPath)
{ {
@ -7409,7 +7405,7 @@ qemuDomainSetupAllDisks(virQEMUDriverPtr driver,
VIR_DEBUG("Setting up disks"); VIR_DEBUG("Setting up disks");
for (i = 0; i < vm->def->ndisks; i++) { for (i = 0; i < vm->def->ndisks; i++) {
if (qemuDomainSetupDisk(driver, if (qemuDomainSetupDisk(cfg,
vm->def->disks[i], vm->def->disks[i],
devPath) < 0) devPath) < 0)
return -1; return -1;
@ -7421,7 +7417,7 @@ qemuDomainSetupAllDisks(virQEMUDriverPtr driver,
static int static int
qemuDomainSetupHostdev(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, qemuDomainSetupHostdev(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED,
virDomainHostdevDefPtr dev, virDomainHostdevDefPtr dev,
const char *devPath) const char *devPath)
{ {
@ -7447,7 +7443,7 @@ qemuDomainSetupHostdev(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
static int static int
qemuDomainSetupAllHostdevs(virQEMUDriverPtr driver, qemuDomainSetupAllHostdevs(virQEMUDriverConfigPtr cfg,
virDomainObjPtr vm, virDomainObjPtr vm,
const char *devPath) const char *devPath)
{ {
@ -7455,7 +7451,7 @@ qemuDomainSetupAllHostdevs(virQEMUDriverPtr driver,
VIR_DEBUG("Setting up hostdevs"); VIR_DEBUG("Setting up hostdevs");
for (i = 0; i < vm->def->nhostdevs; i++) { for (i = 0; i < vm->def->nhostdevs; i++) {
if (qemuDomainSetupHostdev(driver, if (qemuDomainSetupHostdev(cfg,
vm->def->hostdevs[i], vm->def->hostdevs[i],
devPath) < 0) devPath) < 0)
return -1; return -1;
@ -7480,7 +7476,7 @@ qemuDomainSetupChardev(virDomainDefPtr def ATTRIBUTE_UNUSED,
static int static int
qemuDomainSetupAllChardevs(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, qemuDomainSetupAllChardevs(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED,
virDomainObjPtr vm, virDomainObjPtr vm,
const char *devPath) const char *devPath)
{ {
@ -7498,7 +7494,7 @@ qemuDomainSetupAllChardevs(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
static int static int
qemuDomainSetupTPM(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, qemuDomainSetupTPM(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED,
virDomainObjPtr vm, virDomainObjPtr vm,
const char *devPath) const char *devPath)
{ {
@ -7527,7 +7523,7 @@ qemuDomainSetupTPM(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
static int static int
qemuDomainSetupGraphics(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, qemuDomainSetupGraphics(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED,
virDomainGraphicsDefPtr gfx, virDomainGraphicsDefPtr gfx,
const char *devPath) const char *devPath)
{ {
@ -7543,7 +7539,7 @@ qemuDomainSetupGraphics(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
static int static int
qemuDomainSetupAllGraphics(virQEMUDriverPtr driver, qemuDomainSetupAllGraphics(virQEMUDriverConfigPtr cfg,
virDomainObjPtr vm, virDomainObjPtr vm,
const char *devPath) const char *devPath)
{ {
@ -7551,7 +7547,7 @@ qemuDomainSetupAllGraphics(virQEMUDriverPtr driver,
VIR_DEBUG("Setting up graphics"); VIR_DEBUG("Setting up graphics");
for (i = 0; i < vm->def->ngraphics; i++) { for (i = 0; i < vm->def->ngraphics; i++) {
if (qemuDomainSetupGraphics(driver, if (qemuDomainSetupGraphics(cfg,
vm->def->graphics[i], vm->def->graphics[i],
devPath) < 0) devPath) < 0)
return -1; return -1;
@ -7563,7 +7559,7 @@ qemuDomainSetupAllGraphics(virQEMUDriverPtr driver,
static int static int
qemuDomainSetupInput(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, qemuDomainSetupInput(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED,
virDomainInputDefPtr input, virDomainInputDefPtr input,
const char *devPath) const char *devPath)
{ {
@ -7590,7 +7586,7 @@ qemuDomainSetupInput(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
static int static int
qemuDomainSetupAllInputs(virQEMUDriverPtr driver, qemuDomainSetupAllInputs(virQEMUDriverConfigPtr cfg,
virDomainObjPtr vm, virDomainObjPtr vm,
const char *devPath) const char *devPath)
{ {
@ -7598,7 +7594,7 @@ qemuDomainSetupAllInputs(virQEMUDriverPtr driver,
VIR_DEBUG("Setting up inputs"); VIR_DEBUG("Setting up inputs");
for (i = 0; i < vm->def->ninputs; i++) { for (i = 0; i < vm->def->ninputs; i++) {
if (qemuDomainSetupInput(driver, if (qemuDomainSetupInput(cfg,
vm->def->inputs[i], vm->def->inputs[i],
devPath) < 0) devPath) < 0)
return -1; return -1;
@ -7609,7 +7605,7 @@ qemuDomainSetupAllInputs(virQEMUDriverPtr driver,
static int static int
qemuDomainSetupRNG(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, qemuDomainSetupRNG(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED,
virDomainRNGDefPtr rng, virDomainRNGDefPtr rng,
const char *devPath) const char *devPath)
{ {
@ -7629,7 +7625,7 @@ qemuDomainSetupRNG(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
static int static int
qemuDomainSetupAllRNGs(virQEMUDriverPtr driver, qemuDomainSetupAllRNGs(virQEMUDriverConfigPtr cfg,
virDomainObjPtr vm, virDomainObjPtr vm,
const char *devPath) const char *devPath)
{ {
@ -7637,7 +7633,7 @@ qemuDomainSetupAllRNGs(virQEMUDriverPtr driver,
VIR_DEBUG("Setting up RNGs"); VIR_DEBUG("Setting up RNGs");
for (i = 0; i < vm->def->nrngs; i++) { for (i = 0; i < vm->def->nrngs; i++) {
if (qemuDomainSetupRNG(driver, if (qemuDomainSetupRNG(cfg,
vm->def->rngs[i], vm->def->rngs[i],
devPath) < 0) devPath) < 0)
return -1; return -1;
@ -7649,10 +7645,10 @@ qemuDomainSetupAllRNGs(virQEMUDriverPtr driver,
int int
qemuDomainBuildNamespace(virQEMUDriverPtr driver, qemuDomainBuildNamespace(virQEMUDriverConfigPtr cfg,
virSecurityManagerPtr mgr,
virDomainObjPtr vm) virDomainObjPtr vm)
{ {
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
char *devPath = NULL; char *devPath = NULL;
char **devMountsPath = NULL, **devMountsSavePath = NULL; char **devMountsPath = NULL, **devMountsSavePath = NULL;
size_t ndevMountsPath = 0, i; size_t ndevMountsPath = 0, i;
@ -7663,7 +7659,7 @@ qemuDomainBuildNamespace(virQEMUDriverPtr driver,
goto cleanup; goto cleanup;
} }
if (qemuDomainGetPreservedMounts(driver, vm, if (qemuDomainGetPreservedMounts(cfg, vm,
&devMountsPath, &devMountsSavePath, &devMountsPath, &devMountsSavePath,
&ndevMountsPath) < 0) &ndevMountsPath) < 0)
goto cleanup; goto cleanup;
@ -7684,7 +7680,7 @@ qemuDomainBuildNamespace(virQEMUDriverPtr driver,
if (virProcessSetupPrivateMountNS() < 0) if (virProcessSetupPrivateMountNS() < 0)
goto cleanup; goto cleanup;
if (qemuDomainSetupDev(driver, vm, devPath) < 0) if (qemuDomainSetupDev(cfg, mgr, vm, devPath) < 0)
goto cleanup; goto cleanup;
/* Save some mount points because we want to share them with the host */ /* Save some mount points because we want to share them with the host */
@ -7703,25 +7699,25 @@ qemuDomainBuildNamespace(virQEMUDriverPtr driver,
goto cleanup; goto cleanup;
} }
if (qemuDomainSetupAllDisks(driver, vm, devPath) < 0) if (qemuDomainSetupAllDisks(cfg, vm, devPath) < 0)
goto cleanup; goto cleanup;
if (qemuDomainSetupAllHostdevs(driver, vm, devPath) < 0) if (qemuDomainSetupAllHostdevs(cfg, vm, devPath) < 0)
goto cleanup; goto cleanup;
if (qemuDomainSetupAllChardevs(driver, vm, devPath) < 0) if (qemuDomainSetupAllChardevs(cfg, vm, devPath) < 0)
goto cleanup; goto cleanup;
if (qemuDomainSetupTPM(driver, vm, devPath) < 0) if (qemuDomainSetupTPM(cfg, vm, devPath) < 0)
goto cleanup; goto cleanup;
if (qemuDomainSetupAllGraphics(driver, vm, devPath) < 0) if (qemuDomainSetupAllGraphics(cfg, vm, devPath) < 0)
goto cleanup; goto cleanup;
if (qemuDomainSetupAllInputs(driver, vm, devPath) < 0) if (qemuDomainSetupAllInputs(cfg, vm, devPath) < 0)
goto cleanup; goto cleanup;
if (qemuDomainSetupAllRNGs(driver, vm, devPath) < 0) if (qemuDomainSetupAllRNGs(cfg, vm, devPath) < 0)
goto cleanup; goto cleanup;
if (virFileMoveMount(devPath, "/dev") < 0) if (virFileMoveMount(devPath, "/dev") < 0)
@ -7743,7 +7739,6 @@ qemuDomainBuildNamespace(virQEMUDriverPtr driver,
ret = 0; ret = 0;
cleanup: cleanup:
virObjectUnref(cfg);
for (i = 0; i < ndevMountsPath; i++) for (i = 0; i < ndevMountsPath; i++)
rmdir(devMountsSavePath[i]); rmdir(devMountsSavePath[i]);
virStringListFreeCount(devMountsPath, ndevMountsPath); virStringListFreeCount(devMountsPath, ndevMountsPath);

3
src/qemu/qemu_domain.h
View File

@ -809,7 +809,8 @@ int qemuDomainGetHostdevPath(virDomainDefPtr def,
char ***path, char ***path,
int **perms); int **perms);
int qemuDomainBuildNamespace(virQEMUDriverPtr driver, int qemuDomainBuildNamespace(virQEMUDriverConfigPtr cfg,
virSecurityManagerPtr mgr,
virDomainObjPtr vm); virDomainObjPtr vm);
int qemuDomainCreateNamespace(virQEMUDriverPtr driver, int qemuDomainCreateNamespace(virQEMUDriverPtr driver,

2
src/qemu/qemu_process.c
View File

@ -2636,7 +2636,7 @@ static int qemuProcessHook(void *data)
if (virSecurityManagerClearSocketLabel(h->driver->securityManager, h->vm->def) < 0) if (virSecurityManagerClearSocketLabel(h->driver->securityManager, h->vm->def) < 0)
goto cleanup; goto cleanup;
if (qemuDomainBuildNamespace(h->driver, h->vm) < 0) if (qemuDomainBuildNamespace(h->cfg, h->driver->securityManager, h->vm) < 0)
goto cleanup; goto cleanup;
if (virDomainNumatuneGetMode(h->vm->def->numa, -1, &mode) == 0) { if (virDomainNumatuneGetMode(h->vm->def->numa, -1, &mode) == 0) {