Re: [libvirt] [PATCH 1/4] qemu: export disk snapshot support in capabilities
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(a)redhat.com>
---
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
Attachments:
- signature.asc (application/pgp-signature — 604 bytes)