[libvirt] [PATCH] virsh: rework command parsing
Lai Jiangshan
laijs at cn.fujitsu.com
Tue Sep 14 02:52:01 UTC 2010
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"
>
> 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.
>
I am still waiting for review comments, I will collect comments
and fix the patch for next version.
Thanks, Lai
More information about the libvir-list
mailing list