ui: refactor using a common qemu_pixman_shareable

Use a common shareable type for win32 & unix, and helper functions.
This simplify the code as it avoids a lot of #ifdef'ery.

Note: if it helps review, commits could be reordered to introduce the
common type before introducing shareable memory for unix.

Suggested-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Message-ID: <20241008125028.1177932-19-marcandre.lureau@redhat.com>

hw/display/virtio-gpu.c

@@ -239,20 +239,6 @@ static uint32_t calc_image_hostmem(pixman_format_code_t pformat,
     return height * stride;
 }
 
-static void
-resource_set_image_destroy(struct virtio_gpu_simple_resource *res)
-{
-    if (!res) {
-        return;
-    }
-#ifdef WIN32
-    void *data = res->handle;
-#else
-    void *data = GINT_TO_POINTER(res->shmfd);
-#endif
-    pixman_image_set_destroy_function(res->image, qemu_pixman_shared_image_destroy, data);
-}
-
 static void virtio_gpu_resource_create_2d(VirtIOGPU *g,
                                           struct virtio_gpu_ctrl_command *cmd)
 {
@@ -299,21 +285,17 @@ static void virtio_gpu_resource_create_2d(VirtIOGPU *g,
 
     res->hostmem = calc_image_hostmem(pformat, c2d.width, c2d.height);
     if (res->hostmem + g->hostmem < g->conf_max_hostmem) {
-        void *bits = NULL;
+        if (!qemu_pixman_image_new_shareable(
-#ifdef WIN32
+                &res->image,
-        bits = qemu_win32_map_alloc(res->hostmem, &res->handle, &error_warn);
+                &res->share_handle,
-#else
+                "virtio-gpu res",
-        bits = qemu_memfd_alloc("virtio-gpu-res", res->hostmem, 0, &res->shmfd, &error_warn);
-#endif
-        if (!bits) {
-            goto end;
-        }
-        res->image = pixman_image_create_bits(
                 pformat,
                 c2d.width,
                 c2d.height,
-            bits, c2d.height ? res->hostmem / c2d.height : 0);
+                c2d.height ? res->hostmem / c2d.height : 0,
-        resource_set_image_destroy(res);
+                &error_warn)) {
+            goto end;
+        }
     }
 
 end:
@@ -687,11 +669,7 @@ static bool virtio_gpu_do_set_scanout(VirtIOGPU *g,
 
         /* realloc the surface ptr */
         scanout->ds = qemu_create_displaysurface_pixman(rect);
-#ifdef WIN32
+        qemu_displaysurface_set_share_handle(scanout->ds, res->share_handle, fb->offset);
-        qemu_displaysurface_win32_set_handle(scanout->ds, res->handle, fb->offset);
-#else
-        qemu_displaysurface_set_shmfd(scanout->ds, res->shmfd, fb->offset);
-#endif
 
         pixman_image_unref(rect);
         dpy_gfx_replace_surface(g->parent_obj.scanout[scanout_id].con,
@@ -1287,7 +1265,6 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,
     VirtIOGPU *g = opaque;
     struct virtio_gpu_simple_resource *res;
     uint32_t resource_id, pformat;
-    void *bits = NULL;
     int i;
 
     g->hostmem = 0;
@@ -1314,24 +1291,17 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,
         }
 
         res->hostmem = calc_image_hostmem(pformat, res->width, res->height);
-#ifdef WIN32
+        if (!qemu_pixman_image_new_shareable(&res->image,
-        bits = qemu_win32_map_alloc(res->hostmem, &res->handle, &error_warn);
+                                             &res->share_handle,
-#else
+                                             "virtio-gpu res",
-        bits = qemu_memfd_alloc("virtio-gpu-res", res->hostmem, 0, &res->shmfd, &error_warn);
-#endif
-        if (!bits) {
-            g_free(res);
-            return -EINVAL;
-        }
-        res->image = pixman_image_create_bits(
                                              pformat,
-            res->width, res->height,
+                                             res->width,
-            bits, res->height ? res->hostmem / res->height : 0);
+                                             res->height,
-        if (!res->image) {
+                                             res->height ? res->hostmem / res->height : 0,
+                                             &error_warn)) {
             g_free(res);
             return -EINVAL;
         }
-        resource_set_image_destroy(res);
 
         res->addrs = g_new(uint64_t, res->iov_cnt);
         res->iov = g_new(struct iovec, res->iov_cnt);
@@ -1464,11 +1434,7 @@ static int virtio_gpu_post_load(void *opaque, int version_id)
                 return -EINVAL;
             }
             scanout->ds = qemu_create_displaysurface_pixman(res->image);
-#ifdef WIN32
+            qemu_displaysurface_set_share_handle(scanout->ds, res->share_handle, 0);
-            qemu_displaysurface_win32_set_handle(scanout->ds, res->handle, 0);
-#else
-            qemu_displaysurface_set_shmfd(scanout->ds, res->shmfd, 0);
-#endif
             dpy_gfx_replace_surface(scanout->con, scanout->ds);
         }
 

include/hw/virtio/virtio-gpu.h

@@ -51,11 +51,7 @@ struct virtio_gpu_simple_resource {
     unsigned int iov_cnt;
     uint32_t scanout_bitmask;
     pixman_image_t *image;
-#ifdef WIN32
+    qemu_pixman_shareable share_handle;
-    HANDLE handle;
-#else
-    int shmfd;
-#endif
     uint64_t hostmem;
 
     uint64_t blob_size;

include/ui/qemu-pixman.h

@@ -12,6 +12,8 @@
 #include "pixman-minimal.h"
 #endif
 
+#include "qapi/error.h"
+
 /*
  * pixman image formats are defined to be native endian,
  * that means host byte order on qemu.  So we go define
@@ -97,7 +99,27 @@ void qemu_pixman_glyph_render(pixman_image_t *glyph,
 
 void qemu_pixman_image_unref(pixman_image_t *image);
 
-void qemu_pixman_shared_image_destroy(pixman_image_t *image, void *data);
+#ifdef WIN32
+typedef HANDLE qemu_pixman_shareable;
+#define SHAREABLE_NONE (NULL)
+#define SHAREABLE_TO_PTR(handle) (handle)
+#define PTR_TO_SHAREABLE(ptr) (ptr)
+#else
+typedef int qemu_pixman_shareable;
+#define SHAREABLE_NONE (-1)
+#define SHAREABLE_TO_PTR(handle) GINT_TO_POINTER(handle)
+#define PTR_TO_SHAREABLE(ptr) GPOINTER_TO_INT(ptr)
+#endif
+
+bool qemu_pixman_image_new_shareable(
+    pixman_image_t **image,
+    qemu_pixman_shareable *handle,
+    const char *name,
+    pixman_format_code_t format,
+    int width,
+    int height,
+    int rowstride_bytes,
+    Error **errp);
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(pixman_image_t, qemu_pixman_image_unref)
 

include/ui/surface.h

@@ -23,13 +23,8 @@ typedef struct DisplaySurface {
     GLenum gltype;
     GLuint texture;
 #endif
-#ifdef WIN32
+    qemu_pixman_shareable share_handle;
-    HANDLE handle;
+    uint32_t share_handle_offset;
-    uint32_t handle_offset;
-#else
-    int shmfd;
-    uint32_t shmfd_offset;
-#endif
 } DisplaySurface;
 
 PixelFormat qemu_default_pixelformat(int bpp);
@@ -40,13 +35,10 @@ DisplaySurface *qemu_create_displaysurface_from(int width, int height,
 DisplaySurface *qemu_create_displaysurface_pixman(pixman_image_t *image);
 DisplaySurface *qemu_create_placeholder_surface(int w, int h,
                                                 const char *msg);
-#ifdef WIN32
+
-void qemu_displaysurface_win32_set_handle(DisplaySurface *surface,
+void qemu_displaysurface_set_share_handle(DisplaySurface *surface,
-                                          HANDLE h, uint32_t offset);
+                                          qemu_pixman_shareable handle,
-#else
+                                          uint32_t offset);
-void qemu_displaysurface_set_shmfd(DisplaySurface *surface,
-                                   int shmfd, uint32_t offset);
-#endif
 
 DisplaySurface *qemu_create_displaysurface(int width, int height);
 void qemu_free_displaysurface(DisplaySurface *surface);

ui/console.c

@@ -453,61 +453,26 @@ qemu_graphic_console_init(Object *obj)
 {
 }
 
-#ifdef WIN32
+void qemu_displaysurface_set_share_handle(DisplaySurface *surface,
-void qemu_displaysurface_win32_set_handle(DisplaySurface *surface,
+                                          qemu_pixman_shareable handle,
-                                          HANDLE h, uint32_t offset)
+                                          uint32_t offset)
 {
-    assert(!surface->handle);
+    assert(surface->share_handle == SHAREABLE_NONE);
 
-    surface->handle = h;
+    surface->share_handle = handle;
-    surface->handle_offset = offset;
+    surface->share_handle_offset = offset;
-}
-#else
-void qemu_displaysurface_set_shmfd(DisplaySurface *surface,
-                                   int shmfd, uint32_t offset)
-{
-    assert(surface->shmfd == -1);
 
-    surface->shmfd = shmfd;
-    surface->shmfd_offset = offset;
 }
-#endif
 
 DisplaySurface *qemu_create_displaysurface(int width, int height)
 {
-    DisplaySurface *surface;
-    void *bits = NULL;
-#ifdef WIN32
-    HANDLE handle = NULL;
-#else
-    int shmfd = -1;
-#endif
-
     trace_displaysurface_create(width, height);
 
-#ifdef WIN32
+    return qemu_create_displaysurface_from(
-    bits = qemu_win32_map_alloc(width * height * 4, &handle, &error_abort);
-#else
-    bits = qemu_memfd_alloc("displaysurface", width * height * 4, 0, &shmfd, &error_abort);
-#endif
-
-    surface = qemu_create_displaysurface_from(
         width, height,
         PIXMAN_x8r8g8b8,
-        width * 4, bits
+        width * 4, NULL
     );
-    surface->flags = QEMU_ALLOCATED_FLAG;
-
-#ifdef WIN32
-    qemu_displaysurface_win32_set_handle(surface, handle, 0);
-    void *data = handle;
-#else
-    qemu_displaysurface_set_shmfd(surface, shmfd, 0);
-    void *data = GINT_TO_POINTER(shmfd);
-#endif
-    pixman_image_set_destroy_function(surface->image, qemu_pixman_shared_image_destroy, data);
-
-    return surface;
 }
 
 DisplaySurface *qemu_create_displaysurface_from(int width, int height,
@@ -517,14 +482,25 @@ DisplaySurface *qemu_create_displaysurface_from(int width, int height,
     DisplaySurface *surface = g_new0(DisplaySurface, 1);
 
     trace_displaysurface_create_from(surface, width, height, format);
-#ifndef WIN32
+    surface->share_handle = SHAREABLE_NONE;
-    surface->shmfd = -1;
+
-#endif
+    if (data) {
         surface->image = pixman_image_create_bits(format,
                                                   width, height,
                                                   (void *)data, linesize);
-    assert(surface->image != NULL);
+    } else {
+        qemu_pixman_image_new_shareable(&surface->image,
+                                        &surface->share_handle,
+                                        "displaysurface",
+                                        format,
+                                        width,
+                                        height,
+                                        linesize,
+                                        &error_abort);
+        surface->flags = QEMU_ALLOCATED_FLAG;
+    }
 
+    assert(surface->image != NULL);
     return surface;
 }
 
@@ -533,9 +509,7 @@ DisplaySurface *qemu_create_displaysurface_pixman(pixman_image_t *image)
     DisplaySurface *surface = g_new0(DisplaySurface, 1);
 
     trace_displaysurface_create_pixman(surface);
-#ifndef WIN32
+    surface->share_handle = SHAREABLE_NONE;
-    surface->shmfd = -1;
-#endif
     surface->image = pixman_image_ref(image);
 
     return surface;

ui/dbus-listener.c

@@ -336,13 +336,13 @@ static bool dbus_scanout_map(DBusDisplayListener *ddl)
         return true;
     }
 
-    if (!ddl->can_share_map || !ddl->ds->handle) {
+    if (!ddl->can_share_map || !ddl->ds->share_handle) {
         return false;
     }
 
     success = DuplicateHandle(
         GetCurrentProcess(),
-        ddl->ds->handle,
+        ddl->ds->share_handle,
         ddl->peer_process,
         &target_handle,
         FILE_MAP_READ | SECTION_QUERY,
@@ -359,7 +359,7 @@ static bool dbus_scanout_map(DBusDisplayListener *ddl)
     if (!qemu_dbus_display1_listener_win32_map_call_scanout_map_sync(
             ddl->map_proxy,
             GPOINTER_TO_UINT(target_handle),
-            ddl->ds->handle_offset,
+            ddl->ds->share_handle_offset,
             surface_width(ddl->ds),
             surface_height(ddl->ds),
             surface_stride(ddl->ds),
@@ -453,13 +453,13 @@ static bool dbus_scanout_map(DBusDisplayListener *ddl)
         return true;
     }
 
-    if (!ddl->can_share_map || ddl->ds->shmfd == -1) {
+    if (!ddl->can_share_map || ddl->ds->share_handle == SHAREABLE_NONE) {
         return false;
     }
 
     ddl_discard_display_messages(ddl);
     fd_list = g_unix_fd_list_new();
-    if (g_unix_fd_list_append(fd_list, ddl->ds->shmfd, &err) != 0) {
+    if (g_unix_fd_list_append(fd_list, ddl->ds->share_handle, &err) != 0) {
         g_debug("Failed to setup scanout map fdlist: %s", err->message);
         ddl->can_share_map = false;
         return false;
@@ -468,7 +468,7 @@ static bool dbus_scanout_map(DBusDisplayListener *ddl)
     if (!qemu_dbus_display1_listener_unix_map_call_scanout_map_sync(
             ddl->map_proxy,
             g_variant_new_handle(0),
-            ddl->ds->shmfd_offset,
+            ddl->ds->share_handle_offset,
             surface_width(ddl->ds),
             surface_height(ddl->ds),
             surface_stride(ddl->ds),

ui/qemu-pixman.c

@@ -270,19 +270,71 @@ void qemu_pixman_glyph_render(pixman_image_t *glyph,
 }
 #endif /* CONFIG_PIXMAN */
 
-void
+static void *
-qemu_pixman_shared_image_destroy(pixman_image_t *image, void *data)
+qemu_pixman_shareable_alloc(const char *name, size_t size,
+                            qemu_pixman_shareable *handle,
+                            Error **errp)
 {
-    void *ptr = pixman_image_get_data(image);
-
 #ifdef WIN32
-    HANDLE handle = data;
+    return qemu_win32_map_alloc(size, handle, errp);
-
-    qemu_win32_map_free(ptr, handle, &error_warn);
 #else
-    int shmfd = GPOINTER_TO_INT(data);
+    return qemu_memfd_alloc(name, size, 0, handle, errp);
-    size_t size = pixman_image_get_height(image) * pixman_image_get_stride(image);
-
-    qemu_memfd_free(ptr, size, shmfd);
 #endif
 }
+
+static void
+qemu_pixman_shareable_free(qemu_pixman_shareable handle,
+                           void *ptr, size_t size)
+{
+#ifdef WIN32
+    qemu_win32_map_free(ptr, handle, &error_warn);
+#else
+    qemu_memfd_free(ptr, size, handle);
+#endif
+}
+
+static void
+qemu_pixman_shared_image_destroy(pixman_image_t *image, void *data)
+{
+    qemu_pixman_shareable handle = PTR_TO_SHAREABLE(data);
+    void *ptr = pixman_image_get_data(image);
+    size_t size = pixman_image_get_height(image) * pixman_image_get_stride(image);
+
+    qemu_pixman_shareable_free(handle, ptr, size);
+}
+
+bool
+qemu_pixman_image_new_shareable(pixman_image_t **image,
+                                qemu_pixman_shareable *handle,
+                                const char *name,
+                                pixman_format_code_t format,
+                                int width,
+                                int height,
+                                int rowstride_bytes,
+                                Error **errp)
+{
+    ERRP_GUARD();
+    size_t size = height * rowstride_bytes;
+    void *bits = NULL;
+
+    g_return_val_if_fail(image != NULL, false);
+    g_return_val_if_fail(handle != NULL, false);
+
+    bits = qemu_pixman_shareable_alloc(name, size, handle, errp);
+    if (!bits) {
+        return false;
+    }
+
+    *image = pixman_image_create_bits(format, width, height, bits, rowstride_bytes);
+    if (!*image) {
+        error_setg(errp, "Failed to allocate image");
+        qemu_pixman_shareable_free(*handle, bits, size);
+        return false;
+    }
+
+    pixman_image_set_destroy_function(*image,
+                                      qemu_pixman_shared_image_destroy,
+                                      SHAREABLE_TO_PTR(*handle));
+
+    return true;
+}