diff mbox series

[v3,2/2] block: move blk_exp_close_all() to qemu_cleanup()

Message ID 20210121170700.59734-3-slp@redhat.com
State New
Headers show
Series nbd/server: Quiesce coroutines on context switch | expand

Commit Message

Sergio Lopez Jan. 21, 2021, 5:07 p.m. UTC
Move blk_exp_close_all() from bdrv_close() to qemu_cleanup(), before
bdrv_drain_all_begin().

Export drivers may have coroutines yielding at some point in the block
layer, so we need to shut them down before draining the block layer,
as otherwise they may get stuck blk_wait_while_drained().

RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1900505
Signed-off-by: Sergio Lopez <slp@redhat.com>
---
 block.c            | 1 -
 softmmu/runstate.c | 9 +++++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

Comments

Eric Blake Jan. 21, 2021, 6:02 p.m. UTC | #1
On 1/21/21 11:07 AM, Sergio Lopez wrote:
> Move blk_exp_close_all() from bdrv_close() to qemu_cleanup(), before
> bdrv_drain_all_begin().
> 
> Export drivers may have coroutines yielding at some point in the block
> layer, so we need to shut them down before draining the block layer,
> as otherwise they may get stuck blk_wait_while_drained().

stuck in

> 
> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1900505
> Signed-off-by: Sergio Lopez <slp@redhat.com>
> ---
>  block.c            | 1 -
>  softmmu/runstate.c | 9 +++++++++
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 

> @@ -783,6 +784,14 @@ void qemu_cleanup(void)
>       */
>      migration_shutdown();
>  
> +    /*
> +     * Close the exports before draining the block layer. The export
> +     * drivers may have coroutines yielding on it, so we need to clean
> +     * them up before the drain, as otherwise they may be get stuck in

s/be //

> +     * blk_wait_while_drained().
> +     */
> +    blk_exp_close_all();
> +
>      /*
>       * We must cancel all block jobs while the block layer is drained,
>       * or cancelling will be affected by throttling and thus may block
> 

Reviewed-by: Eric Blake <eblake@redhat.com>
Kevin Wolf Feb. 1, 2021, 12:20 p.m. UTC | #2
Am 21.01.2021 um 18:07 hat Sergio Lopez geschrieben:
> Move blk_exp_close_all() from bdrv_close() to qemu_cleanup(), before
> bdrv_drain_all_begin().
> 
> Export drivers may have coroutines yielding at some point in the block
> layer, so we need to shut them down before draining the block layer,
> as otherwise they may get stuck blk_wait_while_drained().
> 
> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1900505
> Signed-off-by: Sergio Lopez <slp@redhat.com>

This patch loses the call in qemu-nbd and qemu-storage-daemon.

Kevin
Sergio Lopez Feb. 1, 2021, 12:35 p.m. UTC | #3
On Mon, Feb 01, 2021 at 01:20:30PM +0100, Kevin Wolf wrote:
> Am 21.01.2021 um 18:07 hat Sergio Lopez geschrieben:
> > Move blk_exp_close_all() from bdrv_close() to qemu_cleanup(), before
> > bdrv_drain_all_begin().
> > 
> > Export drivers may have coroutines yielding at some point in the block
> > layer, so we need to shut them down before draining the block layer,
> > as otherwise they may get stuck blk_wait_while_drained().
> > 
> > RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1900505
> > Signed-off-by: Sergio Lopez <slp@redhat.com>
> 
> This patch loses the call in qemu-nbd and qemu-storage-daemon.

You're right, I'll prepare a v4 right away.

Thanks,
Sergio.
diff mbox series

Patch

diff --git a/block.c b/block.c
index 3da99312db..9682c82fa8 100644
--- a/block.c
+++ b/block.c
@@ -4435,7 +4435,6 @@  static void bdrv_close(BlockDriverState *bs)
 void bdrv_close_all(void)
 {
     assert(job_next(NULL) == NULL);
-    blk_exp_close_all();
 
     /* Drop references from requests still in flight, such as canceled block
      * jobs whose AIO context has not been polled yet */
diff --git a/softmmu/runstate.c b/softmmu/runstate.c
index 6177693a30..ac4b2e2540 100644
--- a/softmmu/runstate.c
+++ b/softmmu/runstate.c
@@ -25,6 +25,7 @@ 
 #include "qemu/osdep.h"
 #include "audio/audio.h"
 #include "block/block.h"
+#include "block/export.h"
 #include "chardev/char.h"
 #include "crypto/cipher.h"
 #include "crypto/init.h"
@@ -783,6 +784,14 @@  void qemu_cleanup(void)
      */
     migration_shutdown();
 
+    /*
+     * Close the exports before draining the block layer. The export
+     * drivers may have coroutines yielding on it, so we need to clean
+     * them up before the drain, as otherwise they may be get stuck in
+     * blk_wait_while_drained().
+     */
+    blk_exp_close_all();
+
     /*
      * We must cancel all block jobs while the block layer is drained,
      * or cancelling will be affected by throttling and thus may block