| Message ID | 20240522222034.4001-7-farosas@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [PULL,1/9] migration/colo: Minor fix for colo error message | expand |
CC-ing stable, because this already is an issue in 9.0.0 Am 23.05.24 um 00:20 schrieb Fabiano Rosas: > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > Commit dfcf74fa ("virtio-gpu: fix scanout migration post-load") broke > forward/backward version migration. Versioning of nested VMSD structures > is not straightforward, as the wire format doesn't have nested > structures versions. Introduce x-scanout-vmstate-version and a field > test to save/load appropriately according to the machine version. > > Fixes: dfcf74fa ("virtio-gpu: fix scanout migration post-load") > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > Signed-off-by: Peter Xu <peterx@redhat.com> > Reviewed-by: Fiona Ebner <f.ebner@proxmox.com> > Tested-by: Fiona Ebner <f.ebner@proxmox.com> > [fixed long lines] > Signed-off-by: Fabiano Rosas <farosas@suse.de> > --- > hw/core/machine.c | 1 + > hw/display/virtio-gpu.c | 30 ++++++++++++++++++++++-------- > include/hw/virtio/virtio-gpu.h | 1 + > 3 files changed, 24 insertions(+), 8 deletions(-) > > diff --git a/hw/core/machine.c b/hw/core/machine.c > index c7ceb11501..8d6dc69f0e 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -42,6 +42,7 @@ GlobalProperty hw_compat_8_2[] = { > { "migration", "zero-page-detection", "legacy"}, > { TYPE_VIRTIO_IOMMU_PCI, "granule", "4k" }, > { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "64" }, > + { "virtio-gpu-device", "x-scanout-vmstate-version", "1" }, > }; > const size_t hw_compat_8_2_len = G_N_ELEMENTS(hw_compat_8_2); > > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c > index ae831b6b3e..d60b1b2973 100644 > --- a/hw/display/virtio-gpu.c > +++ b/hw/display/virtio-gpu.c > @@ -1166,10 +1166,17 @@ static void virtio_gpu_cursor_bh(void *opaque) > virtio_gpu_handle_cursor(&g->parent_obj.parent_obj, g->cursor_vq); > } > > +static bool scanout_vmstate_after_v2(void *opaque, int version) > +{ > + struct VirtIOGPUBase *base = container_of(opaque, VirtIOGPUBase, scanout); > + struct VirtIOGPU *gpu = container_of(base, VirtIOGPU, parent_obj); > + > + return gpu->scanout_vmstate_version >= 2; > +} > + > static const VMStateDescription vmstate_virtio_gpu_scanout = { > .name = "virtio-gpu-one-scanout", > - .version_id = 2, > - .minimum_version_id = 1, > + .version_id = 1, > .fields = (const VMStateField[]) { > VMSTATE_UINT32(resource_id, struct virtio_gpu_scanout), > VMSTATE_UINT32(width, struct virtio_gpu_scanout), > @@ -1181,12 +1188,18 @@ static const VMStateDescription vmstate_virtio_gpu_scanout = { > VMSTATE_UINT32(cursor.hot_y, struct virtio_gpu_scanout), > VMSTATE_UINT32(cursor.pos.x, struct virtio_gpu_scanout), > VMSTATE_UINT32(cursor.pos.y, struct virtio_gpu_scanout), > - VMSTATE_UINT32_V(fb.format, struct virtio_gpu_scanout, 2), > - VMSTATE_UINT32_V(fb.bytes_pp, struct virtio_gpu_scanout, 2), > - VMSTATE_UINT32_V(fb.width, struct virtio_gpu_scanout, 2), > - VMSTATE_UINT32_V(fb.height, struct virtio_gpu_scanout, 2), > - VMSTATE_UINT32_V(fb.stride, struct virtio_gpu_scanout, 2), > - VMSTATE_UINT32_V(fb.offset, struct virtio_gpu_scanout, 2), > + VMSTATE_UINT32_TEST(fb.format, struct virtio_gpu_scanout, > + scanout_vmstate_after_v2), > + VMSTATE_UINT32_TEST(fb.bytes_pp, struct virtio_gpu_scanout, > + scanout_vmstate_after_v2), > + VMSTATE_UINT32_TEST(fb.width, struct virtio_gpu_scanout, > + scanout_vmstate_after_v2), > + VMSTATE_UINT32_TEST(fb.height, struct virtio_gpu_scanout, > + scanout_vmstate_after_v2), > + VMSTATE_UINT32_TEST(fb.stride, struct virtio_gpu_scanout, > + scanout_vmstate_after_v2), > + VMSTATE_UINT32_TEST(fb.offset, struct virtio_gpu_scanout, > + scanout_vmstate_after_v2), > VMSTATE_END_OF_LIST() > }, > }; > @@ -1659,6 +1672,7 @@ static Property virtio_gpu_properties[] = { > DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags, > VIRTIO_GPU_FLAG_BLOB_ENABLED, false), > DEFINE_PROP_SIZE("hostmem", VirtIOGPU, parent_obj.conf.hostmem, 0), > + DEFINE_PROP_UINT8("x-scanout-vmstate-version", VirtIOGPU, scanout_vmstate_version, 2), > DEFINE_PROP_END_OF_LIST(), > }; > > diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h > index 56d6e821bf..7a59379f5a 100644 > --- a/include/hw/virtio/virtio-gpu.h > +++ b/include/hw/virtio/virtio-gpu.h > @@ -177,6 +177,7 @@ typedef struct VGPUDMABuf { > struct VirtIOGPU { > VirtIOGPUBase parent_obj; > > + uint8_t scanout_vmstate_version; > uint64_t conf_max_hostmem; > > VirtQueue *ctrl_vq;
Fiona Ebner <f.ebner@proxmox.com> writes: > CC-ing stable, because this already is an issue in 9.0.0 > Thank you for pointing this out. I was expecting b4 to find the tag, but I just now noticed that the CC was added by Peter as a reply to the message and not originally via the patch headers, so I should have added it manually.
diff --git a/hw/core/machine.c b/hw/core/machine.c index c7ceb11501..8d6dc69f0e 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -42,6 +42,7 @@ GlobalProperty hw_compat_8_2[] = { { "migration", "zero-page-detection", "legacy"}, { TYPE_VIRTIO_IOMMU_PCI, "granule", "4k" }, { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "64" }, + { "virtio-gpu-device", "x-scanout-vmstate-version", "1" }, }; const size_t hw_compat_8_2_len = G_N_ELEMENTS(hw_compat_8_2); diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index ae831b6b3e..d60b1b2973 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -1166,10 +1166,17 @@ static void virtio_gpu_cursor_bh(void *opaque) virtio_gpu_handle_cursor(&g->parent_obj.parent_obj, g->cursor_vq); } +static bool scanout_vmstate_after_v2(void *opaque, int version) +{ + struct VirtIOGPUBase *base = container_of(opaque, VirtIOGPUBase, scanout); + struct VirtIOGPU *gpu = container_of(base, VirtIOGPU, parent_obj); + + return gpu->scanout_vmstate_version >= 2; +} + static const VMStateDescription vmstate_virtio_gpu_scanout = { .name = "virtio-gpu-one-scanout", - .version_id = 2, - .minimum_version_id = 1, + .version_id = 1, .fields = (const VMStateField[]) { VMSTATE_UINT32(resource_id, struct virtio_gpu_scanout), VMSTATE_UINT32(width, struct virtio_gpu_scanout), @@ -1181,12 +1188,18 @@ static const VMStateDescription vmstate_virtio_gpu_scanout = { VMSTATE_UINT32(cursor.hot_y, struct virtio_gpu_scanout), VMSTATE_UINT32(cursor.pos.x, struct virtio_gpu_scanout), VMSTATE_UINT32(cursor.pos.y, struct virtio_gpu_scanout), - VMSTATE_UINT32_V(fb.format, struct virtio_gpu_scanout, 2), - VMSTATE_UINT32_V(fb.bytes_pp, struct virtio_gpu_scanout, 2), - VMSTATE_UINT32_V(fb.width, struct virtio_gpu_scanout, 2), - VMSTATE_UINT32_V(fb.height, struct virtio_gpu_scanout, 2), - VMSTATE_UINT32_V(fb.stride, struct virtio_gpu_scanout, 2), - VMSTATE_UINT32_V(fb.offset, struct virtio_gpu_scanout, 2), + VMSTATE_UINT32_TEST(fb.format, struct virtio_gpu_scanout, + scanout_vmstate_after_v2), + VMSTATE_UINT32_TEST(fb.bytes_pp, struct virtio_gpu_scanout, + scanout_vmstate_after_v2), + VMSTATE_UINT32_TEST(fb.width, struct virtio_gpu_scanout, + scanout_vmstate_after_v2), + VMSTATE_UINT32_TEST(fb.height, struct virtio_gpu_scanout, + scanout_vmstate_after_v2), + VMSTATE_UINT32_TEST(fb.stride, struct virtio_gpu_scanout, + scanout_vmstate_after_v2), + VMSTATE_UINT32_TEST(fb.offset, struct virtio_gpu_scanout, + scanout_vmstate_after_v2), VMSTATE_END_OF_LIST() }, }; @@ -1659,6 +1672,7 @@ static Property virtio_gpu_properties[] = { DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags, VIRTIO_GPU_FLAG_BLOB_ENABLED, false), DEFINE_PROP_SIZE("hostmem", VirtIOGPU, parent_obj.conf.hostmem, 0), + DEFINE_PROP_UINT8("x-scanout-vmstate-version", VirtIOGPU, scanout_vmstate_version, 2), DEFINE_PROP_END_OF_LIST(), }; diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h index 56d6e821bf..7a59379f5a 100644 --- a/include/hw/virtio/virtio-gpu.h +++ b/include/hw/virtio/virtio-gpu.h @@ -177,6 +177,7 @@ typedef struct VGPUDMABuf { struct VirtIOGPU { VirtIOGPUBase parent_obj; + uint8_t scanout_vmstate_version; uint64_t conf_max_hostmem; VirtQueue *ctrl_vq;