[libvirt] [PATCH] virsh: rework command parsing

Eric Blake eblake at redhat.com
Mon Sep 13 15:03:35 UTC 2010


On 09/13/2010 01:41 AM, Lai Jiangshan wrote:
> And the usage was changed:
> old:
> virsh [options] [commands]
>
> new:
> virsh [options]... [commands string]
> virsh [options]... [<command>  [command options]...]

This needs a tweak, otherwise, executing:

virsh

ambiguously matches both forms.  In general, you should list the 
shortest possible form first.  Also, it seems like passing multiple 
command strings would be easy to do with this format, such that these 
two are equivalent:

virsh "define D.xml" "dumpxml D"
virsh "define D.xml; dumpxml D"

So, I think it should be:

virsh [options]... [<command> [command options]...]
virsh [options]... <commands_string>...

> Misc changes:
> 1) support escape '\'
> 2) a better quoting support, the following commands are now supported:
>       virsh # dumpxml --"update-cpu" vm1
>       virsh # dumpxml --update-cpu vm"1"
> 3) better handling the boolean options, in old code the following commands are equivalent:
>       virsh # dumpxml --update-cpu=vm1
>       virsh # dumpxml --update-cpu vm1
>     after this patch applied, the first one will become illegal.

Should any of these be split into separate patches?  It's easier to 
review a series of small patches that each do one thing than it is to 
review 350 lines of multiple changes smashed together.

>
> Signed-off-by: Lai Jiangshan<laijs at cn.fujitsu.com>
> ---
>   virsh.c |  351 ++++++++++++++++++++++++++++++----------------------------------

Therefore, I haven't closely reviewed this patch yet, but just from the 
commit message, I like the direction it is headed in.

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




More information about the libvir-list mailing list