On 01/17/2014 08:31 AM, Francesco Romani wrote:
> This patch add an element to QEMU's capability XML, to
s/add/adds/
> show if the underlying QEMU binary supports the live disk
> snapshotting or not.
> This allow any client to know ahead of time if the feature
s/allow/allows/
> is available.
>
> Without this information available, the only way to check
> for the snapshot support is to request one and check for
> errors.
>
> Signed-off-by: Francesco Romani <fromani@xxxxxxxxxx>
> ---
> docs/schemas/capability.rng | 6 ++++++
> src/qemu/qemu_capabilities.c | 7 +++++++
> 2 files changed, 13 insertions(+)
It would probably also be good to test this in
tests/capabilityschemadata/, but maybe that happens later in the series...
> </optional>
> + <optional>
> + <element name='disksnapshot'>
Existing capabilities have an interesting mix of spelling: underscores:
<os_type>, squashed: <baselabel>, camelCase: <machine maxCpus=''>. I
would have problaby picked <disk-snapshot> if I were writing it without
knowledge of pre-existing code, but don't see any examples of that. So,
your naming is probably as good as any.
> +++ b/src/qemu/qemu_capabilities.c
> @@ -687,6 +687,7 @@ virQEMUCapsInitGuest(virCapsPtr caps,
> virQEMUCapsPtr qemubinCaps = NULL;
> virQEMUCapsPtr kvmbinCaps = NULL;
> int ret = -1;
> + bool hasdisksnapshot = false;
>
> /* Check for existence of base emulator, or alternate base
> * which can be used with magic cpu choice
> @@ -774,6 +775,12 @@ virQEMUCapsInitGuest(virCapsPtr caps,
> !virCapabilitiesAddGuestFeature(guest, "deviceboot", 1, 0))
That, and you were copying from 'deviceboot' which also uses squashed style.
> goto error;
>
> + if (virQEMUCapsGet(qemubinCaps, QEMU_CAPS_DISK_SNAPSHOT))
> + hasdisksnapshot = true;
> +
> + if (!virCapabilitiesAddGuestFeature(guest, "disksnapshot", hasdisksnapshot, 0))
> + goto error;
I probably would have skipped the temporary variable, but then hit
longer line length:
if (!virCapabilitiesAddGuestFeature(guest, "disksnapshot",
virQEMUCapsGet(qemubinCaps,
QEMU_CAPS_DISK_SNAPSHOT)),
0))
goto error;
so your way is fine as-is.
Preliminary ACK, assuming the rest of the series covers testing and
documentation (and as you said in the cover letter, existing docs are a
bit sparse).
--
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