Re: [PATCH 4/4] migration: Document the BQL behavior of load SaveVMHandlers

[Peter Xu <peterx@redhat.com>]

On Tue, Oct 29, 2024 at 09:46:01PM +0100, Maciej S. Szmigiero wrote:
> On 29.10.2024 21:35, Peter Xu wrote:
> > On Tue, Oct 29, 2024 at 03:58:16PM +0100, Maciej S. Szmigiero wrote:
> > > From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
> > > 
> > > Some of these SaveVMHandlers were missing the BQL behavior annotation,
> > > making people wonder what it exactly is.
> > > 
> > > Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
> > > ---
> > >   include/migration/register.h | 4 ++++
> > >   1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/include/migration/register.h b/include/migration/register.h
> > > index f60e797894e5..c411d84876ec 100644
> > > --- a/include/migration/register.h
> > > +++ b/include/migration/register.h
> > > @@ -210,6 +210,8 @@ typedef struct SaveVMHandlers {
> > >       void (*state_pending_exact)(void *opaque, uint64_t *must_precopy,
> > >                                   uint64_t *can_postcopy);
> > > +    /* This runs inside the BQL. */
> > > +
> > >       /**
> > >        * @load_state
> > >        *
> > > @@ -227,6 +229,8 @@ typedef struct SaveVMHandlers {
> > >        */
> > >       int (*load_state)(QEMUFile *f, void *opaque, int version_id);
> > > +    /* The following handlers run inside the BQL. */
> > 
> > If above also requires BQL, why not move this line upper?
> 
> The reason for this is that my main patch set also adds
> "load_state_buffer" handler, which runs without BQL.
> 
> That handler is ordered next after "load_state" and I tried
> to avoid further comment churn here.
> 
> But if you prefer to change these comments in the patch
> introducing "load_state_buffer" handler instead then it's
> fine.

Considering the current change is so small to start benefit readers.. I
think it's ok we do this in one shot after load_state_buffer() change.

> > OTOH, I think resume_prepare() doesn't require BQL..
> 
> Yes, it seems like resume_prepare() is only called outside BQL.
> Will update the patch.

Thanks,

-- 
Peter Xu