| Message ID | 20240515141557.1277999-1-marcandre.lureau@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fix "virtio-gpu: fix scanout migration post-load" | expand |
On Wed, May 15, 2024 at 06:15:51PM +0400, marcandre.lureau@redhat.com wrote: > 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"). > > v2: > - use a manual version field test (instead of the more complex struct variant) > > v3: > - introduce machine_check_version() > - drop the VMSD version, and use machine version field test Thanks for trying this out already. Last time I mentioned this may for the long term because I remember Dan and Thomas were trying to work on some machine deprecation work, and maybe such things may collapse with that work (and perhaps easier with that work landed, too?). Just to copy them both here so we know where we are now, as I didn't follow that discussion. IOW, patch 3/4 may need separate review from outside migration.. The simpler solution is we stick with the customized field and simple fix to the issue first, then whenever we have that new helper later we simply use the new helper to replace the old, alongside we can drop the new field / property too as long as it is declared with "x-". Might be easier to backport too in this case. Marc-Andre, what do you think? Thanks,
On Wed, May 15, 2024 at 10:07:31AM -0600, Peter Xu wrote: > On Wed, May 15, 2024 at 06:15:51PM +0400, marcandre.lureau@redhat.com wrote: > > 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"). > > > > v2: > > - use a manual version field test (instead of the more complex struct variant) > > > > v3: > > - introduce machine_check_version() > > - drop the VMSD version, and use machine version field test > > Thanks for trying this out already. > > Last time I mentioned this may for the long term because I remember Dan and > Thomas were trying to work on some machine deprecation work, and maybe such > things may collapse with that work (and perhaps easier with that work > landed, too?). Just to copy them both here so we know where we are now, as > I didn't follow that discussion. IOW, patch 3/4 may need separate review > from outside migration.. You'll be refering to my series here: https://lists.nongnu.org/archive/html/qemu-devel/2024-05/msg00084.html Note that series very delibrately did *not* expose the version numbers as accessible fields to code. The version number info is only accessible within the machine type macros, and once the macros are expanded, the version digits remains hidden within the opaque machine type name strings, and/or method names. With regards, Daniel
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"). v2: - use a manual version field test (instead of the more complex struct variant) v3: - introduce machine_check_version() - drop the VMSD version, and use machine version field test Marc-André Lureau (5): migration: add "exists" info to load-state-field trace migration: fix a typo hw/boards: add machine_check_version() Set major/minor for PC and arm machines virtio-gpu: fix v2 migration include/hw/boards.h | 14 ++++++++ include/hw/i386/pc.h | 4 ++- hw/arm/virt.c | 2 ++ hw/display/virtio-gpu.c | 21 +++++++----- hw/i386/pc_piix.c | 74 ++++++++++++++++++++--------------------- hw/i386/pc_q35.c | 62 +++++++++++++++++----------------- migration/vmstate.c | 7 ++-- migration/trace-events | 2 +- 8 files changed, 105 insertions(+), 81 deletions(-)