Re: [PATCH v3 1/7] block/copy-before-write: refactor option parsing

From: Hanna Reitz <hreitz@redhat.com>
To: Vladimir Sementsov-Ogievskiy
	<vladimir.sementsov-ogievskiy@openvz.org>,
	qemu-block@nongnu.org
Cc: kwolf@redhat.com, v.sementsov-og@mail.ru, jsnow@redhat.com,
	qemu-devel@nongnu.org, armbru@redhat.com, vsementsov@openvz.org,
	stefanha@redhat.com, eblake@redhat.com
Subject: Re: [PATCH v3 1/7] block/copy-before-write: refactor option parsing
Date: Thu, 7 Apr 2022 09:08:25 +0200	[thread overview]
Message-ID: <0c2df44a-0fcd-0989-edb3-34c848d8e098@redhat.com> (raw)
In-Reply-To: <20220406180801.374844-2-vsementsov@openvz.org>

On 06.04.22 20:07, Vladimir Sementsov-Ogievskiy wrote:
> We are going to add one more option of enum type. Let's refactor option
> parsing so that we can simply work with BlockdevOptionsCbw object.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
> ---
>   block/copy-before-write.c | 55 ++++++++++++++++++++-------------------
>   1 file changed, 28 insertions(+), 27 deletions(-)
>
> diff --git a/block/copy-before-write.c b/block/copy-before-write.c
> index a8a06fdc09..6877ff893a 100644
> --- a/block/copy-before-write.c
> +++ b/block/copy-before-write.c

[...]

> @@ -376,6 +365,14 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
>       BDRVCopyBeforeWriteState *s = bs->opaque;
>       BdrvDirtyBitmap *bitmap = NULL;
>       int64_t cluster_size;
> +    g_autoptr(BlockdevOptions) full_opts = NULL;
> +    BlockdevOptionsCbw *opts;
> +
> +    full_opts = cbw_parse_options(options, errp);
> +    if (!full_opts) {
> +        return -EINVAL;
> +    }
> +    opts = &full_opts->u.copy_before_write;

I would prefer an `assert(full_opts->driver == 
BLOCKDEV_DRIVER_COPY_BEFORE_WRITE);` here, but, either way:

Reviewed-by: Hanna Reitz <hreitz@redhat.com>