From nobody Mon Feb 9 09:34:21 2026 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.zohomail.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 150532764234482.61121893106304; Wed, 13 Sep 2017 11:34:02 -0700 (PDT) Received: from localhost ([::1]:44070 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dsCU5-0003mU-JR for importer@patchew.org; Wed, 13 Sep 2017 14:34:01 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37181) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dsCHV-0000LU-7D for qemu-devel@nongnu.org; Wed, 13 Sep 2017 14:21:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dsCHS-0005fV-W3 for qemu-devel@nongnu.org; Wed, 13 Sep 2017 14:21:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51504) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dsCHM-0005YV-BH; Wed, 13 Sep 2017 14:20:52 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6A118C047B88; Wed, 13 Sep 2017 18:20:51 +0000 (UTC) Received: from localhost (ovpn-204-23.brq.redhat.com [10.40.204.23]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 202205D763; Wed, 13 Sep 2017 18:20:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 6A118C047B88 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=mreitz@redhat.com From: Max Reitz To: qemu-block@nongnu.org Date: Wed, 13 Sep 2017 20:19:02 +0200 Message-Id: <20170913181910.29688-11-mreitz@redhat.com> In-Reply-To: <20170913181910.29688-1-mreitz@redhat.com> References: <20170913181910.29688-1-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 13 Sep 2017 18:20:51 +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 10/18] block/mirror: Make source the file child 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: Kevin Wolf , Fam Zheng , qemu-devel@nongnu.org, Max Reitz , Stefan Hajnoczi , John Snow 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" Regarding the source BDS, the mirror BDS is arguably a filter node. Therefore, the source BDS should be its "file" child. Signed-off-by: Max Reitz --- block/mirror.c | 127 ++++++++++++++++++++++++++++++++++-------= ---- block/qapi.c | 25 ++++++--- tests/qemu-iotests/141.out | 4 +- 3 files changed, 119 insertions(+), 37 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index 9df4157511..05410c94ca 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -77,8 +77,16 @@ typedef struct MirrorBlockJob { int target_cluster_size; int max_iov; bool initial_zeroing_ongoing; + + /* Signals that we are no longer accessing source and target and the m= irror + * BDS should thus relinquish all permissions */ + bool exiting; } MirrorBlockJob; =20 +typedef struct MirrorBDSOpaque { + MirrorBlockJob *job; +} MirrorBDSOpaque; + struct MirrorOp { MirrorBlockJob *s; QEMUIOVector qiov; @@ -595,12 +603,15 @@ static void mirror_exit(BlockJob *job, void *opaque) { MirrorBlockJob *s =3D container_of(job, MirrorBlockJob, common); MirrorExitData *data =3D opaque; + MirrorBDSOpaque *bs_opaque =3D s->mirror_top_bs->opaque; AioContext *replace_aio_context =3D NULL; BlockDriverState *src =3D s->source->bs; BlockDriverState *target_bs =3D blk_bs(s->target); BlockDriverState *mirror_top_bs =3D s->mirror_top_bs; Error *local_err =3D NULL; =20 + s->exiting =3D true; + bdrv_release_dirty_bitmap(src, s->dirty_bitmap); =20 /* Make sure that the source BDS doesn't go away before we called @@ -622,7 +633,7 @@ static void mirror_exit(BlockJob *job, void *opaque) =20 /* We don't access the source any more. Dropping any WRITE/RESIZE is * required before it could become a backing file of target_bs. */ - bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL, + bdrv_child_try_set_perm(mirror_top_bs->file, 0, BLK_PERM_ALL, &error_abort); if (s->backing_mode =3D=3D MIRROR_SOURCE_BACKING_CHAIN) { BlockDriverState *backing =3D s->is_none_mode ? src : s->base; @@ -673,12 +684,11 @@ static void mirror_exit(BlockJob *job, void *opaque) =20 /* Remove the mirror filter driver from the graph. Before this, get ri= d of * the blockers on the intermediate nodes so that the resulting state = is - * valid. Also give up permissions on mirror_top_bs->backing, which mi= ght + * valid. Also give up permissions on mirror_top_bs->file, which might * block the removal. */ block_job_remove_all_bdrv(job); - bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL, - &error_abort); - bdrv_replace_node(mirror_top_bs, backing_bs(mirror_top_bs), &error_abo= rt); + bdrv_child_try_set_perm(mirror_top_bs->file, 0, BLK_PERM_ALL, &error_a= bort); + bdrv_replace_node(mirror_top_bs, mirror_top_bs->file->bs, &error_abort= ); =20 /* We just changed the BDS the job BB refers to (with either or both o= f the * bdrv_replace_node() calls), so switch the BB back so the cleanup do= es @@ -687,6 +697,7 @@ static void mirror_exit(BlockJob *job, void *opaque) blk_set_perm(job->blk, 0, BLK_PERM_ALL, &error_abort); blk_insert_bs(job->blk, mirror_top_bs, &error_abort); =20 + bs_opaque->job =3D NULL; block_job_completed(&s->common, data->ret); =20 g_free(data); @@ -1102,7 +1113,7 @@ static void mirror_drain(BlockJob *job) { MirrorBlockJob *s =3D container_of(job, MirrorBlockJob, common); =20 - /* Need to keep a reference in case blk_drain triggers execution + /* Need to keep a reference in case bdrv_drain triggers execution * of mirror_complete... */ if (s->target) { @@ -1135,44 +1146,88 @@ static const BlockJobDriver commit_active_job_drive= r =3D { .drain =3D mirror_drain, }; =20 +static void source_child_inherit_fmt_options(int *child_flags, + QDict *child_options, + int parent_flags, + QDict *parent_options) +{ + child_backing.inherit_options(child_flags, child_options, + parent_flags, parent_options); +} + +static char *source_child_get_parent_desc(BdrvChild *c) +{ + return child_backing.get_parent_desc(c); +} + +static void source_child_cb_drained_begin(BdrvChild *c) +{ + BlockDriverState *bs =3D c->opaque; + MirrorBDSOpaque *s =3D bs->opaque; + + if (s && s->job) { + block_job_drained_begin(&s->job->common); + } + bdrv_drained_begin(bs); +} + +static void source_child_cb_drained_end(BdrvChild *c) +{ + BlockDriverState *bs =3D c->opaque; + MirrorBDSOpaque *s =3D bs->opaque; + + if (s && s->job) { + block_job_drained_end(&s->job->common); + } + bdrv_drained_end(bs); +} + +static BdrvChildRole source_child_role =3D { + .inherit_options =3D source_child_inherit_fmt_options, + .get_parent_desc =3D source_child_get_parent_desc, + .drained_begin =3D source_child_cb_drained_begin, + .drained_end =3D source_child_cb_drained_end, +}; + static int coroutine_fn bdrv_mirror_top_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags) { - return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags); + return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags); } =20 static int coroutine_fn bdrv_mirror_top_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags) { - return bdrv_co_pwritev(bs->backing, offset, bytes, qiov, flags); + return bdrv_co_pwritev(bs->file, offset, bytes, qiov, flags); } =20 static int coroutine_fn bdrv_mirror_top_flush(BlockDriverState *bs) { - return bdrv_co_flush(bs->backing->bs); + return bdrv_co_flush(bs->file->bs); } =20 static int coroutine_fn bdrv_mirror_top_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int bytes, BdrvRequestFlags flags) { - return bdrv_co_pwrite_zeroes(bs->backing, offset, bytes, flags); + return bdrv_co_pwrite_zeroes(bs->file, offset, bytes, flags); } =20 static int coroutine_fn bdrv_mirror_top_pdiscard(BlockDriverState *bs, int64_t offset, int bytes) { - return bdrv_co_pdiscard(bs->backing->bs, offset, bytes); + return bdrv_co_pdiscard(bs->file->bs, offset, bytes); } =20 static void bdrv_mirror_top_refresh_filename(BlockDriverState *bs, QDict *= opts) { - bdrv_refresh_filename(bs->backing->bs); pstrcpy(bs->exact_filename, sizeof(bs->exact_filename), - bs->backing->bs->filename); + bs->file->bs->filename); } =20 static void bdrv_mirror_top_close(BlockDriverState *bs) { + bdrv_unref_child(bs, bs->file); + bs->file =3D NULL; } =20 static void bdrv_mirror_top_child_perm(BlockDriverState *bs, BdrvChild *c, @@ -1180,6 +1235,14 @@ static void bdrv_mirror_top_child_perm(BlockDriverSt= ate *bs, BdrvChild *c, uint64_t perm, uint64_t shared, uint64_t *nperm, uint64_t *nshared) { + MirrorBDSOpaque *s =3D bs->opaque; + + if (s->job && s->job->exiting) { + *nperm =3D 0; + *nshared =3D BLK_PERM_ALL; + return; + } + /* Must be able to forward guest writes to the real image */ *nperm =3D 0; if (perm & BLK_PERM_WRITE) { @@ -1190,7 +1253,7 @@ static void bdrv_mirror_top_child_perm(BlockDriverSta= te *bs, BdrvChild *c, } =20 /* 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.= */ + * from its source file and that allows writes on the source file. */ static BlockDriver bdrv_mirror_top =3D { .format_name =3D "mirror_top", .bdrv_co_preadv =3D bdrv_mirror_top_preadv, @@ -1198,7 +1261,7 @@ static BlockDriver bdrv_mirror_top =3D { .bdrv_co_pwrite_zeroes =3D bdrv_mirror_top_pwrite_zeroes, .bdrv_co_pdiscard =3D bdrv_mirror_top_pdiscard, .bdrv_co_flush =3D bdrv_mirror_top_flush, - .bdrv_co_get_block_status =3D bdrv_co_get_block_status_from_backing, + .bdrv_co_get_block_status =3D bdrv_co_get_block_status_from_file, .bdrv_refresh_filename =3D bdrv_mirror_top_refresh_filename, .bdrv_close =3D bdrv_mirror_top_close, .bdrv_child_perm =3D bdrv_mirror_top_child_perm, @@ -1221,6 +1284,7 @@ static void mirror_start_job(const char *job_id, Bloc= kDriverState *bs, Error **errp) { MirrorBlockJob *s; + MirrorBDSOpaque *bs_opaque; BlockDriverState *mirror_top_bs; bool target_graph_mod; bool target_is_backing; @@ -1244,9 +1308,7 @@ static void mirror_start_job(const char *job_id, Bloc= kDriverState *bs, buf_size =3D DEFAULT_MIRROR_BUF_SIZE; } =20 - /* In the case of active commit, add dummy driver to provide consistent - * reads on the top, while disabling it in the intermediate nodes, and= make - * the backing chain writable. */ + /* Create mirror BDS */ mirror_top_bs =3D bdrv_new_open_driver(&bdrv_mirror_top, filter_node_n= ame, BDRV_O_RDWR, errp); if (mirror_top_bs =3D=3D NULL) { @@ -1256,14 +1318,19 @@ static void mirror_start_job(const char *job_id, Bl= ockDriverState *bs, mirror_top_bs->implicit =3D true; } mirror_top_bs->total_sectors =3D bs->total_sectors; + bs_opaque =3D g_new0(MirrorBDSOpaque, 1); + mirror_top_bs->opaque =3D bs_opaque; bdrv_set_aio_context(mirror_top_bs, bdrv_get_aio_context(bs)); =20 - /* bdrv_append takes ownership of the mirror_top_bs reference, need to= keep - * it alive until block_job_create() succeeds even if bs has no parent= . */ - bdrv_ref(mirror_top_bs); - bdrv_drained_begin(bs); - bdrv_append(mirror_top_bs, bs, &local_err); - bdrv_drained_end(bs); + /* Create reference for bdrv_attach_child() */ + bdrv_ref(bs); + mirror_top_bs->file =3D bdrv_attach_child(mirror_top_bs, bs, "file", + &source_child_role, &local_err= ); + if (!local_err) { + bdrv_drained_begin(bs); + bdrv_replace_node(bs, mirror_top_bs, &local_err); + bdrv_drained_end(bs); + } =20 if (local_err) { bdrv_unref(mirror_top_bs); @@ -1280,6 +1347,8 @@ static void mirror_start_job(const char *job_id, Bloc= kDriverState *bs, if (!s) { goto fail; } + bs_opaque->job =3D s; + /* The block job now has a reference to this node */ bdrv_unref(mirror_top_bs); =20 @@ -1329,7 +1398,7 @@ static void mirror_start_job(const char *job_id, Bloc= kDriverState *bs, s->should_complete =3D true; } =20 - s->source =3D mirror_top_bs->backing; + s->source =3D mirror_top_bs->file; s->mirror_top_bs =3D mirror_top_bs; =20 s->dirty_bitmap =3D bdrv_create_dirty_bitmap(bs, granularity, NULL, er= rp); @@ -1373,12 +1442,12 @@ fail: =20 g_free(s->replaces); blk_unref(s->target); - block_job_early_fail(&s->common); + bs_opaque->job =3D NULL; + block_job_unref(&s->common); } =20 - bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL, - &error_abort); - bdrv_replace_node(mirror_top_bs, backing_bs(mirror_top_bs), &error_abo= rt); + bdrv_child_try_set_perm(mirror_top_bs->file, 0, BLK_PERM_ALL, &error_a= bort); + bdrv_replace_node(mirror_top_bs, mirror_top_bs->file->bs, &error_abort= ); =20 bdrv_unref(mirror_top_bs); } diff --git a/block/qapi.c b/block/qapi.c index 7fa2437923..ee792d0cbc 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -147,9 +147,13 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *= blk, =20 /* Skip automatically inserted nodes that the user isn't aware of = for * query-block (blk !=3D NULL), but not for query-named-block-node= s */ - while (blk && bs0->drv && bs0->implicit) { - bs0 =3D backing_bs(bs0); - assert(bs0); + while (blk && bs0 && bs0->drv && bs0->implicit) { + if (bs0->backing) { + bs0 =3D backing_bs(bs0); + } else { + assert(bs0->file); + bs0 =3D bs0->file->bs; + } } } =20 @@ -337,7 +341,12 @@ static void bdrv_query_info(BlockBackend *blk, BlockIn= fo **p_info, =20 /* Skip automatically inserted nodes that the user isn't aware of */ while (bs && bs->drv && bs->implicit) { - bs =3D backing_bs(bs); + if (bs->backing) { + bs =3D backing_bs(bs); + } else { + assert(bs->file); + bs =3D bs->file->bs; + } } =20 info->device =3D g_strdup(blk_name(blk)); @@ -466,8 +475,12 @@ static BlockStats *bdrv_query_bds_stats(BlockDriverSta= te *bs, * a BlockBackend-level command. Stay at the exact node for a node-lev= el * command. */ while (blk_level && bs->drv && bs->implicit) { - bs =3D backing_bs(bs); - assert(bs); + if (bs->backing) { + bs =3D backing_bs(bs); + } else { + assert(bs->file); + bs =3D bs->file->bs; + } } =20 if (bdrv_get_node_name(bs)[0]) { diff --git a/tests/qemu-iotests/141.out b/tests/qemu-iotests/141.out index 82e763b68d..8c4dd6d531 100644 --- a/tests/qemu-iotests/141.out +++ b/tests/qemu-iotests/141.out @@ -20,7 +20,7 @@ Formatting 'TEST_DIR/o.IMGFMT', fmt=3DIMGFMT size=3D10485= 76 backing_file=3DTEST_DIR/t. Formatting 'TEST_DIR/o.IMGFMT', fmt=3DIMGFMT size=3D1048576 backing_file= =3DTEST_DIR/t.IMGFMT backing_fmt=3DIMGFMT {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event"= : "BLOCK_JOB_READY", "data": {"device": "job0", "len": 0, "offset": 0, "spe= ed": 0, "type": "mirror"}} {"return": {}} -{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: node is = used as backing hd of 'NODE_NAME'"}} +{"error": {"class": "GenericError", "desc": "Block device drv0 is in use"}} {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event"= : "BLOCK_JOB_COMPLETED", "data": {"device": "job0", "len": 0, "offset": 0, = "speed": 0, "type": "mirror"}} {"return": {}} @@ -30,7 +30,7 @@ Formatting 'TEST_DIR/o.IMGFMT', fmt=3DIMGFMT size=3D10485= 76 backing_file=3DTEST_DIR/t. {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event"= : "BLOCK_JOB_READY", "data": {"device": "job0", "len": 0, "offset": 0, "spe= ed": 0, "type": "commit"}} {"return": {}} -{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: node is = used as backing hd of 'NODE_NAME'"}} +{"error": {"class": "GenericError", "desc": "Block device drv0 is in use"}} {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event"= : "BLOCK_JOB_COMPLETED", "data": {"device": "job0", "len": 0, "offset": 0, = "speed": 0, "type": "commit"}} {"return": {}} --=20 2.13.5