Re: [PATCH] virsh: rework command parsing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxx>
>> ---
>>   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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]