[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 02/14] snapshot: indent domain xml when nesting, round 2



On Thu, Sep 22, 2011 at 02:34:56PM -0600, Eric Blake wrote:
> <domainsnapshot> is the first public instance of <domain> being
> used as a sub-element, although we have two other private uses
> (runtime state, and migration cookie).  Although indentation has
> no effect on XML parsing, using it makes the output more consistent.
> 
> The overall process of adding indentation will be rather mechanical,
> but breaking it into steps will help review.  This step adds the
> framework to request the indentation.
> 
> * src/conf/domain_conf.h (virDomainDefFormatInternal): New prototype.
> * src/conf/domain_conf.c (virDomainDefFormatInternal): Add
> parameter, and export.
> (virDomainDefFormat, virDomainObjFormat): Update callers.
> * src/libvirt_private.syms (domain_conf.h): Add new export.
> * src/qemu/qemu_migration.c (qemuMigrationCookieXMLFormat): Use
> new function.
> (qemuMigrationCookieXMLFormatStr): Update caller.
> ---
>  src/conf/domain_conf.c    |   24 +++++++++++++++---------
>  src/conf/domain_conf.h    |    4 ++++
>  src/libvirt_private.syms  |    1 +
>  src/qemu/qemu_migration.c |   23 ++++++++++++-----------
>  4 files changed, 32 insertions(+), 20 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 7463d7c..3e3be3c 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -10450,14 +10450,14 @@ verify(((VIR_DOMAIN_XML_INTERNAL_STATUS |
> 
>  /* This internal version can accept VIR_DOMAIN_XML_INTERNAL_*,
>   * whereas the public version cannot.  Also, it appends to an existing
> - * buffer, rather than flattening to string.  Return -1 on failure.  */
> -static int
> + * buffer, rather than flattening to string, as well as allowing
> + * additional indentation.  Return -1 on failure.  */
> +int
>  virDomainDefFormatInternal(virDomainDefPtr def,
> +                           int indent,
>                             unsigned int flags,
>                             virBufferPtr buf)
>  {
> -    /* XXX Also need to take an indentation parameter - either int or
> -     * string prefix, so that snapshot xml gets uniform indentation.  */
>      unsigned char *uuid;
>      char uuidstr[VIR_UUID_STRING_BUFLEN];
>      const char *type = NULL;
> @@ -10477,13 +10477,16 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>      if (def->id == -1)
>          flags |= VIR_DOMAIN_XML_INACTIVE;
> 
> -    virBufferAsprintf(buf, "<domain type='%s'", type);
> +    virBufferAsprintf(buf, "%*s<domain type='%s'", indent, "", type);

  Hum I have never seen that formatting command used before. I would
rather add a virBufferIndentAsprintf() instead to be honnest and avoid
this, that's clearer from my POV.

>      if (!(flags & VIR_DOMAIN_XML_INACTIVE))
>          virBufferAsprintf(buf, " id='%d'", def->id);
>      if (def->namespaceData && def->ns.href)
>          virBufferAsprintf(buf, " %s", (def->ns.href)());
>      virBufferAddLit(buf, ">\n");
> 
> +    indent += 2;
> +    /* XXX Fix indentation of the body */
> +
>      virBufferEscapeString(buf, "  <name>%s</name>\n", def->name);
> 
>      uuid = def->uuid;
> @@ -10895,7 +10898,10 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>              goto cleanup;
>      }
> 
> -    virBufferAddLit(buf, "</domain>\n");
> +    /* XXX Fix indentation of body prior to here */
> +    indent -= 2;
> +
> +    virBufferIndentAddLit(buf, indent, "</domain>\n");
> 
>      if (virBufferError(buf))
>          goto no_memory;
> @@ -10915,7 +10921,7 @@ virDomainDefFormat(virDomainDefPtr def, unsigned int flags)
>      virBuffer buf = VIR_BUFFER_INITIALIZER;
> 
>      virCheckFlags(DUMPXML_FLAGS, NULL);
> -    if (virDomainDefFormatInternal(def, flags, &buf) < 0)
> +    if (virDomainDefFormatInternal(def, 0, flags, &buf) < 0)
>          return NULL;
> 
>      return virBufferContentAndReset(&buf);
> @@ -10947,7 +10953,7 @@ static char *virDomainObjFormat(virCapsPtr caps,
>          ((caps->privateDataXMLFormat)(&buf, obj->privateData)) < 0)
>          goto error;
> 
> -    if (virDomainDefFormatInternal(obj->def, flags, &buf) < 0)
> +    if (virDomainDefFormatInternal(obj->def, 2, flags, &buf) < 0)
>          goto error;
> 
>      virBufferAddLit(&buf, "</domstatus>\n");
> @@ -11963,7 +11969,7 @@ char *virDomainSnapshotDefFormat(char *domain_uuid,
>          virBufferAddLit(&buf, "  </disks>\n");
>      }
>      if (def->dom) {
> -        virDomainDefFormatInternal(def->dom, flags, &buf);
> +        virDomainDefFormatInternal(def->dom, 2, flags, &buf);
>      } else {
>          virBufferAddLit(&buf, "  <domain>\n");
>          virBufferAsprintf(&buf, "    <uuid>%s</uuid>\n", domain_uuid);
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 371f270..7e3c06c 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1643,6 +1643,10 @@ int virDomainDefAddImplicitControllers(virDomainDefPtr def);
> 
>  char *virDomainDefFormat(virDomainDefPtr def,
>                           unsigned int flags);
> +int virDomainDefFormatInternal(virDomainDefPtr def,
> +                               int indent,
> +                               unsigned int flags,
> +                               virBufferPtr buf);
> 
>  int virDomainCpuSetParse(const char **str,
>                           char sep,
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 1523289..f4064c0 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -265,6 +265,7 @@ virDomainDefCheckABIStability;
>  virDomainDefClearDeviceAliases;
>  virDomainDefClearPCIAddresses;
>  virDomainDefFormat;
> +virDomainDefFormatInternal;
>  virDomainDefFree;
>  virDomainDefParseFile;
>  virDomainDefParseNode;
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 0a5a13d..a131c5c 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -378,12 +378,12 @@ static void qemuMigrationCookieGraphicsXMLFormat(virBufferPtr buf,
>  }
> 
> 
> -static void qemuMigrationCookieXMLFormat(virBufferPtr buf,
> -                                         qemuMigrationCookiePtr mig)
> +static int
> +qemuMigrationCookieXMLFormat(virBufferPtr buf,
> +                             qemuMigrationCookiePtr mig)
>  {
>      char uuidstr[VIR_UUID_STRING_BUFLEN];
>      char hostuuidstr[VIR_UUID_STRING_BUFLEN];
> -    char *domXML;
>      int i;
> 
>      virUUIDFormat(mig->uuid, uuidstr);
> @@ -415,15 +415,15 @@ static void qemuMigrationCookieXMLFormat(virBufferPtr buf,
>      }
> 
>      if ((mig->flags & QEMU_MIGRATION_COOKIE_PERSISTENT) &&
> -        mig->persistent) {
> -        domXML = virDomainDefFormat(mig->persistent,
> -                                    VIR_DOMAIN_XML_INACTIVE |
> -                                    VIR_DOMAIN_XML_SECURE);
> -        virBufferAdd(buf, domXML, -1);
> -        VIR_FREE(domXML);
> -    }
> +        mig->persistent &&
> +        virDomainDefFormatInternal(mig->persistent, 2,
> +                                   VIR_DOMAIN_XML_INACTIVE |
> +                                   VIR_DOMAIN_XML_SECURE,
> +                                   buf) < 0)
> +        return -1;
> 
>      virBufferAddLit(buf, "</qemu-migration>\n");
> +    return 0;
>  }
> 
> 
> @@ -431,7 +431,8 @@ static char *qemuMigrationCookieXMLFormatStr(qemuMigrationCookiePtr mig)
>  {
>      virBuffer buf = VIR_BUFFER_INITIALIZER;
> 
> -    qemuMigrationCookieXMLFormat(&buf, mig);
> +    if (qemuMigrationCookieXMLFormat(&buf, mig) < 0)
> +        return NULL;
> 
>      if (virBufferError(&buf)) {
>          virReportOOMError();

  That looks correct, I think pushing this would be fine but a separate
patch implementing and using virBufferIndentAsprintf() would be a nice
improvement,

  ACK,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]