On 10.11.21 13:46, Vladimir Sementsov-Ogievskiy wrote:
04.11.2021 13:38, Hanna Reitz wrote:
The children list is specific to BDS parents. We should not modify it
in the general children modification code, but let BDS parents deal with
it in their .attach() and .detach() methods.
This also has the advantage that a BdrvChild is removed from the
children list before its .bs pointer can become NULL. BDS parents
generally assume that their children's .bs pointer is never NULL, so
this is actually a bug fix.
Signed-off-by: Hanna Reitz <hre...@redhat.com>
---
block.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/block.c b/block.c
index 580cb77a70..243ae206b5 100644
--- a/block.c
+++ b/block.c
@@ -1387,6 +1387,8 @@ static void bdrv_child_cb_attach(BdrvChild *child)
{
BlockDriverState *bs = child->opaque;
+ QLIST_INSERT_HEAD(&bs->children, child, next);
+
if (child->role & BDRV_CHILD_COW) {
bdrv_backing_attach(child);
}
@@ -1403,6 +1405,8 @@ static void bdrv_child_cb_detach(BdrvChild *child)
}
bdrv_unapply_subtree_drain(child, bs);
+
+ QLIST_REMOVE(child, next);
}
static int bdrv_child_cb_update_filename(BdrvChild *c,
BlockDriverState *base,
@@ -2747,7 +2751,7 @@ static void bdrv_child_free(void *opaque)
static void bdrv_remove_empty_child(BdrvChild *child)
{
assert(!child->bs);
- QLIST_SAFE_REMOVE(child, next);
+ assert(!child->next.le_prev); /* not in children list */
bdrv_child_free(child);
}
@@ -2913,7 +2917,6 @@ static int
bdrv_attach_child_noperm(BlockDriverState *parent_bs,
return ret;
}
- QLIST_INSERT_HEAD(&parent_bs->children, *child, next);
The following comment become stale. We should remove it too. With
comment removed:
Ah, right, thanks!
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
