From nobody Thu Nov 6 19:14:35 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1488228760262617.8128416991415; Mon, 27 Feb 2017 12:52:40 -0800 (PST) Received: from localhost ([::1]:56755 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ciSHf-0002hW-0j for importer@patchew.org; Mon, 27 Feb 2017 15:52:39 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57374) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ciRdQ-0000CT-7r for qemu-devel@nongnu.org; Mon, 27 Feb 2017 15:11:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ciRdO-0003ZG-G6 for qemu-devel@nongnu.org; Mon, 27 Feb 2017 15:11:04 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36382) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ciRdL-0003Xt-5X; Mon, 27 Feb 2017 15:10:59 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5AB6661B92; Mon, 27 Feb 2017 20:10:59 +0000 (UTC) Received: from noname.redhat.com (ovpn-116-148.ams2.redhat.com [10.36.116.148]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1RK9n8c003294; Mon, 27 Feb 2017 15:10:57 -0500 From: Kevin Wolf To: qemu-block@nongnu.org Date: Mon, 27 Feb 2017 21:09:26 +0100 Message-Id: <1488226184-9044-26-git-send-email-kwolf@redhat.com> In-Reply-To: <1488226184-9044-1-git-send-email-kwolf@redhat.com> References: <1488226184-9044-1-git-send-email-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Mon, 27 Feb 2017 20:10:59 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v2 25/43] commit: Use real permissions in commit block job X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, jcody@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" This is probably one of the most interesting conversions to the new op blocker system because a commit block job intentionally leaves some intermediate block nodes in the backing chain that aren't valid on their own any more; only the whole chain together results in a valid view. In order to provide the 'consistent read' permission to the parents of the 'top' node of the commit job, a new filter block driver is inserted above 'top' which doesn't require 'consistent read' on its backing chain. Subsequently, the commit job can block 'consistent read' on all intermediate nodes without causing a conflict. Signed-off-by: Kevin Wolf --- block/commit.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++++-----= ---- 1 file changed, 95 insertions(+), 18 deletions(-) diff --git a/block/commit.c b/block/commit.c index b69586f..8de4473 100644 --- a/block/commit.c +++ b/block/commit.c @@ -36,6 +36,7 @@ typedef struct CommitBlockJob { BlockJob common; RateLimit limit; BlockDriverState *active; + BlockDriverState *commit_top_bs; BlockBackend *top; BlockBackend *base; BlockdevOnError on_error; @@ -83,12 +84,23 @@ static void commit_complete(BlockJob *job, void *opaque) BlockDriverState *active =3D s->active; BlockDriverState *top =3D blk_bs(s->top); BlockDriverState *base =3D blk_bs(s->base); - BlockDriverState *overlay_bs =3D bdrv_find_overlay(active, top); + BlockDriverState *overlay_bs =3D bdrv_find_overlay(active, s->commit_t= op_bs); int ret =3D data->ret; + bool remove_commit_top_bs =3D false; + + /* Remove base node parent that still uses BLK_PERM_WRITE/RESIZE before + * the normal backing chain can be restored. */ + blk_unref(s->base); =20 if (!block_job_is_cancelled(&s->common) && ret =3D=3D 0) { /* success */ - ret =3D bdrv_drop_intermediate(active, top, base, s->backing_file_= str); + ret =3D bdrv_drop_intermediate(active, s->commit_top_bs, base, + s->backing_file_str); + } else if (overlay_bs) { + /* XXX Can (or should) we somehow keep 'consistent read' blocked e= ven + * after the failed/cancelled commit job is gone? If we already wr= ote + * something to base, the intermediate images aren't valid any mor= e. */ + remove_commit_top_bs =3D true; } =20 /* restore base open flags here if appropriate (e.g., change the base = back @@ -102,9 +114,15 @@ static void commit_complete(BlockJob *job, void *opaqu= e) } g_free(s->backing_file_str); blk_unref(s->top); - blk_unref(s->base); block_job_completed(&s->common, ret); g_free(data); + + /* If bdrv_drop_intermediate() didn't already do that, remove the comm= it + * filter driver from the backing chain. Do this as the final step so = that + * the 'consistent read' permission can be granted. */ + if (remove_commit_top_bs) { + bdrv_set_backing_hd(overlay_bs, top); + } } =20 static void coroutine_fn commit_run(void *opaque) @@ -208,6 +226,34 @@ static const BlockJobDriver commit_job_driver =3D { .start =3D commit_run, }; =20 +static int coroutine_fn bdrv_commit_top_preadv(BlockDriverState *bs, + uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags) +{ + return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags); +} + +static void bdrv_commit_top_close(BlockDriverState *bs) +{ +} + +static void bdrv_commit_top_child_perm(BlockDriverState *bs, BdrvChild *c, + const BdrvChildRole *role, + uint64_t perm, uint64_t shared, + uint64_t *nperm, uint64_t *nshared) +{ + *nperm =3D 0; + *nshared =3D BLK_PERM_ALL; +} + +/* Dummy node that provides consistent read to its users without requiring= it + * from its backing file and that allows writes on the backing file chain.= */ +static BlockDriver bdrv_commit_top =3D { + .format_name =3D "commit_top", + .bdrv_co_preadv =3D bdrv_commit_top_preadv, + .bdrv_close =3D bdrv_commit_top_close, + .bdrv_child_perm =3D bdrv_commit_top_child_perm, +}; + void commit_start(const char *job_id, BlockDriverState *bs, BlockDriverState *base, BlockDriverState *top, int64_t s= peed, BlockdevOnError on_error, const char *backing_file_str, @@ -219,6 +265,7 @@ void commit_start(const char *job_id, BlockDriverState = *bs, int orig_base_flags; BlockDriverState *iter; BlockDriverState *overlay_bs; + BlockDriverState *commit_top_bs =3D NULL; Error *local_err =3D NULL; int ret; =20 @@ -235,7 +282,6 @@ void commit_start(const char *job_id, BlockDriverState = *bs, return; } =20 - /* FIXME Use real permissions */ s =3D block_job_create(job_id, &commit_job_driver, bs, 0, BLK_PERM_ALL, speed, BLOCK_JOB_DEFAULT, NULL, NULL, errp); if (!s) { @@ -262,34 +308,62 @@ void commit_start(const char *job_id, BlockDriverStat= e *bs, } } =20 + /* Insert commit_top block node above top, so we can block consistent = read + * on the backing chain below it */ + commit_top_bs =3D bdrv_new_open_driver(&bdrv_commit_top, NULL, 0, errp= ); + if (commit_top_bs =3D=3D NULL) { + goto fail; + } + + bdrv_set_backing_hd(commit_top_bs, top); + bdrv_set_backing_hd(overlay_bs, commit_top_bs); + + s->commit_top_bs =3D commit_top_bs; + bdrv_unref(commit_top_bs); =20 /* Block all nodes between top and base, because they will * disappear from the chain after this operation. */ assert(bdrv_chain_contains(top, base)); - for (iter =3D top; iter !=3D backing_bs(base); iter =3D backing_bs(ite= r)) { - /* FIXME Use real permissions */ - block_job_add_bdrv(&s->common, "intermediate node", iter, 0, - BLK_PERM_ALL, &error_abort); + for (iter =3D top; iter !=3D base; iter =3D backing_bs(iter)) { + /* XXX BLK_PERM_WRITE needs to be allowed so we don't block oursel= ves + * at s->base (if writes are blocked for a node, they are also blo= cked + * for its backing file). The other options would be a second filt= er + * driver above s->base. */ + ret =3D block_job_add_bdrv(&s->common, "intermediate node", iter, = 0, + BLK_PERM_WRITE_UNCHANGED | BLK_PERM_WRITE, + errp); + if (ret < 0) { + goto fail; + } } + + ret =3D block_job_add_bdrv(&s->common, "base", base, 0, BLK_PERM_ALL, = errp); + if (ret < 0) { + goto fail; + } + /* overlay_bs must be blocked because it needs to be modified to - * update the backing image string, but if it's the root node then - * don't block it again */ - if (bs !=3D overlay_bs) { - /* FIXME Use real permissions */ - block_job_add_bdrv(&s->common, "overlay of top", overlay_bs, 0, - BLK_PERM_ALL, &error_abort); + * update the backing image string. */ + ret =3D block_job_add_bdrv(&s->common, "overlay of top", overlay_bs, + BLK_PERM_GRAPH_MOD, BLK_PERM_ALL, errp); + if (ret < 0) { + goto fail; } =20 - /* FIXME Use real permissions */ - s->base =3D blk_new(0, BLK_PERM_ALL); + s->base =3D blk_new(BLK_PERM_CONSISTENT_READ + | BLK_PERM_WRITE + | BLK_PERM_RESIZE, + BLK_PERM_CONSISTENT_READ + | BLK_PERM_GRAPH_MOD + | BLK_PERM_WRITE_UNCHANGED); ret =3D blk_insert_bs(s->base, base, errp); if (ret < 0) { goto fail; } =20 - /* FIXME Use real permissions */ + /* Required permissions are already taken with block_job_add_bdrv() */ s->top =3D blk_new(0, BLK_PERM_ALL); - ret =3D blk_insert_bs(s->top, top, errp); + blk_insert_bs(s->top, top, errp); if (ret < 0) { goto fail; } @@ -314,6 +388,9 @@ fail: if (s->top) { blk_unref(s->top); } + if (commit_top_bs) { + bdrv_set_backing_hd(overlay_bs, top); + } block_job_unref(&s->common); } =20 --=20 1.8.3.1