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

From: Eric Blake <eblake@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	qemu-block@nongnu.org
Cc: kwolf@redhat.com, wencongyang2@huawei.com,
	xiechanglong.d@gmail.com, qemu-devel@nongnu.org,
	mreitz@redhat.com, den@openvz.org, jsnow@redhat.com,
	armbru@redhat.com
Subject: Re: [PATCH v3 02/25] qapi: backup: add perf.use-copy-range parameter
Date: Tue, 19 Jan 2021 14:21:34 -0600	[thread overview]
Message-ID: <84902f8a-2c1f-7e71-f59e-c3f485b7295a@redhat.com> (raw)
In-Reply-To: <20201026171815.13233-3-vsementsov@virtuozzo.com>

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

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