Re: [Qemu-devel] Assertion failure taking external snapshot with virtio drive + iothread

[Paolo Bonzini <pbonzini@redhat.com>]

On 17/03/2017 17:55, Ed Swierk wrote:
> I'm running into the same problem taking an external snapshot with a
> virtio-blk drive with iothread, so it's not specific to virtio-scsi.
> Run a Linux guest on qemu master
> 
>   qemu-system-x86_64 -nographic -enable-kvm -monitor
> telnet:0.0.0.0:1234,server,nowait -m 1024 -object
> iothread,id=iothread1 -drive file=/x/drive.qcow2,if=none,id=drive0
> -device virtio-blk-pci,iothread=iothread1,drive=drive0
> 
> Then in the monitor
> 
>   snapshot_blkdev drive0 /x/snap1.qcow2
> 
> qemu bombs with
> 
>   qemu-system-x86_64: /x/qemu/include/block/aio.h:457:
> aio_enable_external: Assertion `ctx->external_disable_cnt > 0' failed.
> 
> whereas without the iothread the assertion failure does not occur.

Please try this patch:

diff --git a/block/block-backend.c b/block/block-backend.c
index 5742c09..1d95879 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1876,6 +1876,7 @@ static void blk_root_drained_begin(BdrvChild *child)
     if (blk->public.io_limits_disabled++ == 0) {
         throttle_group_restart_blk(blk);
     }
+    aio_disable_external(bdrv_get_aio_context(bs));
 }
 
 static void blk_root_drained_end(BdrvChild *child)
@@ -1883,5 +1884,6 @@ static void blk_root_drained_end(BdrvChild *child)
     BlockBackend *blk = child->opaque;
 
     assert(blk->public.io_limits_disabled);
+    aio_enable_external(bdrv_get_aio_context(bs));
     --blk->public.io_limits_disabled;
 }
diff --git a/block/io.c b/block/io.c
index 2709a70..a6dcef5 100644
--- a/block/io.c
+++ b/block/io.c
@@ -224,7 +224,6 @@ void bdrv_drained_begin(BlockDriverState *bs)
     }
 
     if (!bs->quiesce_counter++) {
-        aio_disable_external(bdrv_get_aio_context(bs));
         bdrv_parent_drained_begin(bs);
     }
 
@@ -239,7 +238,6 @@ void bdrv_drained_end(BlockDriverState *bs)
     }
 
     bdrv_parent_drained_end(bs);
-    aio_enable_external(bdrv_get_aio_context(bs));
 }
 
 /*

This is not a proper fix, the right one would move the calls into
virtio-blk and virtio-scsi, but it might be a start.  I think the
issue is that you have one call to aio_disable_external for drive.qcow2
before the snapshot, and two calls to aio_enable_external (one for
drive.qcow2 and one for snap1.qcow2) after.

Paolo