[libvirt] [PATCH v2 1/7] dumpxml: add flag to virDomainGetXMLDesc
Adam Litke
alitke at redhat.com
Tue Nov 18 14:50:09 UTC 2014
On 18/11/14 06:31 -0700, Eric Blake wrote:
>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 <eblake at redhat.com>
>---
> include/libvirt/libvirt-domain.h | 1 +
> src/libvirt-domain-snapshot.c | 5 +++--
> src/libvirt-domain.c | 10 ++++++++--
> tools/virsh-domain.c | 6 ++++++
> tools/virsh.pod | 6 ++++--
> 5 files changed, 22 insertions(+), 6 deletions(-)
>
>diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
>index 1fac2a3..caf1f46 100644
>--- a/include/libvirt/libvirt-domain.h
>+++ b/include/libvirt/libvirt-domain.h
>@@ -1248,6 +1248,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 */
I'll admit I'm not a master API designer but this is a red flag for me
(pun most definitely intended). Up to this point, the individual
virDomainXMLFlags are used to ask for XML for a certain purpose. For
example, VIR_DOMAIN_XML_INACTIVE retrieves XML suitable for a call to
virDomainCreateXML whereas VIR_DOMAIN_XML_MIGRATABLE is used to "dump
XML suitable for migration".
This new flag is somewhat arbitrarily slicing up some of the ACTIVE VM
information. Slippery slope logic would lead us to a future API with
a proliferation of flags that turn various bits of info on and off
which would be very cumbersome to maintain and use. Since this
information is really ACTIVE VM info, I'd prefer it to be provided
whenever VIR_DOMAIN_XML_INACTIVE is not set. Callers who want a
tight XML document can always use VIR_DOMAIN_XML_INACTIVE.
> } virDomainXMLFlags;
>
> char * virDomainGetXMLDesc (virDomainPtr domain,
>diff --git a/src/libvirt-domain-snapshot.c b/src/libvirt-domain-snapshot.c
>index 9feb669..f12af45 100644
>--- a/src/libvirt-domain-snapshot.c
>+++ b/src/libvirt-domain-snapshot.c
>@@ -269,8 +269,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/src/libvirt-domain.c b/src/libvirt-domain.c
>index 2b0defc..2ef1a51 100644
>--- a/src/libvirt-domain.c
>+++ b/src/libvirt-domain.c
>@@ -1086,8 +1086,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.
>@@ -2592,6 +2593,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.
> */
>diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
>index a7e9151..4c63006 100644
>--- a/tools/virsh-domain.c
>+++ b/tools/virsh-domain.c
>@@ -8984,6 +8984,10 @@ static const vshCmdOptDef opts_dumpxml[] = {
> .type = VSH_OT_BOOL,
> .help = N_("provide XML suitable for migrations")
> },
>+ {.name = "block-info",
>+ .type = VSH_OT_BOOL,
>+ .help = N_("include additional block info for each <disk> in XML dump")
>+ },
> {.name = NULL}
> };
>
>@@ -9007,6 +9011,8 @@ cmdDumpXML(vshControl *ctl, const vshCmd *cmd)
> flags |= VIR_DOMAIN_XML_UPDATE_CPU;
> if (migratable)
> flags |= VIR_DOMAIN_XML_MIGRATABLE;
>+ if (vshCommandOptBool(cmd, "block-info"))
>+ flags |= VIR_DOMAIN_XML_BLOCK_INFO;
>
> if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
> return false;
>diff --git a/tools/virsh.pod b/tools/virsh.pod
>index d5608cc..28f928f 100644
>--- a/tools/virsh.pod
>+++ b/tools/virsh.pod
>@@ -1290,7 +1290,7 @@ NOTE: Some hypervisors may require the user to manually ensure proper
> permissions on file and path specified by argument I<corefilepath>.
>
> =item B<dumpxml> I<domain> [I<--inactive>] [I<--security-info>]
>-[I<--update-cpu>] [I<--migratable>]
>+[I<--update-cpu>] [I<--migratable>] [I<--block-info>]
>
> Output the domain information as an XML dump to stdout, this format can be used
> by the B<create> command. Additional options affecting the XML dump may be
>@@ -1301,7 +1301,9 @@ in the XML dump. I<--update-cpu> updates domain CPU requirements according to
> host CPU. With I<--migratable> one can request an XML that is suitable for
> migrations, i.e., compatible with older libvirt releases and possibly amended
> with internal run-time options. This option may automatically enable other
>-options (I<--update-cpu>, I<--security-info>, ...) as necessary.
>+options (I<--update-cpu>, I<--security-info>, ...) as necessary. Using
>+I<--block-info> will add additional information about each <disk> source,
>+comparable to what B<vol-dumpxml> or B<domblkinfo> would show.
>
> =item B<edit> I<domain>
>
>--
>1.9.3
>
--
Adam Litke
More information about the libvir-list
mailing list