9:31 a.m.
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
9:31 a.m.
New subject: [libvirt] [PATCH 1/4] qemu: export disk snapshot support in capabilities
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(a)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
4:30 p.m.
New subject: [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
2:32 a.m.
New subject: [libvirt] [PATCH 1/4] qemu: export disk snapshot support in capabilities
Hi,
thanks for the review!
----- Original Message -----
From: "Eric Blake" <eblake(a)redhat.com>
To: "Francesco Romani" <fromani(a)redhat.com>, libvir-list(a)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
9:31 a.m.
New subject: [libvirt] [PATCH 2/4] qemu: add function to fill capabilities cache
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
9:28 p.m.
New subject: [libvirt] [PATCH 2/4] qemu: add function to fill capabilities cache
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
9:31 a.m.
New subject: [libvirt] [PATCH 3/4] qemu: export the virQEMUCapsInitGuest function.
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
11:20 p.m.
New subject: [libvirt] [PATCH 3/4] qemu: export the virQEMUCapsInitGuest function.
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
9:31 a.m.
New subject: [libvirt] [PATCH 4/4] qemu: add unit tests for the capabilities xml
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
5:49 p.m.
New subject: [libvirt] [PATCH 4/4] qemu: add unit tests for the capabilities xml
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
3:10 a.m.
----- Original Message -----
From: "Francesco Romani" <fromani(a)redhat.com>
To: libvir-list(a)redhat.com
Cc: "Francesco Romani" <fromani(a)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
5:32 a.m.
----- Original Message -----
From: "Francesco Romani" <fromani(a)redhat.com>
To: libvir-list(a)redhat.com
Cc: "Francesco Romani" <fromani(a)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
2:09 a.m.
----- 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
4:17 p.m.
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
5:51 p.m.
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
3923
days inactive
3964
days old
14 comments
2 participants
participants (2)
-
Eric Blake -
Francesco Romani