[libvirt PATCH v2 12/16] api: add virNodeDeviceUndefine()

[Erik Skultety]

On Tue, Aug 18, 2020 at 09:48:02AM -0500, Jonathon Jongsma wrote:
> This interface allows you to undefine a persistently defined (but
> inactive) mediated devices. It is implemented via 'mdevctl'
>
> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
> ---
>  include/libvirt/libvirt-nodedev.h             |  2 +
>  src/driver-nodedev.h                          |  4 ++
>  src/libvirt-nodedev.c                         | 36 ++++++++++
>  src/libvirt_public.syms                       |  1 +
>  src/node_device/node_device_driver.c          | 68 +++++++++++++++++++
>  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 ++
>  .../nodedevmdevctldata/mdevctl-undefine.argv  |  1 +
>  tests/nodedevmdevctltest.c                    | 56 ++++++++++++---
>  12 files changed, 185 insertions(+), 9 deletions(-)
>  create mode 100644 tests/nodedevmdevctldata/mdevctl-undefine.argv

...

>
> +virCommandPtr
> +nodeDeviceGetMdevctlUndefineCommand(const char *uuid)
> +{
> +    return virCommandNewArgList(MDEVCTL,
> +                                "undefine",
> +                                "-u",
> +                                uuid,
> +                                NULL);
> +
> +}
> +
>  static int
>  virMdevctlStop(virNodeDeviceDefPtr def)
>  {
> @@ -848,6 +859,21 @@ virMdevctlStop(virNodeDeviceDefPtr def)
>  }
>
>
> +static int
> +virMdevctlUndefine(virNodeDeviceDefPtr def)
> +{
> +    int status;
> +    g_autoptr(virCommand) cmd = NULL;
> +
> +    cmd = nodeDeviceGetMdevctlUndefineCommand(def->caps->data.mdev.uuid);
> +
> +    if (virCommandRun(cmd, &status) < 0 || status != 0)
> +        return -1;
> +
> +    return 0;
> +}
> +
> +

Depending on 10/16 comments, ^this would need some micro adjustments.

>  virCommandPtr
>  nodeDeviceGetMdevctlListCommand(bool defined,
>                                  char **output)
> @@ -1093,6 +1119,48 @@ nodeDeviceDefineXML(virConnectPtr conn,
>  }
>
>
> +int
> +nodeDeviceUndefine(virNodeDevicePtr device)
> +{
> +    int ret = -1;
> +    virNodeDeviceObjPtr obj = NULL;
> +    virNodeDeviceDefPtr def;
> +    g_autofree char *parent = NULL;
> +
> +    if (nodeDeviceWaitInit() < 0)
> +        return -1;
> +
> +    if (!(obj = nodeDeviceObjFindByName(device->name)))
> +        return -1;
> +    def = virNodeDeviceObjGetDef(obj);
> +
> +    if (virNodeDeviceUndefineEnsureACL(device->conn, def) < 0)
> +        goto cleanup;
> +
> +    if (virNodeDeviceObjIsActive(obj)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       _("node device '%s' is still active"),
> +                       def->name);
> +        goto cleanup;
> +    }
> +
> +    if (nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_MDEV)) {
> +        if (virMdevctlUndefine(def) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("Unable to undefine mediated device"));
> +            goto cleanup;

I haven't had a deep look at the mdevctl thread yet, so I hope we won't get to
a racy kind of situation where we'd try to undefine a device that doesn't
exist either transient or persistent in mdevctl anymore.

> +        }
> +        ret = 0;
> +    } else {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("Unsupported device type"));
> +    }
> +
> + cleanup:
> +    virNodeDeviceObjEndAPI(&obj);
> +    return ret;
> +}
> +

...

>  static int
> -testMdevctlStop(const void *data)
> +testMdevctlUuidCommand(const char *uuid, GetStopUndefineCmdFunc func, const char *outfile)

Hopefully with comments in 10/16 ^this could become testMdevctlCommand and
simplify these test code bits more.

Regards,
Erik