Re: [PATCH v2] nodedev: transient mdev update on nodeDeviceCreateXML

On 6/30/23 6:34 AM, Boris Fiuczynski wrote: > Update the optional mdev attributes by running an mdevctl update on a > new created nodedev object representing an mdev. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2143158 > Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; > --- >   src/node_device/node_device_udev.c | 12 ++++++++++-- >   1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/src/node_device/node_device_udev.c > b/src/node_device/node_device_udev.c > index 4c37ec3189..fce4212728 100644 > --- a/src/node_device/node_device_udev.c > +++ b/src/node_device/node_device_udev.c > @@ -1757,12 +1757,20 @@ nodeStateCleanup(void) >   static int >   udevHandleOneDevice(struct udev_device *device) >   { > +    virNodeDevCapType dev_cap_type; >       const char *action = udev_device_get_action(device); >       VIR_DEBUG("udev action: '%s': %s", action, > udev_device_get_syspath(device)); > -    if (STREQ(action, "add") || STREQ(action, "change")) > -        return udevAddOneDevice(device); > +    if (STREQ(action, "add") || STREQ(action, "change")) { > +        int ret = udevAddOneDevice(device); > +        if (ret == 0 && > +            udevGetDeviceType(device, &dev_cap_type) == 0 && > +            dev_cap_type == VIR_NODE_DEV_CAP_MDEV) > +            if (nodeDeviceUpdateMediatedDevices() < 0) > +                VIR_WARN("mdevctl failed to update mediated devices"); > +        return ret; > +    } >       if (STREQ(action, "remove")) >           return udevRemoveOneDevice(device); So, this should work and roughly matches what we've done for other similar cases. But this is running from within the udev event handler thread, and theoretically there could already be an mdevctl thread running concurrently and updating the internal device list. The device list update functions are thread-safe so I don't think it'll corrupt anything, but it seems better to do all mdevctl updates from  mdevctl update thread rather than calling it directly here. The same actually applies to a couple other existing calls to nodeDeviceUpdateMediatedDevices(). I'll send a couple patches that apply on top of yours and refactor things a bit. Let me know what you think. Jonathon