On Tue, May 25, 2021 at 10:52:49AM +0200, Michal Prívozník wrote: > 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@xxxxxxxxxx> > >> --- > >> > >> 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. Ah, you're stripping the spaces that libxml added, rather than the spaces that virBuffer added. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
