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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list