On Mon, 1 Aug 2022 at 10:17, Dr. David Alan Gilbert <dgilb...@redhat.com> wrote: > > * Peter Maydell (peter.mayd...@linaro.org) wrote: > > 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: > > > > 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. > > I'd rather have something that was explicitly poisoned rather than just > walking off the end of an uninitialised array and having to break out > gdb.
It doesn't walk off the end of the array -- it segfaults because it wants to dereference vmsd->name, which is NULL. If we want to have a more obvious and concrete way to mark "this vmsd is bad and should never be actively used" that's fine, but it seems like a separate patch from this one, which is just fixing the problem that we use a vmsd that we should not be using. -- PMM