On Wed, Nov 17, 2021 at 11:43:42AM +0100, Laszlo Ersek wrote:
> 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>
Thanks (again) for the review (and spot/analyze the issue)!
--
Peter Xu
