hw/display/virtio-gpu-rutabaga.c | 20 +++++------------- hw/display/virtio-gpu-virgl.c | 34 ++++++++++++++++++------------- hw/display/virtio-gpu.c | 35 +++++++++++++++++++++----------- include/hw/virtio/virtio-gpu.h | 4 ++++ 4 files changed, 52 insertions(+), 41 deletions(-)
From: Marc-André Lureau <marcandre.lureau@redhat.com>
All allocation sites used g_new0() which zero-initializes fields.
However share_handle must be SHAREABLE_NONE (-1 on Unix, NULL on
Windows) and dmabuf_fd must be -1, not 0. Centralize allocation and
field initialization in new constructors fix this and reduce code
duplication.
This fixes -display dbus with virtio-gpu blob resources. The other
end is currently receiving qemu fd 0.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
v2: add virgl_gpu_virgl_resource_new()
---
hw/display/virtio-gpu-rutabaga.c | 20 +++++-------------
hw/display/virtio-gpu-virgl.c | 34 ++++++++++++++++++-------------
hw/display/virtio-gpu.c | 35 +++++++++++++++++++++-----------
include/hw/virtio/virtio-gpu.h | 4 ++++
4 files changed, 52 insertions(+), 41 deletions(-)
diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
index 041216a10d04..234fe94b178b 100644
--- a/hw/display/virtio-gpu-rutabaga.c
+++ b/hw/display/virtio-gpu-rutabaga.c
@@ -100,12 +100,8 @@ rutabaga_cmd_create_resource_2d(VirtIOGPU *g,
result = rutabaga_resource_create_3d(vr->rutabaga, c2d.resource_id, &rc_3d);
CHECK(!result, cmd);
- res = g_new0(struct virtio_gpu_simple_resource, 1);
- res->width = c2d.width;
- res->height = c2d.height;
- res->format = c2d.format;
- res->resource_id = c2d.resource_id;
-
+ res = virtio_gpu_simple_resource_new(c2d.resource_id, c2d.width,
+ c2d.height, c2d.format);
QTAILQ_INSERT_HEAD(&g->reslist, res, next);
}
@@ -139,12 +135,8 @@ rutabaga_cmd_create_resource_3d(VirtIOGPU *g,
result = rutabaga_resource_create_3d(vr->rutabaga, c3d.resource_id, &rc_3d);
CHECK(!result, cmd);
- res = g_new0(struct virtio_gpu_simple_resource, 1);
- res->width = c3d.width;
- res->height = c3d.height;
- res->format = c3d.format;
- res->resource_id = c3d.resource_id;
-
+ res = virtio_gpu_simple_resource_new(c3d.resource_id, c3d.width,
+ c3d.height, c3d.format);
QTAILQ_INSERT_HEAD(&g->reslist, res, next);
}
@@ -634,9 +626,7 @@ rutabaga_cmd_resource_create_blob(VirtIOGPU *g,
CHECK(cblob.resource_id != 0, cmd);
- res = g_new0(struct virtio_gpu_simple_resource, 1);
-
- res->resource_id = cblob.resource_id;
+ res = virtio_gpu_simple_resource_new(cblob.resource_id, 0, 0, 0);
res->blob_size = cblob.size;
if (cblob.blob_mem != VIRTIO_GPU_BLOB_MEM_HOST3D) {
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 9bda572426b2..d32801c3893a 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -309,6 +309,23 @@ virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
}
#endif
+static struct virtio_gpu_virgl_resource *
+virtio_gpu_virgl_resource_new(uint32_t resource_id, uint32_t width,
+ uint32_t height, uint32_t format)
+{
+ struct virtio_gpu_virgl_resource *res = g_new0(struct virtio_gpu_virgl_resource, 1);
+
+ res->base.share_handle = SHAREABLE_NONE;
+ res->base.dmabuf_fd = -1;
+
+ res->base.resource_id = resource_id;
+ res->base.width = width;
+ res->base.height = height;
+ res->base.format = format;
+
+ return res;
+}
+
static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
struct virtio_gpu_ctrl_command *cmd)
{
@@ -335,12 +352,7 @@ static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
return;
}
- res = g_new0(struct virtio_gpu_virgl_resource, 1);
- res->base.width = c2d.width;
- res->base.height = c2d.height;
- res->base.format = c2d.format;
- res->base.resource_id = c2d.resource_id;
- res->base.dmabuf_fd = -1;
+ res = virtio_gpu_virgl_resource_new(c2d.resource_id, c2d.width, c2d.height, c2d.format);
QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
args.handle = c2d.resource_id;
@@ -383,12 +395,7 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
return;
}
- res = g_new0(struct virtio_gpu_virgl_resource, 1);
- res->base.width = c3d.width;
- res->base.height = c3d.height;
- res->base.format = c3d.format;
- res->base.resource_id = c3d.resource_id;
- res->base.dmabuf_fd = -1;
+ res = virtio_gpu_virgl_resource_new(c3d.resource_id, c3d.width, c3d.height, c3d.format);
QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
args.handle = c3d.resource_id;
@@ -853,8 +860,7 @@ static void virgl_cmd_resource_create_blob(VirtIOGPU *g,
return;
}
- res = g_new0(struct virtio_gpu_virgl_resource, 1);
- res->base.resource_id = cblob.resource_id;
+ res = virtio_gpu_virgl_resource_new(cblob.resource_id, 0, 0, 0);
res->base.blob_size = cblob.size;
res->base.dmabuf_fd = -1;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 55a1c7f80fb8..d5405f0c715f 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -41,6 +41,24 @@ virtio_gpu_find_check_resource(VirtIOGPU *g, uint32_t resource_id,
static void virtio_gpu_reset_bh(void *opaque);
+struct virtio_gpu_simple_resource *
+virtio_gpu_simple_resource_new(uint32_t resource_id, uint32_t width,
+ uint32_t height, uint32_t format)
+{
+ struct virtio_gpu_simple_resource *res =
+ g_new0(struct virtio_gpu_simple_resource, 1);
+
+ res->share_handle = SHAREABLE_NONE;
+ res->dmabuf_fd = -1;
+
+ res->resource_id = resource_id;
+ res->width = width;
+ res->height = height;
+ res->format = format;
+
+ return res;
+}
+
void virtio_gpu_update_cursor_data(VirtIOGPU *g,
struct virtio_gpu_scanout *s,
uint32_t resource_id)
@@ -259,12 +277,8 @@ static void virtio_gpu_resource_create_2d(VirtIOGPU *g,
return;
}
- res = g_new0(struct virtio_gpu_simple_resource, 1);
-
- res->width = c2d.width;
- res->height = c2d.height;
- res->format = c2d.format;
- res->resource_id = c2d.resource_id;
+ res = virtio_gpu_simple_resource_new(c2d.resource_id, c2d.width,
+ c2d.height, c2d.format);
pformat = virtio_gpu_get_pixman_format(c2d.format);
if (!pformat) {
@@ -345,8 +359,7 @@ static void virtio_gpu_resource_create_blob(VirtIOGPU *g,
return;
}
- res = g_new0(struct virtio_gpu_simple_resource, 1);
- res->resource_id = cblob.resource_id;
+ res = virtio_gpu_simple_resource_new(cblob.resource_id, 0, 0, 0);
res->blob_size = cblob.size;
if (cblob.nr_entries) {
@@ -1442,8 +1455,7 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,
return -EINVAL;
}
- res = g_new0(struct virtio_gpu_simple_resource, 1);
- res->resource_id = resource_id;
+ res = virtio_gpu_simple_resource_new(resource_id, 0, 0, 0);
res->width = qemu_get_be32(f);
res->height = qemu_get_be32(f);
res->format = qemu_get_be32(f);
@@ -1555,8 +1567,7 @@ static int virtio_gpu_blob_load(QEMUFile *f, void *opaque, size_t size,
return -EINVAL;
}
- res = g_new0(struct virtio_gpu_simple_resource, 1);
- res->resource_id = resource_id;
+ res = virtio_gpu_simple_resource_new(resource_id, 0, 0, 0);
res->blob_size = qemu_get_be32(f);
res->iov_cnt = qemu_get_be32(f);
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 69b5ee2e382f..090719aa0522 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -64,6 +64,10 @@ struct virtio_gpu_simple_resource {
QTAILQ_ENTRY(virtio_gpu_simple_resource) next;
};
+struct virtio_gpu_simple_resource *
+virtio_gpu_simple_resource_new(uint32_t resource_id, uint32_t width,
+ uint32_t height, uint32_t format);
+
struct virtio_gpu_framebuffer {
pixman_format_code_t format;
uint32_t width, height;
--
2.55.0.543.g5ebe2ebe4ea8
On 2026/08/28 16:53, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> All allocation sites used g_new0() which zero-initializes fields.
> However share_handle must be SHAREABLE_NONE (-1 on Unix, NULL on
> Windows) and dmabuf_fd must be -1, not 0. Centralize allocation and
> field initialization in new constructors fix this and reduce code
> duplication.
>
> This fixes -display dbus with virtio-gpu blob resources. The other
> end is currently receiving qemu fd 0.
The bug was introduced by 5f899c34af1d (“virtio-gpu: allocate
shareable 2d resources on !win32”). Please add the corresponding Fixes:
tag, as required by: docs/devel/submitting-a-patch.rst
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> v2: add virgl_gpu_virgl_resource_new()
I think you meant virtio_gpu_virgl_resource_new().
> ---
> hw/display/virtio-gpu-rutabaga.c | 20 +++++-------------
> hw/display/virtio-gpu-virgl.c | 34 ++++++++++++++++++-------------
> hw/display/virtio-gpu.c | 35 +++++++++++++++++++++-----------
> include/hw/virtio/virtio-gpu.h | 4 ++++
> 4 files changed, 52 insertions(+), 41 deletions(-)
>
> diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
> index 041216a10d04..234fe94b178b 100644
> --- a/hw/display/virtio-gpu-rutabaga.c
> +++ b/hw/display/virtio-gpu-rutabaga.c
> @@ -100,12 +100,8 @@ rutabaga_cmd_create_resource_2d(VirtIOGPU *g,
> result = rutabaga_resource_create_3d(vr->rutabaga, c2d.resource_id, &rc_3d);
> CHECK(!result, cmd);
>
> - res = g_new0(struct virtio_gpu_simple_resource, 1);
> - res->width = c2d.width;
> - res->height = c2d.height;
> - res->format = c2d.format;
> - res->resource_id = c2d.resource_id;
> -
> + res = virtio_gpu_simple_resource_new(c2d.resource_id, c2d.width,
> + c2d.height, c2d.format);
> QTAILQ_INSERT_HEAD(&g->reslist, res, next);
> }
>
> @@ -139,12 +135,8 @@ rutabaga_cmd_create_resource_3d(VirtIOGPU *g,
> result = rutabaga_resource_create_3d(vr->rutabaga, c3d.resource_id, &rc_3d);
> CHECK(!result, cmd);
>
> - res = g_new0(struct virtio_gpu_simple_resource, 1);
> - res->width = c3d.width;
> - res->height = c3d.height;
> - res->format = c3d.format;
> - res->resource_id = c3d.resource_id;
> -
> + res = virtio_gpu_simple_resource_new(c3d.resource_id, c3d.width,
> + c3d.height, c3d.format);
> QTAILQ_INSERT_HEAD(&g->reslist, res, next);
> }
>
> @@ -634,9 +626,7 @@ rutabaga_cmd_resource_create_blob(VirtIOGPU *g,
>
> CHECK(cblob.resource_id != 0, cmd);
>
> - res = g_new0(struct virtio_gpu_simple_resource, 1);
> -
> - res->resource_id = cblob.resource_id;
> + res = virtio_gpu_simple_resource_new(cblob.resource_id, 0, 0, 0);
> res->blob_size = cblob.size;
>
> if (cblob.blob_mem != VIRTIO_GPU_BLOB_MEM_HOST3D) {
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 9bda572426b2..d32801c3893a 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -309,6 +309,23 @@ virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
> }
> #endif
>
> +static struct virtio_gpu_virgl_resource *
> +virtio_gpu_virgl_resource_new(uint32_t resource_id, uint32_t width,
> + uint32_t height, uint32_t format)
> +{
> + struct virtio_gpu_virgl_resource *res = g_new0(struct virtio_gpu_virgl_resource, 1);
> +
> + res->base.share_handle = SHAREABLE_NONE;
> + res->base.dmabuf_fd = -1;
> +
> + res->base.resource_id = resource_id;
> + res->base.width = width;
> + res->base.height = height;
> + res->base.format = format;
I think it's better to omit the width, height, and format assignments.
They only matter for non-blob resources, so they do not fit well in the
common function. blob_size, which is only valid for blob resources, is
also assigned outside this function, so we can make them consistent.
Having four integer parameters is also slightly confusing. Reducing the
number of parameters may make sense in this regard too.
With the four assignments removed, this and
virtio_gpu_simple_resource_new() will become quite small, so I'm not
sure if we should have virtio_gpu_simple_resource_init() or something
similar to share the remaining assignments; I'm kind of netural in that
aspect.
> +
> + return res;
> +}
> +
> static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
> struct virtio_gpu_ctrl_command *cmd)
> {
> @@ -335,12 +352,7 @@ static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
> return;
> }
>
> - res = g_new0(struct virtio_gpu_virgl_resource, 1);
> - res->base.width = c2d.width;
> - res->base.height = c2d.height;
> - res->base.format = c2d.format;
> - res->base.resource_id = c2d.resource_id;
> - res->base.dmabuf_fd = -1;
> + res = virtio_gpu_virgl_resource_new(c2d.resource_id, c2d.width, c2d.height, c2d.format);
> QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
>
> args.handle = c2d.resource_id;
> @@ -383,12 +395,7 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
> return;
> }
>
> - res = g_new0(struct virtio_gpu_virgl_resource, 1);
> - res->base.width = c3d.width;
> - res->base.height = c3d.height;
> - res->base.format = c3d.format;
> - res->base.resource_id = c3d.resource_id;
> - res->base.dmabuf_fd = -1;
> + res = virtio_gpu_virgl_resource_new(c3d.resource_id, c3d.width, c3d.height, c3d.format);
> QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
>
> args.handle = c3d.resource_id;
> @@ -853,8 +860,7 @@ static void virgl_cmd_resource_create_blob(VirtIOGPU *g,
> return;
> }
>
> - res = g_new0(struct virtio_gpu_virgl_resource, 1);
> - res->base.resource_id = cblob.resource_id;
> + res = virtio_gpu_virgl_resource_new(cblob.resource_id, 0, 0, 0);
> res->base.blob_size = cblob.size;
> res->base.dmabuf_fd = -1;
This dmabuf_fd assignment is now redundant.
Regards,
Akihiko Odaki
>
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 55a1c7f80fb8..d5405f0c715f 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -41,6 +41,24 @@ virtio_gpu_find_check_resource(VirtIOGPU *g, uint32_t resource_id,
>
> static void virtio_gpu_reset_bh(void *opaque);
>
> +struct virtio_gpu_simple_resource *
> +virtio_gpu_simple_resource_new(uint32_t resource_id, uint32_t width,
> + uint32_t height, uint32_t format)
> +{
> + struct virtio_gpu_simple_resource *res =
> + g_new0(struct virtio_gpu_simple_resource, 1);
> +
> + res->share_handle = SHAREABLE_NONE;
> + res->dmabuf_fd = -1;
> +
> + res->resource_id = resource_id;
> + res->width = width;
> + res->height = height;
> + res->format = format;
> +
> + return res;
> +}
> +
> void virtio_gpu_update_cursor_data(VirtIOGPU *g,
> struct virtio_gpu_scanout *s,
> uint32_t resource_id)
> @@ -259,12 +277,8 @@ static void virtio_gpu_resource_create_2d(VirtIOGPU *g,
> return;
> }
>
> - res = g_new0(struct virtio_gpu_simple_resource, 1);
> -
> - res->width = c2d.width;
> - res->height = c2d.height;
> - res->format = c2d.format;
> - res->resource_id = c2d.resource_id;
> + res = virtio_gpu_simple_resource_new(c2d.resource_id, c2d.width,
> + c2d.height, c2d.format);
>
> pformat = virtio_gpu_get_pixman_format(c2d.format);
> if (!pformat) {
> @@ -345,8 +359,7 @@ static void virtio_gpu_resource_create_blob(VirtIOGPU *g,
> return;
> }
>
> - res = g_new0(struct virtio_gpu_simple_resource, 1);
> - res->resource_id = cblob.resource_id;
> + res = virtio_gpu_simple_resource_new(cblob.resource_id, 0, 0, 0);
> res->blob_size = cblob.size;
>
> if (cblob.nr_entries) {
> @@ -1442,8 +1455,7 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,
> return -EINVAL;
> }
>
> - res = g_new0(struct virtio_gpu_simple_resource, 1);
> - res->resource_id = resource_id;
> + res = virtio_gpu_simple_resource_new(resource_id, 0, 0, 0);
> res->width = qemu_get_be32(f);
> res->height = qemu_get_be32(f);
> res->format = qemu_get_be32(f);
> @@ -1555,8 +1567,7 @@ static int virtio_gpu_blob_load(QEMUFile *f, void *opaque, size_t size,
> return -EINVAL;
> }
>
> - res = g_new0(struct virtio_gpu_simple_resource, 1);
> - res->resource_id = resource_id;
> + res = virtio_gpu_simple_resource_new(resource_id, 0, 0, 0);
> res->blob_size = qemu_get_be32(f);
> res->iov_cnt = qemu_get_be32(f);
>
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index 69b5ee2e382f..090719aa0522 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -64,6 +64,10 @@ struct virtio_gpu_simple_resource {
> QTAILQ_ENTRY(virtio_gpu_simple_resource) next;
> };
>
> +struct virtio_gpu_simple_resource *
> +virtio_gpu_simple_resource_new(uint32_t resource_id, uint32_t width,
> + uint32_t height, uint32_t format);
> +
> struct virtio_gpu_framebuffer {
> pixman_format_code_t format;
> uint32_t width, height;
On 28/8/26 09:53, marcandre.lureau--- via qemu development wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> All allocation sites used g_new0() which zero-initializes fields.
> However share_handle must be SHAREABLE_NONE (-1 on Unix, NULL on
> Windows) and dmabuf_fd must be -1, not 0. Centralize allocation and
> field initialization in new constructors fix this and reduce code
> duplication.
>
> This fixes -display dbus with virtio-gpu blob resources. The other
> end is currently receiving qemu fd 0.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> v2: add virgl_gpu_virgl_resource_new()
> ---
> hw/display/virtio-gpu-rutabaga.c | 20 +++++-------------
> hw/display/virtio-gpu-virgl.c | 34 ++++++++++++++++++-------------
> hw/display/virtio-gpu.c | 35 +++++++++++++++++++++-----------
> include/hw/virtio/virtio-gpu.h | 4 ++++
> 4 files changed, 52 insertions(+), 41 deletions(-)
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 9bda572426b2..d32801c3893a 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -309,6 +309,23 @@ virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
> }
> #endif
>
> +static struct virtio_gpu_virgl_resource *
> +virtio_gpu_virgl_resource_new(uint32_t resource_id, uint32_t width,
> + uint32_t height, uint32_t format)
> +{
> + struct virtio_gpu_virgl_resource *res = g_new0(struct virtio_gpu_virgl_resource, 1);
> +
> + res->base.share_handle = SHAREABLE_NONE;
> + res->base.dmabuf_fd = -1;
> +
> + res->base.resource_id = resource_id;
> + res->base.width = width;
> + res->base.height = height;
> + res->base.format = format;
Seeing v2 maybe we should only add a virtio_gpu_virgl_resource_init().
> +
> + return res;
> +}
© 2016 - 2026 Red Hat, Inc.