[PATCH 0/3] nodedev: dynamic parent update on mdev definitions

Mdev definitions can be created regardless of the existence of there parent devices in mdevctl. Parent objects of mdev definitions can also vanish dynamically. This series adds the missing support for these scenarios. Boris Fiuczynski (3): virnodedeviceobj: export virNodeDeviceObjHasCap nodedev: update mdevs on parent change nodedev: trigger mdev device definition update on udev add and remove src/conf/virnodedeviceobj.c | 3 +-- src/conf/virnodedeviceobj.h | 4 ++++ src/libvirt_private.syms | 1 + src/node_device/node_device_driver.c | 4 +++- src/node_device/node_device_udev.c | 9 +++++++++ 5 files changed, 18 insertions(+), 3 deletions(-) -- 2.33.1

The function will be reused in the nodedev drivers udev handling. Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/conf/virnodedeviceobj.c | 3 +-- src/conf/virnodedeviceobj.h | 4 ++++ src/libvirt_private.syms | 1 + 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index ba84dce82b..fc23e6ea01 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -58,7 +58,6 @@ static virClass *virNodeDeviceObjClass; static virClass *virNodeDeviceObjListClass; static void virNodeDeviceObjDispose(void *opaque); static void virNodeDeviceObjListDispose(void *opaque); -static bool virNodeDeviceObjHasCap(const virNodeDeviceObj *obj, int type); static int virNodeDeviceObjOnceInit(void) @@ -684,7 +683,7 @@ virNodeDeviceObjListGetParentHost(virNodeDeviceObjList *devs, } -static bool +bool virNodeDeviceObjHasCap(const virNodeDeviceObj *obj, int type) { diff --git a/src/conf/virnodedeviceobj.h b/src/conf/virnodedeviceobj.h index 5f6d22e1f6..ba2e424998 100644 --- a/src/conf/virnodedeviceobj.h +++ b/src/conf/virnodedeviceobj.h @@ -86,6 +86,10 @@ int virNodeDeviceObjListGetParentHost(virNodeDeviceObjList *devs, virNodeDeviceDef *def); +bool +virNodeDeviceObjHasCap(const virNodeDeviceObj *obj, + int type); + virNodeDeviceObjList * virNodeDeviceObjListNew(void); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6f0d72ca38..ef1c60d103 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1298,6 +1298,7 @@ virNetworkPortDefSaveStatus; # conf/virnodedeviceobj.h virNodeDeviceObjEndAPI; virNodeDeviceObjGetDef; +virNodeDeviceObjHasCap; virNodeDeviceObjIsActive; virNodeDeviceObjIsAutostart; virNodeDeviceObjIsPersistent; -- 2.33.1

The parent of the mdev definition can change due to the existance of the parent device. The parents existance can e.g. depend on the device driver load state. Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/node_device/node_device_driver.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 7b2fb3d953..84dde743df 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -1670,7 +1670,9 @@ removeMissingPersistentMdev(virNodeDeviceObj *obj, for (i = 0; i < data->ndefs; i++) { /* OK, this mdev is still defined by mdevctl */ - if (STREQ(data->defs[i]->name, def->name)) + /* AND the parent object has not changed */ + if (STREQ(data->defs[i]->name, def->name) && + STREQ(data->defs[i]->parent, def->parent)) return false; } -- 2.33.1

When nodedev objects are added and removed if possible check if mdev-types is supported by the object and trigger a mdev device definition update to correct the associated parent nodedevs. Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/node_device/node_device_udev.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index b0a5e6302c..e17373a0b0 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1440,6 +1440,10 @@ udevRemoveOneDeviceSysPath(const char *path) } virNodeDeviceObjEndAPI(&obj); + /* cannot check for mdev_types since they have already been removed */ + if (nodeDeviceUpdateMediatedDevices() < 0) + VIR_WARN("mdevctl failed to updated mediated devices"); + virObjectEventStateQueue(driver->nodeDeviceEventState, event); return 0; } @@ -1507,6 +1511,7 @@ udevAddOneDevice(struct udev_device *device) bool persistent = false; bool autostart = false; bool is_mdev; + bool is_mdev_types = false; def = g_new0(virNodeDeviceDef, 1); @@ -1562,8 +1567,12 @@ udevAddOneDevice(struct udev_device *device) event = virNodeDeviceEventUpdateNew(objdef->name); virNodeDeviceObjSetActive(obj, true); + is_mdev_types = virNodeDeviceObjHasCap(obj, VIR_NODE_DEV_CAP_MDEV_TYPES); virNodeDeviceObjEndAPI(&obj); + if (is_mdev_types && nodeDeviceUpdateMediatedDevices() < 0) + VIR_WARN("mdevctl failed to updated mediated devices"); + ret = 0; cleanup: -- 2.33.1

On 3/15/22 09:56, Boris Fiuczynski wrote:
When nodedev objects are added and removed if possible check if mdev-types is supported by the object and trigger a mdev device definition update to correct the associated parent nodedevs.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/node_device/node_device_udev.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index b0a5e6302c..e17373a0b0 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1440,6 +1440,10 @@ udevRemoveOneDeviceSysPath(const char *path) } virNodeDeviceObjEndAPI(&obj);
+ /* cannot check for mdev_types since they have already been removed */ + if (nodeDeviceUpdateMediatedDevices() < 0) + VIR_WARN("mdevctl failed to updated mediated devices");
failed to update
+ virObjectEventStateQueue(driver->nodeDeviceEventState, event); return 0; } @@ -1507,6 +1511,7 @@ udevAddOneDevice(struct udev_device *device) bool persistent = false; bool autostart = false; bool is_mdev; + bool is_mdev_types = false;
has_mdev_cap perhaps?
def = g_new0(virNodeDeviceDef, 1);
@@ -1562,8 +1567,12 @@ udevAddOneDevice(struct udev_device *device) event = virNodeDeviceEventUpdateNew(objdef->name);
virNodeDeviceObjSetActive(obj, true); + is_mdev_types = virNodeDeviceObjHasCap(obj, VIR_NODE_DEV_CAP_MDEV_TYPES); virNodeDeviceObjEndAPI(&obj);
+ if (is_mdev_types && nodeDeviceUpdateMediatedDevices() < 0) + VIR_WARN("mdevctl failed to updated mediated devices");
s/updated/update/
+ ret = 0;
cleanup:
Otherwise looking good. I'll let Jonathon express his thoughts, but you have my: Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

On 3/16/22 2:32 PM, Michal Prívozník wrote:
On 3/15/22 09:56, Boris Fiuczynski wrote:
When nodedev objects are added and removed if possible check if mdev-types is supported by the object and trigger a mdev device definition update to correct the associated parent nodedevs.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/node_device/node_device_udev.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index b0a5e6302c..e17373a0b0 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1440,6 +1440,10 @@ udevRemoveOneDeviceSysPath(const char *path) } virNodeDeviceObjEndAPI(&obj);
+ /* cannot check for mdev_types since they have already been removed */ + if (nodeDeviceUpdateMediatedDevices() < 0) + VIR_WARN("mdevctl failed to updated mediated devices");
failed to update
Uh, I will need to send another patch to fix the original line as well.
+ virObjectEventStateQueue(driver->nodeDeviceEventState, event); return 0; } @@ -1507,6 +1511,7 @@ udevAddOneDevice(struct udev_device *device) bool persistent = false; bool autostart = false; bool is_mdev; + bool is_mdev_types = false;
has_mdev_cap perhaps?
Yes, makes sense!
def = g_new0(virNodeDeviceDef, 1);
@@ -1562,8 +1567,12 @@ udevAddOneDevice(struct udev_device *device) event = virNodeDeviceEventUpdateNew(objdef->name);
virNodeDeviceObjSetActive(obj, true); + is_mdev_types = virNodeDeviceObjHasCap(obj, VIR_NODE_DEV_CAP_MDEV_TYPES); virNodeDeviceObjEndAPI(&obj);
+ if (is_mdev_types && nodeDeviceUpdateMediatedDevices() < 0) + VIR_WARN("mdevctl failed to updated mediated devices");
s/updated/update/
Yes, see above
+ ret = 0;
cleanup:
Otherwise looking good. I'll let Jonathon express his thoughts, but you have my:
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal
Michael, thanks for your r-b. -- 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

On 3/16/22 8:32 AM, Michal Prívozník wrote:
On 3/15/22 09:56, Boris Fiuczynski wrote:
When nodedev objects are added and removed if possible check if mdev-types is supported by the object and trigger a mdev device definition update to correct the associated parent nodedevs.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/node_device/node_device_udev.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index b0a5e6302c..e17373a0b0 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1440,6 +1440,10 @@ udevRemoveOneDeviceSysPath(const char *path) } virNodeDeviceObjEndAPI(&obj);
+ /* cannot check for mdev_types since they have already been removed */ + if (nodeDeviceUpdateMediatedDevices() < 0) + VIR_WARN("mdevctl failed to updated mediated devices");
failed to update
+ virObjectEventStateQueue(driver->nodeDeviceEventState, event); return 0; } @@ -1507,6 +1511,7 @@ udevAddOneDevice(struct udev_device *device) bool persistent = false; bool autostart = false; bool is_mdev; + bool is_mdev_types = false;
has_mdev_cap perhaps?
def = g_new0(virNodeDeviceDef, 1);
@@ -1562,8 +1567,12 @@ udevAddOneDevice(struct udev_device *device) event = virNodeDeviceEventUpdateNew(objdef->name);
virNodeDeviceObjSetActive(obj, true); + is_mdev_types = virNodeDeviceObjHasCap(obj, VIR_NODE_DEV_CAP_MDEV_TYPES); virNodeDeviceObjEndAPI(&obj);
+ if (is_mdev_types && nodeDeviceUpdateMediatedDevices() < 0) + VIR_WARN("mdevctl failed to updated mediated devices");
s/updated/update/
+ ret = 0;
cleanup:
Otherwise looking good. I'll let Jonathon express his thoughts, but you have my:
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal
looks ok to me with michal's suggestions Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
participants (3)
-
Boris Fiuczynski
-
Jonathon Jongsma
-
Michal Prívozník