[libvirt PATCH 0/4] Some memory leaks related fixes
Patch #1 is a resend of https://listman.redhat.com/archives/libvir-list/2021-April/msg00600.html Patch #2 is a fix for a mistake in https://listman.redhat.com/archives/libvir-list/2021-April/msg00782.html Patches #3 and #4 are the replacement for https://listman.redhat.com/archives/libvir-list/2021-April/msg00643.html Cheers, Tim Tim Wiederhake (4): virxml: Fix schema validation of individual nodes xenParseHypervisorFeatures: Remove superfluous VIR_FREE qemu: Introduce virQEMUCapsUpdateHostCPUModel testUpdateQEMUCaps: Fix memory leak src/conf/cpu_conf.c | 3 +-- src/libxl/xen_common.c | 1 - src/qemu/qemu_capabilities.c | 9 +++++++++ src/qemu/qemu_capspriv.h | 4 ++++ src/util/virxml.c | 13 ++++++------- src/util/virxml.h | 1 - tests/qemuxml2argvtest.c | 8 ++++---- 7 files changed, 24 insertions(+), 15 deletions(-) -- 2.26.3
xmlDocSetRootElement removes the node from its previous document tree, effectively removing the "<cpu>" node from "<domain>" in virCPUDefParseXML. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/cpu_conf.c | 3 +-- src/util/virxml.c | 13 ++++++------- src/util/virxml.h | 1 - 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index c7bea8ae00..58ca1e2019 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -355,8 +355,7 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt, PKGDATADIR "/schemas"))) return -1; - if (virXMLValidateNodeAgainstSchema(schemafile, ctxt->doc, - ctxt->node) < 0) + if (virXMLValidateNodeAgainstSchema(schemafile, ctxt->node) < 0) return -1; } diff --git a/src/util/virxml.c b/src/util/virxml.c index d0d9494009..418be2a898 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -1551,16 +1551,15 @@ virXMLValidateAgainstSchema(const char *schemafile, int -virXMLValidateNodeAgainstSchema(const char *schemafile, - xmlDocPtr doc, - xmlNodePtr node) +virXMLValidateNodeAgainstSchema(const char *schemafile, xmlNodePtr node) { - xmlNodePtr root; int ret; + xmlDocPtr copy = xmlNewDoc(NULL); - root = xmlDocSetRootElement(doc, node); - ret = virXMLValidateAgainstSchema(schemafile, doc); - xmlDocSetRootElement(doc, root); + xmlDocSetRootElement(copy, xmlCopyNode(node, true)); + ret = virXMLValidateAgainstSchema(schemafile, copy); + + xmlFreeDoc(copy); return ret; } diff --git a/src/util/virxml.h b/src/util/virxml.h index 2b40398eee..c0405a39f0 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -296,7 +296,6 @@ virXMLValidateAgainstSchema(const char *schemafile, int virXMLValidateNodeAgainstSchema(const char *schemafile, - xmlDocPtr doc, xmlNodePtr node); void -- 2.26.3
Fixes: 4eb7c621985dad4de911ec394ac628bd1a5b29ab Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/libxl/xen_common.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index 6fa69fbdf0..aeb94e12ad 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -568,7 +568,6 @@ xenParseHypervisorFeatures(virConf *conf, virDomainDef *def) timer->mode = VIR_DOMAIN_TIMER_MODE_PARAVIRT; def->clock.timers[def->clock.ntimers - 1] = timer; - VIR_FREE(tscmode); } if (xenConfigGetString(conf, "passthrough", &passthrough, NULL) < 0) -- 2.26.3
On 4/20/21 1:27 PM, Tim Wiederhake wrote:
Fixes: 4eb7c621985dad4de911ec394ac628bd1a5b29ab Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/libxl/xen_common.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index 6fa69fbdf0..aeb94e12ad 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -568,7 +568,6 @@ xenParseHypervisorFeatures(virConf *conf, virDomainDef *def) timer->mode = VIR_DOMAIN_TIMER_MODE_PARAVIRT;
def->clock.timers[def->clock.ntimers - 1] = timer; - VIR_FREE(tscmode); }
if (xenConfigGetString(conf, "passthrough", &passthrough, NULL) < 0)
That's not the only problem with the function. The pattern above in that if() you're fixing looks funny too: if (tscmode) { ... STREQ_NULLABLE(tscmode, ...); ... } We know that @tscmode is not NULL when we're in the body. Michal
Function will be used by next patch. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_capabilities.c | 9 +++++++++ src/qemu/qemu_capspriv.h | 4 ++++ 2 files changed, 13 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 030779902d..7971a9c557 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3819,6 +3819,15 @@ virQEMUCapsInitHostCPUModel(virQEMUCaps *qemuCaps, } +void +virQEMUCapsUpdateHostCPUModel(virQEMUCaps *qemuCaps, + virArch hostArch, + virDomainVirtType type) +{ + virQEMUCapsHostCPUDataClear(&virQEMUCapsGetAccel(qemuCaps, type)->hostCPU); + virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, type); +} + qemuMonitorCPUModelInfo * virQEMUCapsGetCPUModelInfo(virQEMUCaps *qemuCaps, virDomainVirtType type) diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h index c50b15f193..a54a22685e 100644 --- a/src/qemu/qemu_capspriv.h +++ b/src/qemu/qemu_capspriv.h @@ -60,6 +60,10 @@ virQEMUCapsInitHostCPUModel(virQEMUCaps *qemuCaps, virArch hostArch, virDomainVirtType type); +void +virQEMUCapsUpdateHostCPUModel(virQEMUCaps *qemuCaps, + virArch hostArch, + virDomainVirtType type); int virQEMUCapsInitCPUModel(virQEMUCaps *qemuCaps, virDomainVirtType type, -- 2.26.3
testUpdateQEMUCaps is called multiple times. Use virQEMUCapsUpdateHostCPUModel instead of virQEMUCapsInitHostCPUModel to not overwrite (and leak) the pointers in qemuCaps->kvm.hostCPU and qemuCaps->tcg.hostCPU. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/qemuxml2argvtest.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 572c7b251a..f0efe98d7e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -367,10 +367,10 @@ testUpdateQEMUCaps(const struct testQemuInfo *info, !!(info->flags & FLAG_SKIP_LEGACY_CPUS)) < 0) return -1; - virQEMUCapsInitHostCPUModel(info->qemuCaps, caps->host.arch, - VIR_DOMAIN_VIRT_KVM); - virQEMUCapsInitHostCPUModel(info->qemuCaps, caps->host.arch, - VIR_DOMAIN_VIRT_QEMU); + virQEMUCapsUpdateHostCPUModel(info->qemuCaps, caps->host.arch, + VIR_DOMAIN_VIRT_KVM); + virQEMUCapsUpdateHostCPUModel(info->qemuCaps, caps->host.arch, + VIR_DOMAIN_VIRT_QEMU); return 0; } -- 2.26.3
On 4/20/21 1:27 PM, Tim Wiederhake wrote:
Patch #1 is a resend of https://listman.redhat.com/archives/libvir-list/2021-April/msg00600.html
Patch #2 is a fix for a mistake in https://listman.redhat.com/archives/libvir-list/2021-April/msg00782.html
Patches #3 and #4 are the replacement for https://listman.redhat.com/archives/libvir-list/2021-April/msg00643.html
Cheers, Tim
Tim Wiederhake (4): virxml: Fix schema validation of individual nodes xenParseHypervisorFeatures: Remove superfluous VIR_FREE qemu: Introduce virQEMUCapsUpdateHostCPUModel testUpdateQEMUCaps: Fix memory leak
src/conf/cpu_conf.c | 3 +-- src/libxl/xen_common.c | 1 - src/qemu/qemu_capabilities.c | 9 +++++++++ src/qemu/qemu_capspriv.h | 4 ++++ src/util/virxml.c | 13 ++++++------- src/util/virxml.h | 1 - tests/qemuxml2argvtest.c | 8 ++++---- 7 files changed, 24 insertions(+), 15 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Michal Privoznik -
Tim Wiederhake