On 01/21/2016 07:04 PM, 张敏 wrote: >>> - .args_type = "reuse:-n,full:-f,device:B,target:s,format:s?", >>> - .params = "[-n] [-f] device target [format]", >>> + .args_type = >>> "reuse:-n,full:-f,device:B,target:s,bitmap:s?,format:s?", >>> + .params = "[-n] [-f] device target [bitmap] [format]", >> This is HMP, so it may not matter, but this is not backwards compatible. >> Scripts targetting the old style of passing a format will now have that >> format string interpreted as a bitmap name with no format. Better would >> be to stick [bitmap] at the end, not the middle. > > But I have a question: If I don't want to input a 'format', only use 'bitmap', > it will let 'bitmap' as 'format', This problem how to do it.
Several solutions, some nicer than others, some more complicated than
others. I don't have any strong preference, so much as pointing out the
design space:
1. You don't. If you want to use 'bitmap', you MUST supply 'format'
(supplying format is good anyways, as implicit formats have led to CVEs
in the past).
1.a. Possible variation: Teach the command to allow empty '' format to
be a synonym for an implicit format, so that it could look like:
drive_backup device target '' /path/to/bitmap
2. You modify the HMP parser to accept optionally-named arguments, so
that you can then supply later optional arguments by name without having
to provide the earlier optional arguments. Maybe looking something like:
drive_backup device target --bitmap=/path/to/bitmap
3. Instead of trying to overload an existing command, you create a new
command. Particularly since your overload already declared that '-f'
and 'bitmap' are incompatible.
4. maybe something else?
>> Needs {}. Run your patch through scripts/checkpatch.pl, to flag this
>> and other style violations.
>
> I have checked these patches，but I ignored these warnings.
Not a good idea. And even in the rare case that you plan on ignoring
the warnings, you should at least document in the commit message your
justification for doing so.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
