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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list