Re: [PATCH 0/4] Fix "virtio-gpu: fix scanout migration post-load"

[Fabiano Rosas]

marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Hi,
>
> The aforementioned patch breaks virtio-gpu device migrations for versions
> pre-9.0/9.0, both forwards and backwards. Versioning of `VMS_STRUCT` is more
> complex than it may initially appear, as evidenced in the problematic commit
> dfcf74fa68c ("virtio-gpu: fix scanout migration post-load").
>
> To resolve this, we need to propagate the `vmstate` `version_id` through the
> nested structures. Additionally, we should tie specific machine version to a
> corresponding `version_id` to maintain migration compatibility.
>
> `VMS_VSTRUCT` allows specifying the appropriate version of the nested 
> structure
> to use.

This would have been caught by the migration-compat-x86_64 CI job had we
added the virtio-gpu device to it.

$ cd build-8.2
$ QTEST_TRACE='vmstate_*' QTEST_DEVICE_OPTS='-device virtio-gpu' \
QTEST_QEMU_BINARY=./qemu-system-x86_64 \
QTEST_QEMU_BINARY_DST=../build-9.0/qemu-system-x86_64 
./tests/qtest/migration-test
...
vmstate_n_elems fb.offset: 1
vmstate_subsection_load virtio-gpu-one-scanout
vmstate_subsection_load_good virtio-gpu-one-scanout
vmstate_load_state_end virtio-gpu-one-scanout end/0
vmstate_subsection_load virtio-gpu-scanouts
vmstate_subsection_load_good virtio-gpu-scanouts
vmstate_load_state_end virtio-gpu-scanouts end/0
vmstate_subsection_load virtio-gpu
vmstate_subsection_load_good virtio-gpu
vmstate_load_state_end virtio-gpu end/0
vmstate_downtime_load type=non-iterable idstr=0000:00:03.0/virtio-gpu 
instance_id=0 downtime=32118
qemu-system-x86_64: Missing section footer for 0000:00:03.0/virtio-gpu
vmstate_downtime_checkpoint dst-precopy-loadvm-completed
qemu-system-x86_64: load of migration failed: Invalid argument

Some considerations:

1) Here QTEST_DEVICE_OPTS is a hack I added on top, it doesn't currently
   exist.

2) This only uncovers relatively simple bugs where we don't need the
   guest to access the device, it just needs to be there.

We could take the steps to enable this kind of testing if we think it's
worthwhile. Some downsides are:

a) the item (2) above - situations that depend on guest behavior are out
   of the picture because migration-test runs only a custom program that
   dirties memory;

b) this test only works in CI or in a pre setup environment because it
   needs the previous QEMU version to be built beforehand;

c) the full set of migration tests already runs a few times in CI via
   make check, plus the compat job. We'll probably need to do some
   simplification to avoid taking too much additional time;

d) there's also the obvious maintenance burden of choosing devices and
   doing the eventual upkeep of the QEMU command line for the
   migration-test.