[libvirt] [RFC PATCH] check whether domain is active after qemuDomainObjExitMonitor* returns

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 | 24 ++++- src/qemu/qemu_domain.h | 6 +- src/qemu/qemu_driver.c | 87 +++++++++++++---- src/qemu/qemu_hotplug.c | 230 ++++++++++++++++++++++++++++----------------- src/qemu/qemu_migration.c | 93 +++++++++++++------ src/qemu/qemu_process.c | 61 +++++++++--- 6 files changed, 344 insertions(+), 157 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c2a1f9a..0d40b7e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -578,7 +578,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; @@ -588,11 +588,20 @@ 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) + return -1; + else + return 0; } @@ -621,8 +630,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; @@ -632,12 +641,21 @@ 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) + 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..2d41576 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1452,7 +1452,11 @@ static int qemudDomainShutdown(virDomainPtr dom) { priv = vm->privateData; qemuDomainObjEnterMonitor(vm); ret = qemuMonitorSystemPowerdown(priv->mon); - qemuDomainObjExitMonitor(vm); + if (qemuDomainObjExitMonitor(vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } endjob: if (qemuDomainObjEndJob(vm) == 0) @@ -1659,7 +1663,11 @@ static int qemudDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, priv = vm->privateData; qemuDomainObjEnterMonitor(vm); r = qemuMonitorSetBalloon(priv->mon, newmem); - qemuDomainObjExitMonitor(vm); + if (qemuDomainObjExitMonitor(vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + r = -1; + } qemuAuditMemory(vm, vm->def->mem.cur_balloon, newmem, "update", r == 1); if (r < 0) @@ -1749,7 +1757,11 @@ static int qemudDomainGetInfo(virDomainPtr dom, else { qemuDomainObjEnterMonitor(vm); err = qemuMonitorGetBalloonInfo(priv->mon, &balloon); - qemuDomainObjExitMonitor(vm); + if (qemuDomainObjExitMonitor(vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + err = -1; + } } if (qemuDomainObjEndJob(vm) == 0) { vm = NULL; @@ -2524,7 +2536,11 @@ static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus) ret = 0; cleanup: - qemuDomainObjExitMonitor(vm); + if (qemuDomainObjExitMonitor(vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } vm->def->vcpus = vcpus; qemuAuditVcpu(vm, oldvcpus, nvcpus, "update", rc == 1); return ret; @@ -3295,7 +3311,11 @@ static char *qemudDomainDumpXML(virDomainPtr dom, qemuDomainObjEnterMonitorWithDriver(driver, vm); err = qemuMonitorGetBalloonInfo(priv->mon, &balloon); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + err = -1; + } if (qemuDomainObjEndJob(vm) == 0) { vm = NULL; goto cleanup; @@ -4843,7 +4863,11 @@ qemudDomainBlockStats (virDomainPtr dom, &stats->wr_req, &stats->wr_bytes, &stats->errs); - qemuDomainObjExitMonitor(vm); + if (qemuDomainObjExitMonitor(vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } endjob: if (qemuDomainObjEndJob(vm) == 0) @@ -4944,7 +4968,11 @@ qemudDomainMemoryStats (virDomainPtr dom, qemuDomainObjPrivatePtr priv = vm->privateData; qemuDomainObjEnterMonitor(vm); ret = qemuMonitorGetMemoryStats(priv->mon, stats, nr_stats); - qemuDomainObjExitMonitor(vm); + if (qemuDomainObjExitMonitor(vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } } else { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -5085,17 +5113,18 @@ 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) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } + + if (ret < -1) + goto endjob; /* Read the memory file into buffer. */ if (saferead (fd, buffer, size) == (ssize_t) -1) { @@ -5259,7 +5288,11 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, ret = qemuMonitorGetBlockExtent(priv->mon, disk->info.alias, &info->allocation); - qemuDomainObjExitMonitor(vm); + if (qemuDomainObjExitMonitor(vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } } if (qemuDomainObjEndJob(vm) == 0) @@ -6101,7 +6134,11 @@ qemuDomainSnapshotCreateActive(virConnectPtr conn, qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorCreateSnapshot(priv->mon, snap->def->name); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } cleanup: if (resume && virDomainObjIsActive(vm) && @@ -6434,7 +6471,11 @@ 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) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + rc = -1; + } if (rc < 0) goto endjob; } @@ -6557,7 +6598,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 +6811,11 @@ 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) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + 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..d70e426 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -106,7 +106,11 @@ int qemuDomainChangeEjectableMedia(struct qemud_driver *driver, } else { ret = qemuMonitorEjectMedia(priv->mon, driveAlias, force); } - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } qemuAuditDisk(vm, origdisk, disk, "update", ret >= 0); @@ -201,7 +205,11 @@ int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver, memcpy(&disk->info.addr.pci, &guestAddr, sizeof(guestAddr)); } } - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } qemuAuditDisk(vm, NULL, disk, "attach", ret >= 0); @@ -277,7 +285,11 @@ int qemuDomainAttachPciControllerDevice(struct qemud_driver *driver, type, &controller->info.addr.pci); } - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } if (ret == 0) { controller->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; @@ -433,7 +445,11 @@ int qemuDomainAttachSCSIDisk(struct qemud_driver *driver, memcpy(&disk->info.addr.drive, &driveAddr, sizeof(driveAddr)); } } - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } qemuAuditDisk(vm, NULL, disk, "attach", ret >= 0); @@ -516,7 +532,11 @@ int qemuDomainAttachUsbMassstorageDevice(struct qemud_driver *driver, } else { ret = qemuMonitorAddUSBDisk(priv->mon, disk->src); } - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } qemuAuditDisk(vm, NULL, disk, "attach", ret >= 0); @@ -632,21 +652,22 @@ 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) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + goto cleanup; + } + + qemuAuditNet(vm, NULL, net, "attach", ret >= 0); + + if (ret < 0) + goto cleanup; VIR_FORCE_CLOSE(tapfd); VIR_FORCE_CLOSE(vhostfd); @@ -667,25 +688,26 @@ 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) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + 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 +745,9 @@ try_remove: if (qemuMonitorRemoveNetdev(priv->mon, netdev_name) < 0) VIR_WARN("Failed to remove network backend for netdev %s", netdev_name); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); VIR_FREE(netdev_name); } else { VIR_WARN0("Unable to remove network backend"); @@ -736,7 +760,9 @@ 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); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); VIR_FREE(hostnet_name); } goto cleanup; @@ -795,7 +821,11 @@ 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) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + goto error; + } } else { virDomainDevicePCIAddress guestAddr; @@ -803,7 +833,11 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver, ret = qemuMonitorAddPCIHostDevice(priv->mon, &hostdev->source.subsys.u.pci, &guestAddr); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + goto error; + } hostdev->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; memcpy(&hostdev->info.addr.pci, &guestAddr, sizeof(guestAddr)); @@ -886,7 +920,11 @@ 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) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } qemuAuditHostdev(vm, hostdev, "attach", ret == 0); if (ret < 0) goto error; @@ -1145,25 +1183,27 @@ 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) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + 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 +1275,23 @@ 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) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } qemuAuditDisk(vm, detach, NULL, "detach", ret >= 0); + if (ret < 0) + goto cleanup; + virDomainDiskRemove(vm->def, i); virDomainDiskDefFree(detach); @@ -1364,18 +1409,19 @@ 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); + } + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; } - qemuDomainObjExitMonitorWithDriver(driver, vm); + + if (ret < 0) + goto cleanup; if (vm->def->ncontrollers > 1) { memmove(vm->def->controllers + i, @@ -1452,35 +1498,31 @@ 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); + +exitmonitor: + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } + + if (ret < 0) + goto cleanup; qemuAuditNet(vm, detach, NULL, "detach", true); @@ -1582,7 +1624,11 @@ int qemuDomainDetachHostPciDevice(struct qemud_driver *driver, } else { ret = qemuMonitorRemovePCIDevice(priv->mon, &detach->info.addr.pci); } - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } qemuAuditHostdev(vm, detach, "detach", ret == 0); if (ret < 0) return -1; @@ -1681,7 +1727,11 @@ int qemuDomainDetachHostUsbDevice(struct qemud_driver *driver, qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorDelDevice(priv->mon, detach->info.alias); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } qemuAuditHostdev(vm, detach, "detach", ret == 0); if (ret < 0) return -1; @@ -1798,7 +1848,11 @@ qemuDomainChangeGraphicsPasswords(struct qemud_driver *driver, } cleanup: - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } return ret; } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 43741e1..cc8f428 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -125,7 +125,11 @@ 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) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + rc = -1; + } if (rc < 0) { VIR_WARN0("Unable to cancel job"); } @@ -142,7 +146,11 @@ 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) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + rc = -1; + } if (rc < 0) VIR_WARN0("Unable to set migration downtime"); } else if (priv->jobSignals & QEMU_JOB_SIGNAL_MIGRATE_SPEED) { @@ -153,7 +161,11 @@ 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) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + rc = -1; + } if (rc < 0) VIR_WARN0("Unable to set migration speed"); } @@ -173,7 +185,11 @@ qemuMigrationWaitForCompletion(struct qemud_driver *driver, virDomainObjPtr vm) &memProcessed, &memRemaining, &memTotal); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + rc = -1; + } if (rc < 0) { priv->jobInfo.type = VIR_DOMAIN_JOB_FAILED; @@ -608,24 +624,31 @@ 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); } + if (ret < 0) + goto exitmonitor; + if (flags & VIR_MIGRATE_NON_SHARED_DISK) background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_DISK; 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); - goto cleanup; + ret = qemuMonitorMigrateToHost(priv->mon, background_flags, uribits->server, + uribits->port); + +exitmonitor: + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; } - qemuDomainObjExitMonitorWithDriver(driver, vm); + + if (ret < 0) + goto cleanup; if (qemuMigrationWaitForCompletion(driver, vm) < 0) goto cleanup; @@ -824,7 +847,11 @@ static int doTunnelMigrate(struct qemud_driver *driver, } else { internalret = -1; } - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + internalret = -1; + } if (internalret < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("tunnelled migration monitor command failed")); @@ -844,15 +871,16 @@ 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; + retval = qemuMonitorGetMigrationStatus(priv->mon, + &status, + &transferred, + &remaining, + &total); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + retval = -1; } - qemuDomainObjExitMonitorWithDriver(driver, vm); if (status == QEMU_MONITOR_MIGRATION_STATUS_ERROR) { qemuReportError(VIR_ERR_OPERATION_FAILED, @@ -875,7 +903,10 @@ cancel: if (retval != 0 && virDomainObjIsActive(vm)) { qemuDomainObjEnterMonitorWithDriver(driver, vm); qemuMonitorMigrateCancel(priv->mon); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + } } finish: @@ -1372,12 +1403,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 +1422,13 @@ qemuMigrationToFile(struct qemud_driver *driver, virDomainObjPtr vm, args, path, offset); } } - qemuDomainObjExitMonitorWithDriver(driver, vm); + +exitmonitor: + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + rc = -1; + } if (rc < 0) goto cleanup; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 9ada24d..4ca70bc 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -662,7 +662,11 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm) qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorSetCapabilities(priv->mon); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } error: @@ -1070,7 +1074,11 @@ qemuProcessWaitForMonitor(struct qemud_driver* driver, priv = vm->privateData; qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorGetPtyPaths(priv->mon, paths); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } VIR_DEBUG("qemuMonitorGetPtyPaths returned %i", ret); if (ret == 0) @@ -1122,11 +1130,15 @@ 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); - return -1; + ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ncpupids = -1; } - qemuDomainObjExitMonitorWithDriver(driver, vm); + + if (ncpupids < 0) + return -1; /* Treat failure to get VCPU<->PID mapping as non-fatal */ if (ncpupids == 0) @@ -1322,7 +1334,11 @@ qemuProcessInitPasswords(virConnectPtr conn, qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorSetDrivePassphrase(priv->mon, alias, secret); VIR_FREE(secret); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } if (ret < 0) goto cleanup; } @@ -1713,7 +1729,12 @@ qemuProcessInitPCIAddresses(struct qemud_driver *driver, qemuDomainObjEnterMonitorWithDriver(driver, vm); naddrs = qemuMonitorGetAllPCIAddresses(priv->mon, &addrs); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + VIR_FREE(addrs); + return -1; + } ret = qemuProcessDetectPCIAddresses(vm, addrs, naddrs); @@ -1890,7 +1911,11 @@ qemuProcessStartCPUs(struct qemud_driver *driver, virDomainObjPtr vm, qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorStartCPUs(priv->mon, conn); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } if (ret == 0) { vm->state = VIR_DOMAIN_RUNNING; } @@ -1908,7 +1933,11 @@ 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) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + } if (ret < 0) { vm->state = oldState; } @@ -2389,11 +2418,15 @@ 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); - goto cleanup; + ret = qemuMonitorSetBalloon(priv->mon, cur_balloon); + if (qemuDomainObjExitMonitorWithDriver(driver, vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; } - qemuDomainObjExitMonitorWithDriver(driver, vm); + + if (ret < 0) + goto cleanup; if (!start_paused) { VIR_DEBUG0("Starting domain CPUs"); -- 1.7.1