[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 4/4 v5] virsh: Use virNodeDeviceLookupSCSIHostByWWN



On Mon, Feb 04, 2013 at 10:16:44PM +0800, Osier Yang wrote:
> Only nodedev-destroy and nodedev-dumpxml can benifit from the
> new API, other commands like nodedev-detach only works for
> PCI devices, WWN makes no sense for them.
> 
> ---
> Rebased on Peter's virsh cleanup patches.
> ---
>  tools/virsh-nodedev.c |   84 +++++++++++++++++++++++++++++++++++++++---------
>  tools/virsh.pod       |   15 +++++---
>  2 files changed, 77 insertions(+), 22 deletions(-)
> 
> diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
> index f85bded..7c51a56 100644
> --- a/tools/virsh-nodedev.c
> +++ b/tools/virsh-nodedev.c
> @@ -101,9 +101,14 @@ static const vshCmdInfo info_node_device_destroy[] = {
>  
>  static const vshCmdOptDef opts_node_device_destroy[] = {
>      {.name = "name",
> +     .type = VSH_OT_ALIAS,
> +     .flags = 0,
> +     .help = "device"
> +    },
> +    {.name = "device",
>       .type = VSH_OT_DATA,
>       .flags = VSH_OFLAG_REQ,
> -     .help = N_("name of the device to be destroyed")
> +     .help = N_("device name or wwn pair in 'wwnn,wwpn' format")
>      },
>      {.name = NULL}
>  };
> @@ -112,21 +117,47 @@ static bool
>  cmdNodeDeviceDestroy(vshControl *ctl, const vshCmd *cmd)
>  {
>      virNodeDevicePtr dev = NULL;
> -    bool ret = true;
> -    const char *name = NULL;
> +    bool ret = false;
> +    const char *device_value = NULL;
> +    char **arr = NULL;
> +    int narr;
>  
> -    if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0)
> +    if (vshCommandOptStringReq(ctl, cmd, "device", &device_value) < 0)
>          return false;
>  
> -    dev = virNodeDeviceLookupByName(ctl->conn, name);
> +    if (strchr(device_value, ',')) {
> +        narr = vshStringToArray(device_value, &arr);
> +        if (narr != 2) {
> +            vshError(ctl, _("Malformed device value '%s'"), device_value);
> +            goto cleanup;
> +        }
> +
> +        if (!virValidateWWN(arr[0]) || !virValidateWWN(arr[1]))
> +            goto cleanup;
> +
> +        dev = virNodeDeviceLookupSCSIHostByWWN(ctl->conn, arr[0], arr[1], 0);
> +    } else {
> +        dev = virNodeDeviceLookupByName(ctl->conn, device_value);
> +    }
> +
> +    if (!dev) {
> +        vshError(ctl, "%s '%s'", _("Could not find matching device"), device_value);
> +        goto cleanup;
> +    }
>  
>      if (virNodeDeviceDestroy(dev) == 0) {
> -        vshPrint(ctl, _("Destroyed node device '%s'\n"), name);
> +        vshPrint(ctl, _("Destroyed node device '%s'\n"), device_value);
>      } else {
> -        vshError(ctl, _("Failed to destroy node device '%s'"), name);
> -        ret = false;
> +        vshError(ctl, _("Failed to destroy node device '%s'"), device_value);
> +        goto cleanup;
>      }
>  
> +    ret = true;
> +cleanup:
> +    if (arr) {
> +        VIR_FREE(*arr);

Hmm, IIUC, that is equiv to VIR_FREE(arr[0]), so what is
free'ing arr[1] ?

> +        VIR_FREE(arr);
> +    }


ACK if that leak is fixed, or otherwise explained.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]