Re: [PATCH v6 26/33] block_int-common.h: assertions in the callers of BdrvChildClass function pointers

[Hanna Reitz]

On 21.01.22 18:05, Emanuele Giuseppe Esposito wrote:

Signed-off-by: Emanuele Giuseppe Esposito 
---
  block.c | 9 +
  1 file changed, 9 insertions(+)

diff --git a/block.c b/block.c
index 448fb9d76f..ca16d90627 100644
--- a/block.c
+++ b/block.c


[...]


@@ -2120,6 +2121,7 @@ bool bdrv_is_writable(BlockDriverState *bs)
  
  static char *bdrv_child_user_desc(BdrvChild *c)

  {
+assert(qemu_in_main_thread());
  return c->klass->get_parent_desc(c);
  }


Quick note: Whether we really want this depends on whether we find that 
`.get_parent_desc()` really should be a GS-only function.


Since I leave that up to you, though (and this patch interestingly (and 
correctly) doesn’t add an assertion to bdrv_get_parent_name(), even 
though that calls `.get_name()`, which the previous patch did classify 
as GS):


Reviewed-by: Hanna Reitz 

[PATCH v6 26/33] block_int-common.h: assertions in the callers of BdrvChildClass function pointers

[Emanuele Giuseppe Esposito]

Signed-off-by: Emanuele Giuseppe Esposito 
---
 block.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/block.c b/block.c
index 448fb9d76f..ca16d90627 100644
--- a/block.c
+++ b/block.c
@@ -1491,6 +1491,7 @@ const BdrvChildClass child_of_bds = {
 
 AioContext *bdrv_child_get_parent_aio_context(BdrvChild *c)
 {
+assert(qemu_in_main_thread());
 return c->klass->get_parent_aio_context(c);
 }
 
@@ -2120,6 +2121,7 @@ bool bdrv_is_writable(BlockDriverState *bs)
 
 static char *bdrv_child_user_desc(BdrvChild *c)
 {
+assert(qemu_in_main_thread());
 return c->klass->get_parent_desc(c);
 }
 
@@ -2829,6 +2831,7 @@ static void bdrv_replace_child_noperm(BdrvChild **childp,
 
 assert(!child->frozen);
 assert(old_bs != new_bs);
+assert(qemu_in_main_thread());
 
 if (old_bs && new_bs) {
 assert(bdrv_get_aio_context(old_bs) == bdrv_get_aio_context(new_bs));
@@ -2925,6 +2928,7 @@ static void bdrv_attach_child_common_abort(void *opaque)
 BdrvChild *child = *s->child;
 BlockDriverState *bs = child->bs;
 
+assert(qemu_in_main_thread());
 /*
  * Pass free_empty_child=false, because we still need the child
  * for the AioContext operations on the parent below; those
@@ -3293,6 +3297,7 @@ void bdrv_unref_child(BlockDriverState *parent, BdrvChild 
*child)
 static void bdrv_parent_cb_change_media(BlockDriverState *bs, bool load)
 {
 BdrvChild *c;
+assert(qemu_in_main_thread());
 QLIST_FOREACH(c, &bs->parents, next_parent) {
 if (c->klass->change_media) {
 c->klass->change_media(c, load);
@@ -3789,6 +3794,7 @@ static BlockDriverState *bdrv_open_inherit(const char 
*filename,
 
 assert(!child_class || !flags);
 assert(!child_class == !parent);
+assert(qemu_in_main_thread());
 
 if (reference) {
 bool options_non_empty = options ? qdict_size(options) : false;
@@ -4175,6 +4181,7 @@ static BlockReopenQueue 
*bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
  * important to avoid graph changes between the recursive queuing here and
  * bdrv_reopen_multiple(). */
 assert(bs->quiesce_counter > 0);
+assert(qemu_in_main_thread());
 
 if (bs_queue == NULL) {
 bs_queue = g_new0(BlockReopenQueue, 1);
@@ -7278,6 +7285,7 @@ void bdrv_set_aio_context_ignore(BlockDriverState *bs,
 BdrvChild *child, *parent;
 
 g_assert(qemu_get_current_aio_context() == qemu_get_aio_context());
+assert(qemu_in_main_thread());
 
 if (old_context == new_context) {
 return;
@@ -7350,6 +7358,7 @@ void bdrv_set_aio_context_ignore(BlockDriverState *bs,
 static bool bdrv_parent_can_set_aio_context(BdrvChild *c, AioContext *ctx,
 GSList **ignore, Error **errp)
 {
+assert(qemu_in_main_thread());
 if (g_slist_find(*ignore, c)) {
 return true;
 }
-- 
2.31.1