Re: [PATCH v2] dump-guest-memory: Use BQL to protect dump finalize process

[Laszlo Ersek]

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 <lersek@redhat.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Peter Xu <peterx@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 <lersek@redhat.com>

Thank you!
Laszlo