[libvirt] [PATCHv3 0/3] Fix vmdef usage after ExitMonitor
The patches 1-2, 6-8 from v2 have been pushed already Device removal and device detach are now squashed together. Both attach and detach skip audit when the domain crashed. (Left to be cleaned up by a separate series) Patches checking the return value in all other non-fatal cases were squashed together with the patch adding ATTRIBUTE_RETURN_CHECK. Rebased to master, added qemuProcessUpdateVideoRamSize, and fixed numerous wrong return values and a few missing EndJobs. Ján Tomko (3): Fix vmdef usage after domain crash in monitor on device detach Fix vmdef usage after domain crash in monitor on device attach Always check return value of qemuDomainObjExitMonitor src/qemu/qemu_domain.c | 11 +-- src/qemu/qemu_domain.h | 3 +- src/qemu/qemu_driver.c | 166 ++++++++++++++++++++++--------------- src/qemu/qemu_hotplug.c | 202 ++++++++++++++++++++++++++++++---------------- src/qemu/qemu_hotplug.h | 6 +- src/qemu/qemu_migration.c | 139 ++++++++++++++++--------------- src/qemu/qemu_process.c | 115 +++++++++++++------------- 7 files changed, 373 insertions(+), 269 deletions(-) -- 2.0.4
https://bugzilla.redhat.com/show_bug.cgi?id=1161024 In the device type-specific functions, exit early if the domain has disappeared, because the cleanup should have been done by qemuProcessStop. Check the return value in processDeviceDeletedEvent and qemuProcessUpdateDevices. Fix vmdef usage after domain crash in monitor on device detach https://bugzilla.redhat.com/show_bug.cgi?id=1161024 Skip audit and removing the device from live def because it has already been cleaned up. --- src/qemu/qemu_driver.c | 3 +- src/qemu/qemu_hotplug.c | 91 ++++++++++++++++++++++++++++++------------------- src/qemu/qemu_hotplug.h | 6 ++-- src/qemu/qemu_process.c | 6 ++-- 4 files changed, 65 insertions(+), 41 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 61db194..a0d7d68 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4044,7 +4044,8 @@ processDeviceDeletedEvent(virQEMUDriverPtr driver, if (virDomainDefFindDevice(vm->def, devAlias, &dev, true) < 0) goto endjob; - qemuDomainRemoveDevice(driver, vm, &dev); + if (qemuDomainRemoveDevice(driver, vm, &dev) < 0) + goto endjob; if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) VIR_WARN("unable to save domain status after removing device %s", diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index bf1f69a..32464aa 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2499,8 +2499,9 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); qemuMonitorDriveDel(priv->mon, drivestr); - qemuDomainObjExitMonitor(driver, vm); VIR_FREE(drivestr); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; virDomainAuditDisk(vm, disk->src, NULL, "detach", true); @@ -2615,7 +2616,8 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); qemuMonitorDriveDel(priv->mon, drivestr); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; } event = virDomainEventDeviceRemovedNewFromObj(vm, hostdev->info->alias); @@ -2709,7 +2711,8 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuMonitorRemoveNetdev(priv->mon, hostnet_name) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; virDomainAuditNet(vm, net, NULL, "detach", false); goto cleanup; } @@ -2721,12 +2724,14 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("unable to determine original VLAN")); } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; virDomainAuditNet(vm, net, NULL, "detach", false); goto cleanup; } } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; virDomainAuditNet(vm, net, NULL, "detach", true); @@ -2796,7 +2801,8 @@ qemuDomainRemoveChrDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); rc = qemuMonitorDetachCharDev(priv->mon, charAlias); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; virDomainAuditChardev(vm, chr, NULL, "detach", rc == 0); @@ -2817,27 +2823,28 @@ qemuDomainRemoveChrDevice(virQEMUDriverPtr driver, } -void +int qemuDomainRemoveDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev) { + int ret = -1; switch ((virDomainDeviceType) dev->type) { case VIR_DOMAIN_DEVICE_DISK: - qemuDomainRemoveDiskDevice(driver, vm, dev->data.disk); + ret = qemuDomainRemoveDiskDevice(driver, vm, dev->data.disk); break; case VIR_DOMAIN_DEVICE_CONTROLLER: - qemuDomainRemoveControllerDevice(driver, vm, dev->data.controller); + ret = qemuDomainRemoveControllerDevice(driver, vm, dev->data.controller); break; case VIR_DOMAIN_DEVICE_NET: - qemuDomainRemoveNetDevice(driver, vm, dev->data.net); + ret = qemuDomainRemoveNetDevice(driver, vm, dev->data.net); break; case VIR_DOMAIN_DEVICE_HOSTDEV: - qemuDomainRemoveHostDevice(driver, vm, dev->data.hostdev); + ret = qemuDomainRemoveHostDevice(driver, vm, dev->data.hostdev); break; case VIR_DOMAIN_DEVICE_CHR: - qemuDomainRemoveChrDevice(driver, vm, dev->data.chr); + ret = qemuDomainRemoveChrDevice(driver, vm, dev->data.chr); break; case VIR_DOMAIN_DEVICE_NONE: @@ -2863,6 +2870,7 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver, virDomainDeviceTypeToString(dev->type)); break; } + return ret; } @@ -2986,19 +2994,22 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; virDomainAuditDisk(vm, detach->src, NULL, "detach", false); goto cleanup; } } else { if (qemuMonitorRemovePCIDevice(priv->mon, &detach->info.addr.pci) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; virDomainAuditDisk(vm, detach->src, NULL, "detach", false); goto cleanup; } } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; rc = qemuDomainWaitForDeviceRemoval(vm); if (rc == 0 || rc == 1) @@ -3038,11 +3049,13 @@ qemuDomainDetachDiskDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; virDomainAuditDisk(vm, detach->src, NULL, "detach", false); goto cleanup; } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; rc = qemuDomainWaitForDeviceRemoval(vm); if (rc == 0 || rc == 1) @@ -3219,17 +3232,20 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuMonitorDelDevice(priv->mon, detach->info.alias)) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; goto cleanup; } } else { if (qemuMonitorRemovePCIDevice(priv->mon, &detach->info.addr.pci) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; goto cleanup; } } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; rc = qemuDomainWaitForDeviceRemoval(vm); if (rc == 0 || rc == 1) @@ -3274,7 +3290,8 @@ qemuDomainDetachHostPCIDevice(virQEMUDriverPtr driver, } else { ret = qemuMonitorRemovePCIDevice(priv->mon, &detach->info->addr.pci); } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; return ret; } @@ -3303,7 +3320,8 @@ qemuDomainDetachHostUSBDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorDelDevice(priv->mon, detach->info->alias); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; return ret; } @@ -3331,14 +3349,11 @@ qemuDomainDetachHostSCSIDevice(virQEMUDriverPtr driver, qemuDomainMarkDeviceForRemoval(vm, detach->info); qemuDomainObjEnterMonitor(driver, vm); - if (qemuMonitorDelDevice(priv->mon, detach->info->alias) < 0) { - qemuDomainObjExitMonitor(driver, vm); - goto cleanup; - } - qemuDomainObjExitMonitor(driver, vm); - ret = 0; + ret = qemuMonitorDelDevice(priv->mon, detach->info->alias); + + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; - cleanup: return ret; } @@ -3374,7 +3389,8 @@ qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver, } if (ret < 0) { - virDomainAuditHostdev(vm, detach, "detach", false); + if (virDomainObjIsActive(vm)) + virDomainAuditHostdev(vm, detach, "detach", false); } else { int rc = qemuDomainWaitForDeviceRemoval(vm); if (rc == 0 || rc == 1) @@ -3530,19 +3546,22 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; virDomainAuditNet(vm, detach, NULL, "detach", false); goto cleanup; } } else { if (qemuMonitorRemovePCIDevice(priv->mon, &detach->info.addr.pci) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; virDomainAuditNet(vm, detach, NULL, "detach", false); goto cleanup; } } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; rc = qemuDomainWaitForDeviceRemoval(vm); if (rc == 0 || rc == 1) @@ -3709,10 +3728,12 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); if (devstr && qemuMonitorDelDevice(priv->mon, tmpChr->info.alias) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; goto cleanup; } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; rc = qemuDomainWaitForDeviceRemoval(vm); if (rc == 0 || rc == 1) diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index d13c532..19ab9a0 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -107,9 +107,9 @@ virDomainChrDefPtr qemuDomainChrRemove(virDomainDefPtr vmdef, virDomainChrDefPtr chr); -void qemuDomainRemoveDevice(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainDeviceDefPtr dev); +int qemuDomainRemoveDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev); bool qemuDomainSignalDeviceRemoval(virDomainObjPtr vm, const char *devAlias); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 2aa195f..300efc1 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3629,8 +3629,10 @@ qemuProcessUpdateDevices(virQEMUDriverPtr driver, if ((tmp = old)) { while (*tmp) { if (!virStringArrayHasString(priv->qemuDevices, *tmp) && - virDomainDefFindDevice(vm->def, *tmp, &dev, false) == 0) - qemuDomainRemoveDevice(driver, vm, &dev); + virDomainDefFindDevice(vm->def, *tmp, &dev, false) == 0 && + qemuDomainRemoveDevice(driver, vm, &dev) < 0) { + goto cleanup; + } tmp++; } } -- 2.0.4
On 14.01.2015 19:48, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1161024
In the device type-specific functions, exit early if the domain has disappeared, because the cleanup should have been done by qemuProcessStop.
Check the return value in processDeviceDeletedEvent and qemuProcessUpdateDevices.
Fix vmdef usage after domain crash in monitor on device detach
https://bugzilla.redhat.com/show_bug.cgi?id=1161024
Skip audit and removing the device from live def because it has already been cleaned up.
The commit message seems mangled.
--- src/qemu/qemu_driver.c | 3 +- src/qemu/qemu_hotplug.c | 91 ++++++++++++++++++++++++++++++------------------- src/qemu/qemu_hotplug.h | 6 ++-- src/qemu/qemu_process.c | 6 ++-- 4 files changed, 65 insertions(+), 41 deletions(-)
Michal
https://bugzilla.redhat.com/show_bug.cgi?id=1161024 If the domain crashed while we were in monitor, we cannot rely on the REALLOC done on live definition, since vm->def now points to the persistent definition. Skip adding the attached devices to domain definition if the domain crashed. In AttachChrDevice, the chardev was already added to the live definition and freed by qemuProcessStop in the case of a crash. Skip the device removal in that case. Also skip audit if the domain crashed in the meantime. --- src/qemu/qemu_hotplug.c | 69 +++++++++++++++++++++++++++++++++++++------------ 1 file changed, 52 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 32464aa..748fbfe 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -391,7 +391,11 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, memcpy(&disk->info.addr.pci, &guestAddr, sizeof(guestAddr)); } } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + releaseaddr = false; + ret = -1; + goto error; + } virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0); @@ -477,7 +481,11 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, type, &controller->info.addr.pci); } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + releaseaddr = false; + ret = -1; + goto cleanup; + } if (ret == 0) { if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) @@ -628,7 +636,10 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, disk->info.addr.drive.unit = driveAddr.unit; } } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + ret = -1; + goto error; + } virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0); @@ -708,7 +719,10 @@ qemuDomainAttachUSBMassstorageDevice(virConnectPtr conn, } else { ret = qemuMonitorAddUSBDisk(priv->mon, src); } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + ret = -1; + goto error; + } virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0); @@ -1280,7 +1294,8 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorAddDeviceWithFd(priv->mon, devstr, configfd, configfd_name); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto error; } else { virDevicePCIAddressPtr guestAddr = &hostdev->info->addr.pci; virDevicePCIAddressPtr hostAddr = &hostdev->source.subsys.u.pci.addr; @@ -1296,7 +1311,8 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorAddPCIHostDevice(priv->mon, hostAddr, guestAddr); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto error; hostdev->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; } @@ -1360,12 +1376,11 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver, goto error; qemuDomainObjEnterMonitor(driver, vm); - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) - ret = qemuMonitorAddDevice(priv->mon, devstr); - else + ret = qemuMonitorAddDevice(priv->mon, devstr); + + if (qemuDomainObjExitMonitor(driver, vm) < 0) goto error; - qemuDomainObjExitMonitor(driver, vm); virDomainAuditRedirdev(vm, redirdev, "attach", ret == 0); if (ret < 0) goto error; @@ -1487,17 +1502,29 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); if (qemuMonitorAttachCharDev(priv->mon, charAlias, &chr->source) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + need_remove = false; + ret = -1; + goto cleanup; + } goto audit; } if (devstr && qemuMonitorAddDevice(priv->mon, devstr) < 0) { /* detach associated chardev on error */ qemuMonitorDetachCharDev(priv->mon, charAlias); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + need_remove = false; + ret = -1; + goto cleanup; + } goto audit; } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + need_remove = false; + ret = -1; + goto cleanup; + } ret = 0; audit: @@ -1553,7 +1580,10 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, ret = qemuMonitorAddUSBDeviceExact(priv->mon, hostdev->source.subsys.u.usb.bus, hostdev->source.subsys.u.usb.device); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + ret = -1; + goto cleanup; + } virDomainAuditHostdev(vm, hostdev, "attach", ret == 0); if (ret < 0) goto cleanup; @@ -1656,7 +1686,10 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, } } } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + ret = -1; + goto cleanup; + } virDomainAuditHostdev(vm, hostdev, "attach", ret == 0); if (ret < 0) @@ -1856,7 +1889,8 @@ int qemuDomainChangeNetLinkState(virQEMUDriverPtr driver, dev->linkstate = linkstate; cleanup: - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; return ret; } @@ -3641,7 +3675,8 @@ qemuDomainChangeGraphicsPasswords(virQEMUDriverPtr driver, } end_job: - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; cleanup: virObjectUnref(cfg); return ret; -- 2.0.4
Depending on the context, either error out if the domain has disappeared in the meantime, or just ignore the value to allow marking the function as ATTRIBUTE_RETURN_CHECK. --- src/qemu/qemu_domain.c | 11 ++-- src/qemu/qemu_domain.h | 3 +- src/qemu/qemu_driver.c | 163 ++++++++++++++++++++++++++++------------------ src/qemu/qemu_hotplug.c | 42 ++++++------ src/qemu/qemu_migration.c | 139 ++++++++++++++++++++------------------- src/qemu/qemu_process.c | 109 ++++++++++++++++--------------- 6 files changed, 256 insertions(+), 211 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0b4913b..99c46d4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2317,7 +2317,7 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); /* we continue on even in the face of error */ qemuMonitorDeleteSnapshot(priv->mon, snap->def->name); - qemuDomainObjExitMonitor(driver, vm); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); } } @@ -2752,17 +2752,18 @@ qemuDomainUpdateDeviceList(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; char **aliases; + int rc; if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT)) return 0; if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return -1; - if (qemuMonitorGetDeviceAliases(priv->mon, &aliases) < 0) { - qemuDomainObjExitMonitor(driver, vm); + rc = qemuMonitorGetDeviceAliases(priv->mon, &aliases); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; + if (rc < 0) return -1; - } - qemuDomainObjExitMonitor(driver, vm); virStringFreeList(priv->qemuDevices); priv->qemuDevices = aliases; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index fd91d83..b2c3881 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -247,7 +247,8 @@ void qemuDomainObjEnterMonitor(virQEMUDriverPtr driver, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int qemuDomainObjExitMonitor(virQEMUDriverPtr driver, virDomainObjPtr obj) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) + ATTRIBUTE_RETURN_CHECK; int qemuDomainObjEnterMonitorAsync(virQEMUDriverPtr driver, virDomainObjPtr obj, qemuDomainAsyncJob asyncJob) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a0d7d68..6591517 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1978,7 +1978,8 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorSystemPowerdown(priv->mon); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; } endjob: @@ -2073,7 +2074,8 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags) } else { qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorSystemPowerdown(priv->mon); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; if (ret == 0) qemuDomainSetFakeReboot(driver, vm, isReboot); @@ -2116,7 +2118,8 @@ qemuDomainReset(virDomainPtr dom, unsigned int flags) priv = vm->privateData; qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorSystemReset(priv->mon); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; priv->fakeReboot = false; @@ -2333,7 +2336,8 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, priv = vm->privateData; qemuDomainObjEnterMonitor(driver, vm); r = qemuMonitorSetBalloon(priv->mon, newmem); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto endjob; virDomainAuditMemory(vm, vm->def->mem.cur_balloon, newmem, "update", r == 1); if (r < 0) @@ -2415,7 +2419,8 @@ static int qemuDomainSetMemoryStatsPeriod(virDomainPtr dom, int period, qemuDomainObjEnterMonitor(driver, vm); r = qemuMonitorSetMemoryStatsPeriod(priv->mon, period); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto endjob; if (r < 0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("unable to set balloon driver collection period")); @@ -2479,7 +2484,8 @@ static int qemuDomainInjectNMI(virDomainPtr domain, unsigned int flags) qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorInjectNMI(priv->mon); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; endjob: qemuDomainObjEndJob(driver, vm); @@ -2541,7 +2547,8 @@ static int qemuDomainSendKey(virDomainPtr domain, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorSendKey(priv->mon, holdtime, keycodes, nkeycodes); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; endjob: qemuDomainObjEndJob(driver, vm); @@ -2596,7 +2603,10 @@ static int qemuDomainGetInfo(virDomainPtr dom, } else { qemuDomainObjEnterMonitor(driver, vm); err = qemuMonitorGetBalloonInfo(priv->mon, &balloon); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + qemuDomainObjEndJob(driver, vm); + goto cleanup; + } } qemuDomainObjEndJob(driver, vm); @@ -3492,7 +3502,7 @@ static int qemuDumpToFd(virQEMUDriverPtr driver, virDomainObjPtr vm, ret = qemuMonitorDumpToFd(priv->mon, fd, dumpformat); cleanup: - qemuDomainObjExitMonitor(driver, vm); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); return ret; } @@ -3691,7 +3701,8 @@ qemuDomainCoreDumpWithFormat(virDomainPtr dom, priv = vm->privateData; qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorSystemReset(priv->mon); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; } if (resume && virDomainObjIsActive(vm)) { @@ -3788,10 +3799,11 @@ qemuDomainScreenshot(virDomainPtr dom, qemuDomainObjEnterMonitor(driver, vm); if (qemuMonitorScreendump(priv->mon, tmp) < 0) { - qemuDomainObjExitMonitor(driver, vm); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); goto endjob; } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto endjob; if (VIR_CLOSE(tmp_fd) < 0) { virReportSystemError(errno, _("unable to close %s"), tmp); @@ -4216,7 +4228,8 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorQueryRxFilter(priv->mon, devAlias, &guestFilter); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; if (ret < 0) goto endjob; @@ -6229,7 +6242,8 @@ static char *qemuDomainGetXMLDesc(virDomainPtr dom, qemuDomainObjEnterMonitor(driver, vm); err = qemuMonitorGetBalloonInfo(priv->mon, &balloon); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + err = -1; endjob: qemuDomainObjEndJob(driver, vm); @@ -6992,7 +7006,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, } if (ret == 0) - qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE); + ret = qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE); return ret; } @@ -7068,7 +7082,7 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, } if (ret == 0) - qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE); + ret = qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE); return ret; } @@ -10093,10 +10107,11 @@ qemuDomainBlockResize(virDomainPtr dom, qemuDomainObjEnterMonitor(driver, vm); if (qemuMonitorBlockResize(priv->mon, device, size) < 0) { - qemuDomainObjExitMonitor(driver, vm); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); goto endjob; } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto endjob; ret = 0; @@ -10750,7 +10765,8 @@ qemuDomainMemoryStats(virDomainPtr dom, qemuDomainObjPrivatePtr priv = vm->privateData; qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorGetMemoryStats(priv->mon, stats, nr_stats); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; if (ret >= 0 && ret < nr_stats) { long rss; @@ -10884,16 +10900,17 @@ qemuDomainMemoryPeek(virDomainPtr dom, qemuDomainObjEnterMonitor(driver, vm); if (flags == VIR_MEMORY_VIRTUAL) { if (qemuMonitorSaveVirtualMemory(priv->mon, offset, size, tmp) < 0) { - qemuDomainObjExitMonitor(driver, vm); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); goto endjob; } } else { if (qemuMonitorSavePhysicalMemory(priv->mon, offset, size, tmp) < 0) { - qemuDomainObjExitMonitor(driver, vm); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); goto endjob; } } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto endjob; /* Read the memory file into buffer. */ if (saferead(fd, buffer, size) == (ssize_t) -1) { @@ -11131,7 +11148,8 @@ qemuDomainGetBlockInfo(virDomainPtr dom, ret = qemuMonitorGetBlockExtent(priv->mon, disk->info.alias, &src->allocation); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; } if (ret == 0) { @@ -12371,7 +12389,8 @@ static int qemuDomainAbortJob(virDomainPtr dom) qemuDomainObjAbortAsyncJob(vm); qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorMigrateCancel(priv->mon); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; endjob: qemuDomainObjEndJob(driver, vm); @@ -12414,7 +12433,8 @@ qemuDomainMigrateSetMaxDowntime(virDomainPtr dom, VIR_DEBUG("Setting migration downtime to %llums", downtime); qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorSetMigrationDowntime(priv->mon, downtime); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; endjob: qemuDomainObjEndJob(driver, vm); @@ -12467,7 +12487,8 @@ qemuDomainMigrateGetCompressionCache(virDomainPtr dom, ret = qemuMonitorGetMigrationCacheSize(priv->mon, cacheSize); } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; endjob: qemuDomainObjEndJob(driver, vm); @@ -12521,7 +12542,8 @@ qemuDomainMigrateSetCompressionCache(virDomainPtr dom, ret = qemuMonitorSetMigrationCacheSize(priv->mon, cacheSize); } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; endjob: qemuDomainObjEndJob(driver, vm); @@ -12571,7 +12593,8 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom, VIR_DEBUG("Setting migration bandwidth to %luMbs", bandwidth); qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorSetMigrationSpeed(priv->mon, bandwidth); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; if (ret == 0) priv->migMaxBandwidth = bandwidth; @@ -12887,7 +12910,8 @@ qemuDomainSnapshotCreateActiveInternal(virConnectPtr conn, } ret = qemuMonitorCreateSnapshot(priv->mon, snap->def->name); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; if (ret < 0) goto cleanup; @@ -13343,7 +13367,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, char *device = NULL; char *source = NULL; const char *formatStr = NULL; - int ret = -1; + int ret = -1, rc; bool need_unlink = false; if (snap->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { @@ -13405,18 +13429,14 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) goto cleanup; - ret = qemuMonitorDiskSnapshot(priv->mon, actions, device, source, - formatStr, reuse); + ret = rc = qemuMonitorDiskSnapshot(priv->mon, actions, device, source, + formatStr, reuse); if (!actions) { - qemuDomainObjExitMonitor(driver, vm); - if (!virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("domain crashed while taking the snapshot")); + if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -1; - } } - virDomainAuditDisk(vm, disk->src, snap->src, "snapshot", ret >= 0); + virDomainAuditDisk(vm, disk->src, snap->src, "snapshot", rc >= 0); if (ret < 0) goto cleanup; @@ -13551,12 +13571,8 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, if (ret == 0) { if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) { ret = qemuMonitorTransaction(priv->mon, actions); - qemuDomainObjExitMonitor(driver, vm); - if (!virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("domain crashed while taking the snapshot")); + if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -1; - } } else { /* failed to enter monitor, clean stuff up and quit */ ret = -1; @@ -14649,7 +14665,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } qemuDomainObjEnterMonitor(driver, vm); rc = qemuMonitorLoadSnapshot(priv->mon, snap->def->name); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto endjob; if (rc < 0) { /* XXX resume domain if it was running before the * failed loadvm attempt? */ @@ -15021,7 +15038,8 @@ static int qemuDomainQemuMonitorCommand(virDomainPtr domain, const char *cmd, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorArbitraryCommand(priv->mon, cmd, result, hmp); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; endjob: qemuDomainObjEndJob(driver, vm); @@ -15343,14 +15361,10 @@ qemuDomainBlockPivot(virConnectPtr conn, if (!disk->mirrorState) { qemuDomainObjEnterMonitor(driver, vm); rc = qemuMonitorBlockJobInfo(priv->mon, device, &info, NULL); - qemuDomainObjExitMonitor(driver, vm); - if (rc < 0) + if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; - if (!virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("domain is not running")); + if (rc < 0) goto cleanup; - } if (rc == 1 && info.cur == info.end && info.type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY; @@ -15427,7 +15441,10 @@ qemuDomainBlockPivot(virConnectPtr conn, disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_PIVOT; qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorDrivePivot(priv->mon, device, disk->mirror->path, format); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + ret = -1; + goto cleanup; + } if (ret < 0) { /* On failure, qemu abandons the mirror, and reverts back to @@ -15630,7 +15647,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorBlockJob(priv->mon, device, basePath, backingPath, speed, mode, async); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; if (ret < 0) { if (mode == BLOCK_JOB_ABORT && disk->mirror) disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; @@ -15675,7 +15693,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorBlockJobInfo(priv->mon, device, &dummy, NULL); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; if (ret <= 0) break; @@ -15779,7 +15798,8 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom, const char *path, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorBlockJobInfo(priv->mon, device, info, &bandwidth); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; if (ret < 0) goto endjob; @@ -15997,7 +16017,8 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, ret = qemuMonitorDriveMirror(priv->mon, device, mirror->path, format, bandwidth, granularity, buf_size, flags); virDomainAuditDisk(vm, NULL, mirror, "mirror", ret >= 0); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; if (ret < 0) { qemuDomainPrepareDiskChainElement(driver, vm, mirror, VIR_DISK_CHAIN_NO_ACCESS); @@ -16394,7 +16415,10 @@ qemuDomainBlockCommit(virDomainPtr dom, ret = qemuMonitorBlockCommit(priv->mon, device, topPath, basePath, backingPath, speed); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + ret = -1; + goto endjob; + } if (mirror) { if (ret == 0) { @@ -16488,7 +16512,10 @@ qemuDomainOpenGraphics(virDomainPtr dom, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorOpenGraphics(priv->mon, protocol, fd, "graphicsfd", (flags & VIR_DOMAIN_OPEN_GRAPHICS_SKIPAUTH) != 0); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + ret = -1; + goto cleanup; + } qemuDomainObjEndJob(driver, vm); cleanup: @@ -16557,7 +16584,10 @@ qemuDomainOpenGraphicsFD(virDomainPtr dom, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorOpenGraphics(priv->mon, protocol, pair[1], "graphicsfd", (flags & VIR_DOMAIN_OPEN_GRAPHICS_SKIPAUTH)); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + ret = -1; + goto cleanup; + } qemuDomainObjEndJob(driver, vm); if (ret < 0) goto cleanup; @@ -17000,7 +17030,8 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, goto endjob; qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, &reply, supportMaxOptions); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto endjob; if (ret < 0) goto endjob; } @@ -17172,7 +17203,8 @@ qemuDomainGetDiskErrors(virDomainPtr dom, qemuDomainObjEnterMonitor(driver, vm); table = qemuMonitorGetBlockInfo(priv->mon); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto endjob; if (!table) goto endjob; @@ -17451,7 +17483,8 @@ qemuDomainPMWakeup(virDomainPtr dom, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorSystemWakeup(priv->mon); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; endjob: qemuDomainObjEndJob(driver, vm); @@ -17882,7 +17915,8 @@ qemuDomainSetTime(virDomainPtr dom, qemuDomainObjEnterMonitor(driver, vm); rv = qemuMonitorRTCResetReinjection(priv->mon); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto endjob; if (rv < 0) goto endjob; @@ -18527,7 +18561,8 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, visitBacking); ignore_value(qemuMonitorBlockStatsUpdateCapacity(priv->mon, stats, visitBacking)); - qemuDomainObjExitMonitor(driver, dom); + if (qemuDomainObjExitMonitor(driver, dom) < 0) + goto cleanup; if (rc < 0) { virResetLastError(); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 748fbfe..158b3c5 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -193,7 +193,10 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorEjectMedia(priv->mon, driveAlias, force); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + ret = -1; + goto cleanup; + } if (ret < 0) goto error; @@ -236,7 +239,10 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, driveAlias, sourcestr, format); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + ret = -1; + goto cleanup; + } } virDomainAuditDisk(vm, disk->src, newsrc, "update", ret >= 0); @@ -275,7 +281,8 @@ qemuDomainCheckEjectableMedia(virQEMUDriverPtr driver, if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) { table = qemuMonitorGetBlockInfo(priv->mon); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; } if (!table) @@ -1031,7 +1038,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, if (qemuMonitorAddNetdev(priv->mon, netstr, tapfd, tapfdName, tapfdSize, vhostfd, vhostfdName, vhostfdSize) < 0) { - qemuDomainObjExitMonitor(driver, vm); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); virDomainAuditNet(vm, NULL, net, "attach", false); goto cleanup; } @@ -1039,24 +1046,19 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, if (qemuMonitorAddHostNetwork(priv->mon, netstr, tapfd, tapfdName, tapfdSize, vhostfd, vhostfdName, vhostfdSize) < 0) { - qemuDomainObjExitMonitor(driver, vm); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); virDomainAuditNet(vm, NULL, net, "attach", false); goto cleanup; } } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; for (i = 0; i < tapfdSize; i++) VIR_FORCE_CLOSE(tapfd[i]); for (i = 0; i < vhostfdSize; i++) VIR_FORCE_CLOSE(vhostfd[i]); - if (!virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("guest unexpectedly quit")); - goto cleanup; - } - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (!(nicstr = qemuBuildNicDevStr(vm->def, net, vlan, 0, vhostfdSize, priv->qemuCaps))) @@ -1069,7 +1071,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, qemuDomainObjEnterMonitor(driver, vm); if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuMonitorAddDevice(priv->mon, nicstr) < 0) { - qemuDomainObjExitMonitor(driver, vm); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); virDomainAuditNet(vm, NULL, net, "attach", false); goto try_remove; } @@ -1077,14 +1079,15 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, guestAddr = net->info.addr.pci; if (qemuMonitorAddPCINetwork(priv->mon, nicstr, &guestAddr) < 0) { - qemuDomainObjExitMonitor(driver, vm); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); virDomainAuditNet(vm, NULL, net, "attach", false); goto try_remove; } net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; memcpy(&net->info.addr.pci, &guestAddr, sizeof(guestAddr)); } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; /* set link state */ if (net->linkstate == VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN) { @@ -1096,7 +1099,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV)) { if (qemuMonitorSetLink(priv->mon, net->info.alias, VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN) < 0) { - qemuDomainObjExitMonitor(driver, vm); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); virDomainAuditNet(vm, NULL, net, "attach", false); goto try_remove; } @@ -1105,7 +1108,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, _("setting of link state not supported: Link is up")); } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; } /* link set to down */ } @@ -1179,7 +1183,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, if (qemuMonitorRemoveNetdev(priv->mon, netdev_name) < 0) VIR_WARN("Failed to remove network backend for netdev %s", netdev_name); - qemuDomainObjExitMonitor(driver, vm); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); VIR_FREE(netdev_name); } else { VIR_WARN("Unable to remove network backend"); @@ -1192,7 +1196,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, if (qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0) VIR_WARN("Failed to remove network backend for vlan %d, net %s", vlan, hostnet_name); - qemuDomainObjExitMonitor(driver, vm); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); VIR_FREE(hostnet_name); } goto cleanup; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index a878652..35a15b7 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -550,7 +550,7 @@ qemuMigrationCookieAddNBD(qemuMigrationCookiePtr mig, qemuDomainObjPrivatePtr priv = vm->privateData; virHashTablePtr stats = NULL; size_t i; - int ret = -1; + int ret = -1, rc; /* It is not a bug if there already is a NBD data */ if (!mig->nbd && @@ -571,18 +571,11 @@ qemuMigrationCookieAddNBD(qemuMigrationCookiePtr mig, goto cleanup; qemuDomainObjEnterMonitor(driver, vm); - if (qemuMonitorBlockStatsUpdateCapacity(priv->mon, stats, - false) < 0) { - qemuDomainObjExitMonitor(driver, vm); + rc = qemuMonitorBlockStatsUpdateCapacity(priv->mon, stats, false); + if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; - } - qemuDomainObjExitMonitor(driver, vm); - - if (!virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("domain exited meanwhile")); + if (rc < 0) goto cleanup; - } } if (!disk->info.alias || @@ -1655,15 +1648,13 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver, if (!port && ((virPortAllocatorAcquire(driver->migrationPorts, &port) < 0) || (qemuMonitorNBDServerStart(priv->mon, listenAddr, port) < 0))) { - qemuDomainObjExitMonitor(driver, vm); - goto cleanup; + goto exit_monitor; } - if (qemuMonitorNBDServerAdd(priv->mon, diskAlias, true) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuMonitorNBDServerAdd(priv->mon, diskAlias, true) < 0) + goto exit_monitor; + if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; - } - qemuDomainObjExitMonitor(driver, vm); } priv->nbdPort = port; @@ -1674,6 +1665,10 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver, if (ret < 0) virPortAllocatorRelease(driver->migrationPorts, port); return ret; + + exit_monitor: + ignore_value(qemuDomainObjExitMonitor(driver, vm)); + goto cleanup; } /** @@ -1763,7 +1758,8 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, goto error; mon_ret = qemuMonitorDriveMirror(priv->mon, diskAlias, nbd_dest, NULL, speed, 0, 0, mirror_flags); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto error; if (mon_ret < 0) goto error; @@ -1784,7 +1780,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, /* explicitly do this *after* we entered the monitor, * as this is a critical section so we are guaranteed * priv->job.asyncAbort will not change */ - qemuDomainObjExitMonitor(driver, vm); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); priv->job.current->type = VIR_DOMAIN_JOB_CANCELLED; virReportError(VIR_ERR_OPERATION_ABORTED, _("%s: %s"), qemuDomainAsyncJobTypeToString(priv->job.asyncJob), @@ -1793,7 +1789,8 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, } mon_ret = qemuMonitorBlockJobInfo(priv->mon, diskAlias, &info, NULL); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto error; if (mon_ret < 0) goto error; @@ -1848,7 +1845,8 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, BLOCK_JOB_ABORT, true) < 0) { VIR_WARN("Unable to cancel block-job on '%s'", diskAlias); } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + break; } else { VIR_WARN("Unable to enter monitor. No block job cancelled"); } @@ -1860,7 +1858,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, } -static void +static int qemuMigrationStopNBDServer(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuMigrationCookiePtr mig) @@ -1868,22 +1866,23 @@ qemuMigrationStopNBDServer(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; if (!mig->nbd) - return; + return 0; if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN) < 0) - return; + return -1; if (qemuMonitorNBDServerStop(priv->mon) < 0) VIR_WARN("Unable to stop NBD server"); - - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; virPortAllocatorRelease(driver->migrationPorts, priv->nbdPort); priv->nbdPort = 0; + return 0; } -static void +static int qemuMigrationCancelDriveMirror(qemuMigrationCookiePtr mig, virQEMUDriverPtr driver, virDomainObjPtr vm) @@ -1891,6 +1890,7 @@ qemuMigrationCancelDriveMirror(qemuMigrationCookiePtr mig, qemuDomainObjPrivatePtr priv = vm->privateData; size_t i; char *diskAlias = NULL; + int ret = 0; VIR_DEBUG("mig=%p nbdPort=%d", mig->nbd, priv->nbdPort); @@ -1914,12 +1914,15 @@ qemuMigrationCancelDriveMirror(qemuMigrationCookiePtr mig, if (qemuMonitorBlockJob(priv->mon, diskAlias, NULL, NULL, 0, BLOCK_JOB_ABORT, true) < 0) VIR_WARN("Unable to stop block job on %s", diskAlias); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + ret = -1; + goto cleanup; + } } cleanup: VIR_FREE(diskAlias); - return; + return ret; } /* Validate whether the domain is safe to migrate. If vm is NULL, @@ -2125,7 +2128,8 @@ qemuMigrationSetCompression(virQEMUDriverPtr driver, state); cleanup: - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; return ret; } @@ -2164,7 +2168,8 @@ qemuMigrationSetAutoConverge(virQEMUDriverPtr driver, state); cleanup: - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; return ret; } @@ -2210,7 +2215,8 @@ qemuMigrationSetPinAll(virQEMUDriverPtr driver, state); cleanup: - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; return ret; } @@ -2222,6 +2228,7 @@ qemuMigrationWaitForSpice(virQEMUDriverPtr driver, bool wait_for_spice = false; bool spice_migrated = false; size_t i = 0; + int rc; if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SEAMLESS_MIGRATION)) { for (i = 0; i < vm->def->ngraphics; i++) { @@ -2243,12 +2250,11 @@ qemuMigrationWaitForSpice(virQEMUDriverPtr driver, QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) return -1; - if (qemuMonitorGetSpiceMigrationStatus(priv->mon, - &spice_migrated) < 0) { - qemuDomainObjExitMonitor(driver, vm); + rc = qemuMonitorGetSpiceMigrationStatus(priv->mon, &spice_migrated); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; + if (rc < 0) return -1; - } - qemuDomainObjExitMonitor(driver, vm); virObjectUnlock(vm); nanosleep(&ts, NULL); virObjectLock(vm); @@ -2278,7 +2284,8 @@ qemuMigrationUpdateJobStatus(virQEMUDriverPtr driver, } ret = qemuMonitorGetMigrationStatus(priv->mon, &status); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; if (ret < 0 || qemuDomainJobInfoUpdateTime(priv->job.current) < 0) @@ -2476,7 +2483,8 @@ qemuDomainMigrateGraphicsRelocate(virQEMUDriverPtr driver, QEMU_ASYNC_JOB_MIGRATION_OUT) == 0) { ret = qemuMonitorGraphicsRelocate(priv->mon, type, listenAddress, port, tlsPort, tlsSubject); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; } cleanup: @@ -3874,7 +3882,7 @@ qemuMigrationRun(virQEMUDriverPtr driver, /* explicitly do this *after* we entered the monitor, * as this is a critical section so we are guaranteed * priv->job.asyncAbort will not change */ - qemuDomainObjExitMonitor(driver, vm); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); priv->job.current->type = VIR_DOMAIN_JOB_CANCELLED; virReportError(VIR_ERR_OPERATION_ABORTED, _("%s: %s"), qemuDomainAsyncJobTypeToString(priv->job.asyncJob), @@ -3882,24 +3890,20 @@ qemuMigrationRun(virQEMUDriverPtr driver, goto cleanup; } - if (qemuMonitorSetMigrationSpeed(priv->mon, migrate_speed) < 0) { - qemuDomainObjExitMonitor(driver, vm); - goto cleanup; - } + if (qemuMonitorSetMigrationSpeed(priv->mon, migrate_speed) < 0) + goto exit_monitor; /* connect to the destination qemu if needed */ if (spec->destType == MIGRATION_DEST_CONNECT_HOST && qemuMigrationConnect(driver, vm, spec) < 0) { - qemuDomainObjExitMonitor(driver, vm); - goto cleanup; + goto exit_monitor; } switch (spec->destType) { case MIGRATION_DEST_HOST: if (STREQ(spec->dest.host.protocol, "rdma") && virProcessSetMaxMemLock(vm->pid, vm->def->mem.hard_limit << 10) < 0) { - qemuDomainObjExitMonitor(driver, vm); - goto cleanup; + goto exit_monitor; } ret = qemuMonitorMigrateToHost(priv->mon, migrate_flags, spec->dest.host.protocol, @@ -3933,17 +3937,12 @@ qemuMigrationRun(virQEMUDriverPtr driver, VIR_FORCE_CLOSE(spec->dest.fd.qemu); break; } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; if (ret < 0) goto cleanup; ret = -1; - if (!virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("guest unexpectedly quit")); - goto cleanup; - } - /* From this point onwards we *must* call cancel to abort the * migration on source if anything goes wrong */ @@ -4000,7 +3999,7 @@ qemuMigrationRun(virQEMUDriverPtr driver, /* cancel any outstanding NBD jobs */ if (mig) - qemuMigrationCancelDriveMirror(mig, driver, vm); + ignore_value(qemuMigrationCancelDriveMirror(mig, driver, vm)); if (spec->fwdType != MIGRATION_FWD_DIRECT) { if (iothread && qemuMigrationStopTunnel(iothread, ret < 0) < 0) @@ -4035,6 +4034,10 @@ qemuMigrationRun(virQEMUDriverPtr driver, return ret; + exit_monitor: + ignore_value(qemuDomainObjExitMonitor(driver, vm)); + goto cleanup; + cancel: orig_err = virSaveLastError(); @@ -4042,7 +4045,7 @@ qemuMigrationRun(virQEMUDriverPtr driver, if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT) == 0) { qemuMonitorMigrateCancel(priv->mon); - qemuDomainObjExitMonitor(driver, vm); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); } } goto cleanup; @@ -5142,7 +5145,8 @@ qemuMigrationFinish(virQEMUDriverPtr driver, VIR_WARN("unable to provide network data for relocation"); } - qemuMigrationStopNBDServer(driver, vm, mig); + if (qemuMigrationStopNBDServer(driver, vm, mig) < 0) + goto endjob; if (flags & VIR_MIGRATE_PERSIST_DEST) { virDomainDefPtr vmdef; @@ -5319,7 +5323,8 @@ qemuMigrationToFile(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuMonitorSetMigrationSpeed(priv->mon, QEMU_DOMAIN_MIG_BANDWIDTH_MAX); priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX; - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; } if (!virDomainObjIsActive(vm)) { @@ -5395,11 +5400,11 @@ qemuMigrationToFile(virQEMUDriverPtr driver, virDomainObjPtr vm, if (virSetCloseExec(pipeFD[1]) < 0) { virReportSystemError(errno, "%s", _("Unable to set cloexec flag")); - qemuDomainObjExitMonitor(driver, vm); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); goto cleanup; } if (virCommandRunAsync(cmd, NULL) < 0) { - qemuDomainObjExitMonitor(driver, vm); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); goto cleanup; } rc = qemuMonitorMigrateToFd(priv->mon, @@ -5414,14 +5419,8 @@ qemuMigrationToFile(virQEMUDriverPtr driver, virDomainObjPtr vm, args, path, offset); } } - qemuDomainObjExitMonitor(driver, vm); - - if (!virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("guest unexpectedly quit")); + if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; - } - if (rc < 0) goto cleanup; @@ -5434,7 +5433,7 @@ qemuMigrationToFile(virQEMUDriverPtr driver, virDomainObjPtr vm, if (virDomainObjIsActive(vm) && qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) { qemuMonitorMigrateCancel(priv->mon); - qemuDomainObjExitMonitor(driver, vm); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); } } goto cleanup; @@ -5454,7 +5453,7 @@ qemuMigrationToFile(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) { qemuMonitorSetMigrationSpeed(priv->mon, saveMigBandwidth); priv->migMaxBandwidth = saveMigBandwidth; - qemuDomainObjExitMonitor(driver, vm); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); } VIR_FORCE_CLOSE(pipeFD[0]); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 300efc1..97c04f2 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -571,7 +571,7 @@ qemuProcessFakeReboot(void *opaque) virObjectEventPtr event = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virDomainRunningReason reason = VIR_DOMAIN_RUNNING_BOOTED; - int ret = -1; + int ret = -1, rc; VIR_DEBUG("vm=%p", vm); virObjectLock(vm); @@ -585,17 +585,13 @@ qemuProcessFakeReboot(void *opaque) } qemuDomainObjEnterMonitor(driver, vm); - if (qemuMonitorSystemReset(priv->mon) < 0) { - qemuDomainObjExitMonitor(driver, vm); + rc = qemuMonitorSystemReset(priv->mon); + + if (qemuDomainObjExitMonitor(driver, vm) < 0) goto endjob; - } - qemuDomainObjExitMonitor(driver, vm); - if (!virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("guest unexpectedly quit")); + if (rc < 0) goto endjob; - } if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_CRASHED) reason = VIR_DOMAIN_RUNNING_CRASHED; @@ -1662,7 +1658,8 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob, if (ret == 0 && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MONITOR_JSON)) ret = virQEMUCapsProbeQMP(priv->qemuCaps, priv->mon); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; error: @@ -2118,7 +2115,8 @@ qemuProcessReconnectRefreshChannelVirtioState(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorGetChardevInfo(priv->mon, &info); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; if (ret < 0) goto cleanup; @@ -2171,9 +2169,10 @@ qemuProcessWaitForMonitor(virQEMUDriverPtr driver, if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) goto cleanup; ret = qemuMonitorGetChardevInfo(priv->mon, &info); - qemuDomainObjExitMonitor(driver, vm); - VIR_DEBUG("qemuMonitorGetChardevInfo returned %i", ret); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; + if (ret == 0) { if ((ret = qemuProcessFindCharDevicePTYsMonitor(vm, qemuCaps, info)) < 0) @@ -2264,18 +2263,19 @@ qemuProcessDetectVcpuPIDs(virQEMUDriverPtr driver, if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return -1; + ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; /* failure to get the VCPU<-> PID mapping or to execute the query * command will not be treated fatal as some versions of qemu don't * support this command */ - if ((ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids)) <= 0) { - qemuDomainObjExitMonitor(driver, vm); + if (ncpupids <= 0) { virResetLastError(); priv->nvcpupids = 0; priv->vcpupids = NULL; return 0; } - qemuDomainObjExitMonitor(driver, vm); if (ncpupids != vm->def->vcpus) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -2310,7 +2310,8 @@ qemuProcessDetectIOThreadPIDs(virQEMUDriverPtr driver, if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) goto cleanup; niothreads = qemuMonitorGetIOThreads(priv->mon, &iothreads); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; if (niothreads < 0) goto cleanup; @@ -3023,11 +3024,13 @@ qemuProcessInitPCIAddresses(virQEMUDriverPtr driver, return -1; naddrs = qemuMonitorGetAllPCIAddresses(priv->mon, &addrs); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; if (naddrs > 0) ret = qemuProcessDetectPCIAddresses(vm, addrs, naddrs); + cleanup: VIR_FREE(addrs); return ret; @@ -3135,7 +3138,8 @@ qemuProcessUpdateVideoRamSize(virQEMUDriverPtr driver, } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; cfg = virQEMUDriverGetConfig(driver); ret = virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm); @@ -3144,7 +3148,7 @@ qemuProcessUpdateVideoRamSize(virQEMUDriverPtr driver, return ret; error: - qemuDomainObjExitMonitor(driver, vm); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); return -1; } @@ -3256,7 +3260,8 @@ qemuProcessStartCPUs(virQEMUDriverPtr driver, virDomainObjPtr vm, goto release; ret = qemuMonitorStartCPUs(priv->mon, conn); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; if (ret < 0) goto release; @@ -3289,7 +3294,8 @@ int qemuProcessStopCPUs(virQEMUDriverPtr driver, goto cleanup; ret = qemuMonitorStopCPUs(priv->mon); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; if (ret < 0) goto cleanup; @@ -3358,9 +3364,10 @@ qemuProcessUpdateState(virQEMUDriverPtr driver, virDomainObjPtr vm) qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorGetStatus(priv->mon, &running, &reason); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; - if (ret < 0 || !virDomainObjIsActive(vm)) + if (ret < 0) return -1; state = virDomainObjGetState(vm, NULL); @@ -3472,7 +3479,8 @@ qemuProcessRecoverMigration(virQEMUDriverPtr driver, vm->def->name); qemuDomainObjEnterMonitor(driver, vm); ignore_value(qemuMonitorMigrateCancel(priv->mon)); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; /* resume the domain but only if it was paused as a result of * migration */ if (state == VIR_DOMAIN_PAUSED && @@ -3541,7 +3549,8 @@ qemuProcessRecoverJob(virQEMUDriverPtr driver, case QEMU_ASYNC_JOB_SNAPSHOT: qemuDomainObjEnterMonitor(driver, vm); ignore_value(qemuMonitorMigrateCancel(priv->mon)); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; /* resume the domain but only if it was paused as a result of * running a migration-to-file operation. Although we are * recovering an async job, this function is run at startup @@ -4066,7 +4075,8 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver, if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return false; rc = qemuMonitorGetGuestCPU(priv->mon, arch, &guestcpu); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return false; if (rc < 0) { if (rc == -2) @@ -4853,12 +4863,10 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG("Setting network link states"); if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) goto cleanup; - if (qemuProcessSetLinkStates(vm) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuProcessSetLinkStates(vm) < 0) + goto exit_monitor; + if (qemuDomainObjExitMonitor(driver, vm)) goto cleanup; - } - - qemuDomainObjExitMonitor(driver, vm); VIR_DEBUG("Fetching list of active devices"); if (qemuDomainUpdateDeviceList(driver, vm, asyncJob) < 0) @@ -4882,10 +4890,8 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; if (period) qemuMonitorSetMemoryStatsPeriod(priv->mon, period); - if (qemuMonitorSetBalloon(priv->mon, cur_balloon) < 0) { - qemuDomainObjExitMonitor(driver, vm); - goto cleanup; - } + if (qemuMonitorSetBalloon(priv->mon, cur_balloon) < 0) + goto exit_monitor; if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; @@ -4962,6 +4968,10 @@ int qemuProcessStart(virConnectPtr conn, virObjectUnref(caps); return -1; + + exit_monitor: + ignore_value(qemuDomainObjExitMonitor(driver, vm)); + goto cleanup; } @@ -5468,21 +5478,13 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, VIR_DEBUG("Getting initial memory amount"); qemuDomainObjEnterMonitor(driver, vm); - if (qemuMonitorGetBalloonInfo(priv->mon, &vm->def->mem.cur_balloon) < 0) { - qemuDomainObjExitMonitor(driver, vm); - goto error; - } - if (qemuMonitorGetStatus(priv->mon, &running, &reason) < 0) { - qemuDomainObjExitMonitor(driver, vm); - goto error; - } - if (qemuMonitorGetVirtType(priv->mon, &vm->def->virtType) < 0) { - qemuDomainObjExitMonitor(driver, vm); - goto error; - } - qemuDomainObjExitMonitor(driver, vm); - - if (!virDomainObjIsActive(vm)) + if (qemuMonitorGetBalloonInfo(priv->mon, &vm->def->mem.cur_balloon) < 0) + goto exit_monitor; + if (qemuMonitorGetStatus(priv->mon, &running, &reason) < 0) + goto exit_monitor; + if (qemuMonitorGetVirtType(priv->mon, &vm->def->virtType) < 0) + goto exit_monitor; + if (qemuDomainObjExitMonitor(driver, vm) < 0) goto error; if (running) { @@ -5492,7 +5494,8 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, qemuDomainObjEnterMonitor(driver, vm); qemuMonitorSetMemoryStatsPeriod(priv->mon, vm->def->memballoon->period); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto error; } } else { virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, reason); @@ -5527,6 +5530,8 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, return 0; + exit_monitor: + ignore_value(qemuDomainObjExitMonitor(driver, vm)); error: /* We jump here if we failed to attach to the VM for any reason. * Leave the domain running, but pretend we never attempted to -- 2.0.4
On 14.01.2015 19:48, Ján Tomko wrote:
The patches 1-2, 6-8 from v2 have been pushed already
Device removal and device detach are now squashed together. Both attach and detach skip audit when the domain crashed. (Left to be cleaned up by a separate series)
Patches checking the return value in all other non-fatal cases were squashed together with the patch adding ATTRIBUTE_RETURN_CHECK. Rebased to master, added qemuProcessUpdateVideoRamSize, and fixed numerous wrong return values and a few missing EndJobs.
Ján Tomko (3): Fix vmdef usage after domain crash in monitor on device detach Fix vmdef usage after domain crash in monitor on device attach Always check return value of qemuDomainObjExitMonitor
src/qemu/qemu_domain.c | 11 +-- src/qemu/qemu_domain.h | 3 +- src/qemu/qemu_driver.c | 166 ++++++++++++++++++++++--------------- src/qemu/qemu_hotplug.c | 202 ++++++++++++++++++++++++++++++---------------- src/qemu/qemu_hotplug.h | 6 +- src/qemu/qemu_migration.c | 139 ++++++++++++++++--------------- src/qemu/qemu_process.c | 115 +++++++++++++------------- 7 files changed, 373 insertions(+), 269 deletions(-)
ACK series Michal
participants (2)
-
Ján Tomko -
Michal Privoznik