[libvirt] [PATCH v4 06/14] nodedev: Introduce virNodeDeviceObjListNew

In preparation to make things private, make the ->devs be pointers to a virNodeDeviceObjList and then manage everything inside virnodedeviceobj Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/conf/virnodedeviceobj.c | 13 ++++++++++++- src/conf/virnodedeviceobj.h | 5 ++++- src/libvirt_private.syms | 1 + src/node_device/node_device_driver.c | 14 +++++++------- src/node_device/node_device_hal.c | 19 +++++++++++-------- src/node_device/node_device_udev.c | 18 +++++++++++------- src/test/test_driver.c | 29 +++++++++++++++-------------- 7 files changed, 61 insertions(+), 38 deletions(-) diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index 9a2b5de..0c89790 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -274,6 +274,17 @@ virNodeDeviceObjFree(virNodeDeviceObjPtr obj) } +virNodeDeviceObjListPtr +virNodeDeviceObjListNew(void) +{ + virNodeDeviceObjListPtr devs; + + if (VIR_ALLOC(devs) < 0) + return NULL; + return devs; +} + + void virNodeDeviceObjListFree(virNodeDeviceObjListPtr devs) { @@ -281,7 +292,7 @@ virNodeDeviceObjListFree(virNodeDeviceObjListPtr devs) for (i = 0; i < devs->count; i++) virNodeDeviceObjFree(devs->objs[i]); VIR_FREE(devs->objs); - devs->count = 0; + VIR_FREE(devs); } diff --git a/src/conf/virnodedeviceobj.h b/src/conf/virnodedeviceobj.h index 9bc02ee..77250a0 100644 --- a/src/conf/virnodedeviceobj.h +++ b/src/conf/virnodedeviceobj.h @@ -32,7 +32,7 @@ typedef virNodeDeviceDriverState *virNodeDeviceDriverStatePtr; struct _virNodeDeviceDriverState { virMutex lock; - virNodeDeviceObjList devs; /* currently-known devices */ + virNodeDeviceObjListPtr devs; /* currently-known devices */ void *privateData; /* driver-specific private data */ /* Immutable pointer, self-locking APIs */ @@ -68,6 +68,9 @@ virNodeDeviceObjGetParentHost(virNodeDeviceObjListPtr devs, void virNodeDeviceObjFree(virNodeDeviceObjPtr dev); +virNodeDeviceObjListPtr +virNodeDeviceObjListNew(void); + void virNodeDeviceObjListFree(virNodeDeviceObjListPtr devs); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e6ed981..c7140c8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -971,6 +971,7 @@ virNodeDeviceObjGetNames; virNodeDeviceObjGetParentHost; virNodeDeviceObjListExport; virNodeDeviceObjListFree; +virNodeDeviceObjListNew; virNodeDeviceObjLock; virNodeDeviceObjNumOfDevices; virNodeDeviceObjRemove; diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 45eb3f2..1ad2557 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -182,7 +182,7 @@ nodeNumOfDevices(virConnectPtr conn, virCheckFlags(0, -1); nodeDeviceLock(); - ndevs = virNodeDeviceObjNumOfDevices(&driver->devs, conn, cap, + ndevs = virNodeDeviceObjNumOfDevices(driver->devs, conn, cap, virNodeNumOfDevicesCheckACL); nodeDeviceUnlock(); @@ -205,7 +205,7 @@ nodeListDevices(virConnectPtr conn, virCheckFlags(0, -1); nodeDeviceLock(); - nnames = virNodeDeviceObjGetNames(&driver->devs, conn, + nnames = virNodeDeviceObjGetNames(driver->devs, conn, virNodeListDevicesCheckACL, cap, names, maxnames); nodeDeviceUnlock(); @@ -227,7 +227,7 @@ nodeConnectListAllNodeDevices(virConnectPtr conn, return -1; nodeDeviceLock(); - ret = virNodeDeviceObjListExport(conn, &driver->devs, devices, + ret = virNodeDeviceObjListExport(conn, driver->devs, devices, virConnectListAllNodeDevicesCheckACL, flags); nodeDeviceUnlock(); @@ -241,7 +241,7 @@ nodeDeviceObjFindByName(const char *name) virNodeDeviceObjPtr obj; nodeDeviceLock(); - obj = virNodeDeviceObjFindByName(&driver->devs, name); + obj = virNodeDeviceObjFindByName(driver->devs, name); nodeDeviceUnlock(); if (!obj) { @@ -289,7 +289,7 @@ nodeDeviceLookupSCSIHostByWWN(virConnectPtr conn, unsigned int flags) { size_t i; - virNodeDeviceObjListPtr devs = &driver->devs; + virNodeDeviceObjListPtr devs = driver->devs; virNodeDevCapsDefPtr cap = NULL; virNodeDeviceObjPtr obj = NULL; virNodeDeviceDefPtr def; @@ -587,7 +587,7 @@ nodeDeviceCreateXML(virConnectPtr conn, if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) == -1) goto cleanup; - if ((parent_host = virNodeDeviceObjGetParentHost(&driver->devs, def, + if ((parent_host = virNodeDeviceObjGetParentHost(driver->devs, def, CREATE_DEVICE)) < 0) goto cleanup; @@ -639,7 +639,7 @@ nodeDeviceDestroy(virNodeDevicePtr device) * or parent_fabric_wwn) and drop the object lock. */ virNodeDeviceObjUnlock(obj); obj = NULL; - if ((parent_host = virNodeDeviceObjGetParentHost(&driver->devs, def, + if ((parent_host = virNodeDeviceObjGetParentHost(driver->devs, def, EXISTING_DEVICE)) < 0) goto cleanup; diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 6441a3d..fcffaab 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -482,7 +482,7 @@ dev_create(const char *udi) /* Some devices don't have a path in sysfs, so ignore failure */ (void)get_str_prop(ctx, udi, "linux.sysfs_path", &devicePath); - if (!(obj = virNodeDeviceObjAssignDef(&driver->devs, def))) { + if (!(obj = virNodeDeviceObjAssignDef(driver->devs, def))) { VIR_FREE(devicePath); goto failure; } @@ -509,11 +509,11 @@ dev_refresh(const char *udi) virNodeDeviceObjPtr obj; nodeDeviceLock(); - if ((obj = virNodeDeviceObjFindByName(&driver->devs, name))) { + if ((obj = virNodeDeviceObjFindByName(driver->devs, name))) { /* Simply "rediscover" device -- incrementally handling changes * to sub-capabilities (like net.80203) is nasty ... so avoid it. */ - virNodeDeviceObjRemove(&driver->devs, obj); + virNodeDeviceObjRemove(driver->devs, obj); } else { VIR_DEBUG("no device named %s", name); } @@ -542,10 +542,10 @@ device_removed(LibHalContext *ctx ATTRIBUTE_UNUSED, virNodeDeviceObjPtr obj; nodeDeviceLock(); - obj = virNodeDeviceObjFindByName(&driver->devs, name); + obj = virNodeDeviceObjFindByName(driver->devs, name); VIR_DEBUG("%s", name); if (obj) - virNodeDeviceObjRemove(&driver->devs, obj); + virNodeDeviceObjRemove(driver->devs, obj); else VIR_DEBUG("no device named %s", name); nodeDeviceUnlock(); @@ -562,7 +562,7 @@ device_cap_added(LibHalContext *ctx, virNodeDeviceDefPtr def; nodeDeviceLock(); - obj = virNodeDeviceObjFindByName(&driver->devs, name); + obj = virNodeDeviceObjFindByName(driver->devs, name); nodeDeviceUnlock(); VIR_DEBUG("%s %s", cap, name); if (obj) { @@ -627,6 +627,9 @@ nodeStateInitialize(bool privileged ATTRIBUTE_UNUSED, } nodeDeviceLock(); + if (!(driver->devs = virNodeDeviceObjListNew())) + goto failure; + dbus_error_init(&err); if (!(sysbus = virDBusGetSystemBus())) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -701,7 +704,7 @@ nodeStateInitialize(bool privileged ATTRIBUTE_UNUSED, _("%s: %s"), err.name, err.message); dbus_error_free(&err); } - virNodeDeviceObjListFree(&driver->devs); + virNodeDeviceObjListFree(driver->devs); if (hal_ctx) (void)libhal_ctx_free(hal_ctx); nodeDeviceUnlock(); @@ -717,7 +720,7 @@ nodeStateCleanup(void) if (driver) { nodeDeviceLock(); LibHalContext *hal_ctx = DRV_STATE_HAL_CTX(driver); - virNodeDeviceObjListFree(&driver->devs); + virNodeDeviceObjListFree(driver->devs); (void)libhal_ctx_shutdown(hal_ctx, NULL); (void)libhal_ctx_free(hal_ctx); nodeDeviceUnlock(); diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 2672784..b113d93 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1318,7 +1318,7 @@ udevRemoveOneDevice(struct udev_device *device) const char *name = NULL; name = udev_device_get_syspath(device); - if (!(obj = virNodeDeviceObjFindBySysfsPath(&driver->devs, name))) { + if (!(obj = virNodeDeviceObjFindBySysfsPath(driver->devs, name))) { VIR_DEBUG("Failed to find device to remove that has udev name '%s'", name); return -1; @@ -1331,7 +1331,7 @@ udevRemoveOneDevice(struct udev_device *device) VIR_DEBUG("Removing device '%s' with sysfs path '%s'", def->name, name); - virNodeDeviceObjRemove(&driver->devs, obj); + virNodeDeviceObjRemove(driver->devs, obj); virNodeDeviceObjFree(obj); if (event) @@ -1365,7 +1365,7 @@ udevSetParent(struct udev_device *device, goto cleanup; } - if ((obj = virNodeDeviceObjFindBySysfsPath(&driver->devs, + if ((obj = virNodeDeviceObjFindBySysfsPath(driver->devs, parent_sysfs_path))) { objdef = virNodeDeviceObjGetDef(obj); if (VIR_STRDUP(def->parent, objdef->name) < 0) { @@ -1424,14 +1424,14 @@ udevAddOneDevice(struct udev_device *device) if (udevSetParent(device, def) != 0) goto cleanup; - if ((obj = virNodeDeviceObjFindByName(&driver->devs, def->name))) { + if ((obj = virNodeDeviceObjFindByName(driver->devs, def->name))) { virNodeDeviceObjUnlock(obj); new_device = false; } /* If this is a device change, the old definition will be freed * and the current definition will take its place. */ - if (!(obj = virNodeDeviceObjAssignDef(&driver->devs, def))) + if (!(obj = virNodeDeviceObjAssignDef(driver->devs, def))) goto cleanup; objdef = virNodeDeviceObjGetDef(obj); @@ -1585,7 +1585,7 @@ nodeStateCleanup(void) if (udev != NULL) udev_unref(udev); - virNodeDeviceObjListFree(&driver->devs); + virNodeDeviceObjListFree(driver->devs); nodeDeviceUnlock(); virMutexDestroy(&driver->lock); VIR_FREE(driver); @@ -1721,7 +1721,7 @@ udevSetupSystemDev(void) udevGetDMIData(&def->caps->data.system); #endif - if (!(obj = virNodeDeviceObjAssignDef(&driver->devs, def))) + if (!(obj = virNodeDeviceObjAssignDef(driver->devs, def))) goto cleanup; virNodeDeviceObjUnlock(obj); @@ -1790,6 +1790,10 @@ nodeStateInitialize(bool privileged, driver->privateData = priv; nodeDeviceLock(); + + if (!(driver->devs = virNodeDeviceObjListNew())) + goto cleanup; + driver->nodeDeviceEventState = virObjectEventStateNew(); if (udevPCITranslateInit(privileged) < 0) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 2889ffa..48ef92f 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -101,7 +101,7 @@ struct _testDriver { bool transaction_running; virInterfaceObjListPtr backupIfaces; virStoragePoolObjList pools; - virNodeDeviceObjList devs; + virNodeDeviceObjListPtr devs; int numCells; testCell cells[MAX_CELLS]; size_t numAuths; @@ -152,7 +152,7 @@ testDriverFree(testDriverPtr driver) virObjectUnref(driver->caps); virObjectUnref(driver->xmlopt); virObjectUnref(driver->domains); - virNodeDeviceObjListFree(&driver->devs); + virNodeDeviceObjListFree(driver->devs); virObjectUnref(driver->networks); virInterfaceObjListFree(driver->ifaces); virStoragePoolObjListFree(&driver->pools); @@ -418,7 +418,8 @@ testDriverNew(void) !(ret->eventState = virObjectEventStateNew()) || !(ret->ifaces = virInterfaceObjListNew()) || !(ret->domains = virDomainObjListNew()) || - !(ret->networks = virNetworkObjListNew())) + !(ret->networks = virNetworkObjListNew()) || + !(ret->devs = virNodeDeviceObjListNew())) goto error; virAtomicIntSet(&ret->nextDomID, 1); @@ -1169,7 +1170,7 @@ testParseNodedevs(testDriverPtr privconn, if (!def) goto error; - if (!(obj = virNodeDeviceObjAssignDef(&privconn->devs, def))) { + if (!(obj = virNodeDeviceObjAssignDef(privconn->devs, def))) { virNodeDeviceDefFree(def); goto error; } @@ -4519,7 +4520,7 @@ testDestroyVport(testDriverPtr privconn, * * Reaching across the boundaries of space and time into the * Node Device in order to remove */ - if (!(obj = virNodeDeviceObjFindByName(&privconn->devs, "scsi_host12"))) { + if (!(obj = virNodeDeviceObjFindByName(privconn->devs, "scsi_host12"))) { virReportError(VIR_ERR_NO_NODE_DEVICE, "%s", _("no node device with matching name 'scsi_host12'")); return -1; @@ -4529,7 +4530,7 @@ testDestroyVport(testDriverPtr privconn, VIR_NODE_DEVICE_EVENT_DELETED, 0); - virNodeDeviceObjRemove(&privconn->devs, obj); + virNodeDeviceObjRemove(privconn->devs, obj); virNodeDeviceObjFree(obj); testObjectEventQueue(privconn, event); @@ -5261,7 +5262,7 @@ testNodeDeviceObjFindByName(testDriverPtr driver, virNodeDeviceObjPtr obj; testDriverLock(driver); - obj = virNodeDeviceObjFindByName(&driver->devs, name); + obj = virNodeDeviceObjFindByName(driver->devs, name); testDriverUnlock(driver); if (!obj) @@ -5284,7 +5285,7 @@ testNodeNumOfDevices(virConnectPtr conn, virCheckFlags(0, -1); testDriverLock(driver); - ndevs = virNodeDeviceObjNumOfDevices(&driver->devs, conn, cap, NULL); + ndevs = virNodeDeviceObjNumOfDevices(driver->devs, conn, cap, NULL); testDriverUnlock(driver); return ndevs; @@ -5304,7 +5305,7 @@ testNodeListDevices(virConnectPtr conn, virCheckFlags(0, -1); testDriverLock(driver); - nnames = virNodeDeviceObjGetNames(&driver->devs, conn, NULL, + nnames = virNodeDeviceObjGetNames(driver->devs, conn, NULL, cap, names, maxnames); testDriverUnlock(driver); @@ -5451,7 +5452,7 @@ testNodeDeviceMockCreateVport(testDriverPtr driver, * using the scsi_host11 definition, changing the name and the * scsi_host capability fields before calling virNodeDeviceAssignDef * to add the def to the node device objects list. */ - if (!(objcopy = virNodeDeviceObjFindByName(&driver->devs, "scsi_host11"))) + if (!(objcopy = virNodeDeviceObjFindByName(driver->devs, "scsi_host11"))) goto cleanup; xml = virNodeDeviceDefFormat(virNodeDeviceObjGetDef(objcopy)); @@ -5491,7 +5492,7 @@ testNodeDeviceMockCreateVport(testDriverPtr driver, caps = caps->next; } - if (!(obj = virNodeDeviceObjAssignDef(&driver->devs, def))) + if (!(obj = virNodeDeviceObjAssignDef(driver->devs, def))) goto cleanup; def = NULL; objdef = virNodeDeviceObjGetDef(obj); @@ -5536,7 +5537,7 @@ testNodeDeviceCreateXML(virConnectPtr conn, /* Unlike the "real" code we don't need the parent_host in order to * call virVHBAManageVport, but still let's make sure the code finds * something valid and no one messed up the mock environment. */ - if (virNodeDeviceObjGetParentHost(&driver->devs, def, CREATE_DEVICE) < 0) + if (virNodeDeviceObjGetParentHost(driver->devs, def, CREATE_DEVICE) < 0) goto cleanup; /* In the real code, we'd call virVHBAManageVport followed by @@ -5597,7 +5598,7 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) /* 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 (virNodeDeviceObjGetParentHost(&driver->devs, def, + if (virNodeDeviceObjGetParentHost(driver->devs, def, EXISTING_DEVICE) < 0) { obj = NULL; goto cleanup; @@ -5608,7 +5609,7 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) 0); virNodeDeviceObjLock(obj); - virNodeDeviceObjRemove(&driver->devs, obj); + virNodeDeviceObjRemove(driver->devs, obj); virNodeDeviceObjFree(obj); obj = NULL; -- 2.9.4