Roman Kiryanov <[email protected]> writes: > The vmstate_save_state_v() function does not support > NULL in VMStateDescription::fields and will crash if > one is provided. > > This change prevents this situation from happening > by explicitly crashing earlier. > > Suggested-by: Fabiano Rosas <[email protected]> > Reviewed-by: Peter Xu <[email protected]> > Signed-off-by: Roman Kiryanov <[email protected]> > --- > v3: > - Also added assert to virtio.c to validate > VirtioDeviceClass instances which bypass > vmstate_register_with_alias_id. > - Updated the commit message to "Reviewed-by". > v2: > - Moved the assert from vmstate_save_state_v to the registration > path (vmstate_register_with_alias_id) and the qtest validation path > (vmstate_check) to catch missing fields earlier. > --- > hw/virtio/virtio.c | 6 ++++++ > migration/savevm.c | 5 +++++ > 2 files changed, 11 insertions(+) > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 0ba734d0bc..e4543de335 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -4054,6 +4054,12 @@ static void virtio_device_realize(DeviceState *dev, > Error **errp) > /* Devices should either use vmsd or the load/save methods */ > assert(!vdc->vmsd || !vdc->load); > > + /* > + * If a device has vmsd, it either MUST have valid fields > + * or marked unmigratable. > + */ > + assert(!vdc->vmsd || (vdc->vmsd->unmigratable || vdc->vmsd->fields)); > + > if (vdc->realize != NULL) { > vdc->realize(dev, &err); > if (err != NULL) { > diff --git a/migration/savevm.c b/migration/savevm.c > index 197c89e0e6..20c2b99231 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -861,6 +861,8 @@ static void vmstate_check(const VMStateDescription *vmsd) > const VMStateField *field = vmsd->fields; > const VMStateDescription * const *subsection = vmsd->subsections; > > + assert(field || vmsd->unmigratable); > + > if (field) { > while (field->name) { > if (field->flags & (VMS_STRUCT | VMS_VSTRUCT)) { > @@ -900,6 +902,9 @@ int vmstate_register_with_alias_id(VMStateIf *obj, > uint32_t instance_id, > /* If this triggers, alias support can be dropped for the vmsd. */ > assert(alias_id == -1 || required_for_version >= > vmsd->minimum_version_id); > > + /* If vmsd is migratable it MUST have valid fields. */ > + assert(vmsd->fields || vmsd->unmigratable); > + > se = g_new0(SaveStateEntry, 1); > se->version_id = vmsd->version_id; > se->section_id = savevm_state.global_section_id++;
Looks like we'll also need to fix some stubs that define empty vmsds: qemu-system-mips64: ../migration/savevm.c:864: void vmstate_check(const VMStateDescription *): Assertion `field || vmsd->unmigratable' failed. Maybe something like the following. Peter, what do you think? -- >8 -- >From 9bd63109e970ff231eb321e627f622910f9977ca Mon Sep 17 00:00:00 2001 From: Fabiano Rosas <[email protected]> Date: Mon, 16 Mar 2026 11:16:22 -0300 Subject: [PATCH] fixup! vmstate: validate VMStateDescription::fields upon registration --- hw/acpi/acpi-cpu-hotplug-stub.c | 4 +++- hw/acpi/acpi-mem-hotplug-stub.c | 4 +++- hw/acpi/acpi-pci-hotplug-stub.c | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/hw/acpi/acpi-cpu-hotplug-stub.c b/hw/acpi/acpi-cpu-hotplug-stub.c index 72c5f05f5c..4332f3fb7d 100644 --- a/hw/acpi/acpi-cpu-hotplug-stub.c +++ b/hw/acpi/acpi-cpu-hotplug-stub.c @@ -3,7 +3,9 @@ #include "hw/acpi/cpu.h" /* Following stubs are all related to ACPI cpu hotplug */ -const VMStateDescription vmstate_cpu_hotplug; +const VMStateDescription vmstate_cpu_hotplug = { + .fields = (const VMStateField[]) { VMSTATE_END_OF_LIST() }, +}; void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner, CPUHotplugState *state, hwaddr base_addr) diff --git a/hw/acpi/acpi-mem-hotplug-stub.c b/hw/acpi/acpi-mem-hotplug-stub.c index 7ad0fdcdf2..36c12a4e5f 100644 --- a/hw/acpi/acpi-mem-hotplug-stub.c +++ b/hw/acpi/acpi-mem-hotplug-stub.c @@ -2,7 +2,9 @@ #include "hw/acpi/memory_hotplug.h" #include "migration/vmstate.h" -const VMStateDescription vmstate_memory_hotplug; +const VMStateDescription vmstate_memory_hotplug = { + .fields = (const VMStateField[]) { VMSTATE_END_OF_LIST() }, +}; void acpi_memory_hotplug_init(MemoryRegion *as, Object *owner, MemHotplugState *state, hwaddr io_base) diff --git a/hw/acpi/acpi-pci-hotplug-stub.c b/hw/acpi/acpi-pci-hotplug-stub.c index d58ea726a8..3e3a484d3e 100644 --- a/hw/acpi/acpi-pci-hotplug-stub.c +++ b/hw/acpi/acpi-pci-hotplug-stub.c @@ -2,7 +2,9 @@ #include "hw/acpi/pcihp.h" #include "migration/vmstate.h" -const VMStateDescription vmstate_acpi_pcihp_pci_status; +const VMStateDescription vmstate_acpi_pcihp_pci_status = { + .fields = (const VMStateField[]) { VMSTATE_END_OF_LIST() }, +}; void acpi_pcihp_init(Object *owner, AcpiPciHpState *s, MemoryRegion *address_space_io, uint16_t io_base) -- 2.51.0
