On Thu, May 18, 2017 at 10:18:46AM +0200, Kevin Wolf wrote: > Am 17.05.2017 um 19:09 hat Stefan Hajnoczi geschrieben: > > diff --git a/migration/savevm.c b/migration/savevm.c > > index 7f66d58..a70ba20 100644 > > --- a/migration/savevm.c > > +++ b/migration/savevm.c > > @@ -2153,6 +2153,14 @@ int save_vmstate(const char *name) > > goto the_end; > > } > > > > + /* The bdrv_all_create_snapshot() call that follows acquires the > > AioContext > > + * for itself. BDRV_POLL_WHILE() does not support nested locking > > because > > + * it only releases the lock once. Therefore synchronous I/O will > > deadlock > > + * unless we release the AioContext before bdrv_all_create_snapshot(). > > + */ > > + aio_context_release(aio_context); > > + aio_context = NULL; > > + > > ret = bdrv_all_create_snapshot(sn, bs, vm_state_size, &bs); > > if (ret < 0) { > > error_report("Error while creating snapshot on '%s'", > > @@ -2163,7 +2171,9 @@ int save_vmstate(const char *name) > > ret = 0; > > > > the_end: > > - aio_context_release(aio_context); > > + if (aio_context) { > > + aio_context_release(aio_context); > > + } > > if (saved_vm_running) { > > vm_start(); > > } > > It might actually even be true before this patch because the lock is > already only taken for some parts of the function, but don't we need to > call bdrv_drain_all_begin/end() around the whole function now? > > We're stopping the VM, so hopefully no device is continuing to process > requests, but can't we still have block jobs, NBD server requests etc.? > > And the same is probably true for qemu_loadvm_state().
Yes, they currently rely on bdrv_drain_all() but that's not enough. Thanks for the suggestion, will add a patch in v2. Stefan
signature.asc
Description: PGP signature