[PATCH v2] nodedev: transient mdev update on nodeDeviceCreateXML

[Boris Fiuczynski]

Polite ping

On 7/7/23 11:17 AM, Boris Fiuczynski wrote:
> 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