[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 15/21] block: Make bdrv_root_attach_child() unref chi
From: |
Max Reitz |
Subject: |
[Qemu-devel] [PULL 15/21] block: Make bdrv_root_attach_child() unref child_bs on failure |
Date: |
Tue, 28 May 2019 21:28:41 +0200 |
From: Alberto Garcia <address@hidden>
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 <address@hidden>
Message-id: address@hidden
[mreitz: Removed now superfluous BdrvChild * variable in
bdrv_open_child()]
Signed-off-by: Max Reitz <address@hidden>
---
block.c | 30 ++++++++++++++++++------------
block/block-backend.c | 3 +--
block/quorum.c | 1 -
blockjob.c | 2 +-
4 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/block.c b/block.c
index be37280dc7..1a73e310c1 100644
--- a/block.c
+++ b/block.c
@@ -2243,6 +2243,13 @@ static void bdrv_replace_child(BdrvChild *child,
BlockDriverState *new_bs)
}
}
+/*
+ * 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,
@@ -2255,6 +2262,7 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState
*child_bs,
ret = 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;
}
@@ -2274,6 +2282,14 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState
*child_bs,
return child;
}
+/*
+ * 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,
@@ -2401,12 +2417,9 @@ void bdrv_set_backing_hd(BlockDriverState *bs,
BlockDriverState *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 = bs;
}
- if (!bs->backing) {
- bdrv_unref(backing_hd);
- }
out:
bdrv_refresh_limits(bs, NULL);
@@ -2594,7 +2607,6 @@ BdrvChild *bdrv_open_child(const char *filename,
const BdrvChildRole *child_role,
bool allow_none, Error **errp)
{
- BdrvChild *c;
BlockDriverState *bs;
bs = bdrv_open_child_bs(filename, options, bdref_key, parent, child_role,
@@ -2603,13 +2615,7 @@ BdrvChild *bdrv_open_child(const char *filename,
return NULL;
}
- c = bdrv_attach_child(parent, bs, bdref_key, child_role, errp);
- if (!c) {
- bdrv_unref(bs);
- return NULL;
- }
-
- return c;
+ return bdrv_attach_child(parent, bs, bdref_key, child_role, errp);
}
/* TODO Future callers may need to specify parent/child_role in order for
diff --git a/block/block-backend.c b/block/block-backend.c
index 4c0a8ef88d..ad3e1c882d 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -392,7 +392,6 @@ BlockBackend *blk_new_open(const char *filename, const char
*reference,
blk->root = 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;
}
@@ -800,12 +799,12 @@ void blk_remove_bs(BlockBackend *blk)
int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp)
{
ThrottleGroupMember *tgm = &blk->public.throttle_group_member;
+ bdrv_ref(bs);
blk->root = bdrv_root_attach_child(bs, "root", &child_root,
blk->perm, blk->shared_perm, blk, errp);
if (blk->root == NULL) {
return -EPERM;
}
- bdrv_ref(bs);
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,
BlockDriverState *child_bs,
child = bdrv_attach_child(bs, child_bs, indexstr, &child_format, errp);
if (child == NULL) {
s->next_child_index--;
- bdrv_unref(child_bs);
goto out;
}
s->children = g_renew(BdrvChild *, s->children, s->num_children + 1);
diff --git a/blockjob.c b/blockjob.c
index 9ca942ba01..cc5f18e7cd 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -204,6 +204,7 @@ int block_job_add_bdrv(BlockJob *job, const char *name,
BlockDriverState *bs,
{
BdrvChild *c;
+ bdrv_ref(bs);
c = bdrv_root_attach_child(bs, name, &child_job, perm, shared_perm,
job, errp);
if (c == NULL) {
@@ -211,7 +212,6 @@ int block_job_add_bdrv(BlockJob *job, const char *name,
BlockDriverState *bs,
}
job->nodes = g_slist_prepend(job->nodes, c);
- bdrv_ref(bs);
bdrv_op_block_all(bs, job->blocker);
return 0;
--
2.21.0
- [Qemu-devel] [PULL 04/21] qcow2-threads: qcow2_co_do_compress: protect queuing by mutex, (continued)
- [Qemu-devel] [PULL 04/21] qcow2-threads: qcow2_co_do_compress: protect queuing by mutex, Max Reitz, 2019/05/28
- [Qemu-devel] [PULL 03/21] qcow2-threads: use thread_pool_submit_co, Max Reitz, 2019/05/28
- [Qemu-devel] [PULL 05/21] qcow2-threads: split out generic path, Max Reitz, 2019/05/28
- [Qemu-devel] [PULL 02/21] qcow2: add separate file for threaded data processing functions, Max Reitz, 2019/05/28
- [Qemu-devel] [PULL 16/21] qemu-img: rebase: Reuse parent BlockDriverState, Max Reitz, 2019/05/28
- [Qemu-devel] [PULL 13/21] block/backup: refactor: split out backup_calculate_cluster_size, Max Reitz, 2019/05/28
- [Qemu-devel] [PULL 07/21] qcow2: bdrv_co_pwritev: move encryption code out of the lock, Max Reitz, 2019/05/28
- [Qemu-devel] [PULL 06/21] qcow2: qcow2_co_preadv: improve locking, Max Reitz, 2019/05/28
- [Qemu-devel] [PULL 14/21] block: Use bdrv_unref_child() for all children in bdrv_close(), Max Reitz, 2019/05/28
- [Qemu-devel] [PULL 09/21] block/backup: simplify backup_incremental_init_copy_bitmap, Max Reitz, 2019/05/28
- [Qemu-devel] [PULL 15/21] block: Make bdrv_root_attach_child() unref child_bs on failure,
Max Reitz <=
- [Qemu-devel] [PULL 08/21] qcow2: do encryption in threads, Max Reitz, 2019/05/28
- [Qemu-devel] [PULL 10/21] block/backup: move to copy_bitmap with granularity, Max Reitz, 2019/05/28
- [Qemu-devel] [PULL 21/21] blockdev: loosen restrictions on drive-backup source node, Max Reitz, 2019/05/28
- [Qemu-devel] [PULL 18/21] qemu-img: rebase: Reuse in-chain BlockDriverState, Max Reitz, 2019/05/28
- [Qemu-devel] [PULL 17/21] qemu-img: rebase: Reduce reads on in-chain rebase, Max Reitz, 2019/05/28
- [Qemu-devel] [PULL 11/21] block/backup: refactor and tolerate unallocated cluster skipping, Max Reitz, 2019/05/28
- [Qemu-devel] [PULL 12/21] block/backup: unify different modes code path, Max Reitz, 2019/05/28
- [Qemu-devel] [PULL 20/21] qcow2-bitmap: initialize bitmap directory alignment, Max Reitz, 2019/05/28
- [Qemu-devel] [PULL 19/21] qcow2: skip writing zero buffers to empty COW areas, Max Reitz, 2019/05/28
- Re: [Qemu-devel] [PULL 00/21] Block patches, Peter Maydell, 2019/05/30