Re: [libvirt PATCH 2/7] nodedev: implement virNodeDevice(Get|Set)Autostart()

On Thu, Jun 03, 2021 at 03:11:51PM -0500, Jonathon Jongsma wrote: > Implement autostart functionality for mediated devices. > > Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; > --- > src/conf/virnodedeviceobj.c | 16 ++++ > src/conf/virnodedeviceobj.h | 6 ++ > src/libvirt_private.syms | 2 + > src/node_device/node_device_driver.c | 116 +++++++++++++++++++++++++++ > src/node_device/node_device_driver.h | 13 +++ > src/node_device/node_device_udev.c | 19 ++++- > 6 files changed, 171 insertions(+), 1 deletion(-) > > diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c > index b213592b56..deee796440 100644 > --- a/src/conf/virnodedeviceobj.c > +++ b/src/conf/virnodedeviceobj.c > @@ -41,6 +41,7 @@ struct _virNodeDeviceObj { > used by testdriver */ > bool active; > bool persistent; > + bool autostart; > }; > > struct _virNodeDeviceObjList { > @@ -1026,6 +1027,21 @@ virNodeDeviceObjSetPersistent(virNodeDeviceObj *obj, > } > > > +bool > +virNodeDeviceObjIsAutostart(virNodeDeviceObj *obj) > +{ > + return obj->autostart; > +} > + > + > +void > +virNodeDeviceObjSetAutostart(virNodeDeviceObj *obj, > + bool autostart) > +{ > + obj->autostart = autostart; > +} > + > + > struct virNodeDeviceObjListRemoveHelperData > { > virNodeDeviceObjListRemoveIterator callback; > diff --git a/src/conf/virnodedeviceobj.h b/src/conf/virnodedeviceobj.h > index 7353e4619b..78188be2a2 100644 > --- a/src/conf/virnodedeviceobj.h > +++ b/src/conf/virnodedeviceobj.h > @@ -135,6 +135,12 @@ virNodeDeviceObjIsPersistent(virNodeDeviceObj *obj); > void > virNodeDeviceObjSetPersistent(virNodeDeviceObj *obj, > bool persistent); > +bool > +virNodeDeviceObjIsAutostart(virNodeDeviceObj *obj); > + > +void > +virNodeDeviceObjSetAutostart(virNodeDeviceObj *obj, > + bool autostart); > > typedef bool (*virNodeDeviceObjListRemoveIterator)(virNodeDeviceObj *obj, > const void *opaque); > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > index 0ced2a7990..dd46fe8750 100644 > --- a/src/libvirt_private.syms > +++ b/src/libvirt_private.syms > @@ -1278,6 +1278,7 @@ virNetworkPortDefSaveStatus; > virNodeDeviceObjEndAPI; > virNodeDeviceObjGetDef; > virNodeDeviceObjIsActive; > +virNodeDeviceObjIsAutostart; > virNodeDeviceObjIsPersistent; > virNodeDeviceObjListAssignDef; > virNodeDeviceObjListExport; > @@ -1294,6 +1295,7 @@ virNodeDeviceObjListNumOfDevices; > virNodeDeviceObjListRemove; > virNodeDeviceObjListRemoveLocked; > virNodeDeviceObjSetActive; > +virNodeDeviceObjSetAutostart; > virNodeDeviceObjSetPersistent; > > > diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c > index 8a0a2c3847..9ebe609aa4 100644 > --- a/src/node_device/node_device_driver.c > +++ b/src/node_device/node_device_driver.c > @@ -962,6 +962,46 @@ virMdevctlStart(virNodeDeviceDef *def, char **errmsg) > } > > > +/* gets a virCommand object that executes a mdevctl command to set the > + * 'autostart' property of the device to the specified value > + */ > +virCommand* > +nodeDeviceGetMdevctlSetAutostartCommand(virNodeDeviceDef *def, > + bool autostart, > + char **errmsg) > +{ > + virCommand *cmd = virCommandNewArgList(MDEVCTL, > + "modify", > + "--uuid", > + def->caps->data.mdev.uuid, > + NULL); > + > + if (autostart) > + virCommandAddArg(cmd, "--auto"); > + else > + virCommandAddArg(cmd, "--manual"); > + > + virCommandSetErrorBuffer(cmd, errmsg); > + > + return cmd; > +} > + > + > +static int > +virMdevctlSetAutostart(virNodeDeviceDef *def, bool autostart, char **errmsg) > +{ > + int status; > + g_autoptr(virCommand) cmd = NULL; > + > + cmd = nodeDeviceGetMdevctlSetAutostartCommand(def, autostart, errmsg); > + > + if (virCommandRun(cmd, &status) < 0 || status != 0) > + return -1; > + > + return 0; > +} > + > + > virCommand* > nodeDeviceGetMdevctlListCommand(bool defined, > char **output, > @@ -1688,3 +1728,79 @@ nodeDeviceDefCopyFromMdevctl(virNodeDeviceDef *dst, > > return ret; > } > + > + > +int > +nodeDeviceSetAutostart(virNodeDevice *device, > + int autostart) > +{ > + int ret = -1; > + virNodeDeviceObj *obj = NULL; > + virNodeDeviceDef *def = NULL; > + > + if (nodeDeviceInitWait() < 0) > + return -1; > + > + if (!(obj = nodeDeviceObjFindByName(device->name))) > + return -1; > + def = virNodeDeviceObjGetDef(obj); > + > + if (virNodeDeviceSetAutostartEnsureACL(device->conn, def) < 0) > + goto cleanup; > + > + if (nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_MDEV)) { > + if (!virNodeDeviceObjIsPersistent(obj)) { > + virReportError(VIR_ERR_OPERATION_INVALID, > + "%s", _("cannot set autostart for transient device")); > + goto cleanup; > + } > + > + if (autostart != virNodeDeviceObjIsAutostart(obj)) { > + g_autofree char *errmsg = NULL; > + > + if (virMdevctlSetAutostart(def, autostart, &errmsg) < 0) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("Unable to set autostart on '%s': %s"), > + def->name, > + errmsg && errmsg[0] != '\0' ? errmsg : _("Unknown Error")); > + goto cleanup; > + } > + virNodeDeviceObjSetAutostart(obj, autostart); > + } > + ret = 0; > + } else { > + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", > + _("Unsupported device type")); > + } > + > + cleanup: > + virNodeDeviceObjEndAPI(&obj); > + return ret; > +} > + > + > +int > +nodeDeviceGetAutostart(virNodeDevice *device, > + int *autostart) > +{ > + 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 (virNodeDeviceGetAutostartEnsureACL(device->conn, def) < 0) > + goto cleanup; > + > + *autostart = virNodeDeviceObjIsAutostart(obj); > + ret = 0; > + > + cleanup: > + virNodeDeviceObjEndAPI(&obj); > + return ret; > +} > diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h > index edd763f0e4..d178a18180 100644 > --- a/src/node_device/node_device_driver.h > +++ b/src/node_device/node_device_driver.h > @@ -171,3 +171,16 @@ bool nodeDeviceDefCopyFromMdevctl(virNodeDeviceDef *dst, > int > nodeDeviceCreate(virNodeDevice *dev, > unsigned int flags); > + > +int > +nodeDeviceSetAutostart(virNodeDevice *dev, > + int autostart); > + > +int > +nodeDeviceGetAutostart(virNodeDevice *dev, > + int *autostart); > + > +virCommand* > +nodeDeviceGetMdevctlSetAutostartCommand(virNodeDeviceDef *def, > + bool autostart, > + char **errmsg); > diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c > index f99578414d..21273083a6 100644 > --- a/src/node_device/node_device_udev.c > +++ b/src/node_device/node_device_udev.c > @@ -1488,6 +1488,8 @@ udevAddOneDevice(struct udev_device *device) > bool new_device = true; > int ret = -1; > bool was_persistent = false; > + bool autostart = true; > + bool is_mdev; > > def = g_new0(virNodeDeviceDef, 1); > > @@ -1509,17 +1511,28 @@ udevAddOneDevice(struct udev_device *device) > if (udevSetParent(device, def) != 0) > goto cleanup; > > + is_mdev = def->caps->data.type == VIR_NODE_DEV_CAP_MDEV; > + > if ((obj = virNodeDeviceObjListFindByName(driver->devs, def->name))) { > objdef = virNodeDeviceObjGetDef(obj); > > - if (def->caps->data.type == VIR_NODE_DEV_CAP_MDEV) > + if (is_mdev) > nodeDeviceDefCopyFromMdevctl(def, objdef); > was_persistent = virNodeDeviceObjIsPersistent(obj); > + autostart = virNodeDeviceObjIsAutostart(obj); > + > /* If the device was defined by mdevctl and was never instantiated, it > * won't have a sysfs path. We need to emit a CREATED event... */ > new_device = (objdef->sysfs_path == NULL); > > 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. */ > + autostart = !is_mdev; Don't we need to be querying the mdevctl configuration to see if it is autostart or manual ?