Re: [PATCH v6 06/33] block/block-backend.c: assertions for block-backend

[Hanna Reitz]

On 21.01.22 18:05, Emanuele Giuseppe Esposito wrote:

All the global state (GS) API functions will check that
qemu_in_main_thread() returns true. If not, it means
that the safety of BQL cannot be guaranteed, and
they need to be moved to I/O.

Signed-off-by: Emanuele Giuseppe Esposito 
---
  block/block-backend.c  | 79 ++
  softmmu/qdev-monitor.c |  2 ++
  2 files changed, 81 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index 6f91dcc85d..6c80ae54cc 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c


[...]


@@ -1908,6 +1958,7 @@ void blk_lock_medium(BlockBackend *blk, bool locked)
  void blk_eject(BlockBackend *blk, bool eject_flag)
  {
  BlockDriverState *bs = blk_bs(blk);
+
  char *id;
  
  if (bs) {


Left over hunk from when when there was an assertion added here, should 
be dropped, I believe.


Hanna

[PATCH v6 06/33] block/block-backend.c: assertions for block-backend

[Emanuele Giuseppe Esposito]

All the global state (GS) API functions will check that
qemu_in_main_thread() returns true. If not, it means
that the safety of BQL cannot be guaranteed, and
they need to be moved to I/O.

Signed-off-by: Emanuele Giuseppe Esposito 
---
 block/block-backend.c  | 79 ++
 softmmu/qdev-monitor.c |  2 ++
 2 files changed, 81 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index 6f91dcc85d..6c80ae54cc 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -228,6 +228,7 @@ static void blk_root_activate(BdrvChild *child, Error 
**errp)
 
 void blk_set_force_allow_inactivate(BlockBackend *blk)
 {
+assert(qemu_in_main_thread());
 blk->force_allow_inactivate = true;
 }
 
@@ -346,6 +347,8 @@ BlockBackend *blk_new(AioContext *ctx, uint64_t perm, 
uint64_t shared_perm)
 {
 BlockBackend *blk;
 
+assert(qemu_in_main_thread());
+
 blk = g_new0(BlockBackend, 1);
 blk->refcnt = 1;
 blk->ctx = ctx;
@@ -383,6 +386,8 @@ BlockBackend *blk_new_with_bs(BlockDriverState *bs, 
uint64_t perm,
 {
 BlockBackend *blk = blk_new(bdrv_get_aio_context(bs), perm, shared_perm);
 
+assert(qemu_in_main_thread());
+
 if (blk_insert_bs(blk, bs, errp) < 0) {
 blk_unref(blk);
 return NULL;
@@ -411,6 +416,8 @@ BlockBackend *blk_new_open(const char *filename, const char 
*reference,
 uint64_t perm = 0;
 uint64_t shared = BLK_PERM_ALL;
 
+assert(qemu_in_main_thread());
+
 /*
  * blk_new_open() is mainly used in .bdrv_create implementations and the
  * tools where sharing isn't a major concern because the BDS stays private
@@ -488,6 +495,7 @@ static void drive_info_del(DriveInfo *dinfo)
 
 int blk_get_refcnt(BlockBackend *blk)
 {
+assert(qemu_in_main_thread());
 return blk ? blk->refcnt : 0;
 }
 
@@ -498,6 +506,7 @@ int blk_get_refcnt(BlockBackend *blk)
 void blk_ref(BlockBackend *blk)
 {
 assert(blk->refcnt > 0);
+assert(qemu_in_main_thread());
 blk->refcnt++;
 }
 
@@ -508,6 +517,7 @@ void blk_ref(BlockBackend *blk)
  */
 void blk_unref(BlockBackend *blk)
 {
+assert(qemu_in_main_thread());
 if (blk) {
 assert(blk->refcnt > 0);
 if (blk->refcnt > 1) {
@@ -528,6 +538,7 @@ void blk_unref(BlockBackend *blk)
  */
 BlockBackend *blk_all_next(BlockBackend *blk)
 {
+assert(qemu_in_main_thread());
 return blk ? QTAILQ_NEXT(blk, link)
: QTAILQ_FIRST(&block_backends);
 }
@@ -536,6 +547,8 @@ void blk_remove_all_bs(void)
 {
 BlockBackend *blk = NULL;
 
+assert(qemu_in_main_thread());
+
 while ((blk = blk_all_next(blk)) != NULL) {
 AioContext *ctx = blk_get_aio_context(blk);
 
@@ -559,6 +572,7 @@ void blk_remove_all_bs(void)
  */
 BlockBackend *blk_next(BlockBackend *blk)
 {
+assert(qemu_in_main_thread());
 return blk ? QTAILQ_NEXT(blk, monitor_link)
: QTAILQ_FIRST(&monitor_block_backends);
 }
@@ -625,6 +639,7 @@ static void bdrv_next_reset(BdrvNextIterator *it)
 
 BlockDriverState *bdrv_first(BdrvNextIterator *it)
 {
+assert(qemu_in_main_thread());
 bdrv_next_reset(it);
 return bdrv_next(it);
 }
@@ -662,6 +677,7 @@ bool monitor_add_blk(BlockBackend *blk, const char *name, 
Error **errp)
 {
 assert(!blk->name);
 assert(name && name[0]);
+assert(qemu_in_main_thread());
 
 if (!id_wellformed(name)) {
 error_setg(errp, "Invalid device name");
@@ -689,6 +705,8 @@ bool monitor_add_blk(BlockBackend *blk, const char *name, 
Error **errp)
  */
 void monitor_remove_blk(BlockBackend *blk)
 {
+assert(qemu_in_main_thread());
+
 if (!blk->name) {
 return;
 }
@@ -715,6 +733,7 @@ BlockBackend *blk_by_name(const char *name)
 {
 BlockBackend *blk = NULL;
 
+assert(qemu_in_main_thread());
 assert(name);
 while ((blk = blk_next(blk)) != NULL) {
 if (!strcmp(name, blk->name)) {
@@ -749,6 +768,7 @@ static BlockBackend *bdrv_first_blk(BlockDriverState *bs)
  */
 bool bdrv_has_blk(BlockDriverState *bs)
 {
+assert(qemu_in_main_thread());
 return bdrv_first_blk(bs) != NULL;
 }
 
@@ -759,6 +779,7 @@ bool bdrv_is_root_node(BlockDriverState *bs)
 {
 BdrvChild *c;
 
+assert(qemu_in_main_thread());
 QLIST_FOREACH(c, &bs->parents, next_parent) {
 if (c->klass != &child_root) {
 return false;
@@ -808,6 +829,7 @@ BlockBackend *blk_by_legacy_dinfo(DriveInfo *dinfo)
  */
 BlockBackendPublic *blk_get_public(BlockBackend *blk)
 {
+assert(qemu_in_main_thread());
 return &blk->public;
 }
 
@@ -816,6 +838,7 @@ BlockBackendPublic *blk_get_public(BlockBackend *blk)
  */
 BlockBackend *blk_by_public(BlockBackendPublic *public)
 {
+assert(qemu_in_main_thread());
 return container_of(public, BlockBackend, public);
 }
 
@@ -827,6 +850,8 @@ void blk_remove_bs(BlockBackend *blk)
 ThrottleGroupMember *tgm = &blk->public.throttle_group_member;
 BdrvChild *root;
 
+assert(qemu_in_main_thread());
+
 noti