Re: [PATCH v3 02/25] qapi: backup: add perf.use-copy-range parameter

[Max Reitz <mreitz@redhat.com>]

On 20.01.21 07:09, Vladimir Sementsov-Ogievskiy wrote:
> 19.01.2021 23:21, Eric Blake wrote:
>> On 10/26/20 12:17 PM, Vladimir Sementsov-Ogievskiy wrote:
>>> Experiments show, that copy_range is not always making things faster.
>>> So, to make experimentation simpler, let's add a parameter. Some more
>>> perf parameters will be added soon, so here is a new struct.
>>>
>>> For now, add new backup qmp parameter with x- prefix for the following
>>> reasons:
>>>
>>>   - We are going to add more performance parameters, some will be
>>>     related to the whole block-copy process, some only to background
>>>     copying in backup (ignored for copy-before-write operations).
>>>   - On the other hand, we are going to use block-copy interface in other
>>>     block jobs, which will need performance options as well.. And it
>>>     should be the same structure or at least somehow related.
>>>
>>> So, there are too much unclean things about how the interface and now
>>> we need the new options mostly for testing. Let's keep them
>>> experimental for a while.
>>>
>>> In do_backup_common() new x-perf parameter handled in a way to
>>> make further options addition simpler.
>>>
>>> We add use-copy-range with default=true, and we'll change the default
>>> in further patch, after moving backup to use block-copy.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>
>>> +++ b/qapi/block-core.json
>>> @@ -1370,6 +1370,19 @@
>>>   { 'struct': 'BlockdevSnapshot',
>>>     'data': { 'node': 'str', 'overlay': 'str' } }
>>> +##
>>> +# @BackupPerf:
>>> +#
>>> +# Optional parameters for backup. These parameters don't affect
>>> +# functionality, but may significantly affect performance.
>>> +#
>>> +# @use-copy-range: Use copy offloading. Default true.
>>> +#
>>> +# Since: 5.2
>>
>> We've missed 5.2; this will need to be updated to 6.0
>>
>>> +##
>>> +{ 'struct': 'BackupPerf',
>>> +  'data': { '*use-copy-range': 'bool' }}
>>> +
>>>   ##
>>>   # @BackupCommon:
>>>   #
>>> @@ -1425,6 +1438,8 @@
>>>   #                    above node specified by @drive. If this option 
>>> is not given,
>>>   #                    a node name is autogenerated. (Since: 4.2)
>>>   #
>>> +# @x-perf: Performance options. (Since 5.2)
>>
>> and here
>>
> 
> I wanted to say that it's fixed in v4 (which is already in Max's tree), 
> but... It's not fixed. Max, could you touch this up?

Sure.

Max