On 4/12/24 8:36 AM, Marc Hartmayer wrote:
Since @driver->privateData is modified take the lock. Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx> Signed-off-by: Marc Hartmayer <mhartmay@xxxxxxxxxxxxx> --- src/node_device/node_device_udev.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 5d474acdc2e0..0c393b6233a4 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1482,7 +1482,9 @@ udevRemoveOneDeviceSysPath(const char *path) virNodeDeviceObjEndAPI(&obj);/* cannot check for mdev_types since they have already been removed */- scheduleMdevctlUpdate(driver->privateData, false); + VIR_WITH_OBJECT_LOCK_GUARD(driver->privateData) { + scheduleMdevctlUpdate(driver->privateData, false); + }virObjectEventStateQueue(driver->nodeDeviceEventState, event);return 0; @@ -1616,8 +1618,11 @@ udevAddOneDevice(struct udev_device *device) has_mdev_types = virNodeDeviceObjHasCap(obj, VIR_NODE_DEV_CAP_MDEV_TYPES); virNodeDeviceObjEndAPI(&obj);- if (has_mdev_types)- scheduleMdevctlUpdate(driver->privateData, false); + if (has_mdev_types) { + VIR_WITH_OBJECT_LOCK_GUARD(driver->privateData) { + scheduleMdevctlUpdate(driver->privateData, false); + } + }/* The added mdev needs an immediate active config update before* the event is issued to allow sane API usage. */ @@ -2241,7 +2246,9 @@ mdevctlEventHandleCallback(GFileMonitor *monitor G_GNUC_UNUSED, * configuration change, try to coalesce these changes by waiting for the * CHANGES_DONE_HINT event. As a fallback, add a timeout to trigger the * signal if that event never comes */ - scheduleMdevctlUpdate(priv, (event_type == G_FILE_MONITOR_EVENT_CHANGES_DONE_HINT)); + VIR_WITH_OBJECT_LOCK_GUARD(priv) { + scheduleMdevctlUpdate(priv, (event_type == G_FILE_MONITOR_EVENT_CHANGES_DONE_HINT)); + } }
I don't see any cases where we would not want to take the lock (e.g. because it has already been taken), so maybe it would be better simply to lock the object within the scheduleMdevctlUpdate() function rather than requiring each caller to lock it?
Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx> _______________________________________________ Devel mailing list -- devel@xxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
