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

[Jiri Denemark]

On Tue, Apr 12, 2011 at 15:35:06 -0600, Eric Blake wrote:
> The current state of virsh parsing is that:
> 
> 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:

Is example virsh command line missing here?

> 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.  Therefore, 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:

and here

> 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:

and here

> instead of the longer

and here as well.

> * 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.
> ---
>  tools/virsh.c |   20 ++++++++++----------
>  1 files changed, 10 insertions(+), 10 deletions(-)

Makes sense. I went through all virsh.c and didn't find more options that
would need fixing. The only options which don't follow this "optional after
required" rule are bool options which can safely stay where they are.

ACK

Jirka