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