[PATCH v2] nodedev: transient mdev update on nodeDeviceCreateXML

[Boris Fiuczynski]

On 7/6/23 9:40 PM, Jonathon Jongsma wrote:
> 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 at linux.ibm.com>
>> ---
>>   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
> 

LGTM and testing was successful.
See my direct replies on the patches.


-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294