Re: [PATCH v5 06/12] virtio-gpu-dmabuf: Improve error handling by introducing 'Error **'

[Akihiko Odaki]

On 2026/02/03 16:30, Vivek Kasireddy wrote:

Make the error handling more robust in virtio_gpu_init_udmabuf()
by passing in 'Error **' parameter to capture errors from
virtio_gpu_create_udmabuf() and virtio_gpu_remap_dmabuf().
And, since they now take in 'Error **' parameter, have them
return a bool to adhere to best practices.

Cc: Marc-André Lureau <marcandre.lur...@redhat.com>
Cc: Alex Bennée <alex.ben...@linaro.org>
Cc: Akihiko Odaki <od...@rsg.ci.i.u-tokyo.ac.jp>
Cc: Dmitry Osipenko <dmitry.osipe...@collabora.com>
Cc: Alex Williamson <a...@shazbot.org>
Cc: Cédric Le Goater <c...@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasire...@intel.com>
---
  hw/display/virtio-gpu-dmabuf.c | 44 ++++++++++++++++++++++------------
  1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-dmabuf.c
index 8d67ef7c2a..22e66fbb12 100644
--- a/hw/display/virtio-gpu-dmabuf.c
+++ b/hw/display/virtio-gpu-dmabuf.c
@@ -27,7 +27,8 @@
  #include "standard-headers/linux/udmabuf.h"
  #include "standard-headers/drm/drm_fourcc.h"

-static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)

+static bool virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res,
+                                      Error **errp)
  {
      g_autofree struct udmabuf_create_list *list = NULL;
      RAMBlock *rb;
@@ -36,7 +37,8 @@ static void virtio_gpu_create_udmabuf(struct 
virtio_gpu_simple_resource *res)

udmabuf = udmabuf_fd();

      if (udmabuf < 0) {
-        return;
+        error_setg(errp, "udmabuf device not available");
+        return false;
      }

list = g_malloc0(sizeof(struct udmabuf_create_list) +

@@ -45,7 +47,11 @@ static void virtio_gpu_create_udmabuf(struct 
virtio_gpu_simple_resource *res)
      for (i = 0; i < res->iov_cnt; i++) {
          rb = qemu_ram_block_from_host(res->iov[i].iov_base, false, &offset);
          if (!rb || rb->fd < 0) {
-            return;
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: Could not find valid ramblock\n",
+                          __func__);
+            error_setg(errp, "Could not find ramblock for dmabuf segment");

This should stay LOG_GUEST_ERROR as this may be triggered by the guest.

Regards,
Akihiko Odaki

+            return false;
          }

list->list[i].memfd = rb->fd;

@@ -58,20 +64,26 @@ static void virtio_gpu_create_udmabuf(struct 
virtio_gpu_simple_resource *res)

res->dmabuf_fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);

      if (res->dmabuf_fd < 0) {
-        warn_report("%s: UDMABUF_CREATE_LIST: %s", __func__,
-                    strerror(errno));
+        error_setg_errno(errp, -res->dmabuf_fd,
+                         "Could not create dmabuf fd via udmabuf driver");
+        return false;
      }
+    return true;
  }

-static void virtio_gpu_remap_dmabuf(struct virtio_gpu_simple_resource *res)

+static bool virtio_gpu_remap_dmabuf(struct virtio_gpu_simple_resource *res,
+                                    Error **err1)
  {
-    res->remapped = mmap(NULL, res->blob_size, PROT_READ,
-                         MAP_SHARED, res->dmabuf_fd, 0);
-    if (res->remapped == MAP_FAILED) {
-        warn_report("%s: dmabuf mmap failed: %s", __func__,
-                    strerror(errno));
+    void *map;
+
+    map = mmap(NULL, res->blob_size, PROT_READ, MAP_SHARED, res->dmabuf_fd, 0);
+    if (map == MAP_FAILED) {
+        error_setg_errno(err1, errno, "dmabuf mmap failed");
          res->remapped = NULL;
+        return false;
      }
+    res->remapped = map;
+    return true;
  }

static void virtio_gpu_destroy_dmabuf(struct virtio_gpu_simple_resource *res)

@@ -125,6 +137,7 @@ bool virtio_gpu_have_udmabuf(void)

void virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource *res)

  {
+    Error *err1 = NULL;
      void *pdata = NULL;

res->dmabuf_fd = -1;

@@ -132,12 +145,13 @@ void virtio_gpu_init_dmabuf(struct 
virtio_gpu_simple_resource *res)
          res->iov[0].iov_len < 4096) {
          pdata = res->iov[0].iov_base;
      } else {
-        virtio_gpu_create_udmabuf(res);
-        if (res->dmabuf_fd < 0) {
+        if (!virtio_gpu_create_udmabuf(res, &err1)) {
+            error_report_err(err1);
              return;
          }
-        virtio_gpu_remap_dmabuf(res);
-        if (!res->remapped) {
+
+        if (!virtio_gpu_remap_dmabuf(res, &err1)) {
+            error_report_err(err1);
              return;
          }
          pdata = res->remapped;