On 12/7/18 11:25 AM, Eric Blake wrote:
> On 12/6/18 1:25 PM, John Snow wrote:
>> Especially outside of transactions, it is helpful to provide
>> all-or-nothing semantics for bitmap merges. This facilitates
>> the coalescing of multiple bitmaps into a single target for
>> the "checkpoint" interpretation when assembling bitmaps that
>> represent arbitrary points in time from component bitmaps.
>>
>> Signed-off-by: John Snow <js...@redhat.com>
>> ---
>> blockdev.c | 64 +++++++++++++++++++++++++++++---------------
>> qapi/block-core.json | 22 +++++++--------
>> 2 files changed, 53 insertions(+), 33 deletions(-)
>>
>
>> +++ b/qapi/block-core.json
>> @@ -1818,14 +1818,14 @@
>> #
>> # @node: name of device/node which the bitmap is tracking
>> #
>> -# @dst_name: name of the destination dirty bitmap
>> +# @target: name of the destination dirty bitmap
>> #
>> -# @src_name: name of the source dirty bitmap
>> +# @bitmaps: name(s) of the source dirty bitmap(s)
>> #
>> # Since: 3.0
>> ##
>> { 'struct': 'BlockDirtyBitmapMerge',
>> - 'data': { 'node': 'str', 'dst_name': 'str', 'src_name': 'str' } }
>> + 'data': { 'node': 'str', 'target': 'str', 'bitmaps': ['str'] } }
>
> Definitely worthwhile!
>
> I'll update my pending libvirt patches to use this.
>
>> ##
>> # @block-dirty-bitmap-add:
>> @@ -1940,23 +1940,23 @@
>> ##
>> # @x-block-dirty-bitmap-merge:
>> #
>> -# FIXME: Rename @src_name and @dst_name to src-name and dst-name.
>> -#
>> -# Merge @src_name dirty bitmap to @dst_name dirty bitmap. @src_name
>> dirty
>> -# bitmap is unchanged. On error, @dst_name is unchanged.
>> +# Merge dirty bitmaps listed in @bitmaps to the @target dirty bitmap.
>> +# The @bitmaps dirty bitmaps are unchanged.
>
> Well, except in the corner case of when @bitmaps also lists the
> destination (I presume that merging a bitmap into itself silently
> succeeds with no further changes, but therefore the inclusion of the
> destination in the list of sources means that that particular source is
> changing due to merging in the other sources). Not worth rewording this
> sentence, but does make you want to consider ensuring that the
> testsuite covers merging a bitmap into itself.
>
OK, noted.
> Reviewed-by: Eric Blake <ebl...@redhat.com>
>
Thanks!
