[libvirt] [PATCH 1/2] virsh: list required options first

[Eric Blake]

On 04/15/2011 02:33 PM, Eric Blake wrote:
>> Is example virsh command line missing here?
> 
> Aargh.  I wrote my commit message by using sample command prompts:
> 
> # virsh ...
> 
> But # is a comment character, and ate my message (and now I don't
> remember it off the top of my head).  I'll have to figure that out
> again, and use $ prompts instead...
> 
>> ACK
> 
> The review would have been much easier with a non-broken commit message,
> but you caught on to my drift.  I'll post my revised commit message once
> I remember what it was...

Pushed with the following commit message:

commit 6b75a1a5b0d10b42e3fd344b2067a176ee294f46
Author: Eric Blake <eblake at redhat.com>
Date:   Tue Apr 12 14:58:02 2011 -0600

    virsh: list required options first

    The current state of virsh parsing is that:

    $ virsh vol-info /path/to/image
    $ virsh vol-info --pool default /path/to/image
    $ virsh vol-info --pool default --vol /path/to/image

    all lookup the volume by path (technically, the last two also attempt
    a name lookup within a pool, whereas the first skips that step, but
    the end result is the same); meanwhile:

    $ virsh vol-info default /path/to/image

    complains about unexpected data.  Why?  Because the --pool option is
    optional, so default was parsed as the --vol argument, and
    /path/to/image.img doesn't match up with any remaining options that
    require an argument.  For proof, note that:

    $ virsh vol-info default --vol /path/to/image

    complains about looking up 'default' - the parser mis-associated both
    arguments with --vol.  Given the above, the only way to specify pool
    is with an explicit "--pool" argument (you can't specify it
    positionally).  However, named arguments can appear in any order, so:

    $ virsh vol-info /path/to/image --pool default
    $ virsh vol-info --vol /path/to/image --pool default

    have also always worked.  Therefore, this patch has no functional
    change on vol-info option parsing, but only on 'virsh help vol-info'
    synopsis layout.  However, it also allows the next patch to 1) enforce
    that required options are always first (without this patch, the next
    patch would fail the testsuite), and 2) allow the user to omit the
    "--pool" argument.  That is, the next patch makes it possible to do:

    $ virsh vol-info /path/to/image default

    which to date was not possible.

    * tools/virsh.c (opts_vol_create_from, opts_vol_clone)
    (opts_vol_upload, opts_vol_download, opts_vol_delete)
    (opts_vol_wipe, opts_vol_info, opts_vol_dumpxml, opts_vol_key)
    (opts_vol_path): List optional pool parameter after required
    arguments.

-- 
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/20110415/dd541b08/attachment-0001.sig>