Re: [PATCH] memory: Fix qemu crash on starting dirty log twice with stopped VM

[Peter Xu]

On Mon, Feb 07, 2022 at 10:08:44AM +0100, Paolo Bonzini wrote:
> >   void memory_global_dirty_log_start(unsigned int flags)
> >   {
> >       unsigned int old_flags = global_dirty_tracking;
> > -    if (vmstate_change) {
> > -        qemu_del_vm_change_state_handler(vmstate_change);
> > -        vmstate_change = NULL;
> > -    }
> > +    /*
> > +     * If there is postponed dirty log stop flags, do it, so that 
> > start/stop
> > +     * will always be paired.  A smarter thing to do is ignore start 
> > process if
> > +     * the same flag has got postponed on stop, but it shouldn't matter a 
> > lot
> > +     * in major user scenarios, so keep the code simple for now.
> > +     */
> > +    memory_global_dirty_log_stop_postponed_run();
> 
> I think this should be as easy as doing here:
> 
>     postponed_stop_flags &= ~flags;
>     memory_global_dirty_log_stop_postponed_run();
>     flags &= ~global_dirty_tracking_flags;
>     if (!flags) {
>         return;
>     }
> 
> plus adding an "if (postponed_stop_flags)" in
> memory_global_dirty_log_stop_postponed_run?

Yeah I can do.  Though the latter "if (!flags)" check will also start to allow
nesting of memory_global_dirty_log_start(), and it'll make this assert useless:

    assert(!(global_dirty_tracking & flags));

I'll probably drop it too, then.

Curious: do we have any real case of nesting calls of starting dirty log?  I
always thought there's none, but I could miss something.

I'll post a v2 soon.  Thanks for the quick review!

-- 
Peter Xu