[PATCH for 7.4.0] conf: Avoid double indentation of <metadata/> element

[Pavel Hrdina]

On Tue, May 25, 2021 at 09:32:35AM +0200, Michal Privoznik wrote:
> There was a recent change in libxml2 that caused a trouble for
> us. To us, <metadata/> in domain or network XMLs are just opaque
> value where management application can store whatever data it
> finds fit. At XML parser/formatter level, we just make a copy of
> the element during parsing and then format it back. For
> formatting we use xmlNodeDump() which allows caller to specify
> level of indentation. Previously, the indentation was not
> applied onto the very first line, but as of v2.9.12-2-g85b1792e
> libxml2 is applying indentation also on the first line.
> 
> This does not work well with out virBuffer because as soon as we
> call virBufferAsprintf() to append <metadata/> element,
> virBufferAsprintf() will apply another level of indentation.
> 
> Instead of version checking, let's skip any indentation added by
> libxml2 before virBufferAsprintf() is called.
> 
> Note, the problem is only when telling xmlNodeDump() to use
> indentation, i.e. level argument is not zero. Therefore,
> virXMLNodeToString() which also calls xmlNodeDump() is safe as it
> passes zero.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
> 
> I've raised this issue with libxml2 team:
> 
> https://gitlab.gnome.org/GNOME/libxml2/-/commit/85b1792e37b131e7a51af98a37f92472e8de5f3f
> 
> but I'm not sure it'll be fixed. Unfortunately, the patch that's causing
> us trouble is being backported all over the place, because it's
> supposedly fixing a regression.
> 
>  src/conf/domain_conf.c  | 9 ++++++++-
>  src/conf/network_conf.c | 9 ++++++++-
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 4f78b7b43d..84a8c269be 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -27808,6 +27808,7 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def,
>  
>      if (def->metadata) {
>          g_autoptr(xmlBuffer) xmlbuf = NULL;
> +        const char *xmlbufContent = NULL;
>          int oldIndentTreeOutput = xmlIndentTreeOutput;
>  
>          /* Indentation on output requires that we previously set
> @@ -27824,7 +27825,13 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def,
>              xmlIndentTreeOutput = oldIndentTreeOutput;
>              return -1;
>          }
> -        virBufferAsprintf(buf, "%s\n", (char *) xmlBufferContent(xmlbuf));
> +
> +        /* After libxml2-v2.9.12-2-g85b1792e even the first line is indented.
> +         * But virBufferAsprintf() also adds indentation. Skip one of them. */
> +        xmlbufContent = (const char *) xmlBufferContent(xmlbuf);
> +        virSkipSpaces(&xmlbufContent);
> +
> +        virBufferAsprintf(buf, "%s\n", xmlbufContent);
>          xmlIndentTreeOutput = oldIndentTreeOutput;
>      }
>  
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index a9eadff29c..20c6dc091a 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -2488,6 +2488,7 @@ virNetworkDefFormatBuf(virBuffer *buf,
>  
>      if (def->metadata) {
>          g_autoptr(xmlBuffer) xmlbuf = NULL;
> +        const char *xmlbufContent = NULL;
>          int oldIndentTreeOutput = xmlIndentTreeOutput;
>  
>          /* Indentation on output requires that we previously set
> @@ -2504,7 +2505,13 @@ virNetworkDefFormatBuf(virBuffer *buf,
>              xmlIndentTreeOutput = oldIndentTreeOutput;
>              return -1;
>          }
> -        virBufferAsprintf(buf, "%s\n", (char *) xmlBufferContent(xmlbuf));
> +
> +        /* After libxml2-v2.9.12-2-g85b1792e even the first line is indented.
> +         * But virBufferAsprintf() also adds indentation. Skip one of them. */
> +        xmlbufContent = (const char *) xmlBufferContent(xmlbuf);
> +        virSkipSpaces(&xmlbufContent);
> +
> +        virBufferAsprintf(buf, "%s\n", xmlbufContent);

Please create a helper function and put that workaround there instead of
having it open-coded at two different places.

Otherwise looks good.

Pavel

>          xmlIndentTreeOutput = oldIndentTreeOutput;
>      }
>  
> -- 
> 2.26.3
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20210525/1453f31b/attachment-0001.sig>