Re: [PATCH v2 03/20] qapi: backup: add x-use-copy-range parameter

[Max Reitz]

On 01.06.20 20:11, Vladimir Sementsov-Ogievskiy wrote:
> Add parameter to enable/disable copy_range. Keep current default for
> now (enabled).

Why x-, though?  I can’t think of a reason why we would have to remove this.

> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  qapi/block-core.json       | 4 +++-
>  block/backup-top.h         | 1 +
>  include/block/block-copy.h | 2 +-
>  include/block/block_int.h  | 1 +
>  block/backup-top.c         | 4 +++-
>  block/backup.c             | 4 +++-
>  block/block-copy.c         | 4 ++--
>  block/replication.c        | 1 +
>  blockdev.c                 | 5 +++++
>  9 files changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 6fbacddab2..0c7600e4ec 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1405,6 +1405,8 @@
>  #                    above node specified by @drive. If this option is not 
> given,
>  #                    a node name is autogenerated. (Since: 4.2)
>  #
> +# @x-use-copy-range: use copy offloading if possible. Default true. (Since 
> 5.1)

Would it make more sense to invert it to disable-copy-range?  First,
this would make for a cleaner meaning, because it would allow dropping
the “if possible” part.  Setting use-copy-range=true would intuitively
imply to me that I get an error if copy-range cannot be used.  Sure,
there’s this little “if possible” in the documentation, but it goes
against my intuition.  disable-copy-range=true is intuitively clear.
Second, this would give us a default of false, which is marginally nicer.

Max

signature.asc
Description: OpenPGP digital signature