qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
To: qemu-block@nongnu.org
Cc: "Kevin Wolf" <kwolf@redhat.com>,
	"Hanna Reitz" <hreitz@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Ari Sundholm" <ari@tuxera.com>,
	"Pavel Dovgalyuk" <pavel.dovgaluk@ispras.ru>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"John Snow" <jsnow@redhat.com>,
	"Vladimir Sementsov-Ogievskiy" <vsementsov@yandex-team.ru>,
	"Stefan Weil" <sw@weilnetz.de>, "Fam Zheng" <fam@euphon.net>,
	"Ronnie Sahlberg" <ronniesahlberg@gmail.com>,
	"Peter Lieven" <pl@kamp.de>, "Eric Blake" <eblake@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	"Alberto Garcia" <berto@igalia.com>,
	"Ilya Dryomov" <idryomov@gmail.com>,
	"Wen Congyang" <wencongyang2@huawei.com>,
	"Xie Changlong" <xiechanglong.d@gmail.com>,
	"Richard W.M. Jones" <rjones@redhat.com>,
	"Jeff Cody" <codyprime@gmail.com>,
	"Cleber Rosa" <crosa@redhat.com>,
	qemu-devel@nongnu.org, integration@gluster.org,
	"Emanuele Giuseppe Esposito" <eesposit@redhat.com>
Subject: [PATCH 08/15] block: convert bdrv_is_inserted in generated_co_wrapper_simple
Date: Wed, 16 Nov 2022 09:07:23 -0500	[thread overview]
Message-ID: <20221116140730.3056048-9-eesposit@redhat.com> (raw)
In-Reply-To: <20221116140730.3056048-1-eesposit@redhat.com>

BlockDriver->bdrv_is_inserted is categorized as IO callback, and
it currently doesn't run in a coroutine.
This makes very difficult to add the graph rdlock, since the
callback traverses the block nodes graph.

Therefore use generated_co_wrapper_simple to automatically
creates a wrapper with the same name that will take care of
always calling the function in a coroutine.
This is a generated_co_wrapper_simple callback, which means
it assumes that bdrv_is_inserted is never caled in a coroutine
context.

At the same time, add also blk_is_inserted as g_c_w, since it
is called in both coroutine and non contexts.

Because now this function creates a new coroutine and polls,
we need to take the AioContext lock where it is missing,
for the only reason that internally g_c_w calls AIO_WAIT_WHILE
and it expects to release the AioContext lock.
Once the rwlock is ultimated and placed in every place it needs
to be, we will poll using AIO_WAIT_WHILE_UNLOCKED and remove
the AioContext lock.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 block.c                           |  5 +++--
 block/block-backend.c             |  5 +++--
 block/io.c                        | 12 ++++++------
 blockdev.c                        |  8 +++++++-
 include/block/block-io.h          |  5 ++++-
 include/block/block_int-common.h  |  4 ++--
 include/sysemu/block-backend-io.h |  5 ++++-
 7 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/block.c b/block.c
index 2cdede9c01..4205735308 100644
--- a/block.c
+++ b/block.c
@@ -6778,11 +6778,12 @@ out:
 /**
  * Return TRUE if the media is present
  */
-bool bdrv_is_inserted(BlockDriverState *bs)
+bool coroutine_fn bdrv_co_is_inserted(BlockDriverState *bs)
 {
     BlockDriver *drv = bs->drv;
     BdrvChild *child;
     IO_CODE();
+    assert_bdrv_graph_readable();
 
     if (!drv) {
         return false;
@@ -6791,7 +6792,7 @@ bool bdrv_is_inserted(BlockDriverState *bs)
         return drv->bdrv_is_inserted(bs);
     }
     QLIST_FOREACH(child, &bs->children, next) {
-        if (!bdrv_is_inserted(child->bs)) {
+        if (!bdrv_co_is_inserted(child->bs)) {
             return false;
         }
     }
diff --git a/block/block-backend.c b/block/block-backend.c
index 6a84a2a019..9a500fdde3 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1994,12 +1994,13 @@ void blk_activate(BlockBackend *blk, Error **errp)
     bdrv_activate(bs, errp);
 }
 
-bool blk_is_inserted(BlockBackend *blk)
+bool coroutine_fn blk_co_is_inserted(BlockBackend *blk)
 {
     BlockDriverState *bs = blk_bs(blk);
     IO_CODE();
+    assert_bdrv_graph_readable();
 
-    return bs && bdrv_is_inserted(bs);
+    return bs && bdrv_co_is_inserted(bs);
 }
 
 bool blk_is_available(BlockBackend *blk)
diff --git a/block/io.c b/block/io.c
index 99ef9a8cb9..88da9470c3 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1598,7 +1598,7 @@ int coroutine_fn bdrv_co_preadv_part(BdrvChild *child,
 
     trace_bdrv_co_preadv_part(bs, offset, bytes, flags);
 
-    if (!bdrv_is_inserted(bs)) {
+    if (!bdrv_co_is_inserted(bs)) {
         return -ENOMEDIUM;
     }
 
@@ -2044,7 +2044,7 @@ int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
 
     trace_bdrv_co_pwritev_part(child->bs, offset, bytes, flags);
 
-    if (!bdrv_is_inserted(bs)) {
+    if (!bdrv_co_is_inserted(bs)) {
         return -ENOMEDIUM;
     }
 
@@ -2764,7 +2764,7 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
     assert_bdrv_graph_readable();
     bdrv_inc_in_flight(bs);
 
-    if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs) ||
+    if (!bdrv_co_is_inserted(bs) || bdrv_is_read_only(bs) ||
         bdrv_is_sg(bs)) {
         goto early_exit;
     }
@@ -2889,7 +2889,7 @@ int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset,
     IO_CODE();
     assert_bdrv_graph_readable();
 
-    if (!bs || !bs->drv || !bdrv_is_inserted(bs)) {
+    if (!bs || !bs->drv || !bdrv_co_is_inserted(bs)) {
         return -ENOMEDIUM;
     }
 
@@ -3173,7 +3173,7 @@ static int coroutine_fn bdrv_co_copy_range_internal(
     assert(!(read_flags & BDRV_REQ_NO_WAIT));
     assert(!(write_flags & BDRV_REQ_NO_WAIT));
 
-    if (!dst || !dst->bs || !bdrv_is_inserted(dst->bs)) {
+    if (!dst || !dst->bs || !bdrv_co_is_inserted(dst->bs)) {
         return -ENOMEDIUM;
     }
     ret = bdrv_check_request32(dst_offset, bytes, NULL, 0);
@@ -3184,7 +3184,7 @@ static int coroutine_fn bdrv_co_copy_range_internal(
         return bdrv_co_pwrite_zeroes(dst, dst_offset, bytes, write_flags);
     }
 
-    if (!src || !src->bs || !bdrv_is_inserted(src->bs)) {
+    if (!src || !src->bs || !bdrv_co_is_inserted(src->bs)) {
         return -ENOMEDIUM;
     }
     ret = bdrv_check_request32(src_offset, bytes, NULL, 0);
diff --git a/blockdev.c b/blockdev.c
index 8ffb3d9537..bbe5207e8f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1023,6 +1023,7 @@ fail:
 static BlockDriverState *qmp_get_root_bs(const char *name, Error **errp)
 {
     BlockDriverState *bs;
+    AioContext *aio_context;
 
     bs = bdrv_lookup_bs(name, name, errp);
     if (bs == NULL) {
@@ -1034,11 +1035,16 @@ static BlockDriverState *qmp_get_root_bs(const char *name, Error **errp)
         return NULL;
     }
 
+    aio_context = bdrv_get_aio_context(bs);
+    aio_context_acquire(aio_context);
+
     if (!bdrv_is_inserted(bs)) {
         error_setg(errp, "Device has no medium");
-        return NULL;
+        bs = NULL;
     }
 
+    aio_context_release(aio_context);
+
     return bs;
 }
 
diff --git a/include/block/block-io.h b/include/block/block-io.h
index ba60ffa43b..3432e6ad3e 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -120,7 +120,10 @@ bool bdrv_is_read_only(BlockDriverState *bs);
 bool bdrv_is_writable(BlockDriverState *bs);
 bool bdrv_is_sg(BlockDriverState *bs);
 int bdrv_get_flags(BlockDriverState *bs);
-bool bdrv_is_inserted(BlockDriverState *bs);
+
+bool coroutine_fn bdrv_co_is_inserted(BlockDriverState *bs);
+bool generated_co_wrapper_simple bdrv_is_inserted(BlockDriverState *bs);
+
 void bdrv_lock_medium(BlockDriverState *bs, bool locked);
 void bdrv_eject(BlockDriverState *bs, bool eject_flag);
 const char *bdrv_get_format_name(BlockDriverState *bs);
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 5874531617..4cad48b2ad 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -759,8 +759,8 @@ struct BlockDriver {
                                           QEMUIOVector *qiov,
                                           int64_t pos);
 
-    /* removable device specific */
-    bool (*bdrv_is_inserted)(BlockDriverState *bs);
+    /* removable device specific. Called with graph rdlock held. */
+    bool coroutine_fn (*bdrv_is_inserted)(BlockDriverState *bs);
     void (*bdrv_eject)(BlockDriverState *bs, bool eject_flag);
     void (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
 
diff --git a/include/sysemu/block-backend-io.h b/include/sysemu/block-backend-io.h
index 5456822b07..bf88f7699e 100644
--- a/include/sysemu/block-backend-io.h
+++ b/include/sysemu/block-backend-io.h
@@ -53,7 +53,10 @@ BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
 
 void blk_inc_in_flight(BlockBackend *blk);
 void blk_dec_in_flight(BlockBackend *blk);
-bool blk_is_inserted(BlockBackend *blk);
+
+bool coroutine_fn blk_co_is_inserted(BlockBackend *blk);
+bool generated_co_wrapper blk_is_inserted(BlockBackend *blk);
+
 bool blk_is_available(BlockBackend *blk);
 void blk_lock_medium(BlockBackend *blk, bool locked);
 void blk_eject(BlockBackend *blk, bool eject_flag);
-- 
2.31.1



  parent reply	other threads:[~2022-11-16 14:14 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-16 14:07 [PATCH 00/15] Protect the block layer with a rwlock: part 3 Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 01/15] block/qed: add missing graph rdlock in qed_need_check_timer_entry Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 02/15] block: rename refresh_total_sectors in bdrv_refresh_total_sectors Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 03/15] block-backend: use bdrv_getlength instead of blk_getlength Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 04/15] block: convert bdrv_refresh_total_sectors in generated_co_wrapper Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 05/15] block: use bdrv_co_refresh_total_sectors when possible Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 06/15] block: convert bdrv_get_allocated_file_size in generated_co_wrapper_simple Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 07/15] block: convert bdrv_get_info in generated_co_wrapper Emanuele Giuseppe Esposito
2022-11-16 14:07 ` Emanuele Giuseppe Esposito [this message]
2022-11-16 14:07 ` [PATCH 09/15] block-coroutine-wrapper: support void functions Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 10/15] block: convert bdrv_eject in generated_co_wrapper_simple Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 11/15] block: convert bdrv_lock_medium " Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 12/15] block: convert bdrv_debug_event in generated_co_wrapper Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 13/15] block: convert bdrv_io_plug in generated_co_wrapper_simple Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 14/15] block: convert bdrv_io_unplug " Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 15/15] block: rename newly converted BlockDriver IO coroutine functions Emanuele Giuseppe Esposito
2022-11-18 10:57 ` [PATCH 00/15] Protect the block layer with a rwlock: part 3 Paolo Bonzini
2022-11-18 15:01   ` Emanuele Giuseppe Esposito
2022-11-18 15:13     ` Paolo Bonzini
2022-11-23 11:45   ` Emanuele Giuseppe Esposito
2022-11-23 13:45     ` Kevin Wolf
2022-11-23 17:04       ` Paolo Bonzini
2022-11-23 18:12         ` Kevin Wolf
2022-11-21 15:02 ` Emanuele Giuseppe Esposito

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221116140730.3056048-9-eesposit@redhat.com \
    --to=eesposit@redhat.com \
    --cc=ari@tuxera.com \
    --cc=berto@igalia.com \
    --cc=codyprime@gmail.com \
    --cc=crosa@redhat.com \
    --cc=eblake@redhat.com \
    --cc=fam@euphon.net \
    --cc=hreitz@redhat.com \
    --cc=idryomov@gmail.com \
    --cc=integration@gluster.org \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pavel.dovgaluk@ispras.ru \
    --cc=pbonzini@redhat.com \
    --cc=philmd@linaro.org \
    --cc=pl@kamp.de \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rjones@redhat.com \
    --cc=ronniesahlberg@gmail.com \
    --cc=stefanha@redhat.com \
    --cc=sw@weilnetz.de \
    --cc=vsementsov@yandex-team.ru \
    --cc=wencongyang2@huawei.com \
    --cc=xiechanglong.d@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).