On 02/20/2012 01:48 PM, Eric Blake wrote:
> Overall, the idea looks reasonable, but you'll need a v2 to fix the
> memory issues in qemu_command.c.
Actually, there were problems with freeing the hostdevdef on error paths
in all 4 places virDomainHostdevDefAlloc() was used :-( I don't know
what I was thinking...
I've updated the patch with the following diff and will repost the full
patch.
>From ab626a94d03e8129c8b2a041012476bd56330a3d Mon Sep 17 00:00:00 2001
From: Laine Stump <laine@xxxxxxxxx>
Date: Mon, 20 Feb 2012 14:43:06 -0500
Subject: [PATCH] fix memory leaks in "make hostdev info a separate object"
---
src/conf/domain_conf.c | 1 +
src/qemu/qemu_command.c | 42 +++++++++++++++++++-----------------------
src/xenxs/xen_sxpr.c | 1 +
src/xenxs/xen_xm.c | 4 +++-
4 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c0503f4..df7d87d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1321,6 +1321,7 @@ virDomainHostdevDefPtr virDomainHostdevDefAlloc(void)
return NULL;
}
if (VIR_ALLOC(def->info) < 0) {
+ virReportOOMError();
VIR_FREE(def);
return NULL;
}
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a5a0054..0e295cc 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6534,34 +6534,31 @@ qemuParseCommandLinePCI(const char *val)
virDomainHostdevDefPtr def = virDomainHostdevDefAlloc();
if (!def)
- goto cleanup;
+ goto error;
if (!STRPREFIX(val, "host=")) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown PCI device syntax '%s'"), val);
- goto cleanup;
+ goto error;
}
start = val + strlen("host=");
if (virStrToLong_i(start, &end, 16, &bus) < 0 || *end != ':') {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot extract PCI device bus '%s'"), val);
- VIR_FREE(def);
- goto cleanup;
+ goto error;
}
start = end + 1;
if (virStrToLong_i(start, &end, 16, &slot) < 0 || *end != '.') {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot extract PCI device slot '%s'"), val);
- VIR_FREE(def);
- goto cleanup;
+ goto error;
}
start = end + 1;
if (virStrToLong_i(start, NULL, 16, &func) < 0) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot extract PCI device function '%s'"), val);
- VIR_FREE(def);
- goto cleanup;
+ goto error;
}
def->mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS;
@@ -6570,9 +6567,11 @@ qemuParseCommandLinePCI(const char *val)
def->source.subsys.u.pci.bus = bus;
def->source.subsys.u.pci.slot = slot;
def->source.subsys.u.pci.function = func;
-
-cleanup:
return def;
+
+ error:
+ virDomainHostdevDefFree(def);
+ return NULL;
}
@@ -6588,13 +6587,12 @@ qemuParseCommandLineUSB(const char *val)
char *end;
if (!def)
- goto cleanup;
+ goto error;
if (!STRPREFIX(val, "host:")) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown USB device syntax '%s'"), val);
- VIR_FREE(def);
- goto cleanup;
+ goto error;
}
start = val + strlen("host:");
@@ -6602,29 +6600,25 @@ qemuParseCommandLineUSB(const char *val)
if (virStrToLong_i(start, &end, 16, &first) < 0 || *end != ':') {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot extract USB device vendor '%s'"), val);
- VIR_FREE(def);
- goto cleanup;
+ goto error;
}
start = end + 1;
if (virStrToLong_i(start, NULL, 16, &second) < 0) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot extract USB device product '%s'"), val);
- VIR_FREE(def);
- goto cleanup;
+ goto error;
}
} else {
if (virStrToLong_i(start, &end, 10, &first) < 0 || *end != '.') {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot extract USB device bus '%s'"), val);
- VIR_FREE(def);
- goto cleanup;
+ goto error;
}
start = end + 1;
if (virStrToLong_i(start, NULL, 10, &second) < 0) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot extract USB device address '%s'"), val);
- VIR_FREE(def);
- goto cleanup;
+ goto error;
}
}
@@ -6638,9 +6632,11 @@ qemuParseCommandLineUSB(const char *val)
def->source.subsys.u.usb.vendor = first;
def->source.subsys.u.usb.product = second;
}
-
-cleanup:
return def;
+
+ error:
+ virDomainHostdevDefFree(def);
+ return NULL;
}
diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c
index b0e1b36..e5df953 100644
--- a/src/xenxs/xen_sxpr.c
+++ b/src/xenxs/xen_sxpr.c
@@ -1088,6 +1088,7 @@ xenParseSxprPCI(virDomainDefPtr def,
dev->source.subsys.u.pci.function = funcID;
if (VIR_REALLOC_N(def->hostdevs, def->nhostdevs+1) < 0) {
+ virDomainHostdevDefFree(dev);
goto no_memory;
}
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 6e72aea..5862168 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -825,8 +825,10 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
hostdev->source.subsys.u.pci.slot = slotID;
hostdev->source.subsys.u.pci.function = funcID;
- if (VIR_REALLOC_N(def->hostdevs, def->nhostdevs+1) < 0)
+ if (VIR_REALLOC_N(def->hostdevs, def->nhostdevs+1) < 0) {
+ virDomainHostdevDefFree(hostdev);
goto no_memory;
+ }
def->hostdevs[def->nhostdevs++] = hostdev;
hostdev = NULL;
--
1.7.7.6
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list