Re: [PATCH v1] migration/savevm: Allow stub VMSDs

[Peter Xu <peterx@redhat.com>]

On Mon, Mar 16, 2026 at 04:47:59PM -0300, Fabiano Rosas wrote:
> After the referenced commit, empty VMStateDescription objects declared
> as part of stubs have started reaching the vmstate code.
> 
> A valid VMStateDescription must at minimum have a name and either the
> .fields or .unmigratable fields set. Stubs, being empty, have
> none. Code that assumes a non-NULL name field will now cause a
> crash. E.g.
> 
> $ ./build/mips/qemu-system-mipsel -nographic -drive if=none,format=qcow2,file=dummy.qcow2
>     [Type "C-a c" to get the "(qemu)" monitor prompt)]
>     (qemu) savevm foo
> 
>     Backtrace from doing this under gdb:
> 
>       #0  0x0000555555df7d4d in vmsd_can_compress (field=0x5555564f78a0
>       <__compound_literal.3>) at ../../migration/vmstate.c:339
>       #1  0x0000555555df7dbb in vmsd_desc_field_start
>           (vmsd=0x555556431ba0 <vmstate_cpuhp_state>, vmdesc=0x555556918690,
>       field=0x5555564f78a0 <__compound_literal.3>, i=0, max=1) at
>       ../../migration/vmstate.c:362
>       #2  0x0000555555df85a7 in vmstate_save_state_v
>           (f=0x555556b5a0c0, vmsd=0x555556431ba0 <vmstate_cpuhp_state>,
>       opaque=0x555556c9aac0, vmdesc=0x555556918690, version_id=1,
>       errp=0x7fffffffc948) at ../../migration/vmstate.c:528
>       #3  0x0000555555df8032 in vmstate_save_state
>           (f=0x555556b5a0c0, vmsd=0x555556431ba0 <vmstate_cpuhp_state>,
>       opaque=0x555556c9aac0, vmdesc_id=0x555556918690, errp=0x7fffffffc948)
>       at ../../migration/vmstate.c:427
>       #4  0x0000555555df8f83 in vmstate_subsection_save
>           (f=0x555556b5a0c0, vmsd=0x555556431c40 <vmstate_acpi>,
>       opaque=0x555556c9aac0, vmdesc=0x555556918690, errp=0x7fffffffc948)
>           at ../../migration/vmstate.c:695
> 
> Due to their very nature, it's better to allow stubs to be
> completely empty instead of forcing any rules. Teach the code to skip
> them.
> 
> Fixes: 7aa563630b ("pc: Start with modern CPU hotplug interface by default")
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  migration/savevm.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index dd58f2a705..e8d3360877 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -861,6 +861,10 @@ static void vmstate_check(const VMStateDescription *vmsd)
>      const VMStateField *field = vmsd->fields;
>      const VMStateDescription * const *subsection = vmsd->subsections;
>  
> +    if (!vmsd->name) {
> +        return;
> +    }

IIUC this is fine but isn't needed for stubs' use case, because "field" is
checked right below..

> +
>      if (field) {

.. here.

>          while (field->name) {
>              if (field->flags & (VMS_STRUCT | VMS_VSTRUCT)) {
> @@ -897,6 +901,11 @@ int vmstate_register_with_alias_id(VMStateIf *obj, uint32_t instance_id,
>  {
>      SaveStateEntry *se;
>  
> +    if (!vmsd->name) {
> +        /* assume it's a stub and ignore */
> +        return 0;
> +    }

Two questions on this one..

(1) when someone adds some new VMSD to device and forgot to attach a name,
    it'll silently ignore that new VMSD.. of course anyone testing this
    will find something strange, but silently ignoring an VMSD when not
    having name does sound like unexpected..

(2) would this fix the above crash?  The problem is the commit got fixed
    used VMSTATE_CPU_HOTPLUG which is an internal vmsd, this one only
    checks the top VMSD (which should have a name here..).  So I'm not sure
    if it'll work..

Is that MIPS board the only one that is broken?  Do we want to make sure
nothing is missed? We can definitely decide to break the migration, but we
can also choose to just revert the needed() back to normal.  It just that
it looks like we don't strongly need to break it.. as I don't see what we
get from breaking it yet.. (say, it's not like we can get rid of some weird
API or legacy function, we need to support needed() anyway..)

> +
>      /* If this triggers, alias support can be dropped for the vmsd. */
>      assert(alias_id == -1 || required_for_version >= vmsd->minimum_version_id);
>  
> -- 
> 2.51.0
> 

-- 
Peter Xu