[libvirt] [PATCH] qemu: improve errors related to offline domains
Stefan Berger
stefanb at linux.vnet.ibm.com
Thu Apr 26 20:11:21 UTC 2012
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
More information about the libvir-list
mailing list