From nobody Tue May 7 22:12:15 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org ARC-Seal: i=1; a=rsa-sha256; t=1557755591; cv=none; d=zoho.com; s=zohoarc; b=a//hQ2zJ6/44gJ0Ds4zGGT8Wg5ta4C4/lYDy4FSRLEL/EPzDZRQPsj521qgtvszFHH6iX741T8E7rIGruAxCHY/h4E3ZoDaVnTSXG3wFwJJeGnCGqbj0H1Yob7b3x5BsT3lX13GB7bCDUTHNgRzhLOHxaI9M+swRxMssMPn+iTw= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1557755591; h=Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:Message-ID:References:Sender:Subject:To:ARC-Authentication-Results; bh=10jZxo2pQTHlyRjmSoGjzlkHOLb5jm8EaFasG2y36X0=; b=MInrv6b2dtoZ52Eq8biHaBkDGBdAXGUYKvR9HgUXEW8bwkQ4DsGuQW6PrHG7H/VCsSK24yt4tZ4KHEd3ySoAJ2mesubOIloX1TYIDDmVQeeIqlecwkK5Vw0RZ0uhhVRaS8Y69NRLpdvhWcfvyNMmVheSyE07LpnWPoC/8W4kZRs= ARC-Authentication-Results: i=1; mx.zoho.com; dkim=fail; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1557755591807792.0706351797062; Mon, 13 May 2019 06:53:11 -0700 (PDT) Received: from localhost ([127.0.0.1]:57679 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hQBO5-0004jA-Ro for importer@patchew.org; Mon, 13 May 2019 09:53:05 -0400 Received: from eggs.gnu.org ([209.51.188.92]:59662) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hQBLu-0002eJ-NN for qemu-devel@nongnu.org; Mon, 13 May 2019 09:50:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hQBHs-0005e2-B5 for qemu-devel@nongnu.org; Mon, 13 May 2019 09:46:41 -0400 Received: from fanzine.igalia.com ([91.117.99.155]:36074) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hQBHs-0005cj-1o; Mon, 13 May 2019 09:46:40 -0400 Received: from mobile-access-bcee32-86.dhcp.inet.fi ([188.238.50.86] helo=perseus.local) by fanzine.igalia.com with esmtpsa (Cipher TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim) id 1hQBHo-0003gf-ER; Mon, 13 May 2019 15:46:36 +0200 Received: from berto by perseus.local with local (Exim 4.89) (envelope-from ) id 1hQBHb-0003Bw-8e; Mon, 13 May 2019 16:46:23 +0300 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=References:In-Reply-To:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From; bh=10jZxo2pQTHlyRjmSoGjzlkHOLb5jm8EaFasG2y36X0=; b=f+l+AsnFa70e60IZi128u+ezhCl1fLrmvkhoIQf/iWGGVrACCmAlNUfEt3Ii2alEvQ/U+WREaYL1MG9Mhzx7Y3glUyopX1F/bfXy2LKH/ncefYoH+eUh7XPPmD4RY9xJvde3SUeP/ehzH8cABSOUtGPOoKU8WVdtULQwIk7c1D2kO8NE6KnMkzzWw2leIMPSMPMSGa3XpVBEvi8EL4erWJ3IbNb8P6CT9Y8ek6pyDJXTcONl3WAyaDZd2XD/RfuUkRFX6N7NZR+VT8VNtpQRfwweMYjoftYWUH247x7+78kGPTB6exHBUp+oizfcFS6PFOYJqQLA39rxBAf7eONIZA==; From: Alberto Garcia To: qemu-devel@nongnu.org Date: Mon, 13 May 2019 16:46:17 +0300 Message-Id: <6d1d5feaa53aa1ab127adb73d605dc4503e3abd5.1557754872.git.berto@igalia.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: References: In-Reply-To: References: X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x (no timestamps) [generic] [fuzzy] X-Received-From: 91.117.99.155 Subject: [Qemu-devel] [PATCH v3 1/2] block: Use bdrv_unref_child() for all children in bdrv_close() 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 , Alberto Garcia , qemu-block@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" bdrv_unref_child() does the following things: - Updates the child->bs->inherits_from pointer. - Calls bdrv_detach_child() to remove the BdrvChild from bs->children. - Calls bdrv_unref() to unref the child BlockDriverState. When bdrv_unref_child() was introduced in commit 33a604075c it was not used in bdrv_close() because the drivers that had additional children (like quorum or blkverify) had already called bdrv_unref() on their children during their own close functions. This was changed later (in 0bd6e91a7e for quorum, in 3e586be0b2 for blkverify) so there's no reason not to use bdrv_unref_child() in bdrv_close() anymore. After this there's also no need to remove bs->backing and bs->file separately from the rest of the children, so bdrv_close() can be simplified. Now bdrv_close() unrefs all children (before this patch it was only bs->file and bs->backing). As a result, none of the callers of brvd_attach_child() should remove their reference to child_bs (because this function effectively steals that reference). This patch updates a couple of tests that where doing their own bdrv_unref(). Signed-off-by: Alberto Garcia --- block.c | 16 +++------------- tests/test-bdrv-drain.c | 6 ------ tests/test-bdrv-graph-mod.c | 1 - 3 files changed, 3 insertions(+), 20 deletions(-) diff --git a/block.c b/block.c index 5c2c6aa761..3c3bd0f8d2 100644 --- a/block.c +++ b/block.c @@ -3842,22 +3842,12 @@ static void bdrv_close(BlockDriverState *bs) bs->drv =3D NULL; } =20 - bdrv_set_backing_hd(bs, NULL, &error_abort); - - if (bs->file !=3D NULL) { - bdrv_unref_child(bs, bs->file); - bs->file =3D NULL; - } - QLIST_FOREACH_SAFE(child, &bs->children, next, next) { - /* TODO Remove bdrv_unref() from drivers' close function and use - * bdrv_unref_child() here */ - if (child->bs->inherits_from =3D=3D bs) { - child->bs->inherits_from =3D NULL; - } - bdrv_detach_child(child); + bdrv_unref_child(bs, child); } =20 + bs->backing =3D NULL; + bs->file =3D NULL; g_free(bs->opaque); bs->opaque =3D NULL; atomic_set(&bs->copy_on_read, 0); diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index eda90750eb..5534c2adf9 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -1436,12 +1436,6 @@ static void test_detach_indirect(bool by_parent_cb) bdrv_unref(parent_b); blk_unref(blk); =20 - /* XXX Once bdrv_close() unref's children instead of just detaching th= em, - * this won't be necessary any more. */ - bdrv_unref(a); - bdrv_unref(a); - bdrv_unref(c); - g_assert_cmpint(a->refcnt, =3D=3D, 1); g_assert_cmpint(b->refcnt, =3D=3D, 1); g_assert_cmpint(c->refcnt, =3D=3D, 1); diff --git a/tests/test-bdrv-graph-mod.c b/tests/test-bdrv-graph-mod.c index 283dc84869..747c0bf8fc 100644 --- a/tests/test-bdrv-graph-mod.c +++ b/tests/test-bdrv-graph-mod.c @@ -116,7 +116,6 @@ static void test_update_perm_tree(void) g_assert_nonnull(local_err); error_free(local_err); =20 - bdrv_unref(bs); blk_unref(root); } =20 --=20 2.11.0 From nobody Tue May 7 22:12:15 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org ARC-Seal: i=1; a=rsa-sha256; t=1557755730; cv=none; d=zoho.com; s=zohoarc; b=dedV/JmwN2aYok60/qOcr2JCHkkm21rCsI6yJq17fxWJj5enGI5Q61nwngmdRWuxlIMYUZgmMfVakzzF3xk8ybDuKsuvAj3DeN1a6aviU5LSjxRYGxZ9NMEQzE/hpV20IsuSQ3/z3FXFzwwNMqcLVtRuTL9t7wZ0Hg6NHBMLAaU= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1557755730; h=Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:Message-ID:References:Sender:Subject:To:ARC-Authentication-Results; bh=Bp2rDBX4jzQoNNjJIRpWbG7q9lv1oZ+IR+SGedfJbBo=; b=LYn4ZxsQVL3qsUVNJdBkIKlyi7ddl8rCuVRrnpNpB0tBaYLsMja6+L1mQNRNFligQGkDwC/A0HSeaJrxrvch0l/vJAKVmBGHW+LfMADEmNao1E3Gbbs/J7CqsziXZNlt+FIdcj7V2eejs6FYZohzQcbEj+X0IsngUuABId3Gnnk= ARC-Authentication-Results: i=1; mx.zoho.com; dkim=fail; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 155775573092179.05889987969749; Mon, 13 May 2019 06:55:30 -0700 (PDT) Received: from localhost ([127.0.0.1]:57727 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hQBQN-0006es-NW for importer@patchew.org; Mon, 13 May 2019 09:55:27 -0400 Received: from eggs.gnu.org ([209.51.188.92]:59668) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hQBLu-0002eK-UP for qemu-devel@nongnu.org; Mon, 13 May 2019 09:50:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hQBHr-0005dg-Ps for qemu-devel@nongnu.org; Mon, 13 May 2019 09:46:41 -0400 Received: from fanzine.igalia.com ([91.117.99.155]:36075) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hQBHr-0005cl-8A; Mon, 13 May 2019 09:46:39 -0400 Received: from mobile-access-bcee32-86.dhcp.inet.fi ([188.238.50.86] helo=perseus.local) by fanzine.igalia.com with esmtpsa (Cipher TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim) id 1hQBHo-0003gd-Dq; Mon, 13 May 2019 15:46:36 +0200 Received: from berto by perseus.local with local (Exim 4.89) (envelope-from ) id 1hQBHb-0003By-9i; Mon, 13 May 2019 16:46:23 +0300 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=References:In-Reply-To:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From; bh=Bp2rDBX4jzQoNNjJIRpWbG7q9lv1oZ+IR+SGedfJbBo=; b=ikv5K70b8TAi/pKku4NjQPD09aFc5LOUWJvL6zr6AOrPcz9FAAYGouVgYkmg1iH3AODi0hsinfyTnu+VGa9lIe0suRpFYBKZheDA1fmOoAiQlm+zp9uVOjBkj+womiQxqZSZRVDn1yEZCKvmcp+LzNfBWYsIz3pDnT6z8wPkvBVZuZ5Q4IVopPOZmbFdGkLMJQCSB9rj/nQCvYD9hBZqEt/arZBnugAj65Efhodn7K0fdPnPfbgVUPy5XJYSKSF/Lw0829lepzouga1vwmCvISVVzEH+5AOIey6MuPpc1ASvNIrCkL5wr3hpY98Jgm/mujV4Yw+vG8JNH/WezthrKQ==; From: Alberto Garcia To: qemu-devel@nongnu.org Date: Mon, 13 May 2019 16:46:18 +0300 Message-Id: <20dfb3d9ccec559cdd1a9690146abad5d204a186.1557754872.git.berto@igalia.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: References: In-Reply-To: References: X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x (no timestamps) [generic] [fuzzy] X-Received-From: 91.117.99.155 Subject: [Qemu-devel] [PATCH v3 2/2] block: Make bdrv_root_attach_child() unref child_bs on failure 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 , Alberto Garcia , qemu-block@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" A consequence of the previous patch is that bdrv_attach_child() transfers the reference to child_bs from the caller to parent_bs, which will drop it on bdrv_close() or when someone calls bdrv_unref_child(). But this only happens when bdrv_attach_child() succeeds. If it fails then the caller is responsible for dropping the reference to child_bs. This patch makes bdrv_attach_child() take the reference also when there is an error, freeing the caller for having to do it. A similar situation happens with bdrv_root_attach_child(), so the changes on this patch affect both functions. Signed-off-by: Alberto Garcia --- block.c | 25 +++++++++++++++++-------- block/block-backend.c | 3 +-- block/quorum.c | 1 - blockjob.c | 2 +- 4 files changed, 19 insertions(+), 12 deletions(-) diff --git a/block.c b/block.c index 3c3bd0f8d2..df727314ff 100644 --- a/block.c +++ b/block.c @@ -2208,6 +2208,13 @@ static void bdrv_replace_child(BdrvChild *child, Blo= ckDriverState *new_bs) } } =20 +/* + * This function steals the reference to child_bs from the caller. + * That reference is later dropped by bdrv_root_unref_child(). + * + * On failure NULL is returned, errp is set and the reference to + * child_bs is also dropped. + */ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs, const char *child_name, const BdrvChildRole *child_role, @@ -2220,6 +2227,7 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *c= hild_bs, ret =3D bdrv_check_update_perm(child_bs, NULL, perm, shared_perm, NULL= , errp); if (ret < 0) { bdrv_abort_perm_update(child_bs); + bdrv_unref(child_bs); return NULL; } =20 @@ -2239,6 +2247,14 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *= child_bs, return child; } =20 +/* + * This function transfers the reference to child_bs from the caller + * to parent_bs. That reference is later dropped by parent_bs on + * bdrv_close() or if someone calls bdrv_unref_child(). + * + * On failure NULL is returned, errp is set and the reference to + * child_bs is also dropped. + */ BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs, BlockDriverState *child_bs, const char *child_name, @@ -2366,12 +2382,9 @@ void bdrv_set_backing_hd(BlockDriverState *bs, Block= DriverState *backing_hd, /* If backing_hd was already part of bs's backing chain, and * inherits_from pointed recursively to bs then let's update it to * point directly to bs (else it will become NULL). */ - if (update_inherits_from) { + if (bs->backing && update_inherits_from) { backing_hd->inherits_from =3D bs; } - if (!bs->backing) { - bdrv_unref(backing_hd); - } =20 out: bdrv_refresh_limits(bs, NULL); @@ -2569,10 +2582,6 @@ BdrvChild *bdrv_open_child(const char *filename, } =20 c =3D bdrv_attach_child(parent, bs, bdref_key, child_role, errp); - if (!c) { - bdrv_unref(bs); - return NULL; - } =20 return c; } diff --git a/block/block-backend.c b/block/block-backend.c index f78e82a707..7a621597e7 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -383,7 +383,6 @@ BlockBackend *blk_new_open(const char *filename, const = char *reference, blk->root =3D bdrv_root_attach_child(bs, "root", &child_root, perm, BLK_PERM_ALL, blk, errp); if (!blk->root) { - bdrv_unref(bs); blk_unref(blk); return NULL; } @@ -791,12 +790,12 @@ void blk_remove_bs(BlockBackend *blk) int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp) { ThrottleGroupMember *tgm =3D &blk->public.throttle_group_member; + bdrv_ref(bs); blk->root =3D bdrv_root_attach_child(bs, "root", &child_root, blk->perm, blk->shared_perm, blk, e= rrp); if (blk->root =3D=3D NULL) { return -EPERM; } - bdrv_ref(bs); =20 notifier_list_notify(&blk->insert_bs_notifiers, blk); if (tgm->throttle_state) { diff --git a/block/quorum.c b/block/quorum.c index 352f729136..133ee18204 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -1019,7 +1019,6 @@ static void quorum_add_child(BlockDriverState *bs, Bl= ockDriverState *child_bs, child =3D bdrv_attach_child(bs, child_bs, indexstr, &child_format, err= p); if (child =3D=3D NULL) { s->next_child_index--; - bdrv_unref(child_bs); goto out; } s->children =3D g_renew(BdrvChild *, s->children, s->num_children + 1); diff --git a/blockjob.c b/blockjob.c index 730101d282..b68fa4b13e 100644 --- a/blockjob.c +++ b/blockjob.c @@ -208,6 +208,7 @@ int block_job_add_bdrv(BlockJob *job, const char *name,= BlockDriverState *bs, { BdrvChild *c; =20 + bdrv_ref(bs); c =3D bdrv_root_attach_child(bs, name, &child_job, perm, shared_perm, job, errp); if (c =3D=3D NULL) { @@ -215,7 +216,6 @@ int block_job_add_bdrv(BlockJob *job, const char *name,= BlockDriverState *bs, } =20 job->nodes =3D g_slist_prepend(job->nodes, c); - bdrv_ref(bs); bdrv_op_block_all(bs, job->blocker); =20 return 0; --=20 2.11.0