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

[Michal Prívozník]

On 5/25/21 10:47 AM, Daniel P. Berrangé wrote:
> 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);
> 
> Won't this mean that indent is now wrong for the older libxml ?

No, because the older did not put any spaces at the beginning. I mean,
with older libxml we get:

  xmlbufContent = "<metadata>\n    blah blah ...";

with new new libxl we get:

  xmlbufContent = "  <metadata>\n     blah blah ...";

Hence virSkipSpaces() which drops those leading spaces.

Michal