Re: [PATCH 08/14] migration: Make qemu_savevm_query_pending() available anytime

[Juraj Marcin <jmarcin@redhat.com>]

Hi Peter,

On 2026-04-16 14:06, Peter Xu wrote:
> On Thu, Apr 09, 2026 at 07:15:28PM +0200, Juraj Marcin wrote:
> > On 2026-04-08 12:55, Peter Xu wrote:

...

> > > -static int qemu_savevm_state_setup(QEMUFile *f, Error **errp)
> > > +static int qemu_savevm_state_setup(MigrationState *s, QEMUFile *f,
> > > +                                   Error **errp)
> > >  {
> > >      SaveStateEntry *se;
> > >      int ret;
> > > @@ -1409,6 +1410,13 @@ static int qemu_savevm_state_setup(QEMUFile *f, Error **errp)
> > >          }
> > >      }
> > >  
> > > +    /*
> > > +     * Logically, it should be paired with any hook being used who needs to
> > > +     * load_acquire() the flag first.  So far, only save_query_pending()
> > > +     * uses it.
> > > +     */
> > > +    qatomic_store_release(&s->save_setup_ready, true);
> > 
> > What other savevm functions would benefit from this? Would it make sense
> > to include them in this patch/series?
> 
> I don't yet know any other hook would need this.
> 
> AFAIU, the rational is most of the hooks should only be invoked in
> migration_thread(), where it's always guaranteed that save_setup() is
> always done before hand.
> 
> Here, when we make save_query_pending() to be more flexible to be able to
> get invoked in QMP handlers, then it opens the races and only that hook is
> special.
> 
> We actually have another option, which is to keep another cached variable
> for global remaining data, then only update it in migration_thread().  Then
> it will keep the rational as before.
> 
> But then that var will only be used for this purpose, and I thought it
> might also be confusing: remember that we have per-module counter caches
> already for querying the remaining data fields, like what RAM and VFIO do.
> 
> So I chose the current approach.  Let me know what you think.

I agree with your approach, I was just wondering if there are other such
races that are not handled yet. I don't think anything needs to be
changed then, given that only save_query_pending() is susceptible to the
race.

Thanks!

> 
> Thanks,
> 
> -- 
> Peter Xu
>