Re: [PATCH v12 05/14] qapi: create BlockdevOptionsCor structure for COR driver - Vladimir Sementsov-Ogievskiy

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>,
	qemu-block@nongnu.org
Cc: qemu-devel@nongnu.org, fam@euphon.net, stefanha@redhat.com,
	kwolf@redhat.com, mreitz@redhat.com, armbru@redhat.com,
	jsnow@redhat.com, eblake@redhat.com, den@openvz.org
Subject: Re: [PATCH v12 05/14] qapi: create BlockdevOptionsCor structure for COR driver
Date: Fri, 23 Oct 2020 17:51:18 +0300	[thread overview]
Message-ID: <f405f8ba-ffbe-7fa7-d935-6df2b94dca2f@virtuozzo.com> (raw)
In-Reply-To: <1603390423-980205-6-git-send-email-andrey.shinkevich@virtuozzo.com>

22.10.2020 21:13, Andrey Shinkevich wrote:
> Create the BlockdevOptionsCor structure for COR driver specific options
> splitting it off form the BlockdevOptionsGenericFormat. The only option
> 'bottom' node in the structure denotes an image file that limits the
> COR operations in the backing chain.
> 
> Suggested-by: Max Reitz <mreitz@redhat.com>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   qapi/block-core.json | 21 ++++++++++++++++++++-
>   1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 0a64306..bf465f6 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -3938,6 +3938,25 @@
>     'data': { 'throttle-group': 'str',
>               'file' : 'BlockdevRef'
>                } }
> +
> +##
> +# @BlockdevOptionsCor:
> +#
> +# Driver specific block device options for the copy-on-read driver.
> +#
> +# @bottom: the name of a non-filter node (allocation-bearing layer) that limits
> +#          the COR operations in the backing chain (inclusive).
> +#          For the block-stream job, it will be the first non-filter overlay of
> +#          the base node. We do not involve the base node into the COR
> +#          operations because the base may change due to a concurrent
> +#          block-commit job on the same backing chain. 
> +#
> +# Since: 5.2
> +##
> +{ 'struct': 'BlockdevOptionsCor',
> +  'base': 'BlockdevOptionsGenericFormat',
> +  'data': { '*bottom': 'str' } }
> +
>   ##
>   # @BlockdevOptions:
>   #
> @@ -3990,7 +4009,7 @@
>         'bochs':      'BlockdevOptionsGenericFormat',
>         'cloop':      'BlockdevOptionsGenericFormat',
>         'compress':   'BlockdevOptionsGenericFormat',
> -      'copy-on-read':'BlockdevOptionsGenericFormat',
> +      'copy-on-read':'BlockdevOptionsCor',
>         'dmg':        'BlockdevOptionsGenericFormat',
>         'file':       'BlockdevOptionsFile',
>         'ftp':        'BlockdevOptionsCurlFtp',
> 

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Still, I'd prefer this to be merged with further two patches, to not add non-working interfaces even if this will be fixed two commits further. We do similar things sometimes to simplify big commits, but in this case merged 03/04/05 doesn't seem too big.

-- 
Best regards,
Vladimir