[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH 1/2] hmp: add hmp command for incre
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH 1/2] hmp: add hmp command for incremental backup |
Date: |
Fri, 22 Jan 2016 09:39:38 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 |
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