On 11/16/21 14:54, Peter Xu wrote: > When finalizing the dump-guest-memory with detached mode, we'll first set dump > status to either FAIL or COMPLETE before doing the cleanup, however right > after > the dump status change it's possible that another dump-guest-memory qmp > command > is sent so both the main thread and dump thread (which is during cleanup) > could > be accessing dump state in parallel. That's racy. > > Fix it by protecting the finalizing phase of dump-guest-memory using BQL as > well, as qmp_dump_guest_memory() (which is the QEMU main thread) requires BQL. > To do that, we expand the BQL from dump_cleanup() into dump_process(), so we > will take the BQL longer than before. The critical section must cover the > status switch from ACTIVE->{FAIL|COMPLETE} so as to make sure there's no race > any more. > > We can also just introduce a specific mutex to serialize the dump process, but > BQL should be enough for now so far, not to mention vm_start() in dump_cleanup > will need BQL anyway, so maybe easier to just use the same mutex. > > Reported-by: Laszlo Ersek <ler...@redhat.com> > Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com> > Signed-off-by: Peter Xu <pet...@redhat.com> > --- > v2: > - Fix parallel spelling [Marc-Andre] > - Add r-b for Marc-Andre > - Mention that qmp_dump_guest_memory() is with BQL held [Laszlo] > --- > dump/dump.c | 24 ++++++++++++++++++------ > 1 file changed, 18 insertions(+), 6 deletions(-)
Reviewed-by: Laszlo Ersek <ler...@redhat.com> Thank you! Laszlo