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? Michal