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

[Juan Quintela <quintela@redhat.com>]

"Denis V. Lunev" <den@openvz.org> wrote:

Hi

Kevin, Stefan

> 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.
>
> 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 <den@openvz.org>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Juan Quintela <quintela@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
>
> 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(-)

There are more changes on migration/* that on block/*, do you want to
take the changes yourselves or should I take them?

Should they go for 2.5 or 2.6?

Both options are ok with me.

Later, Juan.