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@linux.ibm.com>
> ---
> 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@redhat.com>
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org