[PATCH] qemu: Don't regenerate NVRAM path if parsed from domain XML

Daniel P. Berrangé berrange at redhat.com
Wed Feb 23 12:21:56 UTC 2022


On Wed, Feb 23, 2022 at 12:47:29PM +0100, Michal Prívozník wrote:
> On 2/23/22 11:01, Daniel P. Berrangé wrote:
> > On Wed, Feb 23, 2022 at 10:47:20AM +0100, Michal Prívozník wrote:
> >> On 2/23/22 10:33, Daniel P. Berrangé wrote:
> >>> On Wed, Feb 23, 2022 at 10:16:57AM +0100, Michal Privoznik wrote:
> >>>> After v8.0.0-466-g08101bde5d we unconditionally regenerate per
> >>>> domain NVRAM path even though it might have been parsed earlier
> >>>> from domain XML. The way we do that leads to a memleak:
> >>>>
> >>>>   43 bytes in 1 blocks are definitely lost in loss record 330 of 682
> >>>>   at 0x483F7E5: malloc (vg_replace_malloc.c:381)
> >>>>   by 0x50D5B18: g_malloc (in /usr/lib64/libglib-2.0.so.0.7000.2)
> >>>>   by 0x50EFA4F: g_strdup (in /usr/lib64/libglib-2.0.so.0.7000.2)
> >>>>   by 0x49E774E: virXPathString (virxml.c:88)
> >>>>   by 0x4A3F0E4: virDomainDefParseBootLoaderOptions (domain_conf.c:18226)
> >>>>   by 0x4A3F49C: virDomainDefParseBootOptions (domain_conf.c:18298)
> >>>>   by 0x4A448C3: virDomainDefParseXML (domain_conf.c:19598)
> >>>>   by 0x4A487A1: virDomainDefParseNode (domain_conf.c:20404)
> >>>>   by 0x117FCF: testCompareXMLToArgv (qemuxml2argvtest.c:726)
> >>>>   by 0x142124: virTestRun (testutils.c:142)
> >>>>   by 0x1423D4: virTestRunLog (testutils.c:197)
> >>>>   by 0x140A76: mymain (qemuxml2argvtest.c:3406)
> >>>>
> >>>> If we parsed NVRAM path from domain XML we must refrain from
> >>>> generating new path.
> >>>
> >>> Hmm, so we honour the 'nvram' path from the XML, even when doing
> >>> firmware auto-select ?
> >>>
> >>> That is contrary to the qemuDomainUndefineFlags method expectations
> >>> which unconditionally uses the qemuDomainNVRAMPathFormat result
> >>> when deleting nvram, ignoring 'nvram' path in the XML
> >>>
> >>
> >> Good point. I think the question boils down to, what should happen when
> >> FW autoselection is enabled and user told use where they want to have
> >> NVRAM stored? It's a tricky situation because if the NVRAM file does
> >> exist we won't overwrite it. And yet, if we ever selected a different FW
> >> image the pre-existing NVRAM might be incompatible with the new FW image.
> > 
> > The new RESET_NVRAM flag can recover from this last scenario now.
> > 
> > 
> > So we need to make the qemuDomainUndefineFlags method honour the nvram
> > path, if it was provided.
> 
> Fair enough. So do you prefer a follow up patch for fixing
> qemuDomainUndefineFlags() or reverting this patch and regenerating NVRAM
> path always?

I'll send a patch for qemuDomainUndefineFlags shortly.



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 :|




More information about the libvir-list mailing list