[libvirt] [PATCH 0/3] Couple of resctrl fixes

*** BLURB HERE *** Michal Prívozník (3): qemuDomainGetResctrlMonData: Don't leak @caps qemuDomainGetResctrlMonData: Dereference resctrl monitor iff not NULL qemuDomainGetResctrlMonData: Switch to switch() src/qemu/qemu_driver.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) -- 2.21.0

The capabilities object must be unrefed when no longer needed. use VIR_AUTOUNREF() for that. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3c5ec848ba..d5a8beb134 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20737,7 +20737,7 @@ qemuDomainGetResctrlMonData(virQEMUDriverPtr driver, virDomainResctrlDefPtr resctrl = NULL; virQEMUResctrlMonDataPtr res = NULL; char **features = NULL; - virCapsPtr caps = NULL; + VIR_AUTOUNREF(virCapsPtr) caps = NULL; size_t i = 0; size_t j = 0; -- 2.21.0

On Tue, Aug 06, 2019 at 01:47:44PM +0200, Michal Privoznik wrote:
The capabilities object must be unrefed when no longer needed. use VIR_AUTOUNREF() for that.
s/use/Use/ or s/needed./needed;/
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3c5ec848ba..d5a8beb134 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20737,7 +20737,7 @@ qemuDomainGetResctrlMonData(virQEMUDriverPtr driver, virDomainResctrlDefPtr resctrl = NULL; virQEMUResctrlMonDataPtr res = NULL; char **features = NULL; - virCapsPtr caps = NULL; + VIR_AUTOUNREF(virCapsPtr) caps = NULL; size_t i = 0; size_t j = 0;
-- 2.21.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

If the host doesn't have resctrl then the monitor is going to be NULL and we must avoid dereferencing it. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d5a8beb134..25b6653e38 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20744,7 +20744,8 @@ qemuDomainGetResctrlMonData(virQEMUDriverPtr driver, caps = virQEMUDriverGetCapabilities(driver, false); if (tag == VIR_RESCTRL_MONITOR_TYPE_CACHE) { - features = caps->host.cache.monitor->features; + if (caps->host.cache.monitor) + features = caps->host.cache.monitor->features; } else { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Unsupported resctrl monitor type")); -- 2.21.0

This way it is obvious when adding a new resource control type that stats helper func needs to be updated too. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 25b6653e38..5606639482 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20743,10 +20743,14 @@ qemuDomainGetResctrlMonData(virQEMUDriverPtr driver, caps = virQEMUDriverGetCapabilities(driver, false); - if (tag == VIR_RESCTRL_MONITOR_TYPE_CACHE) { + switch (tag) { + case VIR_RESCTRL_MONITOR_TYPE_CACHE: if (caps->host.cache.monitor) features = caps->host.cache.monitor->features; - } else { + break; + case VIR_RESCTRL_MONITOR_TYPE_MEMBW: + case VIR_RESCTRL_MONITOR_TYPE_UNSUPPORT: + case VIR_RESCTRL_MONITOR_TYPE_LAST: virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Unsupported resctrl monitor type")); return -1; -- 2.21.0

On Tue, Aug 06, 2019 at 01:47:43PM +0200, Michal Privoznik wrote:
*** BLURB HERE ***
Michal Prívozník (3): qemuDomainGetResctrlMonData: Don't leak @caps qemuDomainGetResctrlMonData: Dereference resctrl monitor iff not NULL qemuDomainGetResctrlMonData: Switch to switch()
src/qemu/qemu_driver.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko
-
Michal Privoznik