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

[Paolo Bonzini]

On 17/03/2017 18:11, Paolo Bonzini wrote:
> 
> 
> 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:

Hmm, no.  I'll post the full fix on top of John Snow's patches.

Paolo

> 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
>