Re: [Qemu-devel] [PATCH v2 10/11] block/backup: support bitmap sync modes for non-bitmap backups

[John Snow]

On 7/16/19 1:18 AM, Markus Armbruster wrote:
> John Snow <address@hidden> writes:
> 
>> Accept bitmaps and sync policies for the other backup modes.
>> This allows us to do things like create a bitmap synced to a full backup
>> without a transaction, or start a resumable backup process.
>>
>> Some combinations don't make sense, though:
>>
>> - NEVER policy combined with any non-BITMAP mode doesn't do anything,
>>   because the bitmap isn't used for input or output.
>>   It's harmless, but is almost certainly never what the user wanted.
>>
>> - sync=NONE is more questionable. It can't use on-success because this
>>   job never completes with success anyway, and the resulting artifact
>>   of 'always' is suspect: because we start with a full bitmap and only
>>   copy out segments that get written to, the final output bitmap will
>>   always be ... a fully set bitmap.
>>
>>   Maybe there's contexts in which bitmaps make sense for sync=none,
>>   but not without more severe changes to the current job, and omitting
>>   it here doesn't prevent us from adding it later.
>>
>> Signed-off-by: John Snow <address@hidden>
>> ---
> [...]
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index 5a578806c5..099e4f37b2 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -1352,13 +1352,15 @@
>>  # @speed: the maximum speed, in bytes per second. The default is 0,
>>  #         for unlimited.
>>  #
>> -# @bitmap: the name of a dirty bitmap if sync is "bitmap" or "incremental".
>> +# @bitmap: The name of a dirty bitmap to use.
>>  #          Must be present if sync is "bitmap" or "incremental".
>> +#          Can be present if sync is "full" or "top".
>>  #          Must not be present otherwise.
>>  #          (Since 2.4 (drive-backup), 3.1 (blockdev-backup))
>>  #
>>  # @bitmap-mode: Specifies the type of data the bitmap should contain after
>> -#               the operation concludes. Must be present if sync is 
>> "bitmap".
>> +#               the operation concludes.
>> +#               Must be present if a bitmap was provided,
>>  #               Must NOT be present otherwise. (Since 4.2)
>>  #
>>  # @compress: true to compress data, if the target format supports it.
> 
> Do you expect management applications will want to know about the
> presence of this patch?
> 

It's all going to be queued up for 4.2, so management can gate on the
presence of the "bitmap-mode" field. The text being replaced here is
intermediate only.

Bitmaps can't be used with traditional backup modes without the
bitmap-mode parameter, so I believe this is OK.

--js