There was one major, and a few minor bugs having to do with the reference counting of node devices in daemon/remote.c The major bug was that remoteDispatchNodeDeviceListCaps() was completely failing to unreference node devices; this would lead to many open file descriptors, which would eventually fail. The minor bugs were along the same lines, but were in rarely used error paths. Still, they should be corrected. Signed-off-by: Chris Lalancette <clalance(a)redhat.com&gt; --- daemon/remote.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 91faa9a..ec5f85b 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -5012,11 +5012,13 @@ remoteDispatchNodeDeviceGetParent (struct qemud_server *server ATTRIBUTE_UNUSED, /* remoteDispatchClientRequest will free this. */ char **parent_p; if (VIR_ALLOC(parent_p) < 0) { + virNodeDeviceFree(dev); remoteDispatchOOMError(rerr); return -1; }

On 06/25/2010 03:22 PM, Chris Lalancette wrote: > There was one major, and a few minor bugs having to do with > the reference counting of node devices in daemon/remote.c > The major bug was that remoteDispatchNodeDeviceListCaps() > was completely failing to unreference node devices; this > would lead to many open file descriptors, which would eventually > fail. > > The minor bugs were along the same lines, but were in rarely > used error paths. Still, they should be corrected. > > Signed-off-by: Chris Lalancette <clalance(a)redhat.com&gt; > --- > daemon/remote.c | 7 +++++++ > 1 files changed, 7 insertions(+), 0 deletions(-) > > diff --git a/daemon/remote.c b/daemon/remote.c > index 91faa9a..ec5f85b 100644 > --- a/daemon/remote.c > +++ b/daemon/remote.c > @@ -5012,11 +5012,13 @@ remoteDispatchNodeDeviceGetParent (struct qemud_server *server ATTRIBUTE_UNUSED, > /* remoteDispatchClientRequest will free this. */ > char **parent_p; > if (VIR_ALLOC(parent_p) < 0) { > + virNodeDeviceFree(dev); > remoteDispatchOOMError(rerr); > return -1; > } ACK. But given the number of places where you had to duplicate this line insertion, would it be better to change all the return -1 into goto cleanup, where the cleanup: label guarantees this cleanup for all possible exit paths?

There was one major, and a few minor bugs having to do with the reference counting of node devices in daemon/remote.c The major bug was that remoteDispatchNodeDeviceListCaps() was completely failing to unreference node devices; this would lead to many open file descriptors, which would eventually fail. The minor bugs were along the same lines, but were in rarely used error paths. Still, they should be corrected. Signed-off-by: Chris Lalancette&lt;clalance(a)redhat.com&gt; --- daemon/remote.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 91faa9a..ec5f85b 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -5012,11 +5012,13 @@ remoteDispatchNodeDeviceGetParent (struct qemud_server *server ATTRIBUTE_UNUSED, /* remoteDispatchClientRequest will free this. */ char **parent_p; if (VIR_ALLOC(parent_p)< 0) { + virNodeDeviceFree(dev); remoteDispatchOOMError(rerr); return -1; } *parent_p = strdup(parent); if (*parent_p == NULL) { + virNodeDeviceFree(dev); remoteDispatchOOMError(rerr); return -1; } @@ -5048,6 +5050,7 @@ remoteDispatchNodeDeviceNumOfCaps (struct qemud_server *server ATTRIBUTE_UNUSED, ret->num = virNodeDeviceNumOfCaps(dev); if (ret->num< 0) { + virNodeDeviceFree(dev); remoteDispatchConnError(rerr, conn); return -1; } @@ -5076,6 +5079,7 @@ remoteDispatchNodeDeviceListCaps (struct qemud_server *server ATTRIBUTE_UNUSED, } if (args->maxnames> REMOTE_NODE_DEVICE_NAME_LIST_MAX) { + virNodeDeviceFree(dev); remoteDispatchFormatError(rerr, "%s", _("maxnames> REMOTE_NODE_DEVICE_NAME_LIST_MAX")); return -1; @@ -5083,6 +5087,7 @@ remoteDispatchNodeDeviceListCaps (struct qemud_server *server ATTRIBUTE_UNUSED, /* Allocate return buffer. */ if (VIR_ALLOC_N(ret->names.names_val, args->maxnames)< 0) { + virNodeDeviceFree(dev); remoteDispatchOOMError(rerr); return -1; } @@ -5091,11 +5096,13 @@ remoteDispatchNodeDeviceListCaps (struct qemud_server *server ATTRIBUTE_UNUSED, virNodeDeviceListCaps (dev, ret->names.names_val, args->maxnames); if (ret->names.names_len == -1) { + virNodeDeviceFree(dev); remoteDispatchConnError(rerr, conn); VIR_FREE(ret->names.names_val); return -1; } + virNodeDeviceFree(dev); return 0; }