From nobody Wed Dec 17 05:36:48 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 1487961649061398.7011270313443; Fri, 24 Feb 2017 10:40:49 -0800 (PST) Received: from localhost ([::1]:39294 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1chKnP-0005f6-NM for importer@patchew.org; Fri, 24 Feb 2017 13:40:47 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50167) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1chKR1-0006hd-15 for qemu-devel@nongnu.org; Fri, 24 Feb 2017 13:17:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1chKQy-0002tv-Du for qemu-devel@nongnu.org; Fri, 24 Feb 2017 13:17:39 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39496) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1chKQt-0002pC-Tl; Fri, 24 Feb 2017 13:17:32 -0500 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (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 2BE2680F8E; Fri, 24 Feb 2017 18:17:32 +0000 (UTC) Received: from noname.redhat.com (ovpn-117-128.ams2.redhat.com [10.36.117.128]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1OIHDCM015460; Fri, 24 Feb 2017 13:17:31 -0500 From: Kevin Wolf To: qemu-block@nongnu.org Date: Fri, 24 Feb 2017 19:17:04 +0100 Message-Id: <1487960230-18054-14-git-send-email-kwolf@redhat.com> In-Reply-To: <1487960230-18054-1-git-send-email-kwolf@redhat.com> References: <1487960230-18054-1-git-send-email-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Fri, 24 Feb 2017 18:17:32 +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] [PULL 13/19] block: Attach bs->file only during .bdrv_open() 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, qemu-devel@nongnu.org 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" The way that attaching bs->file worked was a bit unusual in that it was the only child that would be attached to a node which is not opened yet. Because of this, the block layer couldn't know yet which permissions the driver would eventually need. This patch moves the point where bs->file is attached to the beginning of the individual .bdrv_open() implementations, so drivers already know what they are going to do with the child. This is also more consistent with how driver-specific children work. For a moment, bdrv_open() gets its own BdrvChild to perform image probing, but instead of directly assigning this BdrvChild to the BDS, it becomes a temporary one and the node name is passed as an option to the drivers, so that they can simply use bdrv_open_child() to create another reference for their own use. This duplicated child for (the not opened yet) bs is not the final state, a follow-up patch will change the image probing code to use a BlockBackend, which is completely independent of bs. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- block.c | 35 ++++++++++++++++++++++++----------- block/bochs.c | 6 ++++++ block/cloop.c | 6 ++++++ block/crypto.c | 6 ++++++ block/dmg.c | 6 ++++++ block/parallels.c | 6 ++++++ block/qcow.c | 6 ++++++ block/qcow2.c | 18 +++++++++++++++--- block/qed.c | 18 +++++++++++++++--- block/raw-format.c | 6 ++++++ block/replication.c | 6 ++++++ block/vdi.c | 6 ++++++ block/vhdx.c | 6 ++++++ block/vmdk.c | 6 ++++++ block/vpc.c | 6 ++++++ tests/qemu-iotests/051.out | 4 ++-- tests/qemu-iotests/051.pc.out | 4 ++-- 17 files changed, 130 insertions(+), 21 deletions(-) diff --git a/block.c b/block.c index d951b5d..40c4dee 100644 --- a/block.c +++ b/block.c @@ -1103,13 +1103,6 @@ static int bdrv_open_common(BlockDriverState *bs, Bd= rvChild *file, assert(!drv->bdrv_needs_filename || filename !=3D NULL); ret =3D drv->bdrv_file_open(bs, options, open_flags, &local_err); } else { - if (file =3D=3D NULL) { - error_setg(errp, "Can't use '%s' as a block driver for the " - "protocol level", drv->format_name); - ret =3D -EINVAL; - goto free_and_fail; - } - bs->file =3D file; ret =3D drv->bdrv_open(bs, options, open_flags, &local_err); } =20 @@ -1145,7 +1138,6 @@ static int bdrv_open_common(BlockDriverState *bs, Bdr= vChild *file, return 0; =20 free_and_fail: - bs->file =3D NULL; g_free(bs->opaque); bs->opaque =3D NULL; bs->drv =3D NULL; @@ -1368,7 +1360,18 @@ void bdrv_unref_child(BlockDriverState *parent, Bdrv= Child *child) } =20 if (child->bs->inherits_from =3D=3D parent) { - child->bs->inherits_from =3D NULL; + BdrvChild *c; + + /* Remove inherits_from only when the last reference between paren= t and + * child->bs goes away. */ + QLIST_FOREACH(c, &parent->children, next) { + if (c !=3D child && c->bs =3D=3D child->bs) { + break; + } + } + if (c =3D=3D NULL) { + child->bs->inherits_from =3D NULL; + } } =20 bdrv_root_unref_child(child); @@ -1789,13 +1792,20 @@ static BlockDriverState *bdrv_open_inherit(const ch= ar *filename, qdict_del(options, "backing"); } =20 - /* Open image file without format layer */ + /* Open image file without format layer. This BdrvChild is only used f= or + * probing, the block drivers will do their own bdrv_open_child() for = the + * same BDS, which is why we put the node name back into options. */ if ((flags & BDRV_O_PROTOCOL) =3D=3D 0) { + /* FIXME Shouldn't attach a child to a node that isn't opened yet.= */ file =3D bdrv_open_child(filename, options, "file", bs, &child_file, true, &local_err); if (local_err) { goto fail; } + if (file !=3D NULL) { + qdict_put(options, "file", + qstring_from_str(bdrv_get_node_name(file->bs))); + } } =20 /* Image format probing */ @@ -1835,7 +1845,7 @@ static BlockDriverState *bdrv_open_inherit(const char= *filename, goto fail; } =20 - if (file && (bs->file !=3D file)) { + if (file) { bdrv_unref_child(bs, file); file =3D NULL; } @@ -1901,6 +1911,9 @@ fail: if (file !=3D NULL) { bdrv_unref_child(bs, file); } + if (bs->file !=3D NULL) { + bdrv_unref_child(bs, bs->file); + } QDECREF(snapshot_options); QDECREF(bs->explicit_options); QDECREF(bs->options); diff --git a/block/bochs.c b/block/bochs.c index 8c9652e..7dd2ac4 100644 --- a/block/bochs.c +++ b/block/bochs.c @@ -104,6 +104,12 @@ static int bochs_open(BlockDriverState *bs, QDict *opt= ions, int flags, struct bochs_header bochs; int ret; =20 + bs->file =3D bdrv_open_child(NULL, options, "file", bs, &child_file, + false, errp); + if (!bs->file) { + return -EINVAL; + } + bs->read_only =3D true; /* no write support yet */ =20 ret =3D bdrv_pread(bs->file, 0, &bochs, sizeof(bochs)); diff --git a/block/cloop.c b/block/cloop.c index 7b75f7e..877c9b0 100644 --- a/block/cloop.c +++ b/block/cloop.c @@ -66,6 +66,12 @@ static int cloop_open(BlockDriverState *bs, QDict *optio= ns, int flags, uint32_t offsets_size, max_compressed_block_size =3D 1, i; int ret; =20 + bs->file =3D bdrv_open_child(NULL, options, "file", bs, &child_file, + false, errp); + if (!bs->file) { + return -EINVAL; + } + bs->read_only =3D true; =20 /* read header */ diff --git a/block/crypto.c b/block/crypto.c index e05e4dd..7cb2ff2 100644 --- a/block/crypto.c +++ b/block/crypto.c @@ -300,6 +300,12 @@ static int block_crypto_open_generic(QCryptoBlockForma= t format, QCryptoBlockOpenOptions *open_opts =3D NULL; unsigned int cflags =3D 0; =20 + bs->file =3D bdrv_open_child(NULL, options, "file", bs, &child_file, + false, errp); + if (!bs->file) { + return -EINVAL; + } + opts =3D qemu_opts_create(opts_spec, NULL, 0, &error_abort); qemu_opts_absorb_qdict(opts, options, &local_err); if (local_err) { diff --git a/block/dmg.c b/block/dmg.c index 58a3ae8..8e387cd 100644 --- a/block/dmg.c +++ b/block/dmg.c @@ -413,6 +413,12 @@ static int dmg_open(BlockDriverState *bs, QDict *optio= ns, int flags, int64_t offset; int ret; =20 + bs->file =3D bdrv_open_child(NULL, options, "file", bs, &child_file, + false, errp); + if (!bs->file) { + return -EINVAL; + } + block_module_load_one("dmg-bz2"); bs->read_only =3D true; =20 diff --git a/block/parallels.c b/block/parallels.c index ac94dfb..b2ec09f 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -581,6 +581,12 @@ static int parallels_open(BlockDriverState *bs, QDict = *options, int flags, Error *local_err =3D NULL; char *buf; =20 + bs->file =3D bdrv_open_child(NULL, options, "file", bs, &child_file, + false, errp); + if (!bs->file) { + return -EINVAL; + } + ret =3D bdrv_pread(bs->file, 0, &ph, sizeof(ph)); if (ret < 0) { goto fail; diff --git a/block/qcow.c b/block/qcow.c index 4534515..038b05a 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -106,6 +106,12 @@ static int qcow_open(BlockDriverState *bs, QDict *opti= ons, int flags, QCowHeader header; Error *local_err =3D NULL; =20 + bs->file =3D bdrv_open_child(NULL, options, "file", bs, &child_file, + false, errp); + if (!bs->file) { + return -EINVAL; + } + ret =3D bdrv_pread(bs->file, 0, &header, sizeof(header)); if (ret < 0) { goto fail; diff --git a/block/qcow2.c b/block/qcow2.c index 3e1172b..21e6142 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -814,8 +814,8 @@ static int qcow2_update_options(BlockDriverState *bs, Q= Dict *options, return ret; } =20 -static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, - Error **errp) +static int qcow2_do_open(BlockDriverState *bs, QDict *options, int flags, + Error **errp) { BDRVQcow2State *s =3D bs->opaque; unsigned int len, i; @@ -1205,6 +1205,18 @@ static int qcow2_open(BlockDriverState *bs, QDict *o= ptions, int flags, return ret; } =20 +static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, + Error **errp) +{ + bs->file =3D bdrv_open_child(NULL, options, "file", bs, &child_file, + false, errp); + if (!bs->file) { + return -EINVAL; + } + + return qcow2_do_open(bs, options, flags, errp); +} + static void qcow2_refresh_limits(BlockDriverState *bs, Error **errp) { BDRVQcow2State *s =3D bs->opaque; @@ -1785,7 +1797,7 @@ static void qcow2_invalidate_cache(BlockDriverState *= bs, Error **errp) options =3D qdict_clone_shallow(bs->options); =20 flags &=3D ~BDRV_O_INACTIVE; - ret =3D qcow2_open(bs, options, flags, &local_err); + ret =3D qcow2_do_open(bs, options, flags, &local_err); QDECREF(options); if (local_err) { error_propagate(errp, local_err); diff --git a/block/qed.c b/block/qed.c index 0b62c77..62a0a09 100644 --- a/block/qed.c +++ b/block/qed.c @@ -415,8 +415,8 @@ static void bdrv_qed_drain(BlockDriverState *bs) } } =20 -static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags, - Error **errp) +static int bdrv_qed_do_open(BlockDriverState *bs, QDict *options, int flag= s, + Error **errp) { BDRVQEDState *s =3D bs->opaque; QEDHeader le_header; @@ -550,6 +550,18 @@ out: return ret; } =20 +static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags, + Error **errp) +{ + bs->file =3D bdrv_open_child(NULL, options, "file", bs, &child_file, + false, errp); + if (!bs->file) { + return -EINVAL; + } + + return bdrv_qed_do_open(bs, options, flags, errp); +} + static void bdrv_qed_refresh_limits(BlockDriverState *bs, Error **errp) { BDRVQEDState *s =3D bs->opaque; @@ -1629,7 +1641,7 @@ static void bdrv_qed_invalidate_cache(BlockDriverStat= e *bs, Error **errp) bdrv_qed_close(bs); =20 memset(s, 0, sizeof(BDRVQEDState)); - ret =3D bdrv_qed_open(bs, NULL, bs->open_flags, &local_err); + ret =3D bdrv_qed_do_open(bs, NULL, bs->open_flags, &local_err); if (local_err) { error_propagate(errp, local_err); error_prepend(errp, "Could not reopen qed layer: "); diff --git a/block/raw-format.c b/block/raw-format.c index 0ddffbd..ce34d1b 100644 --- a/block/raw-format.c +++ b/block/raw-format.c @@ -384,6 +384,12 @@ static int raw_open(BlockDriverState *bs, QDict *optio= ns, int flags, BDRVRawState *s =3D bs->opaque; int ret; =20 + bs->file =3D bdrv_open_child(NULL, options, "file", bs, &child_file, + false, errp); + if (!bs->file) { + return -EINVAL; + } + bs->sg =3D bs->file->bs->sg; bs->supported_write_flags =3D BDRV_REQ_FUA & bs->file->bs->supported_write_flags; diff --git a/block/replication.c b/block/replication.c index 729dd12..eff85c7 100644 --- a/block/replication.c +++ b/block/replication.c @@ -86,6 +86,12 @@ static int replication_open(BlockDriverState *bs, QDict = *options, const char *mode; const char *top_id; =20 + bs->file =3D bdrv_open_child(NULL, options, "file", bs, &child_file, + false, errp); + if (!bs->file) { + return -EINVAL; + } + ret =3D -EINVAL; opts =3D qemu_opts_create(&replication_runtime_opts, NULL, 0, &error_a= bort); qemu_opts_absorb_qdict(opts, options, &local_err); diff --git a/block/vdi.c b/block/vdi.c index 0aeb940..18b4773 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -363,6 +363,12 @@ static int vdi_open(BlockDriverState *bs, QDict *optio= ns, int flags, int ret; Error *local_err =3D NULL; =20 + bs->file =3D bdrv_open_child(NULL, options, "file", bs, &child_file, + false, errp); + if (!bs->file) { + return -EINVAL; + } + logout("\n"); =20 ret =3D bdrv_read(bs->file, 0, (uint8_t *)&header, 1); diff --git a/block/vhdx.c b/block/vhdx.c index c67772e..9918ee9 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -898,6 +898,12 @@ static int vhdx_open(BlockDriverState *bs, QDict *opti= ons, int flags, uint64_t signature; Error *local_err =3D NULL; =20 + bs->file =3D bdrv_open_child(NULL, options, "file", bs, &child_file, + false, errp); + if (!bs->file) { + return -EINVAL; + } + s->bat =3D NULL; s->first_visible_write =3D true; =20 diff --git a/block/vmdk.c b/block/vmdk.c index 393c84d..9d68ec5 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -943,6 +943,12 @@ static int vmdk_open(BlockDriverState *bs, QDict *opti= ons, int flags, uint32_t magic; Error *local_err =3D NULL; =20 + bs->file =3D bdrv_open_child(NULL, options, "file", bs, &child_file, + false, errp); + if (!bs->file) { + return -EINVAL; + } + buf =3D vmdk_read_desc(bs->file, 0, errp); if (!buf) { return -EINVAL; diff --git a/block/vpc.c b/block/vpc.c index ed6353d..d0df2a1 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -220,6 +220,12 @@ static int vpc_open(BlockDriverState *bs, QDict *optio= ns, int flags, int disk_type =3D VHD_DYNAMIC; int ret; =20 + bs->file =3D bdrv_open_child(NULL, options, "file", bs, &child_file, + false, errp); + if (!bs->file) { + return -EINVAL; + } + opts =3D qemu_opts_create(&vpc_runtime_opts, NULL, 0, &error_abort); qemu_opts_absorb_qdict(opts, options, &local_err); if (local_err) { diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out index 42bf416..7524c62 100644 --- a/tests/qemu-iotests/051.out +++ b/tests/qemu-iotests/051.out @@ -225,7 +225,7 @@ Testing: -drive driver=3Dnbd QEMU_PROG: -drive driver=3Dnbd: NBD server address missing =20 Testing: -drive driver=3Draw -QEMU_PROG: -drive driver=3Draw: Can't use 'raw' as a block driver for the = protocol level +QEMU_PROG: -drive driver=3Draw: A block device must be specified for "file" =20 Testing: -drive file.driver=3Dfile QEMU_PROG: -drive file.driver=3Dfile: The 'file' block driver requires a f= ile name @@ -234,7 +234,7 @@ Testing: -drive file.driver=3Dnbd QEMU_PROG: -drive file.driver=3Dnbd: NBD server address missing =20 Testing: -drive file.driver=3Draw -QEMU_PROG: -drive file.driver=3Draw: Can't use 'raw' as a block driver for= the protocol level +QEMU_PROG: -drive file.driver=3Draw: A block device must be specified for = "file" =20 Testing: -drive foo=3Dbar QEMU_PROG: -drive foo=3Dbar: Must specify either driver or file diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out index f8047a2..e206ad6 100644 --- a/tests/qemu-iotests/051.pc.out +++ b/tests/qemu-iotests/051.pc.out @@ -323,7 +323,7 @@ Testing: -drive driver=3Dnbd QEMU_PROG: -drive driver=3Dnbd: NBD server address missing =20 Testing: -drive driver=3Draw -QEMU_PROG: -drive driver=3Draw: Can't use 'raw' as a block driver for the = protocol level +QEMU_PROG: -drive driver=3Draw: A block device must be specified for "file" =20 Testing: -drive file.driver=3Dfile QEMU_PROG: -drive file.driver=3Dfile: The 'file' block driver requires a f= ile name @@ -332,7 +332,7 @@ Testing: -drive file.driver=3Dnbd QEMU_PROG: -drive file.driver=3Dnbd: NBD server address missing =20 Testing: -drive file.driver=3Draw -QEMU_PROG: -drive file.driver=3Draw: Can't use 'raw' as a block driver for= the protocol level +QEMU_PROG: -drive file.driver=3Draw: A block device must be specified for = "file" =20 Testing: -drive foo=3Dbar QEMU_PROG: -drive foo=3Dbar: Must specify either driver or file --=20 1.8.3.1