Re: [PATCH v2] hw/display/virtio-gpu: Reject non-migratable large blobs
Marc-André Lureau <[email protected]>
| Newsgroups | gmane.comp.emulators.qemu |
|---|---|
| Message-ID | <CAJ+F1CK3DKbjKUco_XLxgMFhvfqFXHnneM_ee3odf+hp5WHTrA@mail.gmail.com> |
Hi On Tue, Jul 28, 2026 at 10:26 AM Akihiko Odaki <[email protected]> wrote: > > On 2026/07/27 18:30, Daniel P. Berrangé wrote: > > On Mon, Jul 27, 2026 at 06:23:25PM +0900, Akihiko Odaki wrote: > >> The blob size is encoded as a 64-bit integer in the virtio protocol, but > >> it is encoded as a 32-bit integer in the migration stream, and > >> a large blob whose size cannot be expressed in a 32-bit integer will > >> be corrupted after migration. Deny creating such large blobs. > > > > Is there a valid use case for such large blobs ? If so, rather than > > rejecting the blob, we could dynamically add a migration blocker. > > That would allow the valid use cases, while preventing the migration > > problem. > > For the QEMU code path changed here, there should be no practical use > case for a blob larger than 4 GiB. The non-virgl implementation uses > blobs as scanout or cursor backing. Such a size is excessive for those uses. > > A dynamic migration blocker would also let the guest make an otherwise > migratable VM non-migratable at runtime merely by creating such a > resource. This could interfere with host-level VM management. So I think > rejecting sizes that the current migration format cannot represent is > better for now. What's the difference? The dynamic blocker would simply happen earlier or be queryable, no? > If a concrete use case emerges, extending the migration format would be > preferable to making migratability depend on guest-controlled state. > > Regards, > Akihiko Odaki > > > > >> > >> Fixes: f66767f75c9c ("virtio-gpu: add virtio-gpu/blob vmstate subsection") > >> Fixes: 10b9ddbc83b9 ("Revert "virtio-gpu: block migration of VMs with blob=true"") > >> Signed-off-by: Akihiko Odaki <[email protected]> > >> --- > >> Changes in v2: > >> - Add the Fixes: tag for f66767f75c9c. > >> - Add a TODO to encode blob_size as 64 bits in virtio-gpu/blob > >> VMState version 2. > >> - Link to v1: https://lore.kernel.org/qemu-devel/[email protected] > >> --- > >> hw/display/virtio-gpu.c | 15 +++++++++++++++ > >> 1 file changed, 15 insertions(+) > >> > >> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c > >> index 718ba3039290..987b1b3ea860 100644 > >> --- a/hw/display/virtio-gpu.c > >> +++ b/hw/display/virtio-gpu.c > >> @@ -352,6 +352,14 @@ static void virtio_gpu_resource_create_blob(VirtIOGPU *g, > >> return; > >> } > >> > >> + if (cblob.size > UINT32_MAX) { > >> + qemu_log_mask(LOG_GUEST_ERROR, > >> + "%s: blob too large (%" PRIu64 "> %" PRIu32 ")\n", > >> + __func__, cblob.size, UINT32_MAX); > >> + cmd->error = VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY; > >> + return; > >> + } > >> + > >> if (virtio_gpu_find_resource(g, cblob.resource_id)) { > >> qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n", > >> __func__, cblob.resource_id); > >> @@ -1435,7 +1443,14 @@ static int virtio_gpu_blob_save(QEMUFile *f, void *opaque, size_t size, > >> } > >> assert(!res->image); > >> qemu_put_be32(f, res->resource_id); > >> + > >> + /* > >> + * The migration stream encodes blob_size as a 32-bit integer for > >> + * compatibility though virtio encodes it as a 64-bit integer. A newer > >> + * version of the migration stream should encode it as a 64-bit integer. > >> + */ > >> qemu_put_be32(f, res->blob_size); > >> + > >> qemu_put_be32(f, res->iov_cnt); > >> for (i = 0; i < res->iov_cnt; i++) { > >> qemu_put_be64(f, res->addrs[i]); > >> > >> --- > >> base-commit: 006a22cb26998998385b104db1ff9466ef2f3153 > >> change-id: 20260725-blob-8a6a1a85601a > >> > >> Best regards, > >> -- > >> Akihiko Odaki <[email protected]> > >> > >> > > > > With regards, > > Daniel > > -- Marc-André Lureau