|
From: | Vladimir Sementsov-Ogievskiy |
Subject: | Re: [PATCH 16/21] block/copy-before-write: cbw_init(): use options |
Date: | Tue, 18 May 2021 17:24:58 +0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 |
18.05.2021 16:56, Max Reitz wrote:
On 17.05.21 08:44, Vladimir Sementsov-Ogievskiy wrote:One more step closer to .bdrv_open(): use options instead of plain arguments. Move to bdrv_open_child() calls, native for drive open handlers. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- block/copy-before-write.c | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/block/copy-before-write.c b/block/copy-before-write.c index ddd79b3686..9ff1bf676c 100644 --- a/block/copy-before-write.c +++ b/block/copy-before-write.c @@ -144,27 +144,20 @@ static void cbw_child_perm(BlockDriverState *bs, BdrvChild *c, } } -static int cbw_init(BlockDriverState *bs, BlockDriverState *source, - BlockDriverState *target, bool compress, Error **errp) +static int cbw_init(BlockDriverState *bs, QDict *options, Error **errp) { BDRVCopyBeforeWriteState *s = bs->opaque; - bdrv_ref(target); - s->target = bdrv_attach_child(bs, target, "target", &child_of_bds, - BDRV_CHILD_DATA, errp); - if (!s->target) { - error_prepend(errp, "Cannot attach target child: "); - bdrv_unref(target); + bs->file = bdrv_open_child(NULL, options, "file", bs, &child_of_bds, + BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY, + false, errp); + if (!bs->file) { return -EINVAL; } - bdrv_ref(source); - bs->file = bdrv_attach_child(bs, source, "file", &child_of_bds, - BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY, - errp); - if (!bs->file) { - error_prepend(errp, "Cannot attach file child: "); - bdrv_unref(source); + s->target = bdrv_open_child(NULL, options, "target", bs, &child_of_bds, + BDRV_CHILD_DATA, false, errp); + if (!s->target) { return -EINVAL; } @@ -175,7 +168,10 @@ static int cbw_init(BlockDriverState *bs, BlockDriverState *source, ((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) & bs->file->bs->supported_zero_flags); - s->bcs = block_copy_state_new(bs->file, s->target, false, compress, errp); + qdict_del(options, "cluster-size");What is this about?
accidental, will drop. (it's a remaining of my first solution where I tried to pass cluster-size, then I decided that's better move cluster-size detection to block_copy)
+ s->bcs = block_copy_state_new(bs->file, s->target, false, + qdict_get_try_bool(options, "x-deprecated-compress", false), errp);First, I’d keep the `compress` variable and use it to store the value, because this doesn’t look very nice.
OK
Second, what’s the story here? “deprecated” sounds to me like you’re planning to use a different interface eventually, but looking ahead for a bit I didn’t find anything yet.
I should have described it in commit message. We have "compress" filter driver. So instead adding "compress" option to every block job or filter, user should use "compress" filter. That's why I don't want to publish compress option for copy-before-write filter. Still we need it to maintain "compress" option of backup job. I also want to deprecate "compress" option in backup, then everything will be clear.
+ qdict_del(options, "x-deprecated-compress"); if (!s->bcs) { error_prepend(errp, "Cannot create block-copy-state: "); return -EINVAL; @@ -212,6 +208,7 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source, int ret; BDRVCopyBeforeWriteState *state; BlockDriverState *top; + QDict *opts; assert(source->total_sectors == target->total_sectors); @@ -223,7 +220,13 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source, } state = top->opaque; - ret = cbw_init(top, source, target, compress, errp); + opts = qdict_new(); + qdict_put_str(opts, "file", bdrv_get_node_name(source)); + qdict_put_str(opts, "target", bdrv_get_node_name(target)); + qdict_put_bool(opts, "x-deprecated-compress", compress); + + ret = cbw_init(top, opts, errp); + qobject_unref(opts); if (ret < 0) { goto fail; }
-- Best regards, Vladimir
[Prev in Thread] | Current Thread | [Next in Thread] |