node_device_udev: Pass the driver state as parameter in preparation for the next commit

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.

Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
This commit is contained in:
Marc Hartmayer 2024-04-23 20:09:01 +02:00 committed by Jonathon Jongsma
parent 2c3e4a0f6e
commit 01ab7047e9
3 changed files with 45 additions and 36 deletions

View File

@ -1885,7 +1885,7 @@ removeMissingPersistentMdev(virNodeDeviceObj *obj,
int int
nodeDeviceUpdateMediatedDevices(void) nodeDeviceUpdateMediatedDevices(virNodeDeviceDriverState *node_driver)
{ {
g_autofree virNodeDeviceDef **defs = NULL; g_autofree virNodeDeviceDef **defs = NULL;
g_autofree virNodeDeviceDef **act_defs = NULL; g_autofree virNodeDeviceDef **act_defs = NULL;
@ -1909,7 +1909,7 @@ nodeDeviceUpdateMediatedDevices(void)
/* Any mdevs that were previously defined but were not returned in the /* Any mdevs that were previously defined but were not returned in the
* latest mdevctl query should be removed from the device list */ * latest mdevctl query should be removed from the device list */
data.defs = defs; data.defs = defs;
virNodeDeviceObjListForEachRemove(driver->devs, virNodeDeviceObjListForEachRemove(node_driver->devs,
removeMissingPersistentMdev, &data); removeMissingPersistentMdev, &data);
for (i = 0; i < data.ndefs; i++) for (i = 0; i < data.ndefs; i++)
@ -2372,7 +2372,7 @@ nodeDeviceUpdate(virNodeDevice *device,
cleanup: cleanup:
virNodeDeviceObjEndAPI(&obj); virNodeDeviceObjEndAPI(&obj);
if (updated) if (updated)
nodeDeviceUpdateMediatedDevices(); nodeDeviceUpdateMediatedDevices(driver);
return ret; return ret;
} }

View File

@ -147,7 +147,7 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring,
bool defined); bool defined);
int int
nodeDeviceUpdateMediatedDevices(void); nodeDeviceUpdateMediatedDevices(virNodeDeviceDriverState *driver);
void void
nodeDeviceGenerateName(virNodeDeviceDef *def, nodeDeviceGenerateName(virNodeDeviceDef *def,

View File

@ -361,7 +361,8 @@ udevTranslatePCIIds(unsigned int vendor,
static int static int
udevProcessPCI(struct udev_device *device, udevProcessPCI(virNodeDeviceDriverState *driver_state,
struct udev_device *device,
virNodeDeviceDef *def) virNodeDeviceDef *def)
{ {
virNodeDevCapPCIDev *pci_dev = &def->caps->data.pci_dev; virNodeDevCapPCIDev *pci_dev = &def->caps->data.pci_dev;
@ -372,8 +373,8 @@ udevProcessPCI(struct udev_device *device,
char *p; char *p;
bool privileged = false; bool privileged = false;
VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) { VIR_WITH_MUTEX_LOCK_GUARD(&driver_state->lock) {
privileged = driver->privileged; privileged = driver_state->privileged;
} }
pci_dev->klass = -1; pci_dev->klass = -1;
@ -1391,12 +1392,13 @@ udevGetDeviceType(struct udev_device *device,
static int static int
udevGetDeviceDetails(struct udev_device *device, udevGetDeviceDetails(virNodeDeviceDriverState *driver_state,
struct udev_device *device,
virNodeDeviceDef *def) virNodeDeviceDef *def)
{ {
switch (def->caps->data.type) { switch (def->caps->data.type) {
case VIR_NODE_DEV_CAP_PCI_DEV: case VIR_NODE_DEV_CAP_PCI_DEV:
return udevProcessPCI(device, def); return udevProcessPCI(driver_state, device, def);
case VIR_NODE_DEV_CAP_USB_DEV: case VIR_NODE_DEV_CAP_USB_DEV:
return udevProcessUSBDevice(device, def); return udevProcessUSBDevice(device, def);
case VIR_NODE_DEV_CAP_USB_INTERFACE: case VIR_NODE_DEV_CAP_USB_INTERFACE:
@ -1444,13 +1446,14 @@ udevGetDeviceDetails(struct udev_device *device,
static int static int
udevRemoveOneDeviceSysPath(const char *path) udevRemoveOneDeviceSysPath(virNodeDeviceDriverState *driver_state,
const char *path)
{ {
virNodeDeviceObj *obj = NULL; virNodeDeviceObj *obj = NULL;
virNodeDeviceDef *def; virNodeDeviceDef *def;
virObjectEvent *event = NULL; 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'", VIR_DEBUG("Failed to find device to remove that has udev path '%s'",
path); path);
return -1; return -1;
@ -1471,20 +1474,21 @@ udevRemoveOneDeviceSysPath(const char *path)
} else { } else {
VIR_DEBUG("Removing device '%s' with sysfs path '%s'", VIR_DEBUG("Removing device '%s' with sysfs path '%s'",
def->name, path); def->name, path);
virNodeDeviceObjListRemove(driver->devs, obj); virNodeDeviceObjListRemove(driver_state->devs, obj);
} }
virNodeDeviceObjEndAPI(&obj); virNodeDeviceObjEndAPI(&obj);
/* cannot check for mdev_types since they have already been removed */ /* cannot check for mdev_types since they have already been removed */
if (nodeDeviceUpdateMediatedDevices() < 0) if (nodeDeviceUpdateMediatedDevices(driver_state) < 0)
VIR_WARN("mdevctl failed to update mediated devices"); VIR_WARN("mdevctl failed to update mediated devices");
virObjectEventStateQueue(driver->nodeDeviceEventState, event); virObjectEventStateQueue(driver_state->nodeDeviceEventState, event);
return 0; return 0;
} }
static int static int
udevSetParent(struct udev_device *device, udevSetParent(virNodeDeviceDriverState *driver_state,
struct udev_device *device,
virNodeDeviceDef *def) virNodeDeviceDef *def)
{ {
struct udev_device *parent_device = NULL; struct udev_device *parent_device = NULL;
@ -1507,7 +1511,7 @@ udevSetParent(struct udev_device *device,
return -1; return -1;
} }
if ((obj = virNodeDeviceObjListFindBySysfsPath(driver->devs, if ((obj = virNodeDeviceObjListFindBySysfsPath(driver_state->devs,
parent_sysfs_path))) { parent_sysfs_path))) {
objdef = virNodeDeviceObjGetDef(obj); objdef = virNodeDeviceObjGetDef(obj);
def->parent = g_strdup(objdef->name); def->parent = g_strdup(objdef->name);
@ -1525,7 +1529,8 @@ udevSetParent(struct udev_device *device,
} }
static int static int
udevAddOneDevice(struct udev_device *device) udevAddOneDevice(virNodeDeviceDriverState *driver_state,
struct udev_device *device)
{ {
g_autofree char *sysfs_path = NULL; g_autofree char *sysfs_path = NULL;
virNodeDeviceDef *def = NULL; virNodeDeviceDef *def = NULL;
@ -1556,15 +1561,15 @@ udevAddOneDevice(struct udev_device *device)
if (udevGetDeviceNodes(device, def) != 0) if (udevGetDeviceNodes(device, def) != 0)
goto cleanup; goto cleanup;
if (udevGetDeviceDetails(device, def) != 0) if (udevGetDeviceDetails(driver_state, device, def) != 0)
goto cleanup; goto cleanup;
if (udevSetParent(device, def) != 0) if (udevSetParent(driver_state, device, def) != 0)
goto cleanup; goto cleanup;
is_mdev = def->caps->data.type == VIR_NODE_DEV_CAP_MDEV; 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); objdef = virNodeDeviceObjGetDef(obj);
if (is_mdev) if (is_mdev)
@ -1582,7 +1587,7 @@ udevAddOneDevice(struct udev_device *device)
/* If this is a device change, the old definition will be freed /* If this is a device change, the old definition will be freed
* and the current definition will take its place. */ * and the current definition will take its place. */
if (!(obj = virNodeDeviceObjListAssignDef(driver->devs, def))) if (!(obj = virNodeDeviceObjListAssignDef(driver_state->devs, def)))
goto cleanup; goto cleanup;
/* @def is now owned by @obj */ /* @def is now owned by @obj */
def = NULL; def = NULL;
@ -1604,7 +1609,7 @@ udevAddOneDevice(struct udev_device *device)
/* The added mdev needs an immediate active config update before the event /* 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 * is issued so that full device information is available at the time that
* the 'created' event is emitted. */ * the 'created' event is emitted. */
if ((has_mdev_types || is_mdev) && (nodeDeviceUpdateMediatedDevices() < 0)) { if ((has_mdev_types || is_mdev) && (nodeDeviceUpdateMediatedDevices(driver_state) < 0)) {
VIR_WARN("Update of mediated device %s failed", VIR_WARN("Update of mediated device %s failed",
NULLSTR_EMPTY(sysfs_path)); NULLSTR_EMPTY(sysfs_path));
} }
@ -1612,7 +1617,7 @@ udevAddOneDevice(struct udev_device *device)
ret = 0; ret = 0;
cleanup: cleanup:
virObjectEventStateQueue(driver->nodeDeviceEventState, event); virObjectEventStateQueue(driver_state->nodeDeviceEventState, event);
if (ret != 0) { if (ret != 0) {
VIR_DEBUG("Discarding device %d %p %s", ret, def, VIR_DEBUG("Discarding device %d %p %s", ret, def,
@ -1625,7 +1630,8 @@ udevAddOneDevice(struct udev_device *device)
static int static int
udevProcessDeviceListEntry(struct udev *udev, udevProcessDeviceListEntry(virNodeDeviceDriverState *driver_state,
struct udev *udev,
struct udev_list_entry *list_entry) struct udev_list_entry *list_entry)
{ {
struct udev_device *device; struct udev_device *device;
@ -1637,7 +1643,7 @@ udevProcessDeviceListEntry(struct udev *udev,
device = udev_device_new_from_syspath(udev, name); device = udev_device_new_from_syspath(udev, name);
if (device != NULL) { if (device != NULL) {
if (udevAddOneDevice(device) != 0) { if (udevAddOneDevice(driver_state, device) != 0) {
VIR_DEBUG("Failed to create node device for udev device '%s'", VIR_DEBUG("Failed to create node device for udev device '%s'",
name); name);
} }
@ -1675,7 +1681,8 @@ udevEnumerateAddMatches(struct udev_enumerate *udev_enumerate)
static int static int
udevEnumerateDevices(struct udev *udev) udevEnumerateDevices(virNodeDeviceDriverState *driver_state,
struct udev *udev)
{ {
struct udev_enumerate *udev_enumerate = NULL; struct udev_enumerate *udev_enumerate = NULL;
struct udev_list_entry *list_entry = NULL; struct udev_list_entry *list_entry = NULL;
@ -1691,7 +1698,7 @@ udevEnumerateDevices(struct udev *udev)
udev_list_entry_foreach(list_entry, udev_list_entry_foreach(list_entry,
udev_enumerate_get_list_entry(udev_enumerate)) { udev_enumerate_get_list_entry(udev_enumerate)) {
udevProcessDeviceListEntry(udev, list_entry); udevProcessDeviceListEntry(driver_state, udev, list_entry);
} }
ret = 0; ret = 0;
@ -1746,12 +1753,12 @@ udevHandleOneDevice(struct udev_device *device)
VIR_DEBUG("udev action: '%s': %s", action, udev_device_get_syspath(device)); VIR_DEBUG("udev action: '%s': %s", action, udev_device_get_syspath(device));
if (STREQ(action, "add") || STREQ(action, "change")) if (STREQ(action, "add") || STREQ(action, "change"))
return udevAddOneDevice(device); return udevAddOneDevice(driver, device);
if (STREQ(action, "remove")) { if (STREQ(action, "remove")) {
const char *path = udev_device_get_syspath(device); const char *path = udev_device_get_syspath(device);
return udevRemoveOneDeviceSysPath(path); return udevRemoveOneDeviceSysPath(driver, path);
} }
if (STREQ(action, "move")) { if (STREQ(action, "move")) {
@ -1760,10 +1767,10 @@ udevHandleOneDevice(struct udev_device *device)
if (devpath_old) { if (devpath_old) {
g_autofree char *devpath_old_fixed = g_strdup_printf("/sys%s", 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; return 0;
@ -1989,11 +1996,11 @@ nodeStateInitializeEnumerate(void *opaque)
udevEventData *priv = driver->privateData; udevEventData *priv = driver->privateData;
/* Populate with known devices */ /* Populate with known devices */
if (udevEnumerateDevices(udev) != 0) if (udevEnumerateDevices(driver, udev) != 0)
goto error; goto error;
/* Load persistent mdevs (which might not be activated yet) and additional /* Load persistent mdevs (which might not be activated yet) and additional
* information about active mediated devices from mdevctl */ * information about active mediated devices from mdevctl */
if (nodeDeviceUpdateMediatedDevices() != 0) if (nodeDeviceUpdateMediatedDevices(driver) != 0)
goto error; goto error;
cleanup: cleanup:
@ -2041,9 +2048,11 @@ udevPCITranslateInit(bool privileged G_GNUC_UNUSED)
static void 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"); VIR_WARN("mdevctl failed to update mediated devices");
} }
@ -2060,7 +2069,7 @@ launchMdevctlUpdateThread(int timer G_GNUC_UNUSED, void *opaque)
} }
if (virThreadCreateFull(&thread, false, mdevctlUpdateThreadFunc, if (virThreadCreateFull(&thread, false, mdevctlUpdateThreadFunc,
"mdevctl-thread", false, NULL) < 0) { "mdevctl-thread", false, driver) < 0) {
virReportSystemError(errno, "%s", virReportSystemError(errno, "%s",
_("failed to create mdevctl thread")); _("failed to create mdevctl thread"));
} }