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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]