Re: [Qemu-devel] [PATCH for 2.9 v8 0/10] dataplane snapshot fixes

[Greg Kurz]

On Tue, 17 Nov 2015 12:08:20 +0300
"Denis V. Lunev" <address@hidden> wrote:

> with test
>     while /bin/true ; do
>         virsh snapshot-create rhel7
>         sleep 10
>         virsh snapshot-delete rhel7 --current
>     done
> with enabled iothreads on a running VM leads to a lot of troubles: hangs,
> asserts, errors.
> 

In my case, when using a virtio-blk-dataplane device, calling savevm *always*
result in a QEMU hang.

With this series (plus the s/bs/bs_vm_state/ change in patch 11), savevm/loadvm
now works like a charm.

I saw that Juan does not like aio_context being used in migration code, but
in case this series gets applied anyway:

Tested-by: Greg Kurz <address@hidden>

> Anyway, I think that the construction like
>     assert(aio_context_is_locked(aio_context));
> should be widely used to ensure proper locking.
> 
> Changes from v8:
> - split patch 5 into 2 separate patches making changes better to understand 
> and
>   review
> 
> Changes from v7:
> - patch 5: fixes for bdrv_all_find_snapshot. Changed parameter name to
>   skip_read_only which better reflects the meaning of the value and
>   fixed checks inside to conform the note from Stefan
> 
> Changes from v6:
> - tricky part dropped from patch 7
> - patch 5 reworked to process snapshot list differently in info commands
>   and on savevm
> 
> Changes from v5:
> - dropped already merged patch 11
> - fixed spelling in patch 1
> - changed order of condition in loops in all patches. Thank you Stefan
> - dropped patch 9
> - aio_context is not acquired any more in bdrv_all_find_vmstate_bs by request
>   of Stefan
> - patch 10 is implemented in completely different way
> 
> Changes from v4:
> - only migration/savevm.c code and monitor is affected now. Generic block
>   layer stuff will be sent separately to speedup merging. The approach
>   in general was negotiated with Juan and Stefan.
> 
> Changes from v3:
> - more places found
> - new aio_poll concept, see patch 10
> 
> Changes from v2:
> - droppped patch 5 as already merged
> - changed locking scheme in patch 4 by suggestion of Juan
> 
> Changes from v1:
> - aio-context locking added
> - comment is rewritten
> 
> Signed-off-by: Denis V. Lunev <address@hidden>
> CC: Stefan Hajnoczi <address@hidden>
> CC: Juan Quintela <address@hidden>
> CC: Kevin Wolf <address@hidden>
> 
> Denis V. Lunev (10):
>   snapshot: create helper to test that block drivers supports snapshots
>   snapshot: return error code from bdrv_snapshot_delete_by_id_or_name
>   snapshot: create bdrv_all_delete_snapshot helper
>   snapshot: create bdrv_all_goto_snapshot helper
>   snapshot: create bdrv_all_find_snapshot helper
>   migration: drop find_vmstate_bs check in hmp_delvm
>   snapshot: create bdrv_all_create_snapshot helper
>   migration: reorder processing in hmp_savevm
>   migration: implement bdrv_all_find_vmstate_bs helper
>   migration: normalize locking in migration/savevm.c
> 
>  block/snapshot.c         | 135 ++++++++++++++++++++++++++++++-
>  include/block/snapshot.h |  25 +++++-
>  migration/savevm.c       | 207 
> +++++++++++++++--------------------------------
>  3 files changed, 217 insertions(+), 150 deletions(-)