On 04/26/2012 03:50 PM, Eric Blake wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=816662 pointed out
that attempting 'virsh blockpull' on an offline domain gave a
misleading error message about qemu lacking support for the
operation, even when qemu was specifically updated to support it.
The real problem is that we have several capabilities that are
only determined when starting a domain, and therefore are still
clear when first working with an inactive domain (namely, any
capability set by qemuMonitorJSONCheckCommands).
[...]
* src/qemu/qemu_driver.c (qemuDomainPMSuspendForDuration)
(qemuDomainSnapshotCreateXML, qemuDomainBlockJobImpl): Check for
offline state before checking an online-only cap.
---
src/qemu/qemu_driver.c | 24 ++++++++++++++++++------
1 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d3aa34d..3a1c1c4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10389,6 +10389,12 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
goto cleanup;
if (flags& VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) {
+ if (!virDomainObjIsActive(vm)) {
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("disk snapshots of inactive domains not "
+ "implemented yet"));
+ goto cleanup;
+ }
if (virDomainSnapshotAlignDisks(def,
VIR_DOMAIN_DISK_SNAPSHOT_EXTERNAL,
false)< 0)
@@ -10443,12 +10449,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
* makes sense, such as checking that qemu-img recognizes the
* snapshot name in at least one of the domain's disks? */
} else if (flags& VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) {
- if (!virDomainObjIsActive(vm)) {
- qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("disk snapshots of inactive domains not "
- "implemented yet"));
- goto cleanup;
- }
if (qemuDomainSnapshotCreateDiskActive(domain->conn, driver,
&vm, snap, flags)< 0)
goto cleanup;
So this moves the check to an earlier place.
@@ -11642,6 +11642,12 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char
*path, const char *base,
_("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
+ if (!virDomainObjIsActive(vm)) {
+ qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("domain is not running"));
+ goto endjob;
+ }
+
priv = vm->privateData;
if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC)) {
async = true;
'goto cleanup' here -- same comment as below.
@@ -12637,6 +12643,12 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom,
priv = vm->privateData;
+ if (!virDomainObjIsActive(vm)) {
+ qemuReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("domain is not running"));
+ goto endjob;
+ }
+
Nit, you could move this above the priv =. Neither endjob nor cleanup
need this priv.
But 'goto endjob' is wrong (replace with 'goto cleanup'), since only
later (~line 12655) the
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
is run.
Stefan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
