Re: [libvirt] [PATCH 0/4] qemu: export disk snapshot capability
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
Attachments:
- signature.asc (application/pgp-signature — 604 bytes)