[libvirt] [PATCH] qemu: nicer error message if live disk snapshot unsupported

Eric Blake eblake at redhat.com
Tue Dec 4 20:20:04 UTC 2012


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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20121204/161fb835/attachment-0001.sig>


More information about the libvir-list mailing list