Re: [libvirt] [PATCH] virsh: rework command parsing
On 09/13/2010 11:03 PM, Eric Blake wrote:
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"
virsh "define D.xml" "dumpxml D"
is not supported in old parser nor new parser,
in new parser, "define D.xml" is command-name
"dumpxml D" is its parameter, we have no such command-name.
So, I think it should be:
virsh [options]... [<command> [command options]...]
virsh [options]... <commands_string>...
It looks good.
> 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.
It is hard to split them into separate patches. because "Misc change 1)"
and "Misc change 2)" is done by new parser's step 1.
In old parser, '=' is missed after we have got VSH_TK_OPTION token,
if we fix it for "Misc change 3)", we need a lot code, and this code
will be removed after this new parser applied.
>
> Signed-off-by: Lai Jiangshan<laijs(a)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.
Thanks, Lai.