6:03 p.m.
This patch reduces the number of return paths in the node device driver
methods
In doing this I noticed an annoying problem in the public API contract
for virNodeDeviceGetParent. It returns a 'const char *', but the remote
driver is actually returning an malloc'd string requiring the caller to
free it, while the node_device driver is returning a const string the
caller must not free. Ideally we would not have the 'const' annotation
but changing that now is ABI change, so instead I work around it, by
making all drivers return a malloc'd string, but then caching this in
the virNodeDevicePtr object. This mallc'd string is then free'd when
the object is released. This allows us to work around the public API
contract problem
datatypes.c | 1
datatypes.h | 1
libvirt.c | 14 ++++++++----
node_device.c | 65 ++++++++++++++++++++++++++++++++++++++--------------------
4 files changed, 54 insertions(+), 27 deletions(-)
Daniel
diff --git a/src/datatypes.c b/src/datatypes.c
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -861,6 +861,7 @@ virReleaseNodeDevice(virNodeDevicePtr de
dev->magic = -1;
VIR_FREE(dev->name);
+ VIR_FREE(dev->parent);
VIR_FREE(dev);
DEBUG("unref connection %p %d", conn, conn->refs);
diff --git a/src/datatypes.h b/src/datatypes.h
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -199,6 +199,7 @@ struct _virNodeDevice {
int refs; /* reference count */
virConnectPtr conn; /* pointer back to the connection */
char *name; /* device name (unique on node) */
+ char *parent; /* parent device name */
};
diff --git a/src/libvirt.c b/src/libvirt.c
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -5689,11 +5689,15 @@ const char *virNodeDeviceGetParent(virNo
return NULL;
}
- if (dev->conn->deviceMonitor &&
dev->conn->deviceMonitor->deviceGetParent)
- return dev->conn->deviceMonitor->deviceGetParent (dev);
-
- virLibConnError (dev->conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
- return NULL;
+ if (!dev->parent) {
+ if (dev->conn->deviceMonitor &&
dev->conn->deviceMonitor->deviceGetParent) {
+ dev->parent = dev->conn->deviceMonitor->deviceGetParent (dev);
+ } else {
+ virLibConnError (dev->conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+ return NULL;
+ }
+ }
+ return dev->parent;
}
/**
diff --git a/src/node_device.c b/src/node_device.c
--- a/src/node_device.c
+++ b/src/node_device.c
@@ -91,66 +91,84 @@ static virNodeDevicePtr nodeDeviceLookup
const char *name)
{
virDeviceMonitorStatePtr driver = conn->devMonPrivateData;
- virNodeDeviceObjPtr obj = virNodeDeviceFindByName(&driver->devs, name);
+ virNodeDeviceObjPtr obj;
+ virNodeDevicePtr ret = NULL;
+ obj = virNodeDeviceFindByName(&driver->devs, name);
if (!obj) {
virNodeDeviceReportError(conn, VIR_ERR_INVALID_NODE_DEVICE,
"%s", _("no node device with matching
name"));
- return NULL;
+ goto cleanup;
}
- return virGetNodeDevice(conn, name);
+ ret = virGetNodeDevice(conn, name);
+cleanup:
+ return ret;
}
static char *nodeDeviceDumpXML(virNodeDevicePtr dev,
unsigned int flags ATTRIBUTE_UNUSED)
{
virDeviceMonitorStatePtr driver = dev->conn->devMonPrivateData;
- virNodeDeviceObjPtr obj = virNodeDeviceFindByName(&driver->devs,
dev->name);
+ virNodeDeviceObjPtr obj;
+ char *ret = NULL;
+ obj = virNodeDeviceFindByName(&driver->devs, dev->name);
if (!obj) {
virNodeDeviceReportError(dev->conn, VIR_ERR_INVALID_NODE_DEVICE,
"%s", _("no node device with matching
name"));
- return NULL;
+ goto cleanup;
}
- return virNodeDeviceDefFormat(dev->conn, obj->def);
+ ret = virNodeDeviceDefFormat(dev->conn, obj->def);
+
+cleanup:
+ return ret;
}
static char *nodeDeviceGetParent(virNodeDevicePtr dev)
{
virDeviceMonitorStatePtr driver = dev->conn->devMonPrivateData;
- virNodeDeviceObjPtr obj = virNodeDeviceFindByName(&driver->devs,
dev->name);
+ virNodeDeviceObjPtr obj;
+ char *ret = NULL;
+ obj = virNodeDeviceFindByName(&driver->devs, dev->name);
if (!obj) {
virNodeDeviceReportError(dev->conn, VIR_ERR_INVALID_NODE_DEVICE,
"%s", _("no node device with matching
name"));
- return NULL;
+ goto cleanup;
}
- return obj->def->parent;
+ ret = strdup(obj->def->parent);
+
+cleanup:
+ return ret;
}
static int nodeDeviceNumOfCaps(virNodeDevicePtr dev)
{
virDeviceMonitorStatePtr driver = dev->conn->devMonPrivateData;
- virNodeDeviceObjPtr obj = virNodeDeviceFindByName(&driver->devs,
dev->name);
+ virNodeDeviceObjPtr obj;
virNodeDevCapsDefPtr caps;
int ncaps = 0;
+ int ret = -1;
+ obj = virNodeDeviceFindByName(&driver->devs, dev->name);
if (!obj) {
virNodeDeviceReportError(dev->conn, VIR_ERR_INVALID_NODE_DEVICE,
"%s", _("no node device with matching
name"));
- return -1;
+ goto cleanup;
}
for (caps = obj->def->caps; caps; caps = caps->next)
++ncaps;
+ ret = ncaps;
- return ncaps;
+cleanup:
+ return ret;
}
@@ -158,29 +176,32 @@ nodeDeviceListCaps(virNodeDevicePtr dev,
nodeDeviceListCaps(virNodeDevicePtr dev, char **const names, int maxnames)
{
virDeviceMonitorStatePtr driver = dev->conn->devMonPrivateData;
- virNodeDeviceObjPtr obj = virNodeDeviceFindByName(&driver->devs,
dev->name);
+ virNodeDeviceObjPtr obj;
virNodeDevCapsDefPtr caps;
int ncaps = 0;
+ int ret = -1;
+ obj = virNodeDeviceFindByName(&driver->devs, dev->name);
if (!obj) {
virNodeDeviceReportError(dev->conn, VIR_ERR_INVALID_NODE_DEVICE,
"%s", _("no node device with matching
name"));
- return -1;
+ goto cleanup;
}
for (caps = obj->def->caps; caps && ncaps < maxnames; caps =
caps->next) {
names[ncaps] = strdup(virNodeDevCapTypeToString(caps->type));
if (names[ncaps++] == NULL)
- goto failure;
+ goto cleanup;
}
+ ret = ncaps;
- return ncaps;
-
- failure:
- --ncaps;
- while (--ncaps >= 0)
- VIR_FREE(names[ncaps]);
- return -1;
+cleanup:
+ if (ret == -1) {
+ --ncaps;
+ while (--ncaps >= 0)
+ VIR_FREE(names[ncaps]);
+ }
+ return ret;
}
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|