Re: [libvirt] [PATCH 0/4] qemu: export disk snapshot capability

2014-02-26 Thread Eric Blake
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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/4] qemu: export disk snapshot capability

2014-02-26 Thread Eric Blake
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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/4] qemu: export disk snapshot capability

2014-02-19 Thread Francesco Romani
- 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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 0/4] qemu: export disk snapshot capability

2014-02-10 Thread Francesco Romani
- Original Message -
 From: Francesco Romani from...@redhat.com
 To: libvir-list@redhat.com
 Cc: Francesco Romani from...@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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 0/4] qemu: export disk snapshot capability

2014-01-27 Thread Francesco Romani
- Original Message -
 From: Francesco Romani from...@redhat.com
 To: libvir-list@redhat.com
 Cc: Francesco Romani from...@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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 0/4] qemu: export disk snapshot capability

2014-01-17 Thread Francesco Romani
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list