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.szmigi...@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.szmigi...@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
