[libvirt] [PATCH] Adds the missing vol-pool command to virsh.

[Eric Blake]

On 06/08/2010 06:03 AM, Justin Clift wrote:
> Hi all,

Much better - git am liked this patch.  And a tip for your matching your
commit message style to other commits: list a category first, use all
lower-case, and end without a period.  While not essential, doing these
things will make 'git log --oneline' produce more readable output.

> 
> This patch adds a "vol-pool" command to virsh, to round out the conversion functions for vols in virsh.
> 
> At the moment, if we start with a volume name and a pool id, we're all good.  We can convert from that pair to either a volume key or volume path.

Also, wrap commit messages, like you would email (72 or 80 columns is
typical).

> 
> But, if we're given just a volume key or path we have no good way to look up the storage pool for it.  Kind of makes it a one way association. :/
> 
> This patch fixes that, making it possible to work from just having a volume key or path. :)
> 
> Regards and best wishes,
> 
> Justin Clift
> 
> ---
>  tools/virsh.c |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 48 insertions(+), 0 deletions(-)

No change to tools/virsh.pod?  I know you have other cleanup patches to
virsh.pod, but for a new feature, I'd rather get in the habit of
checking in the doc change at the same time as the new feature.

> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 1279f41..95aea0f 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -5975,6 +5975,53 @@ cmdVolName(vshControl *ctl, const vshCmd *cmd)
>  }
>  
>  
> +/*
> + * "vol-pool" command
> + */
> +static const vshCmdInfo info_vol_pool[] = {
> +    {"help", N_("returns the storage pool for a given volume key or path")},
> +    {"desc", ""},
> +    {NULL, NULL}
> +};
> +
> +static const vshCmdOptDef opts_vol_pool[] = {
> +    {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("volume key or path")},
> +    {NULL, 0, 0, NULL}
> +};
> +
> +static int
> +cmdVolPool(vshControl *ctl, const vshCmd *cmd)
> +{
> +    virStoragePoolPtr pool;
> +    virStorageVolPtr vol;
> +
> +    // Check the connection to libvirtd daemon is still working

The rest of this file used /* */ comments.  It's nice to be consistent
(even though we require C99 for various other reasons, we're still stuck
on some C89 syntax constructs for style reasons).

> +    if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
> +        return FALSE;
> +
> +    // Use the supplied string to locate the volume
> +    if (!(vol = vshCommandOptVolBy(ctl, cmd, "vol", "pool", NULL,
> +                                   VSH_BYUUID))) {
> +        return FALSE;
> +    }
> +
> +    // Look up the parent storage pool for the volume
> +    pool = virStoragePoolLookupByVolume(vol);
> +    if (pool == NULL) {
> +        vshError(ctl, "%s", _("failed to get parent pool"));
> +        virStorageVolFree(vol);
> +        return FALSE;
> +    }
> +
> +    // Return the name of the parent storage pool
> +    vshPrint(ctl, "%s\n", virStoragePoolGetName(pool));
> +
> +    // Cleanup
> +    virStorageVolFree(vol);
> +    virStoragePoolFree(pool);
> +    return TRUE;
> +}
> +
>  
>  /*
>   * "vol-key" command
> @@ -8837,6 +8884,7 @@ static const vshCmdDef commands[] = {
>      {"vol-dumpxml", cmdVolDumpXML, opts_vol_dumpxml, info_vol_dumpxml},
>      {"vol-info", cmdVolInfo, opts_vol_info, info_vol_info},
>      {"vol-list", cmdVolList, opts_vol_list, info_vol_list},
> +    {"vol-pool", cmdVolPool, opts_vol_pool, info_vol_pool},
>      {"vol-path", cmdVolPath, opts_vol_path, info_vol_path},
>      {"vol-name", cmdVolName, opts_vol_name, info_vol_name},
>      {"vol-key", cmdVolKey, opts_vol_key, info_vol_key},

In general, this looks okay to me, but I'll wait for the next round
addressing the nits I raised before pushing anything.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100608/41e4c40a/attachment-0001.sig>