Re: [libvirt] [PATCH] virsh: Strip XML declaration when extracting CPU XMLs

On Thu, Nov 22, 2018 at 17:39:16 +0100, Ján Tomko wrote: > On Thu, Nov 22, 2018 at 04:46:33PM +0100, Jiri Denemark wrote: > >Since commit v4.3.0-336-gc84726fbdd all > >{hypervisor-,}cpu-{baseline,compare} commands use a generic > >vshExtractCPUDefXMLs helper for extracting individual CPU definitions > >from the provided input file. The helper wraps the input file in a > ><container> element so that several independent elements can be easily > >parsed from the file. This works fine except when the file starts with > >XML declaration (<?xml version="1.0" ... ?>) because the XML declaration > >cannot be put inside any element. In fact it has to be at the very > >beginning of the XML document without any preceding white space > >characters. We can just simply skip the XML declaration. > > What if someone specifies a doctype? O:) > Also, does libvirt produce such files? I don't think we should bother > doing extra work to undo the extra work done by the user. Of course, we don't generate XML declarations, but I can imagine tools formatting DOM into a file could just automatically output the XML declaration unless you explicitly tell them not to do so. On the other hand, nothing would output a doctype or <?xml-stylesheet ?> without an explicit action. Moreover, libvirt itself doesn't mind XML declarations and virsh before 4.4.0 didn't mind either. I agree, it's probably a corner case and I was thinking about not fixing it, but it turned out to be really easy thanks to the strict XML specification. > >https://bugzilla.redhat.com/show_bug.cgi?id=1595993 > > I only see a relation between the bug summary and this patch. > There's no mention of the XML declaration there and no mention of the > other issues mentioned there here in the patch. Oops, the patch is supposed to fix another bug https://bugzilla.redhat.com/show_bug.cgi?id=1592737