[PATCH] migration: check is_active for old-style vmstate

[PATCH] migration: check is_active for old-style vmstate

[Yanfei Xu]

old-style vmstate doesn't use vmsd and instead rely on SaveStateEntry
ops. Check is_active for old-style vmstate to determine whether they
should be skipped during migration.

Signed-off-by: Yanfei Xu <yanfei.xu@bytedance.com>
---
 migration/savevm.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 197c89e0e6..7eee83ffca 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1030,15 +1030,27 @@ static int vmstate_save(QEMUFile *f, SaveStateEntry *se, JSONWriter *vmdesc,
                         Error **errp)
 {
     int ret;
+    const bool has_old_style = se->ops && se->ops->save_state;
 
-    if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
+    if (!has_old_style && !se->vmsd) {
         return 0;
+    } else if (has_old_style && se->vmsd) {
+        error_report("%s: '%s' (section_id=%u): unexpected: both vmsd (%s) and"
+                     " old-style save_state are set", __func__, se->idstr,
+                     se->section_id, se->vmsd->name);
+        return -EINVAL;
     }
+
     if (se->vmsd && !vmstate_section_needed(se->vmsd, se->opaque)) {
         trace_savevm_section_skip(se->idstr, se->section_id);
         return 0;
     }
 
+    if (has_old_style && se->ops->is_active && !se->ops->is_active(se->opaque)) {
+        trace_savevm_section_skip(se->idstr, se->section_id);
+        return 0;
+    }
+
     trace_savevm_section_start(se->idstr, se->section_id);
     save_section_header(f, se, QEMU_VM_SECTION_FULL);
     if (vmdesc) {
-- 
2.20.1

Re: [PATCH] migration: check is_active for old-style vmstate

[Peter Xu]

On Wed, Mar 11, 2026 at 01:16:58PM +0800, Yanfei Xu wrote:
> old-style vmstate doesn't use vmsd and instead rely on SaveStateEntry
> ops. Check is_active for old-style vmstate to determine whether they
> should be skipped during migration.

Would you please dscribe the use case?

We only have two users in upstream tree that provided is_active(), and
neither of them provided save_state().  IOW, so far it works kind of mutual
exclusively v.s. save_state(), afaict.

> 
> Signed-off-by: Yanfei Xu <yanfei.xu@bytedance.com>
> ---
>  migration/savevm.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 197c89e0e6..7eee83ffca 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1030,15 +1030,27 @@ static int vmstate_save(QEMUFile *f, SaveStateEntry *se, JSONWriter *vmdesc,
>                          Error **errp)
>  {
>      int ret;
> +    const bool has_old_style = se->ops && se->ops->save_state;
>  
> -    if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
> +    if (!has_old_style && !se->vmsd) {
>          return 0;
> +    } else if (has_old_style && se->vmsd) {
> +        error_report("%s: '%s' (section_id=%u): unexpected: both vmsd (%s) and"
> +                     " old-style save_state are set", __func__, se->idstr,
> +                     se->section_id, se->vmsd->name);
> +        return -EINVAL;
>      }
> +
>      if (se->vmsd && !vmstate_section_needed(se->vmsd, se->opaque)) {
>          trace_savevm_section_skip(se->idstr, se->section_id);
>          return 0;
>      }
>  
> +    if (has_old_style && se->ops->is_active && !se->ops->is_active(se->opaque)) {
> +        trace_savevm_section_skip(se->idstr, se->section_id);
> +        return 0;
> +    }
> +
>      trace_savevm_section_start(se->idstr, se->section_id);
>      save_section_header(f, se, QEMU_VM_SECTION_FULL);
>      if (vmdesc) {
> -- 
> 2.20.1
> 

-- 
Peter Xu

Re: [PATCH] migration: check is_active for old-style vmstate

[Yanfei Xu]

> On Wed, Mar 11, 2026 at 01:16:58PM +0800, Yanfei Xu wrote:
>>/old-style vmstate doesn't use vmsd and instead rely on SaveStateEntry/
>>/ops. Check is_active for old-style vmstate to determine whether they/
>>/should be skipped during migration./
>
> Would you please dscribe the use case?
>
> We only have two users in upstream tree that provided is_active(), and
> neither of them provided save_state().  IOW, so far it works kind of mutual
> exclusively v.s. save_state(), afaict.

Sorry for the lack of background information.

We follow the vfio SaveVMHandlers and add an extra vfio SaveVMHandlers
variant with is_active. In our internal codebase, we set it inactive
during migration but active in a specific scenario.

You are right that upstream tree works well. I sent this patch because I
assumed the semantics of is_active is similiar to the "needed" field in
vmsd.

I'd like to get your opinion on this. Perhaps this case we used won't
occur in upstream?

Thanks,
Yanfei

Re: [PATCH] migration: check is_active for old-style vmstate

[Peter Xu]

On Thu, Mar 12, 2026 at 01:25:04PM +0800, Yanfei Xu wrote:
> > On Wed, Mar 11, 2026 at 01:16:58PM +0800, Yanfei Xu wrote:
> > > /old-style vmstate doesn't use vmsd and instead rely on SaveStateEntry/
> > > /ops. Check is_active for old-style vmstate to determine whether they/
> > > /should be skipped during migration./
> > 
> > Would you please dscribe the use case?
> > 
> > We only have two users in upstream tree that provided is_active(), and
> > neither of them provided save_state().  IOW, so far it works kind of mutual
> > exclusively v.s. save_state(), afaict.
> 
> Sorry for the lack of background information.
> 
> We follow the vfio SaveVMHandlers and add an extra vfio SaveVMHandlers
> variant with is_active. In our internal codebase, we set it inactive
> during migration but active in a specific scenario.
> 
> You are right that upstream tree works well. I sent this patch because I
> assumed the semantics of is_active is similiar to the "needed" field in
> vmsd.
> 
> I'd like to get your opinion on this. Perhaps this case we used won't
> occur in upstream?

Thanks for explaining.  That makes sense on its own, but I think you're
also correct since it won't occur in upstream then it means we're adding
dead code into upstream code that has zero use.

I suggest you stick your patch with your downstream, until you plan to
upstream the whole thing.

-- 
Peter Xu

Re: [PATCH] migration: check is_active for old-style vmstate

[Peter Maydell]

On Thu, 12 Mar 2026 at 15:37, Peter Xu <peterx@redhat.com> wrote:
>
> On Thu, Mar 12, 2026 at 01:25:04PM +0800, Yanfei Xu wrote:
> > Sorry for the lack of background information.
> >
> > We follow the vfio SaveVMHandlers and add an extra vfio SaveVMHandlers
> > variant with is_active. In our internal codebase, we set it inactive
> > during migration but active in a specific scenario.
> >
> > You are right that upstream tree works well. I sent this patch because I
> > assumed the semantics of is_active is similiar to the "needed" field in
> > vmsd.
> >
> > I'd like to get your opinion on this. Perhaps this case we used won't
> > occur in upstream?
>
> Thanks for explaining.  That makes sense on its own, but I think you're
> also correct since it won't occur in upstream then it means we're adding
> dead code into upstream code that has zero use.

Incidentally we're now down to very few users left of the legacy
SaveVMHandlers mechanism: less than half a dozen, I think.
(I imagine the main pain point for getting rid of the rest is
retaining migration compat.)

-- PMM