Re: [libvirt] [PATCH] qemu: nicer error message if live disk snapshot unsupported
On 12/04/2012 08:43 AM, Osier Yang wrote:
On 2012年12月04日 05:33, Eric Blake wrote:
> Without this patch, attempts to create a disk snapshot when qemu
> is too old results in a cryptic message:
>
> virsh # snapshot-create 23 --disk-only
> error: operation failed: Failed to take snapshot: unknown command:
> 'snapshot_blkdev'
>
> Now it reports:
>
> virsh # snapshot-create 23 --disk-only
> error: unsupported configuration: live disk snapshot not supported
> with this QEMU binary
>
> * src/qemu/qemu_capabilities.h (QEMU_CAPS_DISK_SNAPSHOT): New
> capability.
ACK'ed a bit earlier, there is no testing for this new flag,
Alas, we really don't have any tests already in place for capabilities
that are set only by probing the existence of a QMP command. However,
Dan has done enough framework that we can fake a QMP sequence, so maybe
it is worth expanding that concept to be able to fake the list of QMP
commands supported by various qemu releases. But I think it's big
enough to save for another patch.
and...
"ret" can be used without initialization.
Good catch.
>>
>> }
>> + } else if (!qemuCapsGet(priv->caps, QEMU_CAPS_DISK_SNAPSHOT)) {
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>
> Shouldn't this be VIR_ERR_OPERATION_INVALID instead? As it's not
> related with config.
The right code is actually VIR_ERR_OPERATION_UNSUPPORTED.
This was from copy-and-paste from elsewhere in qemu_driver.c, but indeed
OPERATION_UNSUPPORTED sounds best.
I've fixed those problems, and will push shortly.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 619 bytes)