Re: [Qemu-block] [Qemu-devel] [PATCH 1/2] qapi: support external bitmaps in block-dirty-bitmap-merge

[Eric Blake]

On 5/16/19 7:32 PM, John Snow wrote:
> 
> 
> On 5/16/19 8:27 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Add new optional parameter making possible to merge bitmaps from
>> different nodes. It is needed to maintain external snapshots during
>> incremental backup chain history.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>> ---
>>  qapi/block-core.json | 13 ++++++++++---
>>  block/dirty-bitmap.c |  9 ++++++---
>>  blockdev.c           | 46 ++++++++++++++++++++++++++++++--------------
>>  3 files changed, 48 insertions(+), 20 deletions(-)

>> -# @bitmaps: name(s) of the source dirty bitmap(s)
>> +# @bitmaps: name(s) of the source dirty bitmap(s). The field is optional
>> +#           since 4.1.
>> +#
>> +# @external-bitmaps: additional list of source dirty bitmaps with specified
>> +#                    nodes, which allows merging bitmaps between different
>> +#                    nodes. (Since: 4.1)
>>  #
>>  # Since: 4.0
>>  ##
>>  { 'struct': 'BlockDirtyBitmapMerge',
>> -  'data': { 'node': 'str', 'target': 'str', 'bitmaps': ['str'] } }
>> +  'data': { 'node': 'str', 'target': 'str', '*bitmaps': ['str'],
>> +            '*external-bitmaps': ['BlockDirtyBitmap'] } }
>>  
> 
> I guess you can specify one, or both, or maybe neither! Seems fine.


> 
> I don't think I like the name "external-bitmaps" but I guess I don't
> really have a better suggestion.

I do - we could use an alternate type instead:

{ 'alternate': 'BitmapSource',
  'data': { 'local': 'str',
             'external': 'BlockDirtyBitmap' } }

then use 'bitmaps': ['BitmapSource']

so that the caller can pass:

"bitmaps": [ "bitmap1", { "node": "other", "name", "bitmap2" } ]

and we only have to deal with one array at all times, and not have the
name 'external-bitmaps' to worry about.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature