Re: [Qemu-devel] [PATCH 2.5 v5 0/11] dataplane snapshot fixes

[Stefan Hajnoczi]

On Wed, Nov 04, 2015 at 08:19:31PM +0300, Denis V. Lunev 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.
> 
> Anyway, I think that the construction like
>     assert(aio_context_is_locked(aio_context));
> should be widely used to ensure proper locking.
> 
> 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 (11):
>   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
>   migration: reorder processing in hmp_savevm
>   migration: implement bdrv_all_find_vmstate_bs and bdrv_unlock helpers
>   migration: add missed aio_context_acquire for state writing/reading
>   snapshot: create bdrv_all_create_snapshot helper
>   monitor: add missed aio_context_acquire into vm_completion call
> 
>  block/snapshot.c         | 141 +++++++++++++++++++++++++++++-
>  include/block/snapshot.h |  23 ++++-
>  migration/savevm.c       | 219 
> +++++++++++++++--------------------------------
>  monitor.c                |  11 ++-
>  4 files changed, 234 insertions(+), 160 deletions(-)

There is a lot going on here.  For the 2.5 release, I'd prefer to limit
changes to fixing bugs like in the last patch (which I've merged) and
refactoring to a minimum (because that's where there will be discussions
and it may take a while).

By the way, the reason these issues haven't been noticed is that savevm
isn't used much by libvirt users (backing files are used for snapshots
instead).  That means the savevm code is only called from live migration
code paths.

Dataplane temporarily disables itself during live migration so no
problems are hit in that case.

The HMP monitor is legacy and also not used by modern libvirt.

I think the affected use cases are restricted to savevm+dataplane and
HMP+dataplane.

Stefan

signature.asc
Description: PGP signature