qemu may quited unexpectedly when invoking a monitor command. And priv->mon will be NULL after qemuDomainObjExitMonitor* returns. So we must not use it. Unfortunately we still use it, and it will cause libvirtd crashed. As Eric suggested, qemuDomainObjExitMonitor* should be made to return the value of vm active after regaining lock, and marked ATTRIBUTE_RETURN_CHECK, to force all other callers to detect the case of a monitor command completing successfully but then the VM disappearing in the window between command completion and regaining the vm lock. --- src/qemu/qemu_domain.c | 28 +++++++- src/qemu/qemu_domain.h | 6 +- src/qemu/qemu_driver.c | 51 ++++++++------ src/qemu/qemu_hotplug.c | 177 ++++++++++++++++++++++----------------------- src/qemu/qemu_migration.c | 73 +++++++++++-------- src/qemu/qemu_process.c | 38 ++++++---- 6 files changed, 212 insertions(+), 161 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a947b4e..44ad6a3 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -580,7 +580,7 @@ void qemuDomainObjEnterMonitor(virDomainObjPtr obj) * * Should be paired with an earlier qemuDomainObjEnterMonitor() call */ -void qemuDomainObjExitMonitor(virDomainObjPtr obj) +int qemuDomainObjExitMonitor(virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; int refs; @@ -590,11 +590,22 @@ void qemuDomainObjExitMonitor(virDomainObjPtr obj) if (refs > 0) qemuMonitorUnlock(priv->mon); + /* Note: qemu may quited unexpectedly here, and the monitor will be freed. + * If it happened, priv->mon will be null. + */ + virDomainObjLock(obj); if (refs == 0) { priv->mon = NULL; } + + if (priv->mon == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + return -1; + } else + return 0; } @@ -623,8 +634,8 @@ void qemuDomainObjEnterMonitorWithDriver(struct qemud_driver *driver, * * Should be paired with an earlier qemuDomainObjEnterMonitorWithDriver() call */ -void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, - virDomainObjPtr obj) +int qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, + virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; int refs; @@ -634,12 +645,23 @@ void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, if (refs > 0) qemuMonitorUnlock(priv->mon); + /* Note: qemu may quited unexpectedly here, and the monitor will be freed. + * If it happened, priv->mon will be null. + */ + qemuDriverLock(driver); virDomainObjLock(obj); if (refs == 0) { priv->mon = NULL; } + + if (priv->mon == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + return -1; + } else + return 0; } void qemuDomainObjEnterRemoteWithDriver(struct qemud_driver *driver, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 8258900..92fccda 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -99,11 +99,11 @@ int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver, virDomainObjPtr obj) ATTRIBUTE_RETURN_CHECK; int qemuDomainObjEndJob(virDomainObjPtr obj) ATTRIBUTE_RETURN_CHECK; void qemuDomainObjEnterMonitor(virDomainObjPtr obj); -void qemuDomainObjExitMonitor(virDomainObjPtr obj); +int qemuDomainObjExitMonitor(virDomainObjPtr obj) ATTRIBUTE_RETURN_CHECK; void qemuDomainObjEnterMonitorWithDriver(struct qemud_driver *driver, virDomainObjPtr obj); -void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, - virDomainObjPtr obj); +int qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, + virDomainObjPtr obj) ATTRIBUTE_RETURN_CHECK; void qemuDomainObjEnterRemoteWithDriver(struct qemud_driver *driver, virDomainObjPtr obj); void qemuDomainObjExitRemoteWithDriver(struct qemud_driver *driver, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 04a5f65..cb9f3fa 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1452,7 +1452,8 @@ static int qemudDomainShutdown(virDomainPtr dom) { priv = vm->privateData; qemuDomainObjEnterMonitor(vm); ret = qemuMonitorSystemPowerdown(priv->mon); - qemuDomainObjExitMonitor(vm); + if (qemuDomainObjExitMonitor(vm) < 0) + ret = -1; endjob: if (qemuDomainObjEndJob(vm) == 0) @@ -1659,7 +1660,8 @@ static int qemudDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, priv = vm->privateData; qemuDomainObjEnterMonitor(vm); r = qemuMonitorSetBalloon(priv->mon, newmem); - qemuDomainObjExitMonitor(vm); + if (qemuDomainObjExitMonitor(vm) < 0) + r = -1; qemuAuditMemory(vm, vm->def->mem.cur_balloon, newmem, "update", r == 1); if (r < 0) @@ -1749,7 +1751,8 @@ static int qemudDomainGetInfo(virDomainPtr dom, else { qemuDomainObjEnterMonitor(vm); err = qemuMonitorGetBalloonInfo(priv->mon, &balloon); - qemuDomainObjExitMonitor(vm); + if (qemuDomainObjExitMonitor(vm) < 0) + err = -1; } if (qemuDomainObjEndJob(vm) == 0) { vm = NULL; @@ -2524,7 +2527,8 @@ static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus) ret = 0; cleanup: - qemuDomainObjExitMonitor(vm); + if (qemuDomainObjExitMonitor(vm) < 0) + ret = -1; vm->def->vcpus = vcpus; qemuAuditVcpu(vm, oldvcpus, nvcpus, "update", rc == 1); return ret; @@ -3295,7 +3299,8 @@ static char *qemudDomainDumpXML(virDomainPtr dom, qemuDomainObjEnterMonitorWithDriver(driver, vm); err = qemuMonitorGetBalloonInfo(priv->mon, &balloon); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + err = -1; if (qemuDomainObjEndJob(vm) == 0) { vm = NULL; goto cleanup; @@ -4843,7 +4848,8 @@ qemudDomainBlockStats (virDomainPtr dom, &stats->wr_req, &stats->wr_bytes, &stats->errs); - qemuDomainObjExitMonitor(vm); + if (qemuDomainObjExitMonitor(vm) < 0) + ret = -1; endjob: if (qemuDomainObjEndJob(vm) == 0) @@ -4944,7 +4950,8 @@ qemudDomainMemoryStats (virDomainPtr dom, qemuDomainObjPrivatePtr priv = vm->privateData; qemuDomainObjEnterMonitor(vm); ret = qemuMonitorGetMemoryStats(priv->mon, stats, nr_stats); - qemuDomainObjExitMonitor(vm); + if (qemuDomainObjExitMonitor(vm) < 0) + ret = -1; } else { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -5085,17 +5092,15 @@ qemudDomainMemoryPeek (virDomainPtr dom, priv = vm->privateData; qemuDomainObjEnterMonitor(vm); if (flags == VIR_MEMORY_VIRTUAL) { - if (qemuMonitorSaveVirtualMemory(priv->mon, offset, size, tmp) < 0) { - qemuDomainObjExitMonitor(vm); - goto endjob; - } + ret = qemuMonitorSaveVirtualMemory(priv->mon, offset, size, tmp); } else { - if (qemuMonitorSavePhysicalMemory(priv->mon, offset, size, tmp) < 0) { - qemuDomainObjExitMonitor(vm); - goto endjob; - } + ret = qemuMonitorSavePhysicalMemory(priv->mon, offset, size, tmp); } - qemuDomainObjExitMonitor(vm); + if (qemuDomainObjExitMonitor(vm) < 0) + ret = -1; + + if (ret < -1) + goto endjob; /* Read the memory file into buffer. */ if (saferead (fd, buffer, size) == (ssize_t) -1) { @@ -5259,7 +5264,8 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, ret = qemuMonitorGetBlockExtent(priv->mon, disk->info.alias, &info->allocation); - qemuDomainObjExitMonitor(vm); + if (qemuDomainObjExitMonitor(vm) < 0) + ret = -1; } if (qemuDomainObjEndJob(vm) == 0) @@ -6101,7 +6107,8 @@ qemuDomainSnapshotCreateActive(virConnectPtr conn, qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorCreateSnapshot(priv->mon, snap->def->name); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; cleanup: if (resume && virDomainObjIsActive(vm) && @@ -6434,7 +6441,8 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, priv = vm->privateData; qemuDomainObjEnterMonitorWithDriver(driver, vm); rc = qemuMonitorLoadSnapshot(priv->mon, snap->def->name); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + rc = -1; if (rc < 0) goto endjob; } @@ -6557,7 +6565,7 @@ static int qemuDomainSnapshotDiscard(struct qemud_driver *driver, qemuDomainObjEnterMonitorWithDriver(driver, vm); /* we continue on even in the face of error */ qemuMonitorDeleteSnapshot(priv->mon, snap->def->name); - qemuDomainObjExitMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjExitMonitorWithDriver(driver, vm)); } if (snap == vm->current_snapshot) { @@ -6770,7 +6778,8 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd, goto cleanup; qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorArbitraryCommand(priv->mon, cmd, result, hmp); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; if (qemuDomainObjEndJob(vm) == 0) { vm = NULL; goto cleanup; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index b03f774..1ecac80 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -106,7 +106,8 @@ int qemuDomainChangeEjectableMedia(struct qemud_driver *driver, } else { ret = qemuMonitorEjectMedia(priv->mon, driveAlias, force); } - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; qemuAuditDisk(vm, origdisk, disk, "update", ret >= 0); @@ -201,7 +202,8 @@ int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver, memcpy(&disk->info.addr.pci, &guestAddr, sizeof(guestAddr)); } } - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; qemuAuditDisk(vm, NULL, disk, "attach", ret >= 0); @@ -277,7 +279,8 @@ int qemuDomainAttachPciControllerDevice(struct qemud_driver *driver, type, &controller->info.addr.pci); } - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; if (ret == 0) { controller->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; @@ -433,7 +436,8 @@ int qemuDomainAttachSCSIDisk(struct qemud_driver *driver, memcpy(&disk->info.addr.drive, &driveAddr, sizeof(driveAddr)); } } - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; qemuAuditDisk(vm, NULL, disk, "attach", ret >= 0); @@ -516,7 +520,8 @@ int qemuDomainAttachUsbMassstorageDevice(struct qemud_driver *driver, } else { ret = qemuMonitorAddUSBDisk(priv->mon, disk->src); } - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; qemuAuditDisk(vm, NULL, disk, "attach", ret >= 0); @@ -632,21 +637,19 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuCapsGet(qemuCaps, QEMU_CAPS_NETDEV) && qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - if (qemuMonitorAddNetdev(priv->mon, netstr, tapfd, tapfd_name, - vhostfd, vhostfd_name) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); - qemuAuditNet(vm, NULL, net, "attach", false); - goto cleanup; - } + ret = qemuMonitorAddNetdev(priv->mon, netstr, tapfd, tapfd_name, + vhostfd, vhostfd_name); } else { - if (qemuMonitorAddHostNetwork(priv->mon, netstr, tapfd, tapfd_name, - vhostfd, vhostfd_name) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); - qemuAuditNet(vm, NULL, net, "attach", false); - goto cleanup; - } + ret = qemuMonitorAddHostNetwork(priv->mon, netstr, tapfd, tapfd_name, + vhostfd, vhostfd_name); } - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; + + qemuAuditNet(vm, NULL, net, "attach", ret >= 0); + + if (ret < 0) + goto cleanup; VIR_FORCE_CLOSE(tapfd); VIR_FORCE_CLOSE(vhostfd); @@ -667,25 +670,23 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - if (qemuMonitorAddDevice(priv->mon, nicstr) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); - qemuAuditNet(vm, NULL, net, "attach", false); - goto try_remove; - } + ret = qemuMonitorAddDevice(priv->mon, nicstr); } else { - if (qemuMonitorAddPCINetwork(priv->mon, nicstr, - &guestAddr) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); - qemuAuditNet(vm, NULL, net, "attach", false); - goto try_remove; + ret = qemuMonitorAddPCINetwork(priv->mon, nicstr, + &guestAddr); + if (ret == 0) { + net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + memcpy(&net->info.addr.pci, &guestAddr, sizeof(guestAddr)); } - net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; - memcpy(&net->info.addr.pci, &guestAddr, sizeof(guestAddr)); } - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; qemuAuditNet(vm, NULL, net, "attach", true); + if (ret < 0) + goto try_remove; + ret = 0; vm->def->nets[vm->def->nnets++] = net; @@ -723,7 +724,7 @@ try_remove: if (qemuMonitorRemoveNetdev(priv->mon, netdev_name) < 0) VIR_WARN("Failed to remove network backend for netdev %s", netdev_name); - qemuDomainObjExitMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjExitMonitorWithDriver(driver, vm)); VIR_FREE(netdev_name); } else { VIR_WARN0("Unable to remove network backend"); @@ -736,7 +737,7 @@ try_remove: if (qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0) VIR_WARN("Failed to remove network backend for vlan %d, net %s", vlan, hostnet_name); - qemuDomainObjExitMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjExitMonitorWithDriver(driver, vm)); VIR_FREE(hostnet_name); } goto cleanup; @@ -795,7 +796,8 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver, qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorAddDeviceWithFd(priv->mon, devstr, configfd, configfd_name); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; } else { virDomainDevicePCIAddress guestAddr; @@ -803,7 +805,8 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver, ret = qemuMonitorAddPCIHostDevice(priv->mon, &hostdev->source.subsys.u.pci, &guestAddr); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; hostdev->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; memcpy(&hostdev->info.addr.pci, &guestAddr, sizeof(guestAddr)); @@ -886,7 +889,8 @@ int qemuDomainAttachHostUsbDevice(struct qemud_driver *driver, ret = qemuMonitorAddUSBDeviceExact(priv->mon, hostdev->source.subsys.u.usb.bus, hostdev->source.subsys.u.usb.device); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; qemuAuditHostdev(vm, hostdev, "attach", ret == 0); if (ret < 0) goto error; @@ -1145,25 +1149,24 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver, qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { - qemuDomainObjExitMonitor(vm); - goto cleanup; - } + ret = qemuMonitorDelDevice(priv->mon, detach->info.alias); } else { - if (qemuMonitorRemovePCIDevice(priv->mon, - &detach->info.addr.pci) < 0) { - qemuDomainObjExitMonitor(vm); - goto cleanup; - } + ret = qemuMonitorRemovePCIDevice(priv->mon, + &detach->info.addr.pci); } /* disconnect guest from host device */ - qemuMonitorDriveDel(priv->mon, drivestr); + if (ret == 0) + qemuMonitorDriveDel(priv->mon, drivestr); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitor(vm) < 0) + ret = -1; qemuAuditDisk(vm, detach, NULL, "detach", ret >= 0); + if (ret < 0) + goto cleanup; + if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE) && qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &detach->info) < 0) VIR_WARN("Unable to release PCI address on %s", dev->data.disk->src); @@ -1235,18 +1238,20 @@ int qemuDomainDetachDiskDevice(struct qemud_driver *driver, } qemuDomainObjEnterMonitorWithDriver(driver, vm); - if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { - qemuDomainObjExitMonitor(vm); - goto cleanup; - } + ret = qemuMonitorDelDevice(priv->mon, detach->info.alias); /* disconnect guest from host device */ - qemuMonitorDriveDel(priv->mon, drivestr); + if (ret == 0) + qemuMonitorDriveDel(priv->mon, drivestr); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; qemuAuditDisk(vm, detach, NULL, "detach", ret >= 0); + if (ret < 0) + goto cleanup; + virDomainDiskRemove(vm->def, i); virDomainDiskDefFree(detach); @@ -1364,18 +1369,16 @@ int qemuDomainDetachPciControllerDevice(struct qemud_driver *driver, qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - if (qemuMonitorDelDevice(priv->mon, detach->info.alias)) { - qemuDomainObjExitMonitor(vm); - goto cleanup; - } + ret = qemuMonitorDelDevice(priv->mon, detach->info.alias); } else { - if (qemuMonitorRemovePCIDevice(priv->mon, - &detach->info.addr.pci) < 0) { - qemuDomainObjExitMonitor(vm); - goto cleanup; - } + ret = qemuMonitorRemovePCIDevice(priv->mon, + &detach->info.addr.pci); } - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; + + if (ret < 0) + goto cleanup; if (vm->def->ncontrollers > 1) { memmove(vm->def->controllers + i, @@ -1452,37 +1455,30 @@ int qemuDomainDetachNetDevice(struct qemud_driver *driver, qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { - qemuDomainObjExitMonitor(vm); - qemuAuditNet(vm, detach, NULL, "detach", false); - goto cleanup; - } + ret = qemuMonitorDelDevice(priv->mon, detach->info.alias); } else { - if (qemuMonitorRemovePCIDevice(priv->mon, - &detach->info.addr.pci) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); - qemuAuditNet(vm, detach, NULL, "detach", false); - goto cleanup; - } + ret = qemuMonitorRemovePCIDevice(priv->mon, + &detach->info.addr.pci); } + if (ret < 0) + goto exitmonitor; + if (qemuCapsGet(qemuCaps, QEMU_CAPS_NETDEV) && qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - if (qemuMonitorRemoveNetdev(priv->mon, hostnet_name) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); - qemuAuditNet(vm, detach, NULL, "detach", false); - goto cleanup; - } + ret = qemuMonitorRemoveNetdev(priv->mon, hostnet_name); } else { - if (qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); - qemuAuditNet(vm, detach, NULL, "detach", false); - goto cleanup; - } + ret = qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name); } - qemuDomainObjExitMonitorWithDriver(driver, vm); - qemuAuditNet(vm, detach, NULL, "detach", true); +exitmonitor: + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; + + qemuAuditNet(vm, detach, NULL, "detach", ret >= 0); + + if (ret < 0) + goto cleanup; if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE) && qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &detach->info) < 0) @@ -1582,7 +1578,8 @@ int qemuDomainDetachHostPciDevice(struct qemud_driver *driver, } else { ret = qemuMonitorRemovePCIDevice(priv->mon, &detach->info.addr.pci); } - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; qemuAuditHostdev(vm, detach, "detach", ret == 0); if (ret < 0) return -1; @@ -1681,7 +1678,8 @@ int qemuDomainDetachHostUsbDevice(struct qemud_driver *driver, qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorDelDevice(priv->mon, detach->info.alias); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; qemuAuditHostdev(vm, detach, "detach", ret == 0); if (ret < 0) return -1; @@ -1798,7 +1796,8 @@ qemuDomainChangeGraphicsPasswords(struct qemud_driver *driver, } cleanup: - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; return ret; } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 43741e1..bdc05c0 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -125,7 +125,8 @@ qemuMigrationWaitForCompletion(struct qemud_driver *driver, virDomainObjPtr vm) VIR_DEBUG0("Cancelling job at client request"); qemuDomainObjEnterMonitorWithDriver(driver, vm); rc = qemuMonitorMigrateCancel(priv->mon); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + rc = -1; if (rc < 0) { VIR_WARN0("Unable to cancel job"); } @@ -142,7 +143,8 @@ qemuMigrationWaitForCompletion(struct qemud_driver *driver, virDomainObjPtr vm) VIR_DEBUG("Setting migration downtime to %llums", ms); qemuDomainObjEnterMonitorWithDriver(driver, vm); rc = qemuMonitorSetMigrationDowntime(priv->mon, ms); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + rc = -1; if (rc < 0) VIR_WARN0("Unable to set migration downtime"); } else if (priv->jobSignals & QEMU_JOB_SIGNAL_MIGRATE_SPEED) { @@ -153,7 +155,8 @@ qemuMigrationWaitForCompletion(struct qemud_driver *driver, virDomainObjPtr vm) VIR_DEBUG("Setting migration bandwidth to %luMbs", bandwidth); qemuDomainObjEnterMonitorWithDriver(driver, vm); rc = qemuMonitorSetMigrationSpeed(priv->mon, bandwidth); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + rc = -1; if (rc < 0) VIR_WARN0("Unable to set migration speed"); } @@ -173,7 +176,8 @@ qemuMigrationWaitForCompletion(struct qemud_driver *driver, virDomainObjPtr vm) &memProcessed, &memRemaining, &memTotal); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + rc = -1; if (rc < 0) { priv->jobInfo.type = VIR_DOMAIN_JOB_FAILED; @@ -608,11 +612,13 @@ static int doNativeMigrate(struct qemud_driver *driver, } qemuDomainObjEnterMonitorWithDriver(driver, vm); - if (resource > 0 && - qemuMonitorSetMigrationSpeed(priv->mon, resource) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); - goto cleanup; - } + if (resource > 0) + ret = qemuMonitorSetMigrationSpeed(priv->mon, resource); + else + ret = 0; + + if (ret < 0) + goto exitmonitor; if (flags & VIR_MIGRATE_NON_SHARED_DISK) background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_DISK; @@ -620,12 +626,15 @@ static int doNativeMigrate(struct qemud_driver *driver, if (flags & VIR_MIGRATE_NON_SHARED_INC) background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_INC; - if (qemuMonitorMigrateToHost(priv->mon, background_flags, uribits->server, - uribits->port) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); + ret = qemuMonitorMigrateToHost(priv->mon, background_flags, uribits->server, + uribits->port); + +exitmonitor: + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; + + if (ret < 0) goto cleanup; - } - qemuDomainObjExitMonitorWithDriver(driver, vm); if (qemuMigrationWaitForCompletion(driver, vm) < 0) goto cleanup; @@ -824,7 +833,8 @@ static int doTunnelMigrate(struct qemud_driver *driver, } else { internalret = -1; } - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + internalret = -1; if (internalret < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("tunnelled migration monitor command failed")); @@ -844,17 +854,15 @@ static int doTunnelMigrate(struct qemud_driver *driver, * rather failed later on. Check the output of "info migrate" */ qemuDomainObjEnterMonitorWithDriver(driver, vm); - if (qemuMonitorGetMigrationStatus(priv->mon, - &status, - &transferred, - &remaining, - &total) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); - goto cancel; - } - qemuDomainObjExitMonitorWithDriver(driver, vm); + retval = qemuMonitorGetMigrationStatus(priv->mon, + &status, + &transferred, + &remaining, + &total); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + retval = -1; - if (status == QEMU_MONITOR_MIGRATION_STATUS_ERROR) { + if (retval < 0 || status == QEMU_MONITOR_MIGRATION_STATUS_ERROR) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",_("migrate failed")); goto cancel; @@ -875,7 +883,7 @@ cancel: if (retval != 0 && virDomainObjIsActive(vm)) { qemuDomainObjEnterMonitorWithDriver(driver, vm); qemuMonitorMigrateCancel(priv->mon); - qemuDomainObjExitMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjExitMonitorWithDriver(driver, vm)); } finish: @@ -1372,12 +1380,12 @@ qemuMigrationToFile(struct qemud_driver *driver, virDomainObjPtr vm, if (virSetCloseExec(pipeFD[1]) < 0) { virReportSystemError(errno, "%s", _("Unable to set cloexec flag")); - qemuDomainObjExitMonitorWithDriver(driver, vm); - goto cleanup; + rc = -1; + goto exitmonitor; } if (virCommandRunAsync(cmd, NULL) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); - goto cleanup; + rc = -1; + goto exitmonitor; } rc = qemuMonitorMigrateToFd(priv->mon, QEMU_MONITOR_MIGRATE_BACKGROUND, @@ -1391,7 +1399,10 @@ qemuMigrationToFile(struct qemud_driver *driver, virDomainObjPtr vm, args, path, offset); } } - qemuDomainObjExitMonitorWithDriver(driver, vm); + +exitmonitor: + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + rc = -1; if (rc < 0) goto cleanup; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7295f9e..374de0f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -662,7 +662,8 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm) qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorSetCapabilities(priv->mon); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; error: @@ -1070,7 +1071,8 @@ qemuProcessWaitForMonitor(struct qemud_driver* driver, priv = vm->privateData; qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorGetPtyPaths(priv->mon, paths); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; VIR_DEBUG("qemuMonitorGetPtyPaths returned %i", ret); if (ret == 0) @@ -1122,11 +1124,12 @@ qemuProcessDetectVcpuPIDs(struct qemud_driver *driver, /* What follows is now all KVM specific */ qemuDomainObjEnterMonitorWithDriver(driver, vm); - if ((ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids)) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); + ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ncpupids = -1; + + if (ncpupids < 0) return -1; - } - qemuDomainObjExitMonitorWithDriver(driver, vm); /* Treat failure to get VCPU<->PID mapping as non-fatal */ if (ncpupids == 0) @@ -1322,7 +1325,8 @@ qemuProcessInitPasswords(virConnectPtr conn, qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorSetDrivePassphrase(priv->mon, alias, secret); VIR_FREE(secret); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; if (ret < 0) goto cleanup; } @@ -1713,7 +1717,10 @@ qemuProcessInitPCIAddresses(struct qemud_driver *driver, qemuDomainObjEnterMonitorWithDriver(driver, vm); naddrs = qemuMonitorGetAllPCIAddresses(priv->mon, &addrs); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + VIR_FREE(addrs); + return -1; + } ret = qemuProcessDetectPCIAddresses(vm, addrs, naddrs); @@ -1890,7 +1897,8 @@ qemuProcessStartCPUs(struct qemud_driver *driver, virDomainObjPtr vm, qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorStartCPUs(priv->mon, conn); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; if (ret == 0) { vm->state = VIR_DOMAIN_RUNNING; } @@ -1908,7 +1916,8 @@ int qemuProcessStopCPUs(struct qemud_driver *driver, virDomainObjPtr vm) vm->state = VIR_DOMAIN_PAUSED; qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorStopCPUs(priv->mon); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; if (ret < 0) { vm->state = oldState; } @@ -2389,11 +2398,12 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG0("Setting initial memory amount"); cur_balloon = vm->def->mem.cur_balloon; qemuDomainObjEnterMonitorWithDriver(driver, vm); - if (qemuMonitorSetBalloon(priv->mon, cur_balloon) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); + ret = qemuMonitorSetBalloon(priv->mon, cur_balloon); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + ret = -1; + + if (ret < 0) goto cleanup; - } - qemuDomainObjExitMonitorWithDriver(driver, vm); if (!start_paused) { VIR_DEBUG0("Starting domain CPUs"); -- 1.7.1