External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-07-17 15:17:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

nodedev: Convert virNodeDeviceObj to use virObjectLockable

Browse Source 
Now that we have a bit more control, let's convert our object into
a lockable object and let that magic handle the create and lock/unlock.

This also involves creating a virNodeDeviceEndAPI in order to handle
the object cleanup for API's that use the Add or Find API's in order
to get a locked/reffed object. The EndAPI will unlock and unref the
object returning NULL to indicate to the caller to not use the obj.

Signed-off-by: John Ferlan <jferlan@redhat.com>
This commit is contained in:
John Ferlan 2017-05-15 11:00:59 -04:00
parent 960d665c1e
commit dae23ec345
7 changed files with 119 additions and 124 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

156
src/conf/virnodedeviceobj.c
View File

@ -33,7 +33,7 @@
VIR_LOG_INIT("conf.virnodedeviceobj");
struct _virNodeDeviceObj {
virMutex lock;
virObjectLockable parent;
virNodeDeviceDefPtr def; /* device definition */
};
@ -44,26 +44,62 @@ struct _virNodeDeviceObjList {
};
static virClassPtr virNodeDeviceObjClass;
static void virNodeDeviceObjDispose(void *opaque);
static int
virNodeDeviceObjOnceInit(void)
{
if (!(virNodeDeviceObjClass = virClassNew(virClassForObjectLockable(),
"virNodeDeviceObj",
sizeof(virNodeDeviceObj),
virNodeDeviceObjDispose)))
return -1;
return 0;
}
VIR_ONCE_GLOBAL_INIT(virNodeDeviceObj)
static void
virNodeDeviceObjDispose(void *opaque)
{
virNodeDeviceObjPtr obj = opaque;
virNodeDeviceDefFree(obj->def);
}
static virNodeDeviceObjPtr
virNodeDeviceObjNew(void)
{
virNodeDeviceObjPtr obj;
if (VIR_ALLOC(obj) < 0)
if (virNodeDeviceObjInitialize() < 0)
return NULL;
if (virMutexInit(&obj->lock) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("cannot initialize mutex"));
VIR_FREE(obj);
if (!(obj = virObjectLockableNew(virNodeDeviceObjClass)))
return NULL;
}
virNodeDeviceObjLock(obj);
virObjectLock(obj);
return obj;
}
void
virNodeDeviceObjEndAPI(virNodeDeviceObjPtr *obj)
{
if (!*obj)
return;
virObjectUnlock(*obj);
virObjectUnref(*obj);
*obj = NULL;
}
virNodeDeviceDefPtr
virNodeDeviceObjGetDef(virNodeDeviceObjPtr obj)
{
@ -185,13 +221,13 @@ virNodeDeviceObjListFindBySysfsPath(virNodeDeviceObjListPtr devs,
virNodeDeviceObjPtr obj = devs->objs[i];
virNodeDeviceDefPtr def;
virNodeDeviceObjLock(obj);
virObjectLock(obj);
def = obj->def;
if ((def->sysfs_path != NULL) &&
(STREQ(def->sysfs_path, sysfs_path))) {
return obj;
return virObjectRef(obj);
}
virNodeDeviceObjUnlock(obj);
virObjectUnlock(obj);
}
return NULL;
@ -208,11 +244,11 @@ virNodeDeviceObjListFindByName(virNodeDeviceObjListPtr devs,
virNodeDeviceObjPtr obj = devs->objs[i];
virNodeDeviceDefPtr def;
virNodeDeviceObjLock(obj);
virObjectLock(obj);
def = obj->def;
if (STREQ(def->name, name))
return obj;
virNodeDeviceObjUnlock(obj);
return virObjectRef(obj);
virObjectUnlock(obj);
}
return NULL;
@ -230,13 +266,13 @@ virNodeDeviceObjListFindByWWNs(virNodeDeviceObjListPtr devs,
virNodeDeviceObjPtr obj = devs->objs[i];
virNodeDevCapsDefPtr cap;
virNodeDeviceObjLock(obj);
virObjectLock(obj);
if ((cap = virNodeDeviceFindFCCapDef(obj)) &&
STREQ_NULLABLE(cap->data.scsi_host.wwnn, parent_wwnn) &&
STREQ_NULLABLE(cap->data.scsi_host.wwpn, parent_wwpn) &&
virNodeDeviceFindVPORTCapDef(obj))
return obj;
virNodeDeviceObjUnlock(obj);
return virObjectRef(obj);
virObjectUnlock(obj);
}
return NULL;
@ -253,12 +289,12 @@ virNodeDeviceObjListFindByFabricWWN(virNodeDeviceObjListPtr devs,
virNodeDeviceObjPtr obj = devs->objs[i];
virNodeDevCapsDefPtr cap;
virNodeDeviceObjLock(obj);
virObjectLock(obj);
if ((cap = virNodeDeviceFindFCCapDef(obj)) &&
STREQ_NULLABLE(cap->data.scsi_host.fabric_wwn, parent_fabric_wwn) &&
virNodeDeviceFindVPORTCapDef(obj))
return obj;
virNodeDeviceObjUnlock(obj);
return virObjectRef(obj);
virObjectUnlock(obj);
}
return NULL;
@ -274,10 +310,10 @@ virNodeDeviceObjListFindByCap(virNodeDeviceObjListPtr devs,
for (i = 0; i < devs->count; i++) {
virNodeDeviceObjPtr obj = devs->objs[i];
virNodeDeviceObjLock(obj);
virObjectLock(obj);
if (virNodeDeviceObjHasCap(obj, cap))
return obj;
virNodeDeviceObjUnlock(obj);
return virObjectRef(obj);
virObjectUnlock(obj);
}
return NULL;
@ -295,7 +331,7 @@ virNodeDeviceObjListFindSCSIHostByWWNs(virNodeDeviceObjListPtr devs,
virNodeDeviceObjPtr obj = devs->objs[i];
virNodeDevCapsDefPtr cap;
virNodeDeviceObjLock(obj);
virObjectLock(obj);
cap = obj->def->caps;
while (cap) {
@ -305,32 +341,18 @@ virNodeDeviceObjListFindSCSIHostByWWNs(virNodeDeviceObjListPtr devs,
VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) {
if (STREQ(cap->data.scsi_host.wwnn, wwnn) &&
STREQ(cap->data.scsi_host.wwpn, wwpn))
return obj;
return virObjectRef(obj);
}
}
cap = cap->next;
}
virNodeDeviceObjUnlock(obj);
virObjectUnlock(obj);
}
return NULL;
}
void
virNodeDeviceObjFree(virNodeDeviceObjPtr obj)
{
if (!obj)
return;
virNodeDeviceDefFree(obj->def);
virMutexDestroy(&obj->lock);
VIR_FREE(obj);
}
virNodeDeviceObjListPtr
virNodeDeviceObjListNew(void)
{
@ -347,7 +369,7 @@ virNodeDeviceObjListFree(virNodeDeviceObjListPtr devs)
{
size_t i;
for (i = 0; i < devs->count; i++)
virNodeDeviceObjFree(devs->objs[i]);
virObjectUnref(devs->objs[i]);
VIR_FREE(devs->objs);
VIR_FREE(devs);
}
@ -369,13 +391,12 @@ virNodeDeviceObjListAssignDef(virNodeDeviceObjListPtr devs,
return NULL;
if (VIR_APPEND_ELEMENT_COPY(devs->objs, devs->count, obj) < 0) {
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjFree(obj);
virNodeDeviceObjEndAPI(&obj);
return NULL;
}
obj->def = def;
return obj;
return virObjectRef(obj);
}
@ -385,17 +406,18 @@ virNodeDeviceObjListRemove(virNodeDeviceObjListPtr devs,
{
size_t i;
virNodeDeviceObjUnlock(obj);
virObjectUnlock(obj);
for (i = 0; i < devs->count; i++) {
virNodeDeviceObjLock(devs->objs[i]);
virObjectLock(devs->objs[i]);
if (devs->objs[i] == obj) {
virNodeDeviceObjUnlock(devs->objs[i]);
virObjectUnlock(devs->objs[i]);
virObjectUnref(devs->objs[i]);
VIR_DELETE_ELEMENT(devs->objs, i, devs->count);
break;
}
virNodeDeviceObjUnlock(devs->objs[i]);
virObjectUnlock(devs->objs[i]);
}
}
@ -446,7 +468,7 @@ virNodeDeviceObjListGetParentHostByParent(virNodeDeviceObjListPtr devs,
ret = virNodeDeviceFindFCParentHost(obj);
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
return ret;
}
@ -471,7 +493,7 @@ virNodeDeviceObjListGetParentHostByWWNs(virNodeDeviceObjListPtr devs,
ret = virNodeDeviceFindFCParentHost(obj);
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
return ret;
}
@ -494,7 +516,7 @@ virNodeDeviceObjListGetParentHostByFabricWWN(virNodeDeviceObjListPtr devs,
ret = virNodeDeviceFindFCParentHost(obj);
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
return ret;
}
@ -515,7 +537,7 @@ virNodeDeviceObjListFindVportParentHost(virNodeDeviceObjListPtr devs)
ret = virNodeDeviceFindFCParentHost(obj);
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
return ret;
}
@ -548,20 +570,6 @@ virNodeDeviceObjListGetParentHost(virNodeDeviceObjListPtr devs,
}
void
virNodeDeviceObjLock(virNodeDeviceObjPtr obj)
{
virMutexLock(&obj->lock);
}
void
virNodeDeviceObjUnlock(virNodeDeviceObjPtr obj)
{
virMutexUnlock(&obj->lock);
}
static bool
virNodeDeviceCapMatch(virNodeDeviceObjPtr obj,
int type)
@ -623,11 +631,11 @@ virNodeDeviceObjListNumOfDevices(virNodeDeviceObjListPtr devs,
for (i = 0; i < devs->count; i++) {
virNodeDeviceObjPtr obj = devs->objs[i];
virNodeDeviceObjLock(obj);
virObjectLock(obj);
if ((!aclfilter || aclfilter(conn, obj->def)) &&
(!cap || virNodeDeviceObjHasCap(obj, cap)))
++ndevs;
virNodeDeviceObjUnlock(obj);
virObjectUnlock(obj);
}
return ndevs;
@ -647,16 +655,16 @@ virNodeDeviceObjListGetNames(virNodeDeviceObjListPtr devs,
for (i = 0; i < devs->count && nnames < maxnames; i++) {
virNodeDeviceObjPtr obj = devs->objs[i];
virNodeDeviceObjLock(obj);
virObjectLock(obj);
if ((!aclfilter || aclfilter(conn, obj->def)) &&
(!cap || virNodeDeviceObjHasCap(obj, cap))) {
if (VIR_STRDUP(names[nnames], obj->def->name) < 0) {
virNodeDeviceObjUnlock(obj);
virObjectUnlock(obj);
goto failure;
}
nnames++;
}
virNodeDeviceObjUnlock(obj);
virObjectUnlock(obj);
}
return nnames;
@ -718,21 +726,21 @@ virNodeDeviceObjListExport(virConnectPtr conn,
for (i = 0; i < devs->count; i++) {
virNodeDeviceObjPtr obj = devs->objs[i];
virNodeDeviceObjLock(obj);
virObjectLock(obj);
if ((!aclfilter || aclfilter(conn, obj->def)) &&
virNodeDeviceMatch(obj, flags)) {
if (devices) {
if (!(device = virGetNodeDevice(conn, obj->def->name)) ||
VIR_STRDUP(device->parent, obj->def->parent) < 0) {
virObjectUnref(device);
virNodeDeviceObjUnlock(obj);
virObjectUnlock(obj);
goto cleanup;
}
tmp_devices[ndevices] = device;
}
ndevices++;
}
virNodeDeviceObjUnlock(obj);
virObjectUnlock(obj);
}
if (tmp_devices) {

11
src/conf/virnodedeviceobj.h
View File

@ -45,6 +45,8 @@ struct _virNodeDeviceDriverState {
virObjectEventStatePtr nodeDeviceEventState;
};
void
virNodeDeviceObjEndAPI(virNodeDeviceObjPtr *obj);
virNodeDeviceDefPtr
virNodeDeviceObjGetDef(virNodeDeviceObjPtr obj);
@ -76,21 +78,12 @@ virNodeDeviceObjListGetParentHost(virNodeDeviceObjListPtr devs,
virNodeDeviceDefPtr def,
int create);
void
virNodeDeviceObjFree(virNodeDeviceObjPtr dev);
virNodeDeviceObjListPtr
virNodeDeviceObjListNew(void);
void
virNodeDeviceObjListFree(virNodeDeviceObjListPtr devs);
void
virNodeDeviceObjLock(virNodeDeviceObjPtr obj);
void
virNodeDeviceObjUnlock(virNodeDeviceObjPtr obj);
typedef bool
(*virNodeDeviceObjListFilter)(virConnectPtr conn,
virNodeDeviceDefPtr def);

4
src/libvirt_private.syms
View File

@ -964,7 +964,7 @@ virNetworkObjUpdateAssignDef;
# conf/virnodedeviceobj.h
virNodeDeviceObjFree;
virNodeDeviceObjEndAPI;
virNodeDeviceObjGetDef;
virNodeDeviceObjListAssignDef;
virNodeDeviceObjListExport;
@ -977,8 +977,6 @@ virNodeDeviceObjListGetParentHost;
virNodeDeviceObjListNew;
virNodeDeviceObjListNumOfDevices;
virNodeDeviceObjListRemove;
virNodeDeviceObjLock;
virNodeDeviceObjUnlock;
# conf/virnwfilterobj.h

18
src/node_device/node_device_driver.c
View File

@ -277,7 +277,7 @@ nodeDeviceLookupByName(virConnectPtr conn,
}
cleanup:
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
return device;
}
@ -314,7 +314,7 @@ nodeDeviceLookupSCSIHostByWWN(virConnectPtr conn,
}
cleanup:
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
return device;
}
@ -345,7 +345,7 @@ nodeDeviceGetXMLDesc(virNodeDevicePtr device,
ret = virNodeDeviceDefFormat(def);
cleanup:
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
return ret;
}
@ -373,7 +373,7 @@ nodeDeviceGetParent(virNodeDevicePtr device)
}
cleanup:
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
return ret;
}
@ -411,7 +411,7 @@ nodeDeviceNumOfCaps(virNodeDevicePtr device)
ret = ncaps;
cleanup:
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
return ret;
}
@ -460,7 +460,7 @@ nodeDeviceListCaps(virNodeDevicePtr device,
ret = ncaps;
cleanup:
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
if (ret == -1) {
--ncaps;
while (--ncaps >= 0)
@ -613,8 +613,7 @@ nodeDeviceDestroy(virNodeDevicePtr device)
* to be taken, so grab the object def which will have the various
* fields used to search (name, parent, parent_wwnn, parent_wwpn,
* or parent_fabric_wwn) and drop the object lock. */
virNodeDeviceObjUnlock(obj);
obj = NULL;
virNodeDeviceObjEndAPI(&obj);
if ((parent_host = virNodeDeviceObjListGetParentHost(driver->devs, def,
EXISTING_DEVICE)) < 0)
goto cleanup;
@ -626,8 +625,7 @@ nodeDeviceDestroy(virNodeDevicePtr device)
cleanup:
nodeDeviceUnlock();
if (obj)
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
VIR_FREE(wwnn);
VIR_FREE(wwpn);
return ret;

8
src/node_device/node_device_hal.c
View File

@ -490,7 +490,7 @@ dev_create(const char *udi)
objdef->sysfs_path = devicePath;
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
nodeDeviceUnlock();
return;
@ -520,7 +520,7 @@ dev_refresh(const char *udi)
nodeDeviceUnlock();
if (obj) {
virNodeDeviceObjFree(obj);
virObjectUnref(obj);
dev_create(udi);
}
}
@ -549,7 +549,7 @@ device_removed(LibHalContext *ctx ATTRIBUTE_UNUSED,
else
VIR_DEBUG("no device named %s", name);
nodeDeviceUnlock();
virNodeDeviceObjFree(obj);
virObjectUnref(obj);
}
@ -568,7 +568,7 @@ device_cap_added(LibHalContext *ctx,
if (obj) {
def = virNodeDeviceObjGetDef(obj);
(void)gather_capability(ctx, udi, cap, &def->caps);
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
} else {
VIR_DEBUG("no device named %s", name);
}

12
src/node_device/node_device_udev.c
View File

@ -1332,7 +1332,7 @@ udevRemoveOneDevice(struct udev_device *device)
VIR_DEBUG("Removing device '%s' with sysfs path '%s'",
def->name, name);
virNodeDeviceObjListRemove(driver->devs, obj);
virNodeDeviceObjFree(obj);
virObjectUnref(obj);
if (event)
virObjectEventStateQueue(driver->nodeDeviceEventState, event);
@ -1369,10 +1369,10 @@ udevSetParent(struct udev_device *device,
parent_sysfs_path))) {
objdef = virNodeDeviceObjGetDef(obj);
if (VIR_STRDUP(def->parent, objdef->name) < 0) {
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
goto cleanup;
}
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
if (VIR_STRDUP(def->parent_sysfs_path, parent_sysfs_path) < 0)
goto cleanup;
@ -1425,7 +1425,7 @@ udevAddOneDevice(struct udev_device *device)
goto cleanup;
if ((obj = virNodeDeviceObjListFindByName(driver->devs, def->name))) {
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
new_device = false;
}
@ -1442,7 +1442,7 @@ udevAddOneDevice(struct udev_device *device)
else
event = virNodeDeviceEventUpdateNew(objdef->name);
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
ret = 0;
@ -1724,7 +1724,7 @@ udevSetupSystemDev(void)
if (!(obj = virNodeDeviceObjListAssignDef(driver->devs, def)))
goto cleanup;
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
ret = 0;

34
src/test/test_driver.c
View File

@ -1177,7 +1177,7 @@ testParseNodedevs(testDriverPtr privconn,
goto error;
}
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
}
ret = 0;
@ -4361,7 +4361,7 @@ testCreateVport(testDriverPtr driver,
* create the vHBA. In the long run the result is the same. */
if (!(obj = testNodeDeviceMockCreateVport(driver, wwnn, wwpn)))
return -1;
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
return 0;
}
@ -4573,7 +4573,7 @@ testDestroyVport(testDriverPtr privconn,
0);
virNodeDeviceObjListRemove(privconn->devs, obj);
virNodeDeviceObjFree(obj);
virObjectUnref(obj);
testObjectEventQueue(privconn, event);
return 0;
@ -5374,7 +5374,7 @@ testNodeDeviceLookupByName(virConnectPtr conn, const char *name)
}
}
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
return ret;
}
@ -5393,7 +5393,7 @@ testNodeDeviceGetXMLDesc(virNodeDevicePtr dev,
ret = virNodeDeviceDefFormat(virNodeDeviceObjGetDef(obj));
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
return ret;
}
@ -5416,7 +5416,7 @@ testNodeDeviceGetParent(virNodeDevicePtr dev)
"%s", _("no parent for this device"));
}
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
return ret;
}
@ -5437,7 +5437,7 @@ testNodeDeviceNumOfCaps(virNodeDevicePtr dev)
for (caps = def->caps; caps; caps = caps->next)
++ncaps;
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
return ncaps;
}
@ -5462,13 +5462,13 @@ testNodeDeviceListCaps(virNodeDevicePtr dev, char **const names, int maxnames)
ncaps++;
}
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
return ncaps;
error:
while (--ncaps >= 0)
VIR_FREE(names[ncaps]);
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
return -1;
}
@ -5499,7 +5499,7 @@ testNodeDeviceMockCreateVport(testDriverPtr driver,
goto cleanup;
xml = virNodeDeviceDefFormat(virNodeDeviceObjGetDef(objcopy));
virNodeDeviceObjUnlock(objcopy);
virNodeDeviceObjEndAPI(&objcopy);
if (!xml)
goto cleanup;
@ -5603,8 +5603,7 @@ testNodeDeviceCreateXML(virConnectPtr conn,
dev = NULL;
cleanup:
if (obj)
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
testDriverUnlock(driver);
virNodeDeviceDefFree(def);
virObjectUnref(dev);
@ -5637,13 +5636,13 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
* taken, so we have to dup the parent's name and drop the lock
* before calling it. We don't need the reference to the object
* any more once we have the parent's name. */
virNodeDeviceObjUnlock(obj);
virObjectUnlock(obj);
/* We do this just for basic validation, but also avoid finding a
* vport capable HBA if for some reason our vHBA doesn't exist */
if (virNodeDeviceObjListGetParentHost(driver->devs, def,
EXISTING_DEVICE) < 0) {
obj = NULL;
virObjectLock(obj);
goto cleanup;
}
@ -5651,14 +5650,13 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
VIR_NODE_DEVICE_EVENT_DELETED,
0);
virNodeDeviceObjLock(obj);
virObjectLock(obj);
virNodeDeviceObjListRemove(driver->devs, obj);
virNodeDeviceObjFree(obj);
virObjectUnref(obj);
obj = NULL;
cleanup:
if (obj)
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjEndAPI(&obj);
testObjectEventQueue(driver, event);
VIR_FREE(parent_name);
VIR_FREE(wwnn);