Re: [libvirt] [PATCH 1/n] dumpxml: add flag to virDomainGetXMLDesc

2014-11-18 Thread Eric Blake
On 10/23/2014 07:52 AM, Adam Litke wrote:
 On 17/09/14 08:43 -0600, Eric Blake wrote:
 + * If @flags contains VIR_DOMAIN_XML_BLOCK_INFO, the listing for each
 + * disk device will contain additional information such as capacity
 + * and allocation, similar to what is displayed by
 virStorageVolGetXMLDesc(),
 + * and avoiding the need to call virDomainGetBlockInfo() separately.
 + *
 
 Eric:  We decided to go with this extra flag since gathering the block
 info was seen as a potentially expensive operation which users would
 want to selectively enable.  Do you have any thoughts on how expensive
 this would be in practice?  We are considering enabling it
 unconditionally in oVirt and am weighing a decision between A)
 Simplicity and Cacheability vs. B) Performance.

I just posted a v2 patch.  The extra expense is that I have to obtain a
job lock in order to call into the qemu monitor.  Thankfully, it's only
a read lock (so it can be done even during a long-running operation like
a migration), but obtaining that lock is not instant.  It's still
probably not noticeable in the long run, but avoiding the flag avoids
the need for the job lock in the first place.

-- 
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 1/n] dumpxml: add flag to virDomainGetXMLDesc

2014-10-23 Thread Adam Litke

On 17/09/14 08:43 -0600, Eric Blake wrote:

+ * If @flags contains VIR_DOMAIN_XML_BLOCK_INFO, the listing for each
+ * disk device will contain additional information such as capacity
+ * and allocation, similar to what is displayed by virStorageVolGetXMLDesc(),
+ * and avoiding the need to call virDomainGetBlockInfo() separately.
+ *


Eric:  We decided to go with this extra flag since gathering the block
info was seen as a potentially expensive operation which users would
want to selectively enable.  Do you have any thoughts on how expensive
this would be in practice?  We are considering enabling it
unconditionally in oVirt and am weighing a decision between A)
Simplicity and Cacheability vs. B) Performance.

Thanks.

--
Adam Litke

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


[libvirt] [PATCH 1/n] dumpxml: add flag to virDomainGetXMLDesc

2014-09-17 Thread Eric Blake
The information in virDomainGetBlockInfo() is important for clients
that use qcow2 format on LVM block devices - by tracking the
allocation in relation to the physical size, management can tell
if a disk needs to be expanded before the guest (file system
contents) and/or qemu (copy-on-write differing more from a backing
file) runs out of space.  Normally, only the active layer matters,
but during a block commit operation, the allocation of the backing
file ALSO grows, and management would like to track that growth.

Right now, virDomainGetBlockInfo() can only convey information
about the active layer of a disk, via a single API call per disk.
It can also be easily extended to support vda[1] notation that
we recently added for blockcommit and friends, to get similar
information about a backing element; but that still implies one
call per layer, which adds up to a lot of overhead.

This API addition will make it possible to grab this information
for ALL guest disks in a single call, by letting the user request
additional information about each disk in the backing chain to be
output as part of the domain XML.  My ultimate goal is to have
this flag and virStorageVolGetXMLDesc() expose the same bits of
information about a given storage volume (there are slight
incompatiblities in the XML naming that we'll have to keep for
back-compat sake, but the idea is to get all the information out
there).

* include/libvirt/libvirt.h.in (virDomainXMLFlags): Add new flag.
* src/libvirt.c (virDomainGetXMLDesc): Document it.
(virDomainSaveImageGetXMLDesc, virDomainSnapshotGetXMLDesc): For
now, mention the flag won't help here.
* tools/virsh-domain.c (cmdDumpXML): Add new flag.
* tools/virsh.pod (dumpxml): Document it.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 include/libvirt/libvirt.h.in |  1 +
 src/libvirt.c| 15 +++
 tools/virsh-domain.c |  6 ++
 tools/virsh.pod  |  6 --
 4 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index c2f9d26..40f4e0c 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2082,6 +2082,7 @@ typedef enum {
 VIR_DOMAIN_XML_INACTIVE = (1  1), /* dump inactive domain 
information */
 VIR_DOMAIN_XML_UPDATE_CPU   = (1  2), /* update guest CPU requirements 
according to host CPU */
 VIR_DOMAIN_XML_MIGRATABLE   = (1  3), /* dump XML suitable for migration 
*/
+VIR_DOMAIN_XML_BLOCK_INFO   = (1  4), /* include storage volume 
information about each disk source */
 } virDomainXMLFlags;

 char *  virDomainGetXMLDesc (virDomainPtr domain,
diff --git a/src/libvirt.c b/src/libvirt.c
index f7e5a37..1020cc5 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -2806,8 +2806,9 @@ virDomainRestoreFlags(virConnectPtr conn, const char 
*from, const char *dxml,
  *
  * No security-sensitive data will be included unless @flags contains
  * VIR_DOMAIN_XML_SECURE; this flag is rejected on read-only
- * connections.  For this API, @flags should not contain either
- * VIR_DOMAIN_XML_INACTIVE or VIR_DOMAIN_XML_UPDATE_CPU.
+ * connections.  For this API, @flags should not contain
+ * VIR_DOMAIN_XML_INACTIVE, VIR_DOMAIN_XML_UPDATE_CPU, or
+ * VIR_DOMAIN_XML_BLOCK_INFO.
  *
  * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of
  * error.  The caller must free() the returned value.
@@ -4354,6 +4355,11 @@ virDomainGetControlInfo(virDomainPtr domain,
  * describing CPU capabilities is modified to match actual
  * capabilities of the host.
  *
+ * If @flags contains VIR_DOMAIN_XML_BLOCK_INFO, the listing for each
+ * disk device will contain additional information such as capacity
+ * and allocation, similar to what is displayed by virStorageVolGetXMLDesc(),
+ * and avoiding the need to call virDomainGetBlockInfo() separately.
+ *
  * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error.
  * the caller must free() the returned value.
  */
@@ -18507,8 +18513,9 @@ virDomainSnapshotCreateXML(virDomainPtr domain,
  *
  * No security-sensitive data will be included unless @flags contains
  * VIR_DOMAIN_XML_SECURE; this flag is rejected on read-only
- * connections.  For this API, @flags should not contain either
- * VIR_DOMAIN_XML_INACTIVE or VIR_DOMAIN_XML_UPDATE_CPU.
+ * connections.  For this API, @flags should not contain
+ * VIR_DOMAIN_XML_INACTIVE, VIR_DOMAIN_XML_UPDATE_CPU, or
+ * VIR_DOMAIN_XML_BLOCK_INFO.
  *
  * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error.
  * the caller must free() the returned value.
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 435d045..8f97c48 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8838,6 +8838,10 @@ static const vshCmdOptDef opts_dumpxml[] = {
  .type = VSH_OT_BOOL,
  .help = N_(provide XML suitable for migrations)
 },
+{.name = block-info,
+ .type =