On 4/19/24 9:49 AM, Marc Hartmayer wrote:
It's better practice for all functions called by the threads to pass the driver
via parameter and not global variables. Easier to test and cleaner.
Signed-off-by: Marc Hartmayer <mhartmay@xxxxxxxxxxxxx>
---
src/node_device/node_device_driver.h | 2 +-
src/node_device/node_device_driver.c | 6 +--
src/node_device/node_device_udev.c | 72 ++++++++++++++--------------
3 files changed, 41 insertions(+), 39 deletions(-)
diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h
index f195cfef9d49..2781ad136d68 100644
--- a/src/node_device/node_device_driver.h
+++ b/src/node_device/node_device_driver.h
@@ -147,7 +147,7 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring,
bool defined);
int
-nodeDeviceUpdateMediatedDevices(void);
+nodeDeviceUpdateMediatedDevices(virNodeDeviceDriverState *driver);
void
nodeDeviceGenerateName(virNodeDeviceDef *def,
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index f623339dc973..59c5f9b417a4 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -1887,7 +1887,7 @@ removeMissingPersistentMdev(virNodeDeviceObj *obj,
int
-nodeDeviceUpdateMediatedDevices(void)
+nodeDeviceUpdateMediatedDevices(virNodeDeviceDriverState *node_driver)
{
g_autofree virNodeDeviceDef **defs = NULL;
g_autofree virNodeDeviceDef **act_defs = NULL;
@@ -1911,7 +1911,7 @@ nodeDeviceUpdateMediatedDevices(void)
/* Any mdevs that were previously defined but were not returned in the
* latest mdevctl query should be removed from the device list */
data.defs = defs;
- virNodeDeviceObjListForEachRemove(driver->devs,
+ virNodeDeviceObjListForEachRemove(node_driver->devs,
removeMissingPersistentMdev, &data);
for (i = 0; i < data.ndefs; i++)
@@ -2374,7 +2374,7 @@ nodeDeviceUpdate(virNodeDevice *device,
cleanup:
virNodeDeviceObjEndAPI(&obj);
if (updated)
- nodeDeviceUpdateMediatedDevices();
+ nodeDeviceUpdateMediatedDevices(driver);
return ret;
}
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 38740033a66e..e4b1532dc385 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -361,7 +361,7 @@ udevTranslatePCIIds(unsigned int vendor,
static int
-udevProcessPCI(struct udev_device *device,
+udevProcessPCI(virNodeDeviceDriverState *driver_state, struct udev_device *device,
virNodeDeviceDef *def)
While there are exceptions, the general coding style is to have a single
argument per line for function definitions.
{
virNodeDevCapPCIDev *pci_dev = &def->caps->data.pci_dev;
@@ -372,8 +372,8 @@ udevProcessPCI(struct udev_device *device,
char *p;
bool privileged = false;
- VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) {
- privileged = driver->privileged;
+ VIR_WITH_MUTEX_LOCK_GUARD(&driver_state->lock) {
+ privileged = driver_state->privileged;
}
pci_dev->klass = -1;
@@ -1391,12 +1391,12 @@ udevGetDeviceType(struct udev_device *device,
static int
-udevGetDeviceDetails(struct udev_device *device,
+udevGetDeviceDetails(virNodeDeviceDriverState *driver_state, struct udev_device *device,
virNodeDeviceDef *def)
same
{
switch (def->caps->data.type) {
case VIR_NODE_DEV_CAP_PCI_DEV:
- return udevProcessPCI(device, def);
+ return udevProcessPCI(driver_state, device, def);
case VIR_NODE_DEV_CAP_USB_DEV:
return udevProcessUSBDevice(device, def);
case VIR_NODE_DEV_CAP_USB_INTERFACE:
@@ -1447,13 +1447,13 @@ static void scheduleMdevctlUpdate(udevEventData *data, bool force);
static int
-udevRemoveOneDeviceSysPath(const char *path)
+udevRemoveOneDeviceSysPath(virNodeDeviceDriverState *driver_state, const char *path)
same
{
virNodeDeviceObj *obj = NULL;
virNodeDeviceDef *def;
virObjectEvent *event = NULL;
- if (!(obj = virNodeDeviceObjListFindBySysfsPath(driver->devs, path))) {
+ if (!(obj = virNodeDeviceObjListFindBySysfsPath(driver_state->devs, path))) {
VIR_DEBUG("Failed to find device to remove that has udev path '%s'",
path);
return -1;
@@ -1474,21 +1474,21 @@ udevRemoveOneDeviceSysPath(const char *path)
} else {
VIR_DEBUG("Removing device '%s' with sysfs path '%s'",
def->name, path);
- virNodeDeviceObjListRemove(driver->devs, obj);
+ virNodeDeviceObjListRemove(driver_state->devs, obj);
}
virNodeDeviceObjEndAPI(&obj);
/* cannot check for mdev_types since they have already been removed */
VIR_WITH_OBJECT_LOCK_GUARD(driver->privateData) {
- scheduleMdevctlUpdate(driver->privateData, false);
+ scheduleMdevctlUpdate(driver_state->privateData, false);
}
- virObjectEventStateQueue(driver->nodeDeviceEventState, event);
+ virObjectEventStateQueue(driver_state->nodeDeviceEventState, event);
return 0;
}
static int
-udevSetParent(struct udev_device *device,
+udevSetParent(virNodeDeviceDriverState *driver_state, struct udev_device *device,
same
virNodeDeviceDef *def)
{
struct udev_device *parent_device = NULL;
@@ -1511,7 +1511,7 @@ udevSetParent(struct udev_device *device,
return -1;
}
- if ((obj = virNodeDeviceObjListFindBySysfsPath(driver->devs,
+ if ((obj = virNodeDeviceObjListFindBySysfsPath(driver_state->devs,
parent_sysfs_path))) {
objdef = virNodeDeviceObjGetDef(obj);
def->parent = g_strdup(objdef->name);
@@ -1529,7 +1529,7 @@ udevSetParent(struct udev_device *device,
}
static int
-udevAddOneDevice(struct udev_device *device)
+udevAddOneDevice(virNodeDeviceDriverState *driver_state, struct udev_device *device)
same
{
g_autofree char *sysfs_path = NULL;
virNodeDeviceDef *def = NULL;
@@ -1560,15 +1560,15 @@ udevAddOneDevice(struct udev_device *device)
if (udevGetDeviceNodes(device, def) != 0)
goto cleanup;
- if (udevGetDeviceDetails(device, def) != 0)
+ if (udevGetDeviceDetails(driver_state, device, def) != 0)
goto cleanup;
- if (udevSetParent(device, def) != 0)
+ if (udevSetParent(driver_state, device, def) != 0)
goto cleanup;
is_mdev = def->caps->data.type == VIR_NODE_DEV_CAP_MDEV;
- if ((obj = virNodeDeviceObjListFindByName(driver->devs, def->name))) {
+ if ((obj = virNodeDeviceObjListFindByName(driver_state->devs, def->name))) {
objdef = virNodeDeviceObjGetDef(obj);
if (is_mdev)
@@ -1586,7 +1586,7 @@ udevAddOneDevice(struct udev_device *device)
/* If this is a device change, the old definition will be freed
* and the current definition will take its place. */
- if (!(obj = virNodeDeviceObjListAssignDef(driver->devs, def)))
+ if (!(obj = virNodeDeviceObjListAssignDef(driver_state->devs, def)))
goto cleanup;
/* @def is now owned by @obj */
def = NULL;
@@ -1606,15 +1606,15 @@ udevAddOneDevice(struct udev_device *device)
virNodeDeviceObjEndAPI(&obj);
if (has_mdev_types) {
- VIR_WITH_OBJECT_LOCK_GUARD(driver->privateData) {
- scheduleMdevctlUpdate(driver->privateData, false);
+ VIR_WITH_OBJECT_LOCK_GUARD(driver_state->privateData) {
+ scheduleMdevctlUpdate(driver_state->privateData, false);
}
}
/* The added mdev needs an immediate active config update before the event
* is issued so that full device information is available at the time that
* the 'created' event is emitted. */
- if (is_mdev && (nodeDeviceUpdateMediatedDevices() < 0)) {
+ if (is_mdev && (nodeDeviceUpdateMediatedDevices(driver_state) < 0)) {
VIR_WARN("Update of mediated device %s failed",
NULLSTR_EMPTY(sysfs_path));
}
@@ -1622,7 +1622,7 @@ udevAddOneDevice(struct udev_device *device)
ret = 0;
cleanup:
- virObjectEventStateQueue(driver->nodeDeviceEventState, event);
+ virObjectEventStateQueue(driver_state->nodeDeviceEventState, event);
if (ret != 0) {
VIR_DEBUG("Discarding device %d %p %s", ret, def,
@@ -1635,7 +1635,7 @@ udevAddOneDevice(struct udev_device *device)
static int
-udevProcessDeviceListEntry(struct udev *udev,
+udevProcessDeviceListEntry(virNodeDeviceDriverState *driver_state, struct udev *udev,
same
struct udev_list_entry *list_entry)
{
struct udev_device *device;
@@ -1647,7 +1647,7 @@ udevProcessDeviceListEntry(struct udev *udev,
device = udev_device_new_from_syspath(udev, name);
if (device != NULL) {
- if (udevAddOneDevice(device) != 0) {
+ if (udevAddOneDevice(driver_state, device) != 0) {
VIR_DEBUG("Failed to create node device for udev device '%s'",
name);
}
@@ -1685,7 +1685,7 @@ udevEnumerateAddMatches(struct udev_enumerate *udev_enumerate)
static int
-udevEnumerateDevices(struct udev *udev)
+udevEnumerateDevices(virNodeDeviceDriverState *driver_state, struct udev *udev)
same
{
struct udev_enumerate *udev_enumerate = NULL;
struct udev_list_entry *list_entry = NULL;
@@ -1701,7 +1701,7 @@ udevEnumerateDevices(struct udev *udev)
udev_list_entry_foreach(list_entry,
udev_enumerate_get_list_entry(udev_enumerate)) {
- udevProcessDeviceListEntry(udev, list_entry);
+ udevProcessDeviceListEntry(driver_state, udev, list_entry);
}
ret = 0;
@@ -1756,12 +1756,12 @@ udevHandleOneDevice(struct udev_device *device)
VIR_DEBUG("udev action: '%s': %s", action, udev_device_get_syspath(device));
if (STREQ(action, "add") || STREQ(action, "change"))
- return udevAddOneDevice(device);
+ return udevAddOneDevice(driver, device);
if (STREQ(action, "remove")) {
const char *path = udev_device_get_syspath(device);
- return udevRemoveOneDeviceSysPath(path);
+ return udevRemoveOneDeviceSysPath(driver, path);
}
if (STREQ(action, "move")) {
@@ -1770,10 +1770,10 @@ udevHandleOneDevice(struct udev_device *device)
if (devpath_old) {
g_autofree char *devpath_old_fixed = g_strdup_printf("/sys%s", devpath_old);
- udevRemoveOneDeviceSysPath(devpath_old_fixed);
+ udevRemoveOneDeviceSysPath(driver, devpath_old_fixed);
}
- return udevAddOneDevice(device);
+ return udevAddOneDevice(driver, device);
}
return 0;
@@ -1995,15 +1995,15 @@ udevSetupSystemDev(void)
static void
nodeStateInitializeEnumerate(void *opaque)
{
- struct udev *udev = opaque;
udevEventData *priv = driver->privateData;
+ struct udev *udev = opaque;
unnecessary change?
/* Populate with known devices */
- if (udevEnumerateDevices(udev) != 0)
+ if (udevEnumerateDevices(driver, udev) != 0)
goto error;
/* Load persistent mdevs (which might not be activated yet) and additional
* information about active mediated devices from mdevctl */
- if (nodeDeviceUpdateMediatedDevices() != 0)
+ if (nodeDeviceUpdateMediatedDevices(driver) != 0)
goto error;
cleanup:
@@ -2051,9 +2051,11 @@ udevPCITranslateInit(bool privileged G_GNUC_UNUSED)
static void
-mdevctlUpdateThreadFunc(void *opaque G_GNUC_UNUSED)
+mdevctlUpdateThreadFunc(void *opaque)
{
- if (nodeDeviceUpdateMediatedDevices() < 0)
+ virNodeDeviceDriverState *driver_state = opaque;
+
+ if (nodeDeviceUpdateMediatedDevices(driver_state) < 0)
VIR_WARN("mdevctl failed to update mediated devices");
}
@@ -2070,7 +2072,7 @@ launchMdevctlUpdateThread(int timer G_GNUC_UNUSED, void *opaque)
}
if (virThreadCreateFull(&thread, false, mdevctlUpdateThreadFunc,
- "mdevctl-thread", false, NULL) < 0) {
+ "mdevctl-thread", false, driver) < 0) {
virReportSystemError(errno, "%s",
_("failed to create mdevctl thread"));
}
Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
