[libvirt] [PATCH 0/4] qemu: export disk snapshot capability
This patch series extend the QEMU capabilities XML to report if the underlying QEMU binary supports, or not, the live disk snapshotting. Without this patch series, the only way to know if QEMU has this support is to actually request a disk snapshot and to see what happens. The change is split in four patches: * patch #1 actually adds the new element in the QEMU capabilities XML. formatcaps.html.in wasn't very detailed about the actual XML format, so I've not updated it. Anyone feel free to point out what should be added, and I'll comply. The new element has the form <disksnapshot default='value' toggle='off'> because I'd like to convey two informations: - disk snapshot is supposed to be here, and it is (default='on') - disk snapshot is supposed to be here, and is NOT (default='off') Put in a different way, I tried to help the client as much as possible. I'm not particolary fond of this format, and I'm really open to alternatives here. Perhaps a simpler <disksnapshot/> element can convey the same meaning? Suggestions welcome. * patches #2, #3 Are trivial and they provide the ground for the last patch which add a new unit tests. They are just dependencies for it. I tried to make them less invasive as possible. * patch #4 add a new unit test, aiming to test not only this new feature but also the whole XML capabilities test. I was under the impression that this kind of test do not really fit into existing one, so I added a new one. Suggestions about possible improvements for this test are welcome Francesco Romani (4): qemu: export disk snapshot support in capabilities qemu: add function to fill capabilities cache qemu: export the virQEMUCapsInitGuest function. qemu: add unit tests for the capabilities xml .gitignore | 1 + docs/schemas/capability.rng | 6 + src/qemu/qemu_capabilities.c | 40 +++- src/qemu/qemu_capabilities.h | 4 + tests/Makefile.am | 10 +- tests/qemucaps2xmldata/all_1.6.0-1.caps | 142 ++++++++++++++ tests/qemucaps2xmldata/all_1.6.0-1.xml | 51 +++++ tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps | 141 +++++++++++++ tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml | 51 +++++ tests/qemucaps2xmltest.c | 217 +++++++++++++++++++++ 10 files changed, 655 insertions(+), 8 deletions(-) create mode 100644 tests/qemucaps2xmldata/all_1.6.0-1.caps create mode 100644 tests/qemucaps2xmldata/all_1.6.0-1.xml create mode 100644 tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps create mode 100644 tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml create mode 100644 tests/qemucaps2xmltest.c -- 1.8.4.2
This patch add an element to QEMU's capability XML, to 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 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@redhat.com> --- docs/schemas/capability.rng | 6 ++++++ src/qemu/qemu_capabilities.c | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index aee03d7..d2d9776 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -371,6 +371,12 @@ <empty/> </element> </optional> + <optional> + <element name='disksnapshot'> + <ref name='featuretoggle'/> + <empty/> + </element> + </optional> </interleave> </element> </define> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 0538115..923b2a3 100644 --- a/src/qemu/qemu_capabilities.c +++ 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)) goto error; + if (virQEMUCapsGet(qemubinCaps, QEMU_CAPS_DISK_SNAPSHOT)) + hasdisksnapshot = true; + + if (!virCapabilitiesAddGuestFeature(guest, "disksnapshot", hasdisksnapshot, 0)) + goto error; + if (virCapabilitiesAddGuestDomain(guest, "qemu", NULL, -- 1.8.4.2
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@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
Hi, thanks for the review! ----- Original Message -----
From: "Eric Blake" <eblake@redhat.com> To: "Francesco Romani" <fromani@redhat.com>, libvir-list@redhat.com Sent: Wednesday, February 26, 2014 11:30:07 PM Subject: 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/
Oops. Both fixed.
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...
Actually not. I will work on this.
</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.
Yes. It looks to me the squashed style was slightly more common, so I adopted it.
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: (snip)
Yep. That's the reason I used the temporary.
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).
Thanks. Will send an amended second version (so far for the typos in the commit message and for the capabilityschemadata tests) once all the patches in the series are reviewed. Thanks and best regards, -- Francesco Romani RedHat Engineering Virtualization R & D Phone: 8261328 IRC: fromani
extract and export the code to add the capabilities for a given QEMU binary in the capabilities cache. The intended use case is for testing, and make writing more test easier. --- src/qemu/qemu_capabilities.c | 31 +++++++++++++++++++++++++------ src/qemu/qemu_capabilities.h | 2 ++ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 923b2a3..125869b 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2847,6 +2847,30 @@ error: return NULL; } +static virQEMUCapsPtr +virQEMUCapsCacheAddUnlocked(virQEMUCapsCachePtr cache, + const char *binary, virQEMUCapsPtr caps) +{ + virQEMUCapsPtr ret = caps; + + VIR_DEBUG("Caching capabilities %p for %s", caps, binary); + + if (virHashAddEntry(cache->binaries, binary, caps) < 0) { + virObjectUnref(caps); + ret = NULL; + } + + return ret; +} + +void +virQEMUCapsCacheAdd(virQEMUCapsCachePtr cache, + const char *binary, virQEMUCapsPtr caps) +{ + virMutexLock(&cache->lock); + virQEMUCapsCacheAddUnlocked(cache, binary, caps); + virMutexUnlock(&cache->lock); +} virQEMUCapsPtr virQEMUCapsCacheLookup(virQEMUCapsCachePtr cache, const char *binary) @@ -2867,12 +2891,7 @@ virQEMUCapsCacheLookup(virQEMUCapsCachePtr cache, const char *binary) ret = virQEMUCapsNewForBinary(binary, cache->libDir, cache->runUid, cache->runGid); if (ret) { - VIR_DEBUG("Caching capabilities %p for %s", - ret, binary); - if (virHashAddEntry(cache->binaries, binary, ret) < 0) { - virObjectUnref(ret); - ret = NULL; - } + ret = virQEMUCapsCacheAddUnlocked(cache, binary, ret); } } VIR_DEBUG("Returning caps %p for %s", ret, binary); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index efb3f43..1814bf7 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -260,6 +260,8 @@ bool virQEMUCapsIsValid(virQEMUCapsPtr qemuCaps); virQEMUCapsCachePtr virQEMUCapsCacheNew(const char *libDir, uid_t uid, gid_t gid); +void virQEMUCapsCacheAdd(virQEMUCapsCachePtr cache, + const char *binary, virQEMUCapsPtr caps); virQEMUCapsPtr virQEMUCapsCacheLookup(virQEMUCapsCachePtr cache, const char *binary); virQEMUCapsPtr virQEMUCapsCacheLookupCopy(virQEMUCapsCachePtr cache, -- 1.8.4.2
On 01/17/2014 08:31 AM, Francesco Romani wrote:
extract and export the code to add the capabilities for a given QEMU binary in the capabilities cache.
The intended use case is for testing, and make writing more test easier. --- src/qemu/qemu_capabilities.c | 31 +++++++++++++++++++++++++------ src/qemu/qemu_capabilities.h | 2 ++ 2 files changed, 27 insertions(+), 6 deletions(-)
Looks like a clean refactor; I'll see how it gets used later, but for this patch: ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
make the function public. the function is useful to XML capabilities unit test. --- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_capabilities.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 125869b..37c1ee4 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -670,7 +670,7 @@ virQEMUCapsIsValidForKVM(virArch hostarch, return false; } -static int +int virQEMUCapsInitGuest(virCapsPtr caps, virQEMUCapsCachePtr cache, virArch hostarch, diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 1814bf7..18a00d6 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -269,6 +269,8 @@ virQEMUCapsPtr virQEMUCapsCacheLookupCopy(virQEMUCapsCachePtr cache, void virQEMUCapsCacheFree(virQEMUCapsCachePtr cache); virCapsPtr virQEMUCapsInit(virQEMUCapsCachePtr cache); +int virQEMUCapsInitGuest(virCapsPtr caps, virQEMUCapsCachePtr cache, + virArch hostarch, virArch guestarch); int virQEMUCapsGetDefaultVersion(virCapsPtr caps, virQEMUCapsCachePtr capsCache, -- 1.8.4.2
On 01/17/2014 08:31 AM, Francesco Romani wrote:
make the function public. the function is useful to XML capabilities unit test. --- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_capabilities.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)
ACK.
+++ b/src/qemu/qemu_capabilities.h @@ -269,6 +269,8 @@ virQEMUCapsPtr virQEMUCapsCacheLookupCopy(virQEMUCapsCachePtr cache, void virQEMUCapsCacheFree(virQEMUCapsCachePtr cache);
virCapsPtr virQEMUCapsInit(virQEMUCapsCachePtr cache); +int virQEMUCapsInitGuest(virCapsPtr caps, virQEMUCapsCachePtr cache, + virArch hostarch, virArch guestarch);
Spurious double space. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
the test is loosely inspired from qemucapabilitiestest and qemuxml2xmltest. Added a new test instead of extending an existing one because the feature being tested don't really fits nicely in any existing place. --- .gitignore | 1 + tests/Makefile.am | 10 +- tests/qemucaps2xmldata/all_1.6.0-1.caps | 142 ++++++++++++++ tests/qemucaps2xmldata/all_1.6.0-1.xml | 51 +++++ tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps | 141 +++++++++++++ tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml | 51 +++++ tests/qemucaps2xmltest.c | 217 +++++++++++++++++++++ 7 files changed, 612 insertions(+), 1 deletion(-) create mode 100644 tests/qemucaps2xmldata/all_1.6.0-1.caps create mode 100644 tests/qemucaps2xmldata/all_1.6.0-1.xml create mode 100644 tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps create mode 100644 tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml create mode 100644 tests/qemucaps2xmltest.c diff --git a/.gitignore b/.gitignore index 7005500..19c8622 100644 --- a/.gitignore +++ b/.gitignore @@ -169,6 +169,7 @@ /tests/qemuagenttest /tests/qemuargv2xmltest /tests/qemucapabilitiestest +/tests/qemucaps2xmltest /tests/qemuhelptest /tests/qemuhotplugtest /tests/qemumonitorjsontest diff --git a/tests/Makefile.am b/tests/Makefile.am index 568b7a0..1496642 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -88,6 +88,7 @@ EXTRA_DIST = \ nwfilterxml2xmlout \ oomtrace.pl \ qemucapabilitiesdata \ + qemucaps2xmldata \ qemuhelpdata \ qemuhotplugtestdata \ qemumonitorjsondata \ @@ -192,7 +193,7 @@ if WITH_QEMU test_programs += qemuxml2argvtest qemuxml2xmltest qemuxmlnstest \ qemuargv2xmltest qemuhelptest domainsnapshotxml2xmltest \ qemumonitortest qemumonitorjsontest qemuhotplugtest \ - qemuagenttest qemucapabilitiestest + qemuagenttest qemucapabilitiestest qemucaps2xmltest endif WITH_QEMU if WITH_LXC @@ -469,6 +470,12 @@ qemucapabilitiestest_SOURCES = \ $(NULL) qemucapabilitiestest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) +qemucaps2xmltest_SOURCES = \ + qemucaps2xmltest.c \ + testutils.c testutils.h \ + $(NULL) +qemucaps2xmltest_LDADD = $(qemu_LDADDS) + qemuagenttest_SOURCES = \ qemuagenttest.c \ testutils.c testutils.h \ @@ -493,6 +500,7 @@ EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c \ qemumonitortest.c testutilsqemu.c testutilsqemu.h \ qemumonitorjsontest.c qemuhotplugtest.c \ qemuagenttest.c qemucapabilitiestest.c \ + qemucaps2xmltest.c \ $(QEMUMONITORTESTUTILS_SOURCES) endif ! WITH_QEMU diff --git a/tests/qemucaps2xmldata/all_1.6.0-1.caps b/tests/qemucaps2xmldata/all_1.6.0-1.caps new file mode 100644 index 0000000..2d50cf9 --- /dev/null +++ b/tests/qemucaps2xmldata/all_1.6.0-1.caps @@ -0,0 +1,142 @@ + <qemuCaps> + <flag name='vnc-colon'/> + <flag name='no-reboot'/> + <flag name='drive'/> + <flag name='name'/> + <flag name='uuid'/> + <flag name='vnet-hdr'/> + <flag name='migrate-qemu-tcp'/> + <flag name='migrate-qemu-exec'/> + <flag name='drive-cache-v2'/> + <flag name='drive-format'/> + <flag name='vga'/> + <flag name='0.10'/> + <flag name='mem-path'/> + <flag name='drive-serial'/> + <flag name='migrate-qemu-unix'/> + <flag name='chardev'/> + <flag name='enable-kvm'/> + <flag name='monitor-json'/> + <flag name='balloon'/> + <flag name='device'/> + <flag name='sdl'/> + <flag name='smp-topology'/> + <flag name='netdev'/> + <flag name='rtc'/> + <flag name='vhost-net'/> + <flag name='no-hpet'/> + <flag name='no-kvm-pit'/> + <flag name='pci-configfd'/> + <flag name='nodefconfig'/> + <flag name='boot-menu'/> + <flag name='fsdev'/> + <flag name='name-process'/> + <flag name='drive-readonly'/> + <flag name='smbios-type'/> + <flag name='vga-qxl'/> + <flag name='spice'/> + <flag name='vga-none'/> + <flag name='migrate-qemu-fd'/> + <flag name='boot-index'/> + <flag name='hda-duplex'/> + <flag name='drive-aio'/> + <flag name='pci-multibus'/> + <flag name='pci-bootindex'/> + <flag name='ccid-emulated'/> + <flag name='ccid-passthru'/> + <flag name='chardev-spicevmc'/> + <flag name='virtio-tx-alg'/> + <flag name='device-qxl-vga'/> + <flag name='pci-multifunction'/> + <flag name='virtio-blk-pci.ioeventfd'/> + <flag name='sga'/> + <flag name='virtio-blk-pci.event_idx'/> + <flag name='virtio-net-pci.event_idx'/> + <flag name='cache-directsync'/> + <flag name='piix3-usb-uhci'/> + <flag name='piix4-usb-uhci'/> + <flag name='usb-ehci'/> + <flag name='ich9-usb-ehci1'/> + <flag name='vt82c686b-usb-uhci'/> + <flag name='pci-ohci'/> + <flag name='usb-hub'/> + <flag name='no-shutdown'/> + <flag name='cache-unsafe'/> + <flag name='rombar'/> + <flag name='ich9-ahci'/> + <flag name='no-acpi'/> + <flag name='fsdev-readonly'/> + <flag name='virtio-blk-pci.scsi'/> + <flag name='blk-sg-io'/> + <flag name='drive-copy-on-read'/> + <flag name='cpu-host'/> + <flag name='fsdev-writeout'/> + <flag name='drive-iotune'/> + <flag name='system_wakeup'/> + <flag name='scsi-disk.channel'/> + <flag name='scsi-block'/> + <flag name='transaction'/> + <flag name='block-job-async'/> + <flag name='scsi-cd'/> + <flag name='ide-cd'/> + <flag name='no-user-config'/> + <flag name='hda-micro'/> + <flag name='dump-guest-memory'/> + <flag name='nec-usb-xhci'/> + <flag name='balloon-event'/> + <flag name='bridge'/> + <flag name='lsi'/> + <flag name='virtio-scsi-pci'/> + <flag name='blockio'/> + <flag name='disable-s3'/> + <flag name='disable-s4'/> + <flag name='ide-drive.wwn'/> + <flag name='scsi-disk.wwn'/> + <flag name='seccomp-sandbox'/> + <flag name='dump-guest-core'/> + <flag name='seamless-migration'/> + <flag name='block-commit'/> + <flag name='vnc'/> + <flag name='drive-mirror'/> + <flag name='usb-host.bootindex'/> + <flag name='blockdev-snapshot-sync'/> + <flag name='qxl'/> + <flag name='VGA'/> + <flag name='cirrus-vga'/> + <flag name='vmware-svga'/> + <flag name='device-video-primary'/> + <flag name='usb-serial'/> + <flag name='usb-net'/> + <flag name='add-fd'/> + <flag name='nbd-server'/> + <flag name='virtio-rng'/> + <flag name='rng-random'/> + <flag name='rng-egd'/> + <flag name='dtb'/> + <flag name='megasas'/> + <flag name='ipv6-migration'/> + <flag name='machine-opt'/> + <flag name='machine-usb-opt'/> + <flag name='pci-bridge'/> + <flag name='vfio-pci'/> + <flag name='vfio-pci.bootindex'/> + <flag name='scsi-generic'/> + <flag name='scsi-generic.bootindex'/> + <flag name='mem-merge'/> + <flag name='vnc-websocket'/> + <flag name='mlock'/> + <flag name='vnc-share-policy'/> + <flag name='device-del-event'/> + <flag name='dmi-to-pci-bridge'/> + <flag name='i440fx-pci-hole64-size'/> + <flag name='q35-pci-hole64-size'/> + <flag name='usb-storage'/> + <flag name='usb-storage.removable'/> + <flag name='virtio-mmio'/> + <flag name='ich9-intel-hda'/> + <flag name='kvm-pit-lost-tick-policy'/> + <flag name='boot-strict'/> + <flag name='pvpanic'/> + <flag name='reboot-timeout'/> + <flag name='enable-fips'/> + </qemuCaps> diff --git a/tests/qemucaps2xmldata/all_1.6.0-1.xml b/tests/qemucaps2xmldata/all_1.6.0-1.xml new file mode 100644 index 0000000..ed9825a --- /dev/null +++ b/tests/qemucaps2xmldata/all_1.6.0-1.xml @@ -0,0 +1,51 @@ +<capabilities> + + <host> + <cpu> + <arch>i686</arch> + </cpu> + <power_management/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='i686'> + <wordsize>32</wordsize> + <emulator>/usr/bin/qemu-system-i386</emulator> + <domain type='qemu'> + </domain> + <domain type='kvm'> + <emulator>/usr/bin/qemu-kvm</emulator> + <machine>pc-i440fx-1.6</machine> + <machine>none</machine> + <machine canonical='pc-i440fx-1.6'>pc</machine> + <machine>pc-i440fx-1.5</machine> + <machine>pc-i440fx-1.4</machine> + <machine>pc-1.3</machine> + <machine>pc-1.2</machine> + <machine>pc-1.1</machine> + <machine>pc-1.0</machine> + <machine>pc-0.15</machine> + <machine>pc-0.14</machine> + <machine>pc-0.13</machine> + <machine>pc-0.12</machine> + <machine>pc-0.11</machine> + <machine>pc-0.10</machine> + <machine>isapc</machine> + <machine canonical='pc-q35-1.6'>q35</machine> + <machine>pc-q35-1.6</machine> + <machine>pc-q35-1.5</machine> + <machine>pc-q35-1.4</machine> + </domain> + </arch> + <features> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + <acpi default='on' toggle='yes'/> + <apic default='on' toggle='no'/> + <pae/> + <nonpae/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps b/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps new file mode 100644 index 0000000..5092e41 --- /dev/null +++ b/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps @@ -0,0 +1,141 @@ + <qemuCaps> + <flag name='vnc-colon'/> + <flag name='no-reboot'/> + <flag name='drive'/> + <flag name='name'/> + <flag name='uuid'/> + <flag name='vnet-hdr'/> + <flag name='migrate-qemu-tcp'/> + <flag name='migrate-qemu-exec'/> + <flag name='drive-cache-v2'/> + <flag name='drive-format'/> + <flag name='vga'/> + <flag name='0.10'/> + <flag name='mem-path'/> + <flag name='drive-serial'/> + <flag name='migrate-qemu-unix'/> + <flag name='chardev'/> + <flag name='enable-kvm'/> + <flag name='monitor-json'/> + <flag name='balloon'/> + <flag name='device'/> + <flag name='sdl'/> + <flag name='smp-topology'/> + <flag name='netdev'/> + <flag name='rtc'/> + <flag name='vhost-net'/> + <flag name='no-hpet'/> + <flag name='no-kvm-pit'/> + <flag name='pci-configfd'/> + <flag name='nodefconfig'/> + <flag name='boot-menu'/> + <flag name='fsdev'/> + <flag name='name-process'/> + <flag name='drive-readonly'/> + <flag name='smbios-type'/> + <flag name='vga-qxl'/> + <flag name='spice'/> + <flag name='vga-none'/> + <flag name='migrate-qemu-fd'/> + <flag name='boot-index'/> + <flag name='hda-duplex'/> + <flag name='drive-aio'/> + <flag name='pci-multibus'/> + <flag name='pci-bootindex'/> + <flag name='ccid-emulated'/> + <flag name='ccid-passthru'/> + <flag name='chardev-spicevmc'/> + <flag name='virtio-tx-alg'/> + <flag name='device-qxl-vga'/> + <flag name='pci-multifunction'/> + <flag name='virtio-blk-pci.ioeventfd'/> + <flag name='sga'/> + <flag name='virtio-blk-pci.event_idx'/> + <flag name='virtio-net-pci.event_idx'/> + <flag name='cache-directsync'/> + <flag name='piix3-usb-uhci'/> + <flag name='piix4-usb-uhci'/> + <flag name='usb-ehci'/> + <flag name='ich9-usb-ehci1'/> + <flag name='vt82c686b-usb-uhci'/> + <flag name='pci-ohci'/> + <flag name='usb-hub'/> + <flag name='no-shutdown'/> + <flag name='cache-unsafe'/> + <flag name='rombar'/> + <flag name='ich9-ahci'/> + <flag name='no-acpi'/> + <flag name='fsdev-readonly'/> + <flag name='virtio-blk-pci.scsi'/> + <flag name='blk-sg-io'/> + <flag name='drive-copy-on-read'/> + <flag name='cpu-host'/> + <flag name='fsdev-writeout'/> + <flag name='drive-iotune'/> + <flag name='system_wakeup'/> + <flag name='scsi-disk.channel'/> + <flag name='scsi-block'/> + <flag name='transaction'/> + <flag name='block-job-async'/> + <flag name='scsi-cd'/> + <flag name='ide-cd'/> + <flag name='no-user-config'/> + <flag name='hda-micro'/> + <flag name='dump-guest-memory'/> + <flag name='nec-usb-xhci'/> + <flag name='balloon-event'/> + <flag name='bridge'/> + <flag name='lsi'/> + <flag name='virtio-scsi-pci'/> + <flag name='blockio'/> + <flag name='disable-s3'/> + <flag name='disable-s4'/> + <flag name='ide-drive.wwn'/> + <flag name='scsi-disk.wwn'/> + <flag name='seccomp-sandbox'/> + <flag name='dump-guest-core'/> + <flag name='seamless-migration'/> + <flag name='block-commit'/> + <flag name='vnc'/> + <flag name='drive-mirror'/> + <flag name='usb-host.bootindex'/> + <flag name='qxl'/> + <flag name='VGA'/> + <flag name='cirrus-vga'/> + <flag name='vmware-svga'/> + <flag name='device-video-primary'/> + <flag name='usb-serial'/> + <flag name='usb-net'/> + <flag name='add-fd'/> + <flag name='nbd-server'/> + <flag name='virtio-rng'/> + <flag name='rng-random'/> + <flag name='rng-egd'/> + <flag name='dtb'/> + <flag name='megasas'/> + <flag name='ipv6-migration'/> + <flag name='machine-opt'/> + <flag name='machine-usb-opt'/> + <flag name='pci-bridge'/> + <flag name='vfio-pci'/> + <flag name='vfio-pci.bootindex'/> + <flag name='scsi-generic'/> + <flag name='scsi-generic.bootindex'/> + <flag name='mem-merge'/> + <flag name='vnc-websocket'/> + <flag name='mlock'/> + <flag name='vnc-share-policy'/> + <flag name='device-del-event'/> + <flag name='dmi-to-pci-bridge'/> + <flag name='i440fx-pci-hole64-size'/> + <flag name='q35-pci-hole64-size'/> + <flag name='usb-storage'/> + <flag name='usb-storage.removable'/> + <flag name='virtio-mmio'/> + <flag name='ich9-intel-hda'/> + <flag name='kvm-pit-lost-tick-policy'/> + <flag name='boot-strict'/> + <flag name='pvpanic'/> + <flag name='reboot-timeout'/> + <flag name='enable-fips'/> + </qemuCaps> diff --git a/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml b/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml new file mode 100644 index 0000000..30c3ef1 --- /dev/null +++ b/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml @@ -0,0 +1,51 @@ +<capabilities> + + <host> + <cpu> + <arch>i686</arch> + </cpu> + <power_management/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='i686'> + <wordsize>32</wordsize> + <emulator>/usr/bin/qemu-system-i386</emulator> + <domain type='qemu'> + </domain> + <domain type='kvm'> + <emulator>/usr/bin/qemu-kvm</emulator> + <machine>pc-i440fx-1.6</machine> + <machine>none</machine> + <machine canonical='pc-i440fx-1.6'>pc</machine> + <machine>pc-i440fx-1.5</machine> + <machine>pc-i440fx-1.4</machine> + <machine>pc-1.3</machine> + <machine>pc-1.2</machine> + <machine>pc-1.1</machine> + <machine>pc-1.0</machine> + <machine>pc-0.15</machine> + <machine>pc-0.14</machine> + <machine>pc-0.13</machine> + <machine>pc-0.12</machine> + <machine>pc-0.11</machine> + <machine>pc-0.10</machine> + <machine>isapc</machine> + <machine canonical='pc-q35-1.6'>q35</machine> + <machine>pc-q35-1.6</machine> + <machine>pc-q35-1.5</machine> + <machine>pc-q35-1.4</machine> + </domain> + </arch> + <features> + <deviceboot/> + <disksnapshot default='off' toggle='no'/> + <acpi default='on' toggle='yes'/> + <apic default='on' toggle='no'/> + <pae/> + <nonpae/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmltest.c b/tests/qemucaps2xmltest.c new file mode 100644 index 0000000..268842a --- /dev/null +++ b/tests/qemucaps2xmltest.c @@ -0,0 +1,217 @@ +/* + * Copyright (C) 2014 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + */ + +#include <config.h> + +#include "testutils.h" +#include "qemu/qemu_capabilities.h" + + +#define VIR_FROM_THIS VIR_FROM_NONE + + +static int +testCompareXMLToXML(const char *inxmldata, const char *outxmldata) +{ + int ret = 0; + + if (STRNEQ(outxmldata, inxmldata)) { + virtTestDifference(stderr, outxmldata, inxmldata); + ret = -1; + } + + return ret; +} + + +typedef struct _testQemuData testQemuData; +typedef testQemuData *testQemuDataPtr; +struct _testQemuData { + const char *base; + virArch hostarch; + virArch guestarch; +}; + +static virQEMUCapsPtr +testQemuGetCaps(char *caps) +{ + virQEMUCapsPtr qemuCaps = NULL; + xmlDocPtr xml; + xmlXPathContextPtr ctxt = NULL; + ssize_t i, n; + xmlNodePtr *nodes = NULL; + + if (!(xml = virXMLParseStringCtxt(caps, "(test caps)", &ctxt))) + goto error; + + if ((n = virXPathNodeSet("/qemuCaps/flag", ctxt, &nodes)) < 0) { + fprintf(stderr, "failed to parse qemu capabilities flags"); + goto error; + } + + if (n > 0) { + if (!(qemuCaps = virQEMUCapsNew())) + goto error; + + for (i = 0; i < n; i++) { + char *str = virXMLPropString(nodes[i], "name"); + if (str) { + int flag = virQEMUCapsTypeFromString(str); + if (flag < 0) { + fprintf(stderr, "Unknown qemu capabilities flag %s", str); + VIR_FREE(str); + goto error; + } + VIR_FREE(str); + virQEMUCapsSet(qemuCaps, flag); + } + } + } + + VIR_FREE(nodes); + xmlFreeDoc(xml); + xmlXPathFreeContext(ctxt); + return qemuCaps; + +error: + VIR_FREE(nodes); + virObjectUnref(qemuCaps); + xmlFreeDoc(xml); + xmlXPathFreeContext(ctxt); + return NULL; +} + +static virCapsPtr +testGetCaps(char *capsData, const testQemuData *data) +{ + virQEMUCapsCachePtr cache = NULL; + virQEMUCapsPtr qemuCaps = NULL; + virCapsPtr caps = NULL; + + if ((cache = virQEMUCapsCacheNew("", -1, -1)) == NULL) { + fprintf(stderr, "failed to create the qemu capabilities cache"); + goto error; + } + + if ((qemuCaps = testQemuGetCaps(capsData)) == NULL) { + fprintf(stderr, "failed to parse qemu capabilities flags"); + goto error; + } + + virQEMUCapsCacheAdd(cache, "kvm", qemuCaps); + virQEMUCapsCacheAdd(cache, "qemu", qemuCaps); + virQEMUCapsCacheAdd(cache, "qemu-system-x86_64", qemuCaps); + virQEMUCapsCacheAdd(cache, "qemu-system-i386", qemuCaps); + virQEMUCapsCacheAdd(cache, "/usr/bin/qemu-system-x86_64", qemuCaps); + virQEMUCapsCacheAdd(cache, "/usr/bin/qemu-system-i386", qemuCaps); + + if ((caps = virCapabilitiesNew(data->hostarch, 0, 0)) == NULL) { + fprintf(stderr, "failed to create the fake capabilities"); + goto error; + } + + if (virQEMUCapsInitGuest(caps, cache, data->hostarch, data->hostarch) < 0) { + fprintf(stderr, "failed to initialize the guest fake capabilities"); + goto error; + } + + return caps; + +error: + VIR_FREE(cache); + virObjectUnref(qemuCaps); + return NULL; +} + +static int +testQemuCapsXML(const void *opaque) +{ + int ret = -1; + const testQemuData *data = opaque; + char *capsFile = NULL, *xmlFile = NULL; + char *capsData = NULL, *xmlData = NULL; + char *capsXml = NULL; + virCapsPtr capsProvided = NULL; + + if (virAsprintf(&xmlFile, "%s/qemucaps2xmldata/%s.xml", + abs_srcdir, data->base) < 0) + goto cleanup; + + if (virAsprintf(&capsFile, "%s/qemucaps2xmldata/%s.caps", + abs_srcdir, data->base) < 0) + goto cleanup; + + if (virtTestLoadFile(xmlFile, &xmlData) < 0) + goto cleanup; + + if (virtTestLoadFile(capsFile, &capsData) < 0) + goto cleanup; + + if (!(capsProvided = testGetCaps(capsData, data))) + goto cleanup; + + capsXml = virCapabilitiesFormatXML(capsProvided); + if (!capsXml) + goto cleanup; + + ret = testCompareXMLToXML(capsXml, xmlData); + +cleanup: + VIR_FREE(xmlFile); + VIR_FREE(capsFile); + VIR_FREE(capsXml); + VIR_FREE(capsData); + VIR_FREE(xmlData); + virObjectUnref(capsProvided); + return ret; +} + +static int +mymain(void) +{ + int ret = 0; + + testQemuData data; + +#if !WITH_YAJL + fputs("libvirt not compiled with yajl, skipping this test\n", stderr); + return EXIT_AM_SKIP; +#endif + + if (virThreadInitialize() < 0) + return EXIT_FAILURE; + + virEventRegisterDefaultImpl(); + +#define DO_TEST_FULL(name, host, guest) \ + data.base = name; \ + data.hostarch = host; \ + data.guestarch = guest; \ + if (virtTestRun(name, testQemuCapsXML, &data) < 0) \ + ret = -1 + +#define DO_TEST(name) DO_TEST_FULL(name, VIR_ARCH_I686, VIR_ARCH_I686) + + DO_TEST("all_1.6.0-1"); + DO_TEST("nodisksnapshot_1.6.0-1"); + + return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIRT_TEST_MAIN(mymain) -- 1.8.4.2
On 01/17/2014 08:31 AM, Francesco Romani wrote:
the test is loosely inspired from qemucapabilitiestest and qemuxml2xmltest.
Added a new test instead of extending an existing one because the feature being tested don't really fits nicely in any
s/don't/doesn't/ s/fits/fit/
existing place. --- .gitignore | 1 + tests/Makefile.am | 10 +- tests/qemucaps2xmldata/all_1.6.0-1.caps | 142 ++++++++++++++ tests/qemucaps2xmldata/all_1.6.0-1.xml | 51 +++++ tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps | 141 +++++++++++++ tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml | 51 +++++ tests/qemucaps2xmltest.c | 217 +++++++++++++++++++++ 7 files changed, 612 insertions(+), 1 deletion(-) create mode 100644 tests/qemucaps2xmldata/all_1.6.0-1.caps create mode 100644 tests/qemucaps2xmldata/all_1.6.0-1.xml create mode 100644 tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps create mode 100644 tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml create mode 100644 tests/qemucaps2xmltest.c
Fails 'make check' for me: $ make -C tests check TESTS=qemucaps2xmltest \ VERBOSE=yes VIR_TEST_DEBUG=1 ... TEST: qemucaps2xmltest 1) all_1.6.0-1 ... Offset 393 Expect [6</machine> <machine>none</machine> <machine canonical='pc-i440fx-1.6'>pc</machine> <machine>pc-i440fx-1.5</machine> <machine>pc-i440fx-1.4</machine> ... Actual [7</machine> <machine>none</machine> <machine canonical='pc-i440fx-1.7'>pc</machine> <machine>pc-i440fx-1.6</machine> <machine>pc-i440fx-1.5</machine> which means you wrote the test by probing the installed qemu, instead of our testsuite helper binary, and my installed qemu 1.7 differs from your setup of 1.6. That has to be fixed before this can be applied.
+++ b/tests/qemucaps2xmltest.c @@ -0,0 +1,217 @@ +/*
+ + virQEMUCapsCacheAdd(cache, "kvm", qemuCaps); + virQEMUCapsCacheAdd(cache, "qemu", qemuCaps); + virQEMUCapsCacheAdd(cache, "qemu-system-x86_64", qemuCaps); + virQEMUCapsCacheAdd(cache, "qemu-system-i386", qemuCaps); + virQEMUCapsCacheAdd(cache, "/usr/bin/qemu-system-x86_64", qemuCaps); + virQEMUCapsCacheAdd(cache, "/usr/bin/qemu-system-i386", qemuCaps);
Is this where the testsuite ended up probing the installed file system instead of using only data from the testsuite? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
----- Original Message -----
From: "Francesco Romani" <fromani@redhat.com> To: libvir-list@redhat.com Cc: "Francesco Romani" <fromani@redhat.com> Sent: Friday, January 17, 2014 4:31:47 PM Subject: [libvirt] [PATCH 0/4] qemu: export disk snapshot capability
This patch series extend the QEMU capabilities XML to report if the underlying QEMU binary supports, or not, the live disk snapshotting.
ping? -- Francesco Romani RedHat Engineering Virtualization R & D Phone: 8261328 IRC: fromani
----- Original Message -----
From: "Francesco Romani" <fromani@redhat.com> To: libvir-list@redhat.com Cc: "Francesco Romani" <fromani@redhat.com> Sent: Friday, January 17, 2014 4:31:47 PM Subject: [PATCH 0/4] qemu: export disk snapshot capability
This patch series extend the QEMU capabilities XML to report if the underlying QEMU binary supports, or not, the live disk snapshotting.
Without this patch series, the only way to know if QEMU has this support is to actually request a disk snapshot and to see what happens.
Hi again! Could please someone take a look and review this? Any comment would be really appreciated! Thanks and best regards, -- Francesco Romani RedHat Engineering Virtualization R & D Phone: 8261328 IRC: fromani
----- Original Message -----
This patch series extend the QEMU capabilities XML to report if the underlying QEMU binary supports, or not, the live disk snapshotting. Could please someone take a look and review this? Any comment would be really appreciated!
ping -- Francesco Romani RedHat Engineering Virtualization R & D Phone: 8261328 IRC: fromani
On 02/19/2014 01:09 AM, Francesco Romani wrote:
----- Original Message -----
This patch series extend the QEMU capabilities XML to report if the underlying QEMU binary supports, or not, the live disk snapshotting. Could please someone take a look and review this? Any comment would be really appreciated!
ping
Apologies for the delay. This series was submitted before the freeze, so I hope it's low enough risk to commit into 1.2.2 if the review goes well. I'm doing the review now... -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/17/2014 08:31 AM, Francesco Romani wrote:
This patch series extend the QEMU capabilities XML to report if the underlying QEMU binary supports, or not, the live disk snapshotting.
Without this patch series, the only way to know if QEMU has this support is to actually request a disk snapshot and to see what happens.
The change is split in four patches:
* patch #1 actually adds the new element in the QEMU capabilities XML. formatcaps.html.in wasn't very detailed about the actual XML format, so I've not updated it. Anyone feel free to point out what should be added, and I'll comply.
The new element has the form <disksnapshot default='value' toggle='off'> because I'd like to convey two informations: - disk snapshot is supposed to be here, and it is (default='on') - disk snapshot is supposed to be here, and is NOT (default='off')
Put in a different way, I tried to help the client as much as possible. I'm not particolary fond of this format, and I'm really open to alternatives here. Perhaps a simpler <disksnapshot/> element can convey the same meaning? Suggestions welcome.
Using merely <disksnapshot/> is too terse - that element can only appear when qemu is new enough, but if it is absent, you don't know whether libvirt is new enough to tell you status but qemu is too old, or whether libvirt is too old to tell you status. The toggle='off' attribute is a bit much, but your choice of XML is at least consistent with other features (that is, seeing the <disksnapshot> element tells the client that this libvirt is new enough to support telling you the capability via the 'default=' attribute; where a missing <disksnapshot> is a sign of an older libvirt where you have to just try the command. The toggle='off' is a bit overkill, since it is not an option that can change state over the life of qemu or via any choice in the <domain> XML, but it is at least consistent with other feature capabilities, so a client can write a generic parser that handles all sorts of features, even the ones with toggle='yes' that can be overridden on a per-<domain> basis).
* patches #2, #3 Are trivial and they provide the ground for the last patch which add a new unit tests. They are just dependencies for it. I tried to make them less invasive as possible.
* patch #4 add a new unit test, aiming to test not only this new feature but also the whole XML capabilities test. I was under the impression that this kind of test do not really fit into existing one, so I added a new one. Suggestions about possible improvements for this test are welcome
Thanks for adding tests as part of your submission, and again I apologize that no one has reviewed this for so long. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Francesco Romani