Re: [PATCH v1 17/20] node_device_udev: Call `nodeDeviceUpdateMediatedDevices` directly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/19/24 9:49 AM, Marc Hartmayer wrote:
When an udev event occurs the mdev active config data requires an update via
mdevctl as the udev does not contain all config data. This update needs to occur
immediate and to be finished before the libvirt nodedev event is issued to keep
the API usage reliable.

This commit message is almost the same as patch 3 (except the 'immediate' was not changed to 'immediately' ;) This patch is just handling the 'remove' and 'adding a parent device that supports mdevs' use cases where patch 3 handled the 'add a new mdev device' case. Should we just squash patch 3 with this patch and handle all cases at the same time?


The only case where a direct `nodeDeviceUpdateMediatedDevices` is not wished is
`mdevctlEventHandleCallback` - see commit 2c57b28191b9 ("nodedev: Refresh mdev
devices when changes are detected") for details, but for this case there are no
nodedev events created so the problem described above does not exist.

`udevAddOneDevice` and `udevRemoveOneDeviceSysPath` are only called by the
worker pool threads therefore it's possible to call the
`nodeDeviceUpdateMediatedDevices` directly without blocking the udev thread.

These function names no longer exist as of the previous patch, so we should probably update the commit log.

I should also note that there is a comment in node_device_driver.c that refers to the udevAddOneDevice() function which is now also out of date.


Signed-off-by: Marc Hartmayer <mhartmay@xxxxxxxxxxxxx>
---
  src/node_device/node_device_udev.c | 16 +++-------------
  1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 67a8b5cd7132..25571ebf708f 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1504,9 +1504,6 @@ udevGetDeviceDetails(virNodeDeviceDriverState *driver_state, struct udev_device
  }
-static void scheduleMdevctlUpdate(udevEventData *data, bool force);
-
-
  static int
  processNodeDeviceRemoveEvent(virNodeDeviceDriverState *driver_state, const char *path)
  {
@@ -1540,9 +1537,8 @@ processNodeDeviceRemoveEvent(virNodeDeviceDriverState *driver_state, const char
      virNodeDeviceObjEndAPI(&obj);
/* cannot check for mdev_types since they have already been removed */
-    VIR_WITH_OBJECT_LOCK_GUARD(driver->privateData) {
-        scheduleMdevctlUpdate(driver_state->privateData, false);
-    }
+    if (nodeDeviceUpdateMediatedDevices(driver_state) < 0)
+        VIR_WARN("mdevctl failed to update mediated devices");
virObjectEventStateQueue(driver_state->nodeDeviceEventState, event);
      return 0;
@@ -1666,16 +1662,10 @@ processNodeDeviceAddAndChangeEvent(virNodeDeviceDriverState *driver_state, struc
      has_mdev_types = virNodeDeviceObjHasCap(obj, VIR_NODE_DEV_CAP_MDEV_TYPES);
      virNodeDeviceObjEndAPI(&obj);
- if (has_mdev_types) {
-        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(driver_state) < 0)) {
+    if ((has_mdev_types || is_mdev) && (nodeDeviceUpdateMediatedDevices(driver_state) < 0)) {
          VIR_WARN("Update of mediated device %s failed",
                   NULLSTR_EMPTY(sysfs_path));
      }

Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux