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. */
+
/**
* @load_setup
*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?
OTOH, I think resume_prepare() doesn't require BQL..
> +
> /**
> * @load_setup
> *
>
--
Peter Xu
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.
> 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,
Maciej
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
"Maciej S. Szmigiero" <mail@maciej.szmigiero.name> writes: > 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> Reviewed-by: Fabiano Rosas <farosas@suse.de>
© 2016 - 2026 Red Hat, Inc.