On Fri, 29 Jul 2022 at 10:57, Igor Mammedov <imamm...@redhat.com> wrote: > > On Thu, 28 Jul 2022 16:12:34 +0100 > Peter Maydell <peter.mayd...@linaro.org> wrote: > > > On Thu, 28 Jul 2022 at 16:09, Dr. David Alan Gilbert > > <dgilb...@redhat.com> wrote: > > > > > > * Igor Mammedov (imamm...@redhat.com) wrote: > > > > On Thu, 28 Jul 2022 15:44:20 +0100 > > > > "Dr. David Alan Gilbert" <dgilb...@redhat.com> wrote: > > > > > > > > > * Igor Mammedov (imamm...@redhat.com) wrote: > > > > > > QEMU crashes trying to save VMSTATE when only MIPS target are > > > > > > compiled in > > > > > > $ qemu-system-mips -monitor stdio > > > > > > (qemu) migrate "exec:gzip -c > STATEFILE.gz" > > > > > > Segmentation fault (core dumped) > > > > > > > > > > > > It happens due to PIIX4_PM trying to parse hotplug vmstate > > > > > > structures > > > > > > which are valid only for x86 and not for MIPS (as it requires ACPI > > > > > > tables support which is not existent for ithe later) > > > > > > > > > > > > Issue was probably exposed by trying to cleanup/compile out unused > > > > > > ACPI bits from MIPS target (but forgetting about migration bits). > > > > > > > > > > > > Disable compiled out features using compat properties as the least > > > > > > risky way to deal with issue. > > > > > > > > > > Isn't the problem partially due to a 'stub' vmsd which isn't > > > > > terminated? > > > > > > > > Not sure what "'stub' vmsd" is, can you explain? > > > > > > In hw/acpi/acpi-pci-hotplug-stub.c there is : > > > const VMStateDescription vmstate_acpi_pcihp_pci_status; > I think that one is there only for linking purposes and not meant > to be actually used.
Yes, exactly. The problem is that without this patch which sets various properties it *does* get used... > > > this seg happens when the migration code walks into that - this should > > > always get populated with some of the minimal fields, in particular the > > > .name and .fields array terminated with VMSTATE_END_OF_LIST(). > > > > Either: > > (1) we should be sure the vmstate struct does not get used if the > > compile-time config has ended up with the stub > > or > > > (2) it needs to actually match the real vmstate struct, otherwise > > migration between a QEMU built with a config that just got the > > stub version and a QEMU built with a config that got the full > > version will break > > > > This patch does the former. Segfaulting if we got something wrong > > and tried to use the vmstate when we weren't expecting to is > > arguably better than producing an incompatible migration stream. > > > (Better still would be if we caught this on machine startup rather > > than only when savevm was invoked.) > Theoretically possible with a bunch of mips and x86 stubs, but ... > we typically don't do this kind of checks for migration sake > as that complicates things a lot in general. > i.e. it's common to let migration fail in case of incompatible > migration stream. It's not exactly friendly to user but it's > graceful failure (assuming code is correct and not crashes QEMU) The point here is that if we ever try to do a migrate with the stub vmstate struct then that's a bug in QEMU. We should prefer to catch those early and clearly. -- PMM