On 6/20/19 11:25 AM, Max Reitz wrote:
> On 20.06.19 03:03, John Snow wrote:
>> This adds a "never" policy for bitmap synchronization. Regardless of if
>> the job succeeds or fails, we never update the bitmap. This can be used
>> to perform differential backups, or simply to avoid the job modifying a
>> bitmap.
>>
>> Signed-off-by: John Snow <js...@redhat.com>
>> ---
>> qapi/block-core.json | 6 +++++-
>> block/backup.c | 5 +++--
>> 2 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index 6d05ad8f47..0332dcaabc 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -1146,10 +1146,14 @@
>> # @conditional: The bitmap is only synchronized when the operation is
>> successul.
>> # This is useful for Incremental semantics.
>> #
>> +# @never: The bitmap is never synchronized with the operation, and is
>> +# treated solely as a manifest of blocks to copy.
>> +# This is useful for Differential semantics.
>> +#
>
> Again, this is too buzzword-y for my taste. I don’t find it as bad
> because there is not much to explain about this mode, and you do explain
> it above, but still.
>
Explained in my response to patch 2, I disagree.
> Like, I (me myself) read this and after the first sentence I think I’ve
> understood what this is. Then I read “for Differential semantics” and
> I’m confused. After a couple of seconds, I realize what you mean
> because I’ve described in my response to patch 1.
>
> One reason it leaves the buzzword-y taste is because “differential” is
> never explained anywhere. bitmaps.rst makes two mentions of it, but it
> too just assumes I know what you mean. Also, incremental backups are
> just a certain kind of differential backups.
>
This, however, is a real shortcoming of the doc. You'll notice I didn't
propose a doc update in this patchset, because secretly it's an RFC and
I did expect a v2+.
> So you need to explain “differential” somewhere and how it differs from
> “incremental” in this regard. Why not here?
>
Too broad of a concept to explain down in qapi comment strings, or I'd
have to explain it everywhere. bitmaps.rst is the correct place.
> “This is useful when you wish to repeatedly perform operations in
> reference to a constant synchronization point (when the bitmap was
> created).”
>
> Or something.
>
> Max
>
>> # Since: 4.1
>> ##
>> { 'enum': 'BitmapSyncMode',
>> - 'data': ['conditional'] }
>> + 'data': ['conditional', 'never'] }
>>
>> ##
>> # @MirrorCopyMode:
>
