Pablo Greco e6a3ae
From 808d2c94f53b9b29f44c2c5d9baea66d63ceddfc Mon Sep 17 00:00:00 2001
Pablo Greco e6a3ae
From: Kevin Wolf <kwolf@redhat.com>
Pablo Greco e6a3ae
Date: Wed, 14 Aug 2019 09:20:43 +0100
Pablo Greco e6a3ae
Subject: [PATCH 07/10] block: Fix AioContext switch for bs->drv == NULL
Pablo Greco e6a3ae
Pablo Greco e6a3ae
RH-Author: Kevin Wolf <kwolf@redhat.com>
Pablo Greco e6a3ae
Message-id: <20190814092043.15421-2-kwolf@redhat.com>
Pablo Greco e6a3ae
Patchwork-id: 89976
Pablo Greco e6a3ae
O-Subject: [RHEL-8.1.0 qemu-kvm PATCH 1/1] block: Fix AioContext switch for bs->drv == NULL
Pablo Greco e6a3ae
Bugzilla: 1716347
Pablo Greco e6a3ae
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Pablo Greco e6a3ae
RH-Acked-by: Max Reitz <mreitz@redhat.com>
Pablo Greco e6a3ae
RH-Acked-by: John Snow <jsnow@redhat.com>
Pablo Greco e6a3ae
Pablo Greco e6a3ae
Even for block nodes with bs->drv == NULL, we can't just ignore a
Pablo Greco e6a3ae
bdrv_set_aio_context() call. Leaving the node in its old context can
Pablo Greco e6a3ae
mean that it's still in an iothread context in bdrv_close_all() during
Pablo Greco e6a3ae
shutdown, resulting in an attempted unlock of the AioContext lock which
Pablo Greco e6a3ae
we don't hold.
Pablo Greco e6a3ae
Pablo Greco e6a3ae
This is an example stack trace of a related crash:
Pablo Greco e6a3ae
Pablo Greco e6a3ae
 #0  0x00007ffff59da57f in raise () at /lib64/libc.so.6
Pablo Greco e6a3ae
 #1  0x00007ffff59c4895 in abort () at /lib64/libc.so.6
Pablo Greco e6a3ae
 #2  0x0000555555b97b1e in error_exit (err=<optimized out="">, msg=msg@entry=0x555555d386d0 <__func__.19059> "qemu_mutex_unlock_impl") at util/qemu-thread-posix.c:36
Pablo Greco e6a3ae
 #3  0x0000555555b97f7f in qemu_mutex_unlock_impl (mutex=mutex@entry=0x5555568002f0, file=file@entry=0x555555d378df "util/async.c", line=line@entry=507) at util/qemu-thread-posix.c:97
Pablo Greco e6a3ae
 #4  0x0000555555b92f55 in aio_context_release (ctx=ctx@entry=0x555556800290) at util/async.c:507
Pablo Greco e6a3ae
 #5  0x0000555555b05cf8 in bdrv_prwv_co (child=child@entry=0x7fffc80012f0, offset=offset@entry=131072, qiov=qiov@entry=0x7fffffffd4f0, is_write=is_write@entry=true, flags=flags@entry=0)
Pablo Greco e6a3ae
         at block/io.c:833
Pablo Greco e6a3ae
 #6  0x0000555555b060a9 in bdrv_pwritev (qiov=0x7fffffffd4f0, offset=131072, child=0x7fffc80012f0) at block/io.c:990
Pablo Greco e6a3ae
 #7  0x0000555555b060a9 in bdrv_pwrite (child=0x7fffc80012f0, offset=131072, buf=<optimized out="">, bytes=<optimized out="">) at block/io.c:990
Pablo Greco e6a3ae
 #8  0x0000555555ae172b in qcow2_cache_entry_flush (bs=bs@entry=0x555556810680, c=c@entry=0x5555568cc740, i=i@entry=0) at block/qcow2-cache.c:51
Pablo Greco e6a3ae
 #9  0x0000555555ae18dd in qcow2_cache_write (bs=bs@entry=0x555556810680, c=0x5555568cc740) at block/qcow2-cache.c:248
Pablo Greco e6a3ae
 #10 0x0000555555ae15de in qcow2_cache_flush (bs=0x555556810680, c=<optimized out="">) at block/qcow2-cache.c:259
Pablo Greco e6a3ae
 #11 0x0000555555ae16b1 in qcow2_cache_flush_dependency (c=0x5555568a1700, c=0x5555568a1700, bs=0x555556810680) at block/qcow2-cache.c:194
Pablo Greco e6a3ae
 #12 0x0000555555ae16b1 in qcow2_cache_entry_flush (bs=bs@entry=0x555556810680, c=c@entry=0x5555568a1700, i=i@entry=0) at block/qcow2-cache.c:194
Pablo Greco e6a3ae
 #13 0x0000555555ae18dd in qcow2_cache_write (bs=bs@entry=0x555556810680, c=0x5555568a1700) at block/qcow2-cache.c:248
Pablo Greco e6a3ae
 #14 0x0000555555ae15de in qcow2_cache_flush (bs=bs@entry=0x555556810680, c=<optimized out="">) at block/qcow2-cache.c:259
Pablo Greco e6a3ae
 #15 0x0000555555ad242c in qcow2_inactivate (bs=bs@entry=0x555556810680) at block/qcow2.c:2124
Pablo Greco e6a3ae
 #16 0x0000555555ad2590 in qcow2_close (bs=0x555556810680) at block/qcow2.c:2153
Pablo Greco e6a3ae
 #17 0x0000555555ab0c62 in bdrv_close (bs=0x555556810680) at block.c:3358
Pablo Greco e6a3ae
 #18 0x0000555555ab0c62 in bdrv_delete (bs=0x555556810680) at block.c:3542
Pablo Greco e6a3ae
 #19 0x0000555555ab0c62 in bdrv_unref (bs=0x555556810680) at block.c:4598
Pablo Greco e6a3ae
 #20 0x0000555555af4d72 in blk_remove_bs (blk=blk@entry=0x5555568103d0) at block/block-backend.c:785
Pablo Greco e6a3ae
 #21 0x0000555555af4dbb in blk_remove_all_bs () at block/block-backend.c:483
Pablo Greco e6a3ae
 #22 0x0000555555aae02f in bdrv_close_all () at block.c:3412
Pablo Greco e6a3ae
 #23 0x00005555557f9796 in main (argc=<optimized out="">, argv=<optimized out="">, envp=<optimized out="">) at vl.c:4776
Pablo Greco e6a3ae
Pablo Greco e6a3ae
The reproducer I used is a qcow2 image on gluster volume, where the
Pablo Greco e6a3ae
virtual disk size (4 GB) is larger than the gluster volume size (64M),
Pablo Greco e6a3ae
so we can easily trigger an ENOSPC. This backend is assigned to a
Pablo Greco e6a3ae
virtio-blk device using an iothread, and then from the guest a
Pablo Greco e6a3ae
'dd if=/dev/zero of=/dev/vda bs=1G count=1' causes the VM to stop
Pablo Greco e6a3ae
because of an I/O error. qemu_gluster_co_flush_to_disk() sets
Pablo Greco e6a3ae
bs->drv = NULL on error, so when virtio-blk stops the dataplane, the
Pablo Greco e6a3ae
block nodes stay in the iothread AioContext. A 'quit' monitor command
Pablo Greco e6a3ae
issued from this paused state crashes the process.
Pablo Greco e6a3ae
Pablo Greco e6a3ae
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1631227
Pablo Greco e6a3ae
Cc: qemu-stable@nongnu.org
Pablo Greco e6a3ae
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Pablo Greco e6a3ae
Reviewed-by: Eric Blake <eblake@redhat.com>
Pablo Greco e6a3ae
Reviewed-by: Max Reitz <mreitz@redhat.com>
Pablo Greco e6a3ae
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Pablo Greco e6a3ae
(cherry picked from commit 1bffe1ae7a7b707c3a14ea2ccd00d3609d3ce4d8)
Pablo Greco e6a3ae
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Pablo Greco e6a3ae
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
Pablo Greco e6a3ae
---
Pablo Greco e6a3ae
 block.c | 12 ++----------
Pablo Greco e6a3ae
 1 file changed, 2 insertions(+), 10 deletions(-)
Pablo Greco e6a3ae
Pablo Greco e6a3ae
diff --git a/block.c b/block.c
Pablo Greco e6a3ae
index 8f3ceea..37af100 100644
Pablo Greco e6a3ae
--- a/block.c
Pablo Greco e6a3ae
+++ b/block.c
Pablo Greco e6a3ae
@@ -4923,10 +4923,6 @@ void bdrv_detach_aio_context(BlockDriverState *bs)
Pablo Greco e6a3ae
     BdrvAioNotifier *baf, *baf_tmp;
Pablo Greco e6a3ae
     BdrvChild *child;
Pablo Greco e6a3ae
 
Pablo Greco e6a3ae
-    if (!bs->drv) {
Pablo Greco e6a3ae
-        return;
Pablo Greco e6a3ae
-    }
Pablo Greco e6a3ae
-
Pablo Greco e6a3ae
     assert(!bs->walking_aio_notifiers);
Pablo Greco e6a3ae
     bs->walking_aio_notifiers = true;
Pablo Greco e6a3ae
     QLIST_FOREACH_SAFE(baf, &bs->aio_notifiers, list, baf_tmp) {
Pablo Greco e6a3ae
@@ -4941,7 +4937,7 @@ void bdrv_detach_aio_context(BlockDriverState *bs)
Pablo Greco e6a3ae
      */
Pablo Greco e6a3ae
     bs->walking_aio_notifiers = false;
Pablo Greco e6a3ae
 
Pablo Greco e6a3ae
-    if (bs->drv->bdrv_detach_aio_context) {
Pablo Greco e6a3ae
+    if (bs->drv && bs->drv->bdrv_detach_aio_context) {
Pablo Greco e6a3ae
         bs->drv->bdrv_detach_aio_context(bs);
Pablo Greco e6a3ae
     }
Pablo Greco e6a3ae
     QLIST_FOREACH(child, &bs->children, next) {
Pablo Greco e6a3ae
@@ -4960,10 +4956,6 @@ void bdrv_attach_aio_context(BlockDriverState *bs,
Pablo Greco e6a3ae
     BdrvAioNotifier *ban, *ban_tmp;
Pablo Greco e6a3ae
     BdrvChild *child;
Pablo Greco e6a3ae
 
Pablo Greco e6a3ae
-    if (!bs->drv) {
Pablo Greco e6a3ae
-        return;
Pablo Greco e6a3ae
-    }
Pablo Greco e6a3ae
-
Pablo Greco e6a3ae
     if (bs->quiesce_counter) {
Pablo Greco e6a3ae
         aio_disable_external(new_context);
Pablo Greco e6a3ae
     }
Pablo Greco e6a3ae
@@ -4973,7 +4965,7 @@ void bdrv_attach_aio_context(BlockDriverState *bs,
Pablo Greco e6a3ae
     QLIST_FOREACH(child, &bs->children, next) {
Pablo Greco e6a3ae
         bdrv_attach_aio_context(child->bs, new_context);
Pablo Greco e6a3ae
     }
Pablo Greco e6a3ae
-    if (bs->drv->bdrv_attach_aio_context) {
Pablo Greco e6a3ae
+    if (bs->drv && bs->drv->bdrv_attach_aio_context) {
Pablo Greco e6a3ae
         bs->drv->bdrv_attach_aio_context(bs, new_context);
Pablo Greco e6a3ae
     }
Pablo Greco e6a3ae
 
Pablo Greco e6a3ae
-- 
Pablo Greco e6a3ae
1.8.3.1
Pablo Greco e6a3ae