[libvirt] [PATCH 0/3] v2: 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 three 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. This patch was alread reviewed in the first version of this changeset and it is unchanged. * patch #2 Extracts the actual QMEU guest capability inizialitation from the binary probe/capabilities code, in order to isolate the actual initialization from the probing part. I added a new function for the real initialization and left the interface unchanged. Existing code will still call the old code which do the probing and after that calls the new initialization function. The main purpose of this patch is to * allow to write an unit test for this change * make the unit test more robust (with respect the first version of this change) and make it independent from the filesystem layout/qemu availability * patch #3 add a new unit test, aiming to test not only this new feature but also the whole output XML capabilities. Francesco Romani (3): qemu: export disk snapshot support in capabilities qemu: extract guest capabilities initialization qemu: add unit tests for the capabilities xml docs/schemas/capability.rng | 6 + src/qemu/qemu_capabilities.c | 50 +++-- src/qemu/qemu_capabilities.h | 7 + tests/Makefile.am | 10 +- tests/qemucaps2xmldata/all_1.6.0-1.caps | 142 ++++++++++++++ tests/qemucaps2xmldata/all_1.6.0-1.xml | 31 ++++ tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps | 141 ++++++++++++++ tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml | 31 ++++ tests/qemucaps2xmltest.c | 206 +++++++++++++++++++++ 9 files changed, 609 insertions(+), 15 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.5.3
This patch adds an element to QEMU's capability XML, to show if the underlying QEMU binary supports the live disk snapshotting or not. This allows 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 47f8bcb..061ddae 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -700,6 +700,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 @@ -787,6 +788,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.5.3
this patch decouples the binary and the capabilities detection from the guest initialization. The purpose is to make testing easier. --- src/qemu/qemu_capabilities.c | 45 +++++++++++++++++++++++++++++--------------- src/qemu/qemu_capabilities.h | 7 +++++++ 2 files changed, 37 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 061ddae..6faef02 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -689,18 +689,12 @@ virQEMUCapsInitGuest(virCapsPtr caps, virArch hostarch, virArch guestarch) { - virCapsGuestPtr guest; size_t i; - bool haskvm = false; - bool haskqemu = false; char *kvmbin = NULL; char *binary = NULL; - virCapsGuestMachinePtr *machines = NULL; - size_t nmachines = 0; 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 @@ -748,6 +742,35 @@ virQEMUCapsInitGuest(virCapsPtr caps, } } + ret = virQEMUCapsInitGuestFromBinary(caps, + binary, qemubinCaps, + kvmbin, kvmbinCaps, + guestarch); + + VIR_FREE(binary); + VIR_FREE(kvmbin); + virObjectUnref(qemubinCaps); + virObjectUnref(kvmbinCaps); + + return ret; +} + +int +virQEMUCapsInitGuestFromBinary(virCapsPtr caps, + const char *binary, + virQEMUCapsPtr qemubinCaps, + const char *kvmbin, + virQEMUCapsPtr kvmbinCaps, + virArch guestarch) +{ + virCapsGuestPtr guest; + bool haskvm = false; + bool haskqemu = false; + virCapsGuestMachinePtr *machines = NULL; + size_t nmachines = 0; + int ret = -1; + bool hasdisksnapshot = false; + if (!binary) return 0; @@ -845,18 +868,10 @@ virQEMUCapsInitGuest(virCapsPtr caps, ret = 0; -cleanup: - VIR_FREE(binary); - VIR_FREE(kvmbin); - virObjectUnref(qemubinCaps); - virObjectUnref(kvmbinCaps); - - return ret; - error: virCapabilitiesFreeMachines(machines, nmachines); - goto cleanup; + return ret; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index a9082d5..df8c978 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -295,4 +295,11 @@ bool virQEMUCapsSupportsChardev(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, virDomainChrDefPtr chr); +int virQEMUCapsInitGuestFromBinary(virCapsPtr caps, + const char *binary, + virQEMUCapsPtr qemubinCaps, + const char *kvmbin, + virQEMUCapsPtr kvmbinCaps, + virArch guestarch); + #endif /* __QEMU_CAPABILITIES_H__*/ -- 1.8.5.3
On 17.03.2014 16:19, Francesco Romani wrote:
this patch decouples the binary and the capabilities detection from the guest initialization.
The purpose is to make testing easier. --- src/qemu/qemu_capabilities.c | 45 +++++++++++++++++++++++++++++--------------- src/qemu/qemu_capabilities.h | 7 +++++++ 2 files changed, 37 insertions(+), 15 deletions(-)
I had to rebase this, but that's not your fault.
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 061ddae..6faef02 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -689,18 +689,12 @@ virQEMUCapsInitGuest(virCapsPtr caps, virArch hostarch, virArch guestarch) { - virCapsGuestPtr guest; size_t i; - bool haskvm = false; - bool haskqemu = false; char *kvmbin = NULL; char *binary = NULL; - virCapsGuestMachinePtr *machines = NULL; - size_t nmachines = 0; 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 @@ -748,6 +742,35 @@ virQEMUCapsInitGuest(virCapsPtr caps, } }
+ ret = virQEMUCapsInitGuestFromBinary(caps, + binary, qemubinCaps, + kvmbin, kvmbinCaps, + guestarch); + + VIR_FREE(binary); + VIR_FREE(kvmbin); + virObjectUnref(qemubinCaps); + virObjectUnref(kvmbinCaps); + + return ret; +} + +int +virQEMUCapsInitGuestFromBinary(virCapsPtr caps, + const char *binary, + virQEMUCapsPtr qemubinCaps, + const char *kvmbin, + virQEMUCapsPtr kvmbinCaps, + virArch guestarch) +{ + virCapsGuestPtr guest; + bool haskvm = false; + bool haskqemu = false; + virCapsGuestMachinePtr *machines = NULL; + size_t nmachines = 0; + int ret = -1; + bool hasdisksnapshot = false; + if (!binary) return 0;
@@ -845,18 +868,10 @@ virQEMUCapsInitGuest(virCapsPtr caps,
ret = 0;
-cleanup: - VIR_FREE(binary); - VIR_FREE(kvmbin); - virObjectUnref(qemubinCaps); - virObjectUnref(kvmbinCaps); - - return ret; - error: virCapabilitiesFreeMachines(machines, nmachines);
- goto cleanup; + return ret; }
We tend to use cleanup in this pattern. Error path should be used only for error not for successful return path too. Such approach, however, requires the substitution of 'error' with 'cleanup' in the rest of the function. I'll take care of that. Michal
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. --- tests/Makefile.am | 10 +- tests/qemucaps2xmldata/all_1.6.0-1.caps | 142 ++++++++++++++ tests/qemucaps2xmldata/all_1.6.0-1.xml | 31 ++++ tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps | 141 ++++++++++++++ tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml | 31 ++++ tests/qemucaps2xmltest.c | 206 +++++++++++++++++++++ 6 files changed, 560 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/tests/Makefile.am b/tests/Makefile.am index 1745469..5705fed 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -90,6 +90,7 @@ EXTRA_DIST = \ nwfilterxml2xmlout \ oomtrace.pl \ qemucapabilitiesdata \ + qemucaps2xmldata \ qemuhelpdata \ qemuhotplugtestdata \ qemumonitorjsondata \ @@ -204,7 +205,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 @@ -507,6 +508,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 \ @@ -531,6 +538,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..425b22e --- /dev/null +++ b/tests/qemucaps2xmldata/all_1.6.0-1.xml @@ -0,0 +1,31 @@ +<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-system-i386</emulator> + </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..cd19814 --- /dev/null +++ b/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml @@ -0,0 +1,31 @@ +<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-system-i386</emulator> + </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..c447af7 --- /dev/null +++ b/tests/qemucaps2xmltest.c @@ -0,0 +1,206 @@ +/* + * 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 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) +{ + virQEMUCapsPtr qemuCaps = NULL; + virCapsPtr caps = NULL; + + if ((qemuCaps = testQemuGetCaps(capsData)) == NULL) { + fprintf(stderr, "failed to parse qemu capabilities flags"); + goto error; + } + + if ((caps = virCapabilitiesNew(data->guestarch, 0, 0)) == NULL) { + fprintf(stderr, "failed to create the fake capabilities"); + goto error; + } + + if (virQEMUCapsInitGuestFromBinary(caps, + "/usr/bin/qemu-system-i386", + qemuCaps, + NULL, + NULL, + data->guestarch) < 0) { + fprintf(stderr, "failed to create the capabilities from qemu"); + goto error; + } + + return caps; + +error: + 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, guest) \ + data.base = name; \ + data.guestarch = guest; \ + if (virtTestRun(name, testQemuCapsXML, &data) < 0) \ + ret = -1 + +#define DO_TEST(name) DO_TEST_FULL(name, 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.5.3
On 17.03.2014 16:19, 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 existing place. --- tests/Makefile.am | 10 +- tests/qemucaps2xmldata/all_1.6.0-1.caps | 142 ++++++++++++++ tests/qemucaps2xmldata/all_1.6.0-1.xml | 31 ++++ tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps | 141 ++++++++++++++ tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml | 31 ++++ tests/qemucaps2xmltest.c | 206 +++++++++++++++++++++ 6 files changed, 560 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/tests/qemucaps2xmltest.c b/tests/qemucaps2xmltest.c new file mode 100644 index 0000000..c447af7 --- /dev/null +++ b/tests/qemucaps2xmltest.c @@ -0,0 +1,206 @@ +/* + * 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/>. + * + */
I'm adding Authors line here among with your name.
+ +#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; +}
While this works, I'm changing it to match the pattern used in the rest of the code.
+ + +typedef struct _testQemuData testQemuData; +typedef testQemuData *testQemuDataPtr; +struct _testQemuData { + const char *base; + 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) {
There's no need for this. I mean, if there's no /qemuCaps/flag the virXPathNodeSet() returns 0 which may be handy if we want to test empty capabilities.
+ 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; +}
Michal
On 17.03.2014 16:19, 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 three 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. This patch was alread reviewed in the first version of this changeset and it is unchanged.
* patch #2 Extracts the actual QMEU guest capability inizialitation from the binary probe/capabilities code, in order to isolate the actual initialization from the probing part. I added a new function for the real initialization and left the interface unchanged. Existing code will still call the old code which do the probing and after that calls the new initialization function. The main purpose of this patch is to * allow to write an unit test for this change * make the unit test more robust (with respect the first version of this change) and make it independent from the filesystem layout/qemu availability
* patch #3 add a new unit test, aiming to test not only this new feature but also the whole output XML capabilities.
Francesco Romani (3): qemu: export disk snapshot support in capabilities qemu: extract guest capabilities initialization qemu: add unit tests for the capabilities xml
docs/schemas/capability.rng | 6 + src/qemu/qemu_capabilities.c | 50 +++-- src/qemu/qemu_capabilities.h | 7 + tests/Makefile.am | 10 +- tests/qemucaps2xmldata/all_1.6.0-1.caps | 142 ++++++++++++++ tests/qemucaps2xmldata/all_1.6.0-1.xml | 31 ++++ tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps | 141 ++++++++++++++ tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml | 31 ++++ tests/qemucaps2xmltest.c | 206 +++++++++++++++++++++ 9 files changed, 609 insertions(+), 15 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
ACKed and pushed. Michal
On 17.03.2014 16:19, 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 three 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. This patch was alread reviewed in the first version of this changeset and it is unchanged.
* patch #2 Extracts the actual QMEU guest capability inizialitation from the binary probe/capabilities code, in order to isolate the actual initialization from the probing part. I added a new function for the real initialization and left the interface unchanged. Existing code will still call the old code which do the probing and after that calls the new initialization function. The main purpose of this patch is to * allow to write an unit test for this change * make the unit test more robust (with respect the first version of this change) and make it independent from the filesystem layout/qemu availability
* patch #3 add a new unit test, aiming to test not only this new feature but also the whole output XML capabilities.
Francesco Romani (3): qemu: export disk snapshot support in capabilities qemu: extract guest capabilities initialization qemu: add unit tests for the capabilities xml
docs/schemas/capability.rng | 6 + src/qemu/qemu_capabilities.c | 50 +++-- src/qemu/qemu_capabilities.h | 7 + tests/Makefile.am | 10 +- tests/qemucaps2xmldata/all_1.6.0-1.caps | 142 ++++++++++++++ tests/qemucaps2xmldata/all_1.6.0-1.xml | 31 ++++ tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps | 141 ++++++++++++++ tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml | 31 ++++ tests/qemucaps2xmltest.c | 206 +++++++++++++++++++++ 9 files changed, 609 insertions(+), 15 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
ACKed and pushed. Michal
participants (2)
-
Francesco Romani -
Michal Privoznik