[libvirt PATCH v6 20/30] api: add virNodeDeviceUndefine()

[Erik Skultety]

On Fri, Mar 26, 2021 at 11:48:16AM -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>

...

>  
>  
> +/**
> + * virNodeDeviceUndefine:
> + * @dev: a device object
> + *
> + * Undefine the device object. The virtual device  is removed from the host
> + * operating system.  This function may require privileged access.
> + *
> + * Returns 0 in case of success and -1 in case of failure.
> + */
> +int
> +virNodeDeviceUndefine(virNodeDevice *dev)

For consistency reasons ^this should remain virNodeDevicePtr

...

>  
> +virCommand*

virCommand *

I noticed this pattern repeating across the whole series, some of the
occurrences I commented on (when I noticed), some of them I forgot...so please
fix all of them.

Reviewed-by: Erik Skultety <eskultet at redhat.com>

> +nodeDeviceGetMdevctlUndefineCommand(const char *uuid, char **errmsg)
> +{
> +    virCommand *cmd = virCommandNewArgList(MDEVCTL,
> +                                           "undefine",
> +                                           "-u",
> +                                           uuid,
> +                                           NULL);
> +    virCommandSetErrorBuffer(cmd, errmsg);
> +    return cmd;
> +}
> +
>  static int
>  virMdevctlStop(virNodeDeviceDefPtr def, char **errmsg)
>  {
> @@ -904,6 +916,22 @@ virMdevctlStop(virNodeDeviceDefPtr def, char **errmsg)
>  }
>  
>  
> +static int
> +virMdevctlUndefine(virNodeDeviceDef *def, char **errmsg)
> +{
> +    int status;
> +    g_autoptr(virCommand) cmd = NULL;
> +
> +    cmd = nodeDeviceGetMdevctlUndefineCommand(def->caps->data.mdev.uuid,
> +                                              errmsg);
> +
> +    if (virCommandRun(cmd, &status) < 0 || status != 0)
> +        return -1;
> +
> +    return 0;
> +}
> +
> +
>  virCommand*
>  nodeDeviceGetMdevctlListCommand(bool defined,
>                                  char **output)
> @@ -1183,6 +1211,51 @@ nodeDeviceDefineXML(virConnect *conn,
>  }
>  
>  
> +int
> +nodeDeviceUndefine(virNodeDevice *device)
> +{
> +    int ret = -1;
> +    virNodeDeviceObj *obj = NULL;
> +    virNodeDeviceDef *def;
> +
> +    if (nodeDeviceWaitInit() < 0)
> +        return -1;
> +
> +    if (!(obj = nodeDeviceObjFindByName(device->name)))
> +        return -1;
> +
> +    def = virNodeDeviceObjGetDef(obj);
> +
> +    if (virNodeDeviceUndefineEnsureACL(device->conn, def) < 0)
> +        goto cleanup;
> +
> +    if (!virNodeDeviceObjIsPersistent(obj)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       _("Node device '%s' is not defined"),
> +                       def->name);
> +        goto cleanup;
> +    }
> +
> +    if (nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_MDEV)) {
> +        g_autofree char *errmsg = NULL;
> +
> +        if (virMdevctlUndefine(def, &errmsg) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Unable to undefine mediated device: %s"),
> +                           errmsg && errmsg[0] ? errmsg : "Unknown Error");

"Unknown Error" case which has already been mentioned...