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

Jonathon Jongsma jjongsma at redhat.com
Thu Jun 10 14:57:44 UTC 2021


ping

On Fri, Jun 4, 2021 at 1:58 PM Jonathon Jongsma <jjongsma at redhat.com> wrote:
>
> Implement autostart functionality for mediated devices.
>
> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
> ---
>  src/conf/node_device_conf.h          |   1 +
>  src/conf/virnodedeviceobj.c          |  16 ++++
>  src/conf/virnodedeviceobj.h          |   6 ++
>  src/libvirt_private.syms             |   2 +
>  src/node_device/node_device_driver.c | 133 ++++++++++++++++++++++++++-
>  src/node_device/node_device_driver.h |  13 +++
>  src/node_device/node_device_udev.c   |  19 +++-
>  7 files changed, 188 insertions(+), 2 deletions(-)
>
> diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
> index a60562e4fe..863b893973 100644
> --- a/src/conf/node_device_conf.h
> +++ b/src/conf/node_device_conf.h
> @@ -153,6 +153,7 @@ struct _virNodeDevCapMdev {
>      char *uuid;
>      virMediatedDeviceAttr **attributes;
>      size_t nattributes;
> +    bool autostart;
>  };
>
>  typedef struct _virNodeDevCapPCIDev virNodeDevCapPCIDev;
> 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..5436b5487d 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -611,11 +611,12 @@ nodeDeviceDefToMdevctlConfig(virNodeDeviceDef *def, char **buf)
>      size_t i;
>      virNodeDevCapMdev *mdev = &def->caps->data.mdev;
>      g_autoptr(virJSONValue) json = virJSONValueNewObject();
> +    const char *startval = mdev->autostart ? "auto" : "manual";
>
>      if (virJSONValueObjectAppendString(json, "mdev_type", mdev->type) < 0)
>          return -1;
>
> -    if (virJSONValueObjectAppendString(json, "start", "manual") < 0)
> +    if (virJSONValueObjectAppendString(json, "start", startval) < 0)
>          return -1;
>
>      if (mdev->attributes) {
> @@ -962,6 +963,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,
> @@ -997,6 +1038,7 @@ nodeDeviceParseMdevctlChildDevice(const char *parent,
>      virJSONValue *props;
>      virJSONValue *attrs;
>      g_autoptr(virNodeDeviceDef) child = g_new0(virNodeDeviceDef, 1);
> +    const char *start = NULL;
>
>      /* the child object should have a single key equal to its uuid.
>       * The value is an object describing the properties of the mdev */
> @@ -1014,6 +1056,8 @@ nodeDeviceParseMdevctlChildDevice(const char *parent,
>      mdev->uuid = g_strdup(uuid);
>      mdev->type =
>          g_strdup(virJSONValueObjectGetString(props, "mdev_type"));
> +    start = virJSONValueObjectGetString(props, "start");
> +    mdev->autostart = STREQ_NULLABLE(start, "auto");
>
>      attrs = virJSONValueObjectGet(props, "attrs");
>
> @@ -1257,6 +1301,7 @@ nodeDeviceUpdateMediatedDevice(virNodeDeviceDef *def)
>
>      /* all devices returned by virMdevctlListDefined() are persistent */
>      virNodeDeviceObjSetPersistent(obj, true);
> +    virNodeDeviceObjSetAutostart(obj, def->caps->data.mdev.autostart);
>
>      if (!defined)
>          event = virNodeDeviceEventLifecycleNew(name,
> @@ -1686,5 +1731,91 @@ nodeDeviceDefCopyFromMdevctl(virNodeDeviceDef *dst,
>      if (virMediatedDeviceAttrsCopy(dstmdev, srcmdev))
>          ret = true;
>
> +    if (dstmdev->autostart != srcmdev->autostart) {
> +        ret = true;
> +        dstmdev->autostart = srcmdev->autostart;
> +    }
> +
> +    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;
> +            }
> +            /* Due to mdevctl performance issues, it may take several seconds
> +             * to re-query mdevctl for the defined devices. Because the mdevctl
> +             * command returned without an error status, assume it was
> +             * successful and set the object status directly here rather than
> +             * waiting for the next query */
> +            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..7e3e03e8f2 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 autostart if
> +         * they are in already in the device list from parsing the mdevctl
> +         * output. */
> +        autostart = !is_mdev;
>      }
>
>      /* If this is a device change, the old definition will be freed
> @@ -1527,6 +1540,7 @@ udevAddOneDevice(struct udev_device *device)
>      if (!(obj = virNodeDeviceObjListAssignDef(driver->devs, def)))
>          goto cleanup;
>      virNodeDeviceObjSetPersistent(obj, was_persistent);
> +    virNodeDeviceObjSetAutostart(obj, autostart);
>      objdef = virNodeDeviceObjGetDef(obj);
>
>      if (new_device)
> @@ -1930,6 +1944,7 @@ udevSetupSystemDev(void)
>          goto cleanup;
>
>      virNodeDeviceObjSetActive(obj, true);
> +    virNodeDeviceObjSetAutostart(obj, true);
>
>      virNodeDeviceObjEndAPI(&obj);
>
> @@ -2331,6 +2346,8 @@ static virNodeDeviceDriver udevNodeDeviceDriver = {
>      .nodeDeviceDefineXML = nodeDeviceDefineXML, /* 7.3.0 */
>      .nodeDeviceUndefine = nodeDeviceUndefine, /* 7.3.0 */
>      .nodeDeviceCreate = nodeDeviceCreate, /* 7.3.0 */
> +    .nodeDeviceSetAutostart = nodeDeviceSetAutostart, /* 7.5.0 */
> +    .nodeDeviceGetAutostart = nodeDeviceGetAutostart, /* 7.5.0 */
>  };
>
>
> --
> 2.31.1
>




More information about the libvir-list mailing list