[libvirt] [PATCH 00/11] Fix vm usage after ExitMonitor
Check if the domain died while we were in the monitor in most of the places (I didn't look at the Snapshot and Blockjob APIs yet). In some places, we've been accessing the data after it could've been freed by qemuProcessStop (if the monitor API itself returned success, but the domain was cleaned up because somehow qemuProcessStop got the vm lock before the ExitMonitor call). A more likely crash happens if qemu crashes in one of the APIs with multiple Monitor calls, like the DetachDevice APIs and AttachCharDevice. Also, some occurences of writing the status XML for a dead domain are fixed. https://bugzilla.redhat.com/show_bug.cgi?id=1161024 Ján Tomko (11): Introduce qemuDomainObjExitMonitorAlive Mark the domain as active in qemuhotplugtest Fix vm usage after ExitMonitor in UpdateDeviceList Fix vm usage after ExitMonitor on device removal Fix error message on redirdev caps detection Fix vm usage after ExitMonitor on AttachDevice Fix vm usage after ExitMonitor in DetachDevice Fix monitor usage in qemuDomainHotplugVcpus Fix vm usage after ExitMonitor in qemu driver Fix vm usage after ExitMonitor in qemu migration Fix vm usage after ExitMonitor in qemu process src/qemu/qemu_command.c | 6 +- src/qemu/qemu_domain.c | 20 +++++- src/qemu/qemu_domain.h | 3 + src/qemu/qemu_driver.c | 49 ++++++++++---- src/qemu/qemu_hotplug.c | 169 +++++++++++++++++++++++++++++++--------------- src/qemu/qemu_migration.c | 41 +++++++---- src/qemu/qemu_process.c | 65 ++++++++++++------ tests/qemuhotplugtest.c | 4 ++ 8 files changed, 251 insertions(+), 106 deletions(-) -- 2.0.4
The domain might disappear during the time in monitor when the virDomainObjPtr is unlocked, so the caller needs to check if it's still alive. Introduce a helper function for the callers that want to report an error in this case. --- src/qemu/qemu_domain.c | 17 +++++++++++++++++ src/qemu/qemu_domain.h | 3 +++ 2 files changed, 20 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8dd427a..d074429 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1606,6 +1606,23 @@ void qemuDomainObjExitMonitor(virQEMUDriverPtr driver, qemuDomainObjExitMonitorInternal(driver, obj); } +/* obj must NOT be locked before calling + * + * Same as qemuDomainObjExitMonitor, but reports an error and + * returns -1 if the domain is no longer alive after exiting the monitor + */ +int qemuDomainObjExitMonitorAlive(virQEMUDriverPtr driver, + virDomainObjPtr obj) +{ + qemuDomainObjExitMonitorInternal(driver, obj); + if (!virDomainObjIsActive(obj)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("domain is no longer running")); + return -1; + } + return 0; +} + /* * obj must be locked before calling * diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index ce5eba3..b8fec34 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -250,6 +250,9 @@ void qemuDomainObjEnterMonitor(virQEMUDriverPtr driver, void qemuDomainObjExitMonitor(virQEMUDriverPtr driver, virDomainObjPtr obj) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int qemuDomainObjExitMonitorAlive(virQEMUDriverPtr driver, + virDomainObjPtr obj) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int qemuDomainObjEnterMonitorAsync(virQEMUDriverPtr driver, virDomainObjPtr obj, qemuDomainAsyncJob asyncJob) -- 2.0.4
On 16.12.2014 17:41, Ján Tomko wrote:
The domain might disappear during the time in monitor when the virDomainObjPtr is unlocked, so the caller needs to check if it's still alive.
Introduce a helper function for the callers that want to report an error in this case. --- src/qemu/qemu_domain.c | 17 +++++++++++++++++ src/qemu/qemu_domain.h | 3 +++ 2 files changed, 20 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8dd427a..d074429 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1606,6 +1606,23 @@ void qemuDomainObjExitMonitor(virQEMUDriverPtr driver, qemuDomainObjExitMonitorInternal(driver, obj); }
+/* obj must NOT be locked before calling + * + * Same as qemuDomainObjExitMonitor, but reports an error and + * returns -1 if the domain is no longer alive after exiting the monitor
Can you state explicitly that this function needs to be called whenever the domain object or vm->def, or privdata (any runtime info held within vm in general) is used after ExitMonitor()?
+ */ +int qemuDomainObjExitMonitorAlive(virQEMUDriverPtr driver, + virDomainObjPtr obj) +{ + qemuDomainObjExitMonitorInternal(driver, obj); + if (!virDomainObjIsActive(obj)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("domain is no longer running")); + return -1; + } + return 0; +} + /* * obj must be locked before calling * diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index ce5eba3..b8fec34 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -250,6 +250,9 @@ void qemuDomainObjEnterMonitor(virQEMUDriverPtr driver, void qemuDomainObjExitMonitor(virQEMUDriverPtr driver, virDomainObjPtr obj) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int qemuDomainObjExitMonitorAlive(virQEMUDriverPtr driver, + virDomainObjPtr obj) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int qemuDomainObjEnterMonitorAsync(virQEMUDriverPtr driver, virDomainObjPtr obj, qemuDomainAsyncJob asyncJob)
ACK Michal
On Wed, Dec 17, 2014 at 01:18:41PM +0100, Michal Privoznik wrote:
On 16.12.2014 17:41, Ján Tomko wrote:
The domain might disappear during the time in monitor when the virDomainObjPtr is unlocked, so the caller needs to check if it's still alive.
Introduce a helper function for the callers that want to report an error in this case. --- src/qemu/qemu_domain.c | 17 +++++++++++++++++ src/qemu/qemu_domain.h | 3 +++ 2 files changed, 20 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8dd427a..d074429 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1606,6 +1606,23 @@ void qemuDomainObjExitMonitor(virQEMUDriverPtr driver, qemuDomainObjExitMonitorInternal(driver, obj); }
+/* obj must NOT be locked before calling + * + * Same as qemuDomainObjExitMonitor, but reports an error and + * returns -1 if the domain is no longer alive after exiting the monitor
Can you state explicitly that this function needs to be called whenever the domain object or vm->def, or privdata (any runtime info held within vm in general) is used after ExitMonitor()?
Are there any cases where that is not true ? eg could we just avoid introducing this new function, by making the existing funtion check liveliness every time. That would avoid the class of bugs you outline above entirely. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 12/17/2014 01:21 PM, Daniel P. Berrange wrote:
On Wed, Dec 17, 2014 at 01:18:41PM +0100, Michal Privoznik wrote:
On 16.12.2014 17:41, Ján Tomko wrote:
The domain might disappear during the time in monitor when the virDomainObjPtr is unlocked, so the caller needs to check if it's still alive.
Introduce a helper function for the callers that want to report an error in this case. --- src/qemu/qemu_domain.c | 17 +++++++++++++++++ src/qemu/qemu_domain.h | 3 +++ 2 files changed, 20 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8dd427a..d074429 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1606,6 +1606,23 @@ void qemuDomainObjExitMonitor(virQEMUDriverPtr driver, qemuDomainObjExitMonitorInternal(driver, obj); }
+/* obj must NOT be locked before calling + * + * Same as qemuDomainObjExitMonitor, but reports an error and + * returns -1 if the domain is no longer alive after exiting the monitor
Can you state explicitly that this function needs to be called whenever the domain object or vm->def, or privdata (any runtime info held within vm in general) is used after ExitMonitor()?
Right, I can add this to qemu/THREADS.txt as well.
Are there any cases where that is not true ? eg could we just avoid introducing this new function, by making the existing funtion check liveliness every time. That would avoid the class of bugs you outline above entirely.
I was worried about backports, but new code checking the return value would fail to compile with the void function in older libvirt. There are functions that only call EndJob and return after exiting the monitor. But if the domain crashed before the API even returned, can we really claim success? Jan
On Wed, Dec 17, 2014 at 01:58:28PM +0100, Ján Tomko wrote:
On 12/17/2014 01:21 PM, Daniel P. Berrange wrote:
On Wed, Dec 17, 2014 at 01:18:41PM +0100, Michal Privoznik wrote:
On 16.12.2014 17:41, Ján Tomko wrote:
The domain might disappear during the time in monitor when the virDomainObjPtr is unlocked, so the caller needs to check if it's still alive.
Introduce a helper function for the callers that want to report an error in this case. --- src/qemu/qemu_domain.c | 17 +++++++++++++++++ src/qemu/qemu_domain.h | 3 +++ 2 files changed, 20 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8dd427a..d074429 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1606,6 +1606,23 @@ void qemuDomainObjExitMonitor(virQEMUDriverPtr driver, qemuDomainObjExitMonitorInternal(driver, obj); }
+/* obj must NOT be locked before calling + * + * Same as qemuDomainObjExitMonitor, but reports an error and + * returns -1 if the domain is no longer alive after exiting the monitor
Can you state explicitly that this function needs to be called whenever the domain object or vm->def, or privdata (any runtime info held within vm in general) is used after ExitMonitor()?
Right, I can add this to qemu/THREADS.txt as well.
Are there any cases where that is not true ? eg could we just avoid introducing this new function, by making the existing funtion check liveliness every time. That would avoid the class of bugs you outline above entirely.
I was worried about backports, but new code checking the return value would fail to compile with the void function in older libvirt.
We should add ATTRIBUTE_RETURNCHECK to force all callers to check the return status too
There are functions that only call EndJob and return after exiting the monitor. But if the domain crashed before the API even returned, can we really claim success?
There are some APIs where we do if (running) do x else do y even with those though, I think we're justified in reporting an error if the domain stopped at any point in the 'do x' code path - after all we're already doing to be doing that if it happens while we're issuing a mointor API call. So this endjob check is only extending that window of opportunity for errors by a couple more instructions. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
This will allow us to call qemuDomainObjIsActive() in the tested functions to check if the domain has crashed. --- tests/qemuhotplugtest.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 9d39968..49e4272 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -90,6 +90,8 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, if (qemuAssignDeviceAliases((*vm)->def, priv->qemuCaps) < 0) goto cleanup; + (*vm)->def->id = 7; + ret = 0; cleanup: return ret; @@ -177,9 +179,11 @@ testQemuHotplugCheckResult(virDomainObjPtr vm, char *actual; int ret; + vm->def->id = -1; actual = virDomainDefFormat(vm->def, VIR_DOMAIN_XML_SECURE); if (!actual) return -1; + vm->def->id = 7; if (STREQ(expected, actual)) { if (fail && virTestGetVerbose()) -- 2.0.4
On 16.12.2014 17:41, Ján Tomko wrote:
This will allow us to call qemuDomainObjIsActive() in the tested functions to check if the domain has crashed. --- tests/qemuhotplugtest.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 9d39968..49e4272 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -90,6 +90,8 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, if (qemuAssignDeviceAliases((*vm)->def, priv->qemuCaps) < 0) goto cleanup;
+ (*vm)->def->id = 7; + ret = 0; cleanup: return ret; @@ -177,9 +179,11 @@ testQemuHotplugCheckResult(virDomainObjPtr vm, char *actual; int ret;
+ vm->def->id = -1; actual = virDomainDefFormat(vm->def, VIR_DOMAIN_XML_SECURE); if (!actual) return -1; + vm->def->id = 7;
if (STREQ(expected, actual)) { if (fail && virTestGetVerbose())
Can you turn the domain ID into a macro? ACK Michal
Error out if the domain has disappeared in the meantime. Explicitly check if the domain is alive in qemuDomain{Attach,Detach}Live, where the return value is ignored. This prevents writing the persistent definition as the domain status and calling qemuDomainRemoveDevice on devices that already have been dealt with in qemuProcessStop. --- src/qemu/qemu_domain.c | 3 ++- src/qemu/qemu_driver.c | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d074429..d9c22da 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2763,7 +2763,8 @@ qemuDomainUpdateDeviceList(virQEMUDriverPtr driver, qemuDomainObjExitMonitor(driver, vm); return -1; } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + return -1; virStringFreeList(priv->qemuDevices); priv->qemuDevices = aliases; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7211d42..99eb7c3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7058,6 +7058,8 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, if (ret == 0) qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE); + if (!virDomainObjIsActive(vm)) + return -1; return ret; } @@ -7134,6 +7136,8 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, if (ret == 0) qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE); + if (!virDomainObjIsActive(vm)) + return -1; return ret; } -- 2.0.4
On 16.12.2014 17:41, Ján Tomko wrote:
Error out if the domain has disappeared in the meantime. Explicitly check if the domain is alive in qemuDomain{Attach,Detach}Live, where the return value is ignored.
This prevents writing the persistent definition as the domain status and calling qemuDomainRemoveDevice on devices that already have been dealt with in qemuProcessStop. --- src/qemu/qemu_domain.c | 3 ++- src/qemu/qemu_driver.c | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d074429..d9c22da 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2763,7 +2763,8 @@ qemuDomainUpdateDeviceList(virQEMUDriverPtr driver, qemuDomainObjExitMonitor(driver, vm); return -1; } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + return -1;
So from now on, qemuDomainUpdateDeviceList() checks domain status after returning from monitor.
virStringFreeList(priv->qemuDevices); priv->qemuDevices = aliases; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7211d42..99eb7c3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7058,6 +7058,8 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
if (ret == 0) qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE); + if (!virDomainObjIsActive(vm)) + return -1;
Therefore this is not needed. Moreover, as seen in the context, the return value of qemuDomainUpdateDeviceList() is ignored. And don't even get me started on the qemuDomainAttachDeviceLive() layout.
return ret; } @@ -7134,6 +7136,8 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
if (ret == 0) qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE); + if (!virDomainObjIsActive(vm)) + return -1;
Ditto.
return ret; }
Michal
In the device type-specific functions, exit early if the domain has disappeared, because the cleanup should have been done by qemuProcessStop. In processDeviceDeletedEvent, only save status XML if the domain is still running and exit early in qemuProcessUpdateDevices if the domain disappeared. --- src/qemu/qemu_driver.c | 3 ++- src/qemu/qemu_hotplug.c | 18 ++++++++++++------ src/qemu/qemu_process.c | 2 ++ 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 99eb7c3..07b061e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4112,7 +4112,8 @@ processDeviceDeletedEvent(virQEMUDriverPtr driver, qemuDomainRemoveDevice(driver, vm, &dev); - if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) + if (virDomainObjIsActive(vm) && + virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) VIR_WARN("unable to save domain status after removing device %s", devAlias); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 5b65283..3fcbef3 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2507,8 +2507,9 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); qemuMonitorDriveDel(priv->mon, drivestr); - qemuDomainObjExitMonitor(driver, vm); VIR_FREE(drivestr); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + return -1; virDomainAuditDisk(vm, disk->src, NULL, "detach", true); @@ -2623,7 +2624,8 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); qemuMonitorDriveDel(priv->mon, drivestr); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; } event = virDomainEventDeviceRemovedNewFromObj(vm, hostdev->info->alias); @@ -2717,7 +2719,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 (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; virDomainAuditNet(vm, net, NULL, "detach", false); goto cleanup; } @@ -2729,12 +2732,14 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("unable to determine original VLAN")); } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; virDomainAuditNet(vm, net, NULL, "detach", false); goto cleanup; } } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; virDomainAuditNet(vm, net, NULL, "detach", true); @@ -2804,7 +2809,8 @@ qemuDomainRemoveChrDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); rc = qemuMonitorDetachCharDev(priv->mon, charAlias); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; virDomainAuditChardev(vm, chr, NULL, "detach", rc == 0); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d683918..d4af85c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3519,6 +3519,8 @@ qemuProcessUpdateDevices(virQEMUDriverPtr driver, if (!virStringArrayHasString(priv->qemuDevices, *tmp) && virDomainDefFindDevice(vm->def, *tmp, &dev, false) == 0) qemuDomainRemoveDevice(driver, vm, &dev); + if (!virDomainObjIsActive(vm)) + goto cleanup; tmp++; } } -- 2.0.4
On 16.12.2014 17:41, Ján Tomko wrote:
In the device type-specific functions, exit early if the domain has disappeared, because the cleanup should have been done by qemuProcessStop.
In processDeviceDeletedEvent, only save status XML if the domain is still running and exit early in qemuProcessUpdateDevices if the domain disappeared. --- src/qemu/qemu_driver.c | 3 ++- src/qemu/qemu_hotplug.c | 18 ++++++++++++------ src/qemu/qemu_process.c | 2 ++ 3 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 99eb7c3..07b061e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4112,7 +4112,8 @@ processDeviceDeletedEvent(virQEMUDriverPtr driver,
qemuDomainRemoveDevice(driver, vm, &dev);
- if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) + if (virDomainObjIsActive(vm) && + virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
Can we rather make qemuDomainRemoveDevice() return -1 on error and check for that in callers? That way we will fix even case when the function is called with unsupported device. ACK to the rest of the patch, though. Michal
--- src/qemu/qemu_command.c | 6 +++++- src/qemu/qemu_hotplug.c | 14 +++++++++----- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 53d6629..11e7cff 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9512,8 +9512,12 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, devstr); VIR_FREE(devstr); - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("redirected devices are not supported by this QEMU")); goto error; + } + virCommandAddArg(cmd, "-device"); if (!(devstr = qemuBuildRedirdevDevStr(def, redirdev, qemuCaps))) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 3fcbef3..38471e3 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1358,13 +1358,17 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver, virDomainDefPtr def = vm->def; char *devstr = NULL; - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { - if (qemuAssignDeviceRedirdevAlias(vm->def, redirdev, -1) < 0) - goto error; - if (!(devstr = qemuBuildRedirdevDevStr(def, redirdev, priv->qemuCaps))) - goto error; + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("redirected devices are not supported by this QEMU")); + goto error; } + if (qemuAssignDeviceRedirdevAlias(vm->def, redirdev, -1) < 0) + goto error; + if (!(devstr = qemuBuildRedirdevDevStr(def, redirdev, priv->qemuCaps))) + goto error; + if (VIR_REALLOC_N(vm->def->redirdevs, vm->def->nredirdevs+1) < 0) goto error; -- 2.0.4
If the domain has died while we were in the monitor, do not clean up what was cleaned up in qemuProcessStop. --- src/qemu/qemu_hotplug.c | 83 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 56 insertions(+), 27 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 38471e3..94bc4a2 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -275,7 +275,8 @@ qemuDomainCheckEjectableMedia(virQEMUDriverPtr driver, if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) { table = qemuMonitorGetBlockInfo(priv->mon); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; } if (!table) @@ -391,7 +392,10 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, memcpy(&disk->info.addr.pci, &guestAddr, sizeof(guestAddr)); } } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) { + ret = -1; + goto cleanup; + } virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0); @@ -485,7 +489,10 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, type, &controller->info.addr.pci); } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) { + ret = -1; + goto cleanup; + } if (ret == 0) { if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) @@ -636,7 +643,8 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, disk->info.addr.drive.unit = driveAddr.unit; } } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0); @@ -724,7 +732,8 @@ qemuDomainAttachUSBMassstorageDevice(virConnectPtr conn, } else { ret = qemuMonitorAddUSBDisk(priv->mon, src); } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0); @@ -1030,7 +1039,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, if (qemuMonitorAddNetdev(priv->mon, netstr, tapfd, tapfdName, tapfdSize, vhostfd, vhostfdName, vhostfdSize) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; virDomainAuditNet(vm, NULL, net, "attach", false); goto cleanup; } @@ -1038,7 +1048,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, if (qemuMonitorAddHostNetwork(priv->mon, netstr, tapfd, tapfdName, tapfdSize, vhostfd, vhostfdName, vhostfdSize) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; virDomainAuditNet(vm, NULL, net, "attach", false); goto cleanup; } @@ -1068,7 +1079,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, qemuDomainObjEnterMonitor(driver, vm); if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuMonitorAddDevice(priv->mon, nicstr) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; virDomainAuditNet(vm, NULL, net, "attach", false); goto try_remove; } @@ -1076,14 +1088,16 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, guestAddr = net->info.addr.pci; if (qemuMonitorAddPCINetwork(priv->mon, nicstr, &guestAddr) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; 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 (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; /* set link state */ if (net->linkstate == VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN) { @@ -1095,7 +1109,8 @@ 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); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; virDomainAuditNet(vm, NULL, net, "attach", false); goto try_remove; } @@ -1104,7 +1119,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, _("setting of link state not supported: Link is up")); } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; } /* link set to down */ } @@ -1116,7 +1132,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, cleanup: if (!ret) { vm->def->nets[vm->def->nnets++] = net; - } else { + } else if (virDomainObjIsActive(vm)) { if (releaseaddr) qemuDomainReleaseDeviceAddress(vm, &net->info, NULL); @@ -1293,7 +1309,8 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorAddDeviceWithFd(priv->mon, devstr, configfd, configfd_name); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; } else { virDevicePCIAddressPtr guestAddr = &hostdev->info->addr.pci; virDevicePCIAddressPtr hostAddr = &hostdev->source.subsys.u.pci.addr; @@ -1309,7 +1326,8 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorAddPCIHostDevice(priv->mon, hostAddr, guestAddr); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; hostdev->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; } @@ -1373,12 +1391,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 (qemuDomainObjExitMonitorAlive(driver, vm) < 0) goto error; - qemuDomainObjExitMonitor(driver, vm); virDomainAuditRedirdev(vm, redirdev, "attach", ret == 0); if (ret < 0) goto error; @@ -1500,17 +1517,26 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); if (qemuMonitorAttachCharDev(priv->mon, charAlias, &chr->source) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) { + need_remove = false; + goto cleanup; + } goto audit; } if (devstr && qemuMonitorAddDevice(priv->mon, devstr) < 0) { /* detach associated chardev on error */ qemuMonitorDetachCharDev(priv->mon, charAlias); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) { + need_remove = false; + goto cleanup; + } goto audit; } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) { + need_remove = false; + goto cleanup; + } ret = 0; audit: @@ -1566,7 +1592,8 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, ret = qemuMonitorAddUSBDeviceExact(priv->mon, hostdev->source.subsys.u.usb.bus, hostdev->source.subsys.u.usb.device); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; virDomainAuditHostdev(vm, hostdev, "attach", ret == 0); if (ret < 0) goto cleanup; @@ -1575,7 +1602,7 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, ret = 0; cleanup: - if (ret < 0) { + if (ret < 0 && virDomainObjIsActive(vm)) { if (teardowncgroup && qemuTeardownHostdevCgroup(vm, hostdev) < 0) VIR_WARN("Unable to remove host device cgroup ACL on hotplug fail"); if (teardownlabel && @@ -1669,7 +1696,8 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, } } } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; virDomainAuditHostdev(vm, hostdev, "attach", ret == 0); if (ret < 0) @@ -1679,7 +1707,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, ret = 0; cleanup: - if (ret < 0) { + if (ret < 0 && virDomainObjIsActive(vm)) { qemuDomainReAttachHostSCSIDevices(driver, vm->def->name, &hostdev, 1); if (teardowncgroup && qemuTeardownHostdevCgroup(vm, hostdev) < 0) VIR_WARN("Unable to remove host device cgroup ACL on hotplug fail"); @@ -1869,7 +1897,8 @@ int qemuDomainChangeNetLinkState(virQEMUDriverPtr driver, dev->linkstate = linkstate; cleanup: - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + return -1; return ret; } -- 2.0.4
On 16.12.2014 17:41, Ján Tomko wrote:
If the domain has died while we were in the monitor, do not clean up what was cleaned up in qemuProcessStop. --- src/qemu/qemu_hotplug.c | 83 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 56 insertions(+), 27 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 38471e3..94bc4a2 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -275,7 +275,8 @@ qemuDomainCheckEjectableMedia(virQEMUDriverPtr driver,
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) { table = qemuMonitorGetBlockInfo(priv->mon); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; }
if (!table) @@ -391,7 +392,10 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, memcpy(&disk->info.addr.pci, &guestAddr, sizeof(guestAddr)); } } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) { + ret = -1; + goto cleanup;
I think this needs to be 'goto error'. Moreover, should ve call virDomainAuditDisk(.., false) to state explicitly that disk attaching went wrong?
+ }
virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0);
@@ -485,7 +489,10 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, type, &controller->info.addr.pci); } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) { + ret = -1; + goto cleanup; + }
if (ret == 0) { if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) @@ -636,7 +643,8 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, disk->info.addr.drive.unit = driveAddr.unit; } } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup;
Ditto.
virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0);
@@ -724,7 +732,8 @@ qemuDomainAttachUSBMassstorageDevice(virConnectPtr conn, } else { ret = qemuMonitorAddUSBDisk(priv->mon, src); } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup;
And here too.
virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0);
@@ -1030,7 +1039,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, if (qemuMonitorAddNetdev(priv->mon, netstr, tapfd, tapfdName, tapfdSize, vhostfd, vhostfdName, vhostfdSize) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup;
And here too.
virDomainAuditNet(vm, NULL, net, "attach", false); goto cleanup; } @@ -1038,7 +1048,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, if (qemuMonitorAddHostNetwork(priv->mon, netstr, tapfd, tapfdName, tapfdSize, vhostfd, vhostfdName, vhostfdSize) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup;
And here too.
virDomainAuditNet(vm, NULL, net, "attach", false); goto cleanup; } @@ -1068,7 +1079,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, qemuDomainObjEnterMonitor(driver, vm); if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuMonitorAddDevice(priv->mon, nicstr) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup;
And here too.
virDomainAuditNet(vm, NULL, net, "attach", false); goto try_remove; } @@ -1076,14 +1088,16 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, guestAddr = net->info.addr.pci; if (qemuMonitorAddPCINetwork(priv->mon, nicstr, &guestAddr) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup;
And here too.
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 (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup;
/* set link state */ if (net->linkstate == VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN) { @@ -1095,7 +1109,8 @@ 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); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup;
And here too. I'm stopping to report this. I'm sure you get the idea. Michal
If the domain died and was cleaned up by qemuProcessStop, we don't need to do the cleanup (and vm->def might have been relpaced). --- src/qemu/qemu_hotplug.c | 54 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 94bc4a2..b45342d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3033,19 +3033,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 (qemuDomainObjExitMonitorAlive(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 (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; virDomainAuditDisk(vm, detach->src, NULL, "detach", false); goto cleanup; } } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; rc = qemuDomainWaitForDeviceRemoval(vm); if (rc == 0 || rc == 1) @@ -3085,11 +3088,13 @@ qemuDomainDetachDiskDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; virDomainAuditDisk(vm, detach->src, NULL, "detach", false); goto cleanup; } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; rc = qemuDomainWaitForDeviceRemoval(vm); if (rc == 0 || rc == 1) @@ -3266,17 +3271,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 (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; goto cleanup; } } else { if (qemuMonitorRemovePCIDevice(priv->mon, &detach->info.addr.pci) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; goto cleanup; } } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; rc = qemuDomainWaitForDeviceRemoval(vm); if (rc == 0 || rc == 1) @@ -3321,7 +3329,8 @@ qemuDomainDetachHostPCIDevice(virQEMUDriverPtr driver, } else { ret = qemuMonitorRemovePCIDevice(priv->mon, &detach->info->addr.pci); } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + ret = -1; return ret; } @@ -3350,7 +3359,8 @@ qemuDomainDetachHostUSBDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorDelDevice(priv->mon, detach->info->alias); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + ret = -1; return ret; } @@ -3382,7 +3392,8 @@ qemuDomainDetachHostSCSIDevice(virQEMUDriverPtr driver, qemuDomainObjExitMonitor(driver, vm); goto cleanup; } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; ret = 0; cleanup: @@ -3421,7 +3432,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) @@ -3577,19 +3589,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 (qemuDomainObjExitMonitorAlive(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 (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; virDomainAuditNet(vm, detach, NULL, "detach", false); goto cleanup; } } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; rc = qemuDomainWaitForDeviceRemoval(vm); if (rc == 0 || rc == 1) @@ -3669,7 +3684,8 @@ qemuDomainChangeGraphicsPasswords(virQEMUDriverPtr driver, } end_job: - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + ret = -1; cleanup: virObjectUnref(cfg); return ret; @@ -3756,10 +3772,12 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); if (devstr && qemuMonitorDelDevice(priv->mon, tmpChr->info.alias) < 0) { - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; goto cleanup; } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; rc = qemuDomainWaitForDeviceRemoval(vm); if (rc == 0 || rc == 1) -- 2.0.4
On 16.12.2014 17:41, Ján Tomko wrote:
If the domain died and was cleaned up by qemuProcessStop, we don't need to do the cleanup (and vm->def might have been relpaced). --- src/qemu/qemu_hotplug.c | 54 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 94bc4a2..b45342d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3033,19 +3033,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 (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; virDomainAuditDisk(vm, detach->src, NULL, "detach", false);
Other than the audit issue, this looks okay. Weak ACK until the time we have a consensus on the audit. Michal
On 16.12.2014 17:41, Ján Tomko wrote:
If the domain died and was cleaned up by qemuProcessStop, we don't need to do the cleanup (and vm->def might have been relpaced). --- src/qemu/qemu_hotplug.c | 54 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 18 deletions(-)
ACK Michal
Exit the monitor right after we've done with it to get the virDomainObjPtr lock back, otherwise we might be accessing vm->def while it's being cleaned up by qemuProcessStop. If the domain crashed while we were in the monitor, exit early instead of changing vm->def which is now the persistent definition. --- src/qemu/qemu_driver.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 07b061e..20b54cc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4443,7 +4443,7 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, if (rc == 0) goto unsupported; if (rc < 0) - goto cleanup; + goto exit_monitor; vcpus++; } @@ -4454,7 +4454,7 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, if (rc == 0) goto unsupported; if (rc < 0) - goto cleanup; + goto exit_monitor; vcpus--; } @@ -4471,6 +4471,10 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, * fatal */ if ((ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids)) <= 0) { virResetLastError(); + goto exit_monitor; + } + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) { + ret = -1; goto cleanup; } @@ -4591,10 +4595,10 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, cpupids = NULL; cleanup: - qemuDomainObjExitMonitor(driver, vm); - vm->def->vcpus = vcpus; VIR_FREE(cpupids); VIR_FREE(mem_mask); + if (virDomainObjIsActive(vm)) + vm->def->vcpus = vcpus; virDomainAuditVcpu(vm, oldvcpus, nvcpus, "update", rc == 1); if (cgroup_vcpu) virCgroupFree(&cgroup_vcpu); @@ -4603,6 +4607,8 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, unsupported: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot change vcpu count of this domain")); + exit_monitor: + qemuDomainObjExitMonitor(driver, vm); goto cleanup; } -- 2.0.4
--- src/qemu/qemu_driver.c | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 20b54cc..035a51d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2360,7 +2360,8 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, priv = vm->privateData; qemuDomainObjEnterMonitor(driver, vm); r = qemuMonitorSetBalloon(priv->mon, newmem); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto endjob; virDomainAuditMemory(vm, vm->def->mem.cur_balloon, newmem, "update", r == 1); if (r < 0) @@ -2444,7 +2445,8 @@ static int qemuDomainSetMemoryStatsPeriod(virDomainPtr dom, int period, qemuDomainObjEnterMonitor(driver, vm); r = qemuMonitorSetMemoryStatsPeriod(priv->mon, period); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto endjob; if (r < 0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("unable to set balloon driver collection period")); @@ -4289,6 +4291,11 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver, if (ret < 0) goto endjob; + if (!virDomainObjIsActive(vm)) { + VIR_WARN("Domain crashed"); + goto endjob; + } + if (virDomainNetGetActualType(def) == VIR_DOMAIN_NET_TYPE_DIRECT) { if (virNetDevGetRxFilter(def->ifname, &hostFilter)) { @@ -10293,11 +10300,11 @@ qemuDomainBlockStatsFlags(virDomainPtr dom, int idx; int tmp, ret = -1; virDomainObjPtr vm; - virDomainDiskDefPtr disk = NULL; qemuDomainObjPrivatePtr priv; long long rd_req, rd_bytes, wr_req, wr_bytes, rd_total_times; long long wr_total_times, flush_req, flush_total_times, errs; virTypedParameterPtr param; + char *diskAlias = NULL; virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1); @@ -10326,6 +10333,7 @@ qemuDomainBlockStatsFlags(virDomainPtr dom, } if (*nparams != 0) { + virDomainDiskDefPtr disk = NULL; if ((idx = virDomainDiskIndexByName(vm->def, path, false)) < 0) { virReportError(VIR_ERR_INVALID_ARG, _("invalid path: %s"), path); @@ -10339,6 +10347,8 @@ qemuDomainBlockStatsFlags(virDomainPtr dom, disk->dst); goto endjob; } + if (VIR_STRDUP(diskAlias, disk->info.alias) < 0) + goto endjob; } priv = vm->privateData; @@ -10354,7 +10364,7 @@ qemuDomainBlockStatsFlags(virDomainPtr dom, } ret = qemuMonitorGetBlockStatsInfo(priv->mon, - disk->info.alias, + diskAlias, &rd_req, &rd_bytes, &rd_total_times, @@ -10453,6 +10463,7 @@ qemuDomainBlockStatsFlags(virDomainPtr dom, cleanup: if (vm) virObjectUnlock(vm); + VIR_FREE(diskAlias); return ret; } @@ -16973,7 +16984,8 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, &info, supportMaxOptions); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto endjob; if (ret < 0) goto endjob; vm->def->disks[idx]->blkdeviotune = info; @@ -17270,7 +17282,8 @@ qemuDomainGetDiskErrors(virDomainPtr dom, qemuDomainObjEnterMonitor(driver, vm); table = qemuMonitorGetBlockInfo(priv->mon); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto endjob; if (!table) goto endjob; @@ -18539,7 +18552,8 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, dom); rc = qemuMonitorGetAllBlockStatsInfo(priv->mon, &stats); ignore_value(qemuMonitorBlockStatsUpdateCapacity(priv->mon, stats)); - qemuDomainObjExitMonitor(driver, dom); + if (qemuDomainObjExitMonitorAlive(driver, dom) < 0) + goto cleanup; if (rc < 0) { virResetLastError(); -- 2.0.4
It should not be needed in all qemuMigrationSet* functions, but I changed them all for consistency. --- src/qemu/qemu_migration.c | 41 ++++++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 0acbb57..ed7d6fe 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1662,7 +1662,8 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver, qemuDomainObjExitMonitor(driver, vm); goto cleanup; } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; } priv->nbdPort = port; @@ -1859,7 +1860,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, } -static void +static int qemuMigrationStopNBDServer(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuMigrationCookiePtr mig) @@ -1867,22 +1868,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); - virPortAllocatorRelease(driver->migrationPorts, priv->nbdPort); priv->nbdPort = 0; + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + return -1; + return 0; } -static void +static int qemuMigrationCancelDriveMirror(qemuMigrationCookiePtr mig, virQEMUDriverPtr driver, virDomainObjPtr vm) @@ -1890,6 +1892,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); @@ -1913,12 +1916,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 (qemuDomainObjExitMonitorAlive(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, @@ -2124,7 +2130,8 @@ qemuMigrationSetCompression(virQEMUDriverPtr driver, state); cleanup: - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + ret = -1; return ret; } @@ -2163,7 +2170,8 @@ qemuMigrationSetAutoConverge(virQEMUDriverPtr driver, state); cleanup: - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + ret = -1; return ret; } @@ -2209,7 +2217,8 @@ qemuMigrationSetPinAll(virQEMUDriverPtr driver, state); cleanup: - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + ret = -1; return ret; } @@ -2475,7 +2484,8 @@ qemuDomainMigrateGraphicsRelocate(virQEMUDriverPtr driver, QEMU_ASYNC_JOB_MIGRATION_OUT) == 0) { ret = qemuMonitorGraphicsRelocate(priv->mon, type, listenAddress, port, tlsPort, tlsSubject); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + ret = -1; } cleanup: @@ -4020,7 +4030,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) @@ -5171,7 +5181,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; -- 2.0.4
On 16.12.2014 17:41, Ján Tomko wrote:
It should not be needed in all qemuMigrationSet* functions, but I changed them all for consistency. --- src/qemu/qemu_migration.c | 41 ++++++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 0acbb57..ed7d6fe 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1662,7 +1662,8 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver, qemuDomainObjExitMonitor(driver, vm); goto cleanup; } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; }
priv->nbdPort = port; @@ -1859,7 +1860,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, }
-static void +static int qemuMigrationStopNBDServer(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuMigrationCookiePtr mig) @@ -1867,22 +1868,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); - virPortAllocatorRelease(driver->migrationPorts, priv->nbdPort); priv->nbdPort = 0; + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + return -1;
I don't thin it's safe to manipulate priv with unlocked domain. Otherwise looking good.
+ return 0; }
Michal
--- src/qemu/qemu_process.c | 63 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 42 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d4af85c..d6de22b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1665,7 +1665,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 (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + return -1; error: @@ -2121,7 +2122,8 @@ qemuProcessReconnectRefreshChannelVirtioState(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorGetChardevInfo(priv->mon, &info); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; if (ret < 0) goto cleanup; @@ -2174,7 +2176,8 @@ qemuProcessWaitForMonitor(virQEMUDriverPtr driver, if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) goto cleanup; ret = qemuMonitorGetChardevInfo(priv->mon, &info); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; VIR_DEBUG("qemuMonitorGetChardevInfo returned %i", ret); if (ret == 0) { @@ -2246,7 +2249,8 @@ qemuProcessDetectVcpuPIDs(virQEMUDriverPtr driver, priv->vcpupids[0] = vm->pid; return 0; } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + return -1; if (ncpupids != vm->def->vcpus) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -2281,7 +2285,8 @@ qemuProcessDetectIOThreadPIDs(virQEMUDriverPtr driver, if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) goto cleanup; niothreads = qemuMonitorGetIOThreads(priv->mon, &iothreads); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; if (niothreads < 0) goto cleanup; @@ -2554,6 +2559,7 @@ qemuProcessInitPasswords(virConnectPtr conn, qemuDomainObjPrivatePtr priv = vm->privateData; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); size_t i; + char *alias = NULL; for (i = 0; i < vm->def->ngraphics; ++i) { virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; @@ -2579,7 +2585,6 @@ qemuProcessInitPasswords(virConnectPtr conn, for (i = 0; i < vm->def->ndisks; i++) { char *secret; size_t secretLen; - const char *alias; if (!vm->def->disks[i]->src->encryption || !virDomainDiskGetSource(vm->def->disks[i])) @@ -2590,20 +2595,26 @@ qemuProcessInitPasswords(virConnectPtr conn, &secret, &secretLen) < 0) goto cleanup; - alias = vm->def->disks[i]->info.alias; + VIR_FREE(alias); + if (VIR_STRDUP(alias, vm->def->disks[i]->info.alias) < 0) + goto cleanup; if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) { VIR_FREE(secret); goto cleanup; } ret = qemuMonitorSetDrivePassphrase(priv->mon, alias, secret); VIR_FREE(secret); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) { + ret = -1; + goto cleanup; + } if (ret < 0) goto cleanup; } } cleanup: + VIR_FREE(alias); virObjectUnref(cfg); return ret; } @@ -2990,11 +3001,13 @@ qemuProcessInitPCIAddresses(virQEMUDriverPtr driver, return -1; naddrs = qemuMonitorGetAllPCIAddresses(priv->mon, &addrs); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; if (naddrs > 0) ret = qemuProcessDetectPCIAddresses(vm, addrs, naddrs); + cleanup: VIR_FREE(addrs); return ret; @@ -3144,7 +3157,8 @@ qemuProcessStartCPUs(virQEMUDriverPtr driver, virDomainObjPtr vm, goto release; ret = qemuMonitorStartCPUs(priv->mon, conn); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto release; if (ret < 0) goto release; @@ -3177,7 +3191,8 @@ int qemuProcessStopCPUs(virQEMUDriverPtr driver, goto cleanup; ret = qemuMonitorStopCPUs(priv->mon); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; if (ret < 0) goto cleanup; @@ -3360,7 +3375,8 @@ qemuProcessRecoverMigration(virQEMUDriverPtr driver, vm->def->name); qemuDomainObjEnterMonitor(driver, vm); ignore_value(qemuMonitorMigrateCancel(priv->mon)); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + return -1; /* resume the domain but only if it was paused as a result of * migration */ if (state == VIR_DOMAIN_PAUSED && @@ -3429,7 +3445,8 @@ qemuProcessRecoverJob(virQEMUDriverPtr driver, case QEMU_ASYNC_JOB_SNAPSHOT: qemuDomainObjEnterMonitor(driver, vm); ignore_value(qemuMonitorMigrateCancel(priv->mon)); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(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 @@ -3971,7 +3988,8 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver, if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return false; rc = qemuMonitorGetGuestCPU(priv->mon, arch, &guestcpu); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + return false; if (rc < 0) { if (rc == -2) @@ -4168,6 +4186,7 @@ int qemuProcessStart(virConnectPtr conn, virCommandPtr cmd = NULL; struct qemuProcessHookData hookData; unsigned long cur_balloon; + unsigned int period = 0; size_t i; bool rawio_set = false; char *nodeset = NULL; @@ -4780,15 +4799,18 @@ int qemuProcessStart(virConnectPtr conn, vm->def->mem.cur_balloon); goto cleanup; } + if (vm->def->memballoon && vm->def->memballoon->period) + period = vm->def->memballoon->period; if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) goto cleanup; - if (vm->def->memballoon && vm->def->memballoon->period) - qemuMonitorSetMemoryStatsPeriod(priv->mon, vm->def->memballoon->period); + if (period) + qemuMonitorSetMemoryStatsPeriod(priv->mon, period); if (qemuMonitorSetBalloon(priv->mon, cur_balloon) < 0) { qemuDomainObjExitMonitor(driver, vm); goto cleanup; } - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto cleanup; if (!(flags & VIR_QEMU_PROCESS_START_PAUSED)) { VIR_DEBUG("Starting domain CPUs"); @@ -5377,9 +5399,7 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, qemuDomainObjExitMonitor(driver, vm); goto error; } - qemuDomainObjExitMonitor(driver, vm); - - if (!virDomainObjIsActive(vm)) + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) goto error; if (running) { @@ -5389,7 +5409,8 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, qemuDomainObjEnterMonitor(driver, vm); qemuMonitorSetMemoryStatsPeriod(priv->mon, vm->def->memballoon->period); - qemuDomainObjExitMonitor(driver, vm); + if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) + goto error; } } else { virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, reason); -- 2.0.4
participants (3)
-
Daniel P. Berrange -
Ján Tomko -
Michal Privoznik