Re: [libvirt PATCH v2 15/16] api: add virNodeDeviceCreate()

[Ján Tomko <jtomko@xxxxxxxxxx>]

On a Tuesday in 2020, Jonathon Jongsma wrote:
> This new API function provides a way to start a persistently-defined
> mediate device that was defined by virNodeDeviceDefineXML() (or one that
> was defined externally via mdevctl)
>
> Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> ---
> include/libvirt/libvirt-nodedev.h            |  2 +
> src/driver-nodedev.h                         |  4 ++
> src/libvirt-nodedev.c                        | 35 +++++++++++
> src/libvirt_public.syms                      |  1 +
> src/node_device/node_device_driver.c         | 64 ++++++++++++++++++++
> src/node_device/node_device_driver.h         |  6 ++
> src/node_device/node_device_udev.c           |  1 +
> src/remote/remote_driver.c                   |  1 +
> src/remote/remote_protocol.x                 | 14 ++++-
> src/remote_protocol-structs                  |  4 ++
> tests/nodedevmdevctldata/mdevctl-create.argv |  1 +
> tests/nodedevmdevctltest.c                   | 11 +++-
> 12 files changed, 141 insertions(+), 3 deletions(-)
> create mode 100644 tests/nodedevmdevctldata/mdevctl-create.argv
>
> @@ -1162,6 +1188,44 @@ nodeDeviceUndefine(virNodeDevicePtr device)
> }
>
>
> +int nodeDeviceCreate(virNodeDevicePtr device)
> +{
> +    int ret = -1;
> +    virNodeDeviceObjPtr obj = NULL;
> +    virNodeDeviceDefPtr def;
> +    g_autofree char *parent = NULL;

../src/node_device/node_device_driver.c:1128:22: error: unused variable 'parent' [-Werror,-Wunused-variable]
    g_autofree char *parent = NULL;
                     ^


> +
> +    if (!(obj = nodeDeviceObjFindByName(device->name)))
> +        return -1;
> +
> +    if (virNodeDeviceObjIsActive(obj)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                       _("Device is already active"));
> +        goto cleanup;
> +    }
> +    def = virNodeDeviceObjGetDef(obj);
> +
> +    if (virNodeDeviceCreateEnsureACL(device->conn, def) < 0)
> +        goto cleanup;
> +
> +    if (nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_MDEV)) {
> +        if (virMdevctlCreate(def) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("Unable to create mediated device"));
> +            goto cleanup;
> +        }
> +        ret = 0;
> +    } else {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("Unsupported device type"));
> +    }
> +
> + cleanup:
> +    virNodeDeviceObjEndAPI(&obj);
> +    return ret;
> +}
> +
> +
> int
> nodeConnectNodeDeviceEventRegisterAny(virConnectPtr conn,
>                                       virNodeDevicePtr device,
> diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
> index d668074204..b019f38efd 100644
> --- a/src/remote/remote_protocol.x
> +++ b/src/remote/remote_protocol.x
> @@ -2152,6 +2152,10 @@ struct remote_node_device_undefine_args {
>     remote_nonnull_string name;
> };
>
> +struct remote_node_device_create_args {
> +    remote_nonnull_string name;
> +};
> +
>
> /*
>  * Events Register/Deregister:
> @@ -6682,7 +6686,6 @@ enum remote_procedure {
>     /**
>      * @generate: both
>      * @acl: node_device:write
> -     * @acl: node_device:start

This change looks fishy.

>      */
>     REMOTE_PROC_NODE_DEVICE_DEFINE_XML = 423,
>
> @@ -6691,6 +6694,13 @@ enum remote_procedure {
>      * @priority: high
>      * @acl: node_device:stop
>      */
> -    REMOTE_PROC_NODE_DEVICE_UNDEFINE = 424
> +    REMOTE_PROC_NODE_DEVICE_UNDEFINE = 424,
> +
> +    /**
> +     * @generate: both
> +     * @priority: high
> +     * @acl: node_device:start
> +     */
> +    REMOTE_PROC_NODE_DEVICE_CREATE = 425
>
> };
> diff --git a/tests/nodedevmdevctldata/mdevctl-create.argv b/tests/nodedevmdevctldata/mdevctl-create.argv
> new file mode 100644
> index 0000000000..802109340c
> --- /dev/null
> +++ b/tests/nodedevmdevctldata/mdevctl-create.argv
> @@ -0,0 +1 @@
> +$MDEVCTL_BINARY$ start -u 8a05ad83-3472-497d-8631-8142f31460e8
> diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c
> index 216c70f1b8..a6726616dc 100644
> --- a/tests/nodedevmdevctltest.c
> +++ b/tests/nodedevmdevctltest.c
> @@ -14,7 +14,8 @@ typedef enum {
>     MDEVCTL_CMD_START,
>     MDEVCTL_CMD_STOP,
>     MDEVCTL_CMD_DEFINE,

> -    MDEVCTL_CMD_UNDEFINE
> +    MDEVCTL_CMD_UNDEFINE,

The last enum entry can be followed by a colon to avoid changes like
these, it's just remote_protocol.x where that does not work.

> +    MDEVCTL_CMD_CREATE
> } MdevctlCmd;

Jano

Attachment: signature.asc
Description: PGP signature