Peter Xu <pet...@redhat.com> wrote: > On Wed, Apr 26, 2023 at 06:36:00PM +0200, Juan Quintela wrote: >> Peter Xu <pet...@redhat.com> wrote: >> > I'm doing some machine type checks to make sure nothing breaks for >> > 7.2<->8.0. Along the way I found one false negative report on e1000e using >> > the static checker, turns out to be an issue in the checker itself. >> > >> > The problem is the checker doesn't take VMS_ARRAY into account when >> > comparing with UNUSED, hence the total size is wrongly calculated. >> > >> > Fix that first in qemu by start dumping size of array as "num", then teach >> > the checker for that. >> > >> > NOTE: the patchset will change both behaviors for either -dump-vmstate on >> > QEMU or the checker, however both patches will be compatible even with old >> > QEMU dumps or even old vmstate-checker script. That's not extremely >> > important, IMHO, but still worth mentioning. >> > >> > Thanks, >> > >> > Peter Xu (2): >> > migration/vmstate-dump: Dump array size too as "num" >> > vmstate-static-checker: Recognize "num" field >> > >> > migration/savevm.c | 3 +++ >> > scripts/vmstate-static-checker.py | 13 ++++++++++--- >> > 2 files changed, 13 insertions(+), 3 deletions(-) >> >> Hi >> >> once that you are working with the static checker. >> >> Could we just run two checks in make check: >> >> - qemu-<whatever> -M <previous-version> against the one from previous >> version, and see that they match. >> - qemu-<whatever> -M <latests> against the one from previous version >> And we save the diffs each time that we add something incompatible and >> fix it on source. > > Normally we don't have "latest machine" but only "previous"? Checking > "previous" would be enough, right? E.g. currently we're at 8.1 dev window, > so we check against 8.0 with whatever new thing coming.
$ qemu-8.0.0 -M pc-q35-8.0.0 > dump-8.0.0-q35 We generate that dump-8.0-q35 on the tree. We will change that once that we release 8.1, until then that is latests. This qemu upstream is whatever is in HEAD $ qemu-upstream -M pc-q35-8.0.0 > dump-8.0.0-upstream-q35 diff dump-8.0.0-q35 dump-8.0.0-upstream-q35 And it should be empty. $ qemu-upstream -M pc-q35-8.1.0 > dump-8.1.0-upstream-q35 diff dump-8.0.0-q35 dump-8.1.0-upstream-q35 Each time that we find a difference, we know that we have to create a property for that to make pc-q35-8.0.0 working. We save that "hunk" somehow. Where I have put diff, we can have something more intelligent that is able to compare json output and have into account that differences that we already know that exist. >> I will start with x86_64. And once that we have it running, the other >> architectures that care about version compatibility can add to it. >> >> What do you think? > > It sounds a good idea to have some way to check compat bits in unit tests. > I'm just not sure whether it's easy to integrate to make check: the > comparision requires building two qemu binaries; one normally with an old > tag that I built manually. No. Single binary. For the old binary we just have saved its output on the tree. > For the static checker itself, it normally also needs some intervention > from human, e.g., it doesn't understand things like field_exists() so it > can report "warnings" only which can be false negative even with ARRAY > issue fixed. This is why I mean that I want the "diff" to be a bit more intelligent and "record" the things that we tell them that are correct. > But ideally e.g. in a CI env we can always keep an old version qemu binary > ready for each arch to be tested, then verify forward+backward migration > with that old machine type with whatever patch applied on top. One trick > here is we need to make sure the test cmdline contains the device/anything > that got changed by the patch. It may not always be the case. I will start with the default machine devices. Once the mechanism is done, we can wonder with more configurations. I will start small and then go from there. Later, Juan.
