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