[libvirt PATCH 6/7] nodedev: Implement virNodeDeviceIsPersistent()/IsActive()

[Boris Fiuczynski]

On 6/3/21 10:11 PM, Jonathon Jongsma wrote:
> Implement these new API functions in the nodedev driver.
> 
> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
> ---
>   src/node_device/node_device_driver.c | 50 ++++++++++++++++++++++++++++
>   src/node_device/node_device_driver.h |  6 ++++
>   src/node_device/node_device_udev.c   | 21 +++++++-----
>   3 files changed, 69 insertions(+), 8 deletions(-)
> 
> diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
> index 9ebe609aa4..75391f18b8 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -1804,3 +1804,53 @@ nodeDeviceGetAutostart(virNodeDevice *device,
>       virNodeDeviceObjEndAPI(&obj);
>       return ret;
>   }
> +
> +
> +int
> +nodeDeviceIsPersistent(virNodeDevice *device)
> +{
> +    virNodeDeviceObj *obj = NULL;
> +    virNodeDeviceDef *def = NULL;
> +    int ret = -1;
> +
> +    if (nodeDeviceInitWait() < 0)
> +        return -1;
> +
> +    if (!(obj = nodeDeviceObjFindByName(device->name)))
> +        return -1;
> +    def = virNodeDeviceObjGetDef(obj);
> +
> +    if (virNodeDeviceIsPersistentEnsureACL(device->conn, def) < 0)
> +        goto cleanup;
> +
> +    ret = virNodeDeviceObjIsPersistent(obj);
> +
> + cleanup:
> +    virNodeDeviceObjEndAPI(&obj);
> +    return ret;
> +}
> +
> +
> +int
> +nodeDeviceIsActive(virNodeDevice *device)
> +{
> +    virNodeDeviceObj *obj = NULL;
> +    virNodeDeviceDef *def = NULL;
> +    int ret = -1;
> +
> +    if (nodeDeviceInitWait() < 0)
> +        return -1;
> +
> +    if (!(obj = nodeDeviceObjFindByName(device->name)))
> +        return -1;
> +    def = virNodeDeviceObjGetDef(obj);
> +
> +    if (virNodeDeviceIsActiveEnsureACL(device->conn, def) < 0)
> +        goto cleanup;
> +
> +    ret = virNodeDeviceObjIsActive(obj);
> +
> + cleanup:
> +    virNodeDeviceObjEndAPI(&obj);
> +    return ret;
> +}
> diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h
> index d178a18180..744dd42632 100644
> --- a/src/node_device/node_device_driver.h
> +++ b/src/node_device/node_device_driver.h
> @@ -180,6 +180,12 @@ int
>   nodeDeviceGetAutostart(virNodeDevice *dev,
>                          int *autostart);
>   
> +int
> +nodeDeviceIsPersistent(virNodeDevice *dev);
> +
> +int
> +nodeDeviceIsActive(virNodeDevice *dev);
> +
>   virCommand*
>   nodeDeviceGetMdevctlSetAutostartCommand(virNodeDeviceDef *def,
>                                           bool autostart,
> diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
> index 21273083a6..eb15ccce7f 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c
> @@ -1487,7 +1487,7 @@ udevAddOneDevice(struct udev_device *device)
>       virObjectEvent *event = NULL;
>       bool new_device = true;
>       int ret = -1;
> -    bool was_persistent = false;
> +    bool persistent = true;
>       bool autostart = true;
>       bool is_mdev;
>   
> @@ -1518,7 +1518,8 @@ udevAddOneDevice(struct udev_device *device)
>   
>           if (is_mdev)
>               nodeDeviceDefCopyFromMdevctl(def, objdef);
> -        was_persistent = virNodeDeviceObjIsPersistent(obj);
> +
> +        persistent = virNodeDeviceObjIsPersistent(obj);
>           autostart = virNodeDeviceObjIsAutostart(obj);
>   
>           /* If the device was defined by mdevctl and was never instantiated, it
> @@ -1527,11 +1528,12 @@ udevAddOneDevice(struct udev_device *device)
>   
>           virNodeDeviceObjEndAPI(&obj);
>       } else {
> -        /* All non-mdev devices report themselves as autostart since they
> -         * should still be present and active after a reboot unless the device
> -         * is removed from the host. Mediated devices can only be persistent if
> -         * they are in already in the device list from parsing the mdevctl
> -         * output. */
> +        /* All non-mdev devices report themselves as persistent and autostart
> +         * since they should still be present and active after a reboot unless
> +         * the device is removed from the host. Mediated devices can only be
> +         * persistent if they are in already in the device list from parsing
> +         * the mdevctl output. */

The assumption for all non-mdev devices ends up very misleading.
For example: The parent device of an mdev needs to be bound to a vfio 
device driver. Without it the device ends up without the capability to 
create mdevs.
If this driver binding is not persisted (e.g. with setting up driverctl) 
but instead the device is just manually being rebound to a vfio device 
driver than after reboot neither the mdev capability on the parent 
device nor the mdev device as a child device will be existing.
A user calling nodedev-info before the reboot gets
on the parent device
	Persistent:     yes
	Autostart:      yes
and on the mdev device
	Persistent:     yes
	Autostart:      yes
After a reboot he ends up with with nodedev-info
on the parent device
	Persistent:     yes
	Autostart:      yes
and the mdev device does not exist.

I suggest to use three states like yes, no, unknown
or not showing Persistent and Autostart on devices that libvirt does not 
manage/know about their persistence and autostart configuration.


> +        persistent = !is_mdev;
>           autostart = !is_mdev;
>       }
>   
> @@ -1539,7 +1541,7 @@ udevAddOneDevice(struct udev_device *device)
>        * and the current definition will take its place. */
>       if (!(obj = virNodeDeviceObjListAssignDef(driver->devs, def)))
>           goto cleanup;
> -    virNodeDeviceObjSetPersistent(obj, was_persistent);
> +    virNodeDeviceObjSetPersistent(obj, persistent);
>       virNodeDeviceObjSetAutostart(obj, autostart);
>       objdef = virNodeDeviceObjGetDef(obj);
>   
> @@ -1945,6 +1947,7 @@ udevSetupSystemDev(void)
>   
>       virNodeDeviceObjSetActive(obj, true);
>       virNodeDeviceObjSetAutostart(obj, true);
> +    virNodeDeviceObjSetPersistent(obj, true);
>   
>       virNodeDeviceObjEndAPI(&obj);
>   
> @@ -2348,6 +2351,8 @@ static virNodeDeviceDriver udevNodeDeviceDriver = {
>       .nodeDeviceCreate = nodeDeviceCreate, /* 7.3.0 */
>       .nodeDeviceSetAutostart = nodeDeviceSetAutostart, /* 7.5.0 */
>       .nodeDeviceGetAutostart = nodeDeviceGetAutostart, /* 7.5.0 */
> +    .nodeDeviceIsPersistent = nodeDeviceIsPersistent, /* 7.5.0 */
> +    .nodeDeviceIsActive = nodeDeviceIsActive, /* 7.5.0 */
>   };
>   
>   
> 


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

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