block: Convert bdrv_refresh_total_sectors() to co_wrapper_mixed
BlockDriver->bdrv_getlength is categorized as IO callback, and it
currently doesn't run in a coroutine. We should let it take a graph
rdlock since the callback traverses the block nodes graph, which however
is only possible in a coroutine.
Therefore turn it into a co_wrapper to move the actual function into a
coroutine where the lock can be taken.
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 co_wrapper calls AIO_WAIT_WHILE and it expects to release the
AioContext lock.
This is especially messy when a co_wrapper creates a coroutine and polls
in bdrv_open_driver, because this function has so many callers in so
many context that it can easily lead to deadlocks. Therefore the new
rule for bdrv_open_driver is that the caller must always hold the
AioContext lock of the given bs (except if it is a coroutine), because
the function calls bdrv_refresh_total_sectors() which is now a
co_wrapper.
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>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230113204212.359076-7-kwolf@redhat.com>
Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
diff --git a/block.c b/block.c
index b6c6b17..ee6f909 100644
--- a/block.c
+++ b/block.c
@@ -1035,7 +1035,8 @@
* Set the current 'total_sectors' value
* Return 0 on success, -errno on error.
*/
-int bdrv_refresh_total_sectors(BlockDriverState *bs, int64_t hint)
+int coroutine_fn bdrv_co_refresh_total_sectors(BlockDriverState *bs,
+ int64_t hint)
{
BlockDriver *drv = bs->drv;
IO_CODE();
@@ -1044,13 +1045,13 @@
return -ENOMEDIUM;
}
- /* Do not attempt drv->bdrv_getlength() on scsi-generic devices */
+ /* Do not attempt drv->bdrv_co_getlength() on scsi-generic devices */
if (bdrv_is_sg(bs))
return 0;
/* query actual device if possible, otherwise just trust the hint */
- if (drv->bdrv_getlength) {
- int64_t length = drv->bdrv_getlength(bs);
+ if (drv->bdrv_co_getlength) {
+ int64_t length = drv->bdrv_co_getlength(bs);
if (length < 0) {
return length;
}
@@ -1601,6 +1602,11 @@
g_free(gen_node_name);
}
+/*
+ * The caller must always hold @bs AioContext lock, because this function calls
+ * bdrv_refresh_total_sectors() which polls when called from non-coroutine
+ * context.
+ */
static int bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv,
const char *node_name, QDict *options,
int open_flags, Error **errp)
@@ -3796,6 +3802,10 @@
* The reference parameter may be used to specify an existing block device which
* should be opened. If specified, neither options nor a filename may be given,
* nor can an existing BDS be reused (that is, *pbs has to be NULL).
+ *
+ * The caller must always hold @filename AioContext lock, because this
+ * function eventually calls bdrv_refresh_total_sectors() which polls
+ * when called from non-coroutine context.
*/
static BlockDriverState *bdrv_open_inherit(const char *filename,
const char *reference,
@@ -4084,6 +4094,11 @@
return NULL;
}
+/*
+ * The caller must always hold @filename AioContext lock, because this
+ * function eventually calls bdrv_refresh_total_sectors() which polls
+ * when called from non-coroutine context.
+ */
BlockDriverState *bdrv_open(const char *filename, const char *reference,
QDict *options, int flags, Error **errp)
{
@@ -5800,7 +5815,7 @@
/**
* Return number of sectors on success, -errno on error.
*/
-int64_t bdrv_nb_sectors(BlockDriverState *bs)
+int64_t coroutine_fn bdrv_co_nb_sectors(BlockDriverState *bs)
{
BlockDriver *drv = bs->drv;
IO_CODE();
@@ -5809,7 +5824,7 @@
return -ENOMEDIUM;
if (drv->has_variable_length) {
- int ret = bdrv_refresh_total_sectors(bs, bs->total_sectors);
+ int ret = bdrv_co_refresh_total_sectors(bs, bs->total_sectors);
if (ret < 0) {
return ret;
}
@@ -5821,11 +5836,12 @@
* Return length in bytes on success, -errno on error.
* The length is always a multiple of BDRV_SECTOR_SIZE.
*/
-int64_t bdrv_getlength(BlockDriverState *bs)
+int64_t coroutine_fn bdrv_co_getlength(BlockDriverState *bs)
{
- int64_t ret = bdrv_nb_sectors(bs);
+ int64_t ret;
IO_CODE();
+ ret = bdrv_co_nb_sectors(bs);
if (ret < 0) {
return ret;
}