ui: improve feedback if gl setup fails

Signed-off-by Cole Robinson <[email protected]>

---
 ui/egl-helpers.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

Signed-off-by Cole Robinson <[email protected]>

---
 ui/egl-helpers.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

ui/egl-helpers.c +14 -14

index 558edfd..6555f5f 100644
@@ -2,6 +2,7 @@
 #include <glob.h>
 #include <dirent.h>
 
+#include "qemu/error-report.h"
 #include "ui/egl-helpers.h"
 
 EGLDisplay *qemu_egl_display;

@@ -77,13 +78,13 @@ int egl_rendernode_init(void)
 
     qemu_egl_rn_fd = qemu_egl_rendernode_open();
     if (qemu_egl_rn_fd == -1) {
-        fprintf(stderr, "egl: no drm render node available\n");
+        error_report("egl: no drm render node available");
         goto err;
     }
 
     qemu_egl_rn_gbm_dev = gbm_create_device(qemu_egl_rn_fd);
     if (!qemu_egl_rn_gbm_dev) {
-        fprintf(stderr, "egl: gbm_create_device failed\n");
+        error_report("egl: gbm_create_device failed");
         goto err;
     }
 

@@ -91,18 +92,18 @@ int egl_rendernode_init(void)
 
     if (!epoxy_has_egl_extension(qemu_egl_display,
                                  "EGL_KHR_surfaceless_context")) {
-        fprintf(stderr, "egl: EGL_KHR_surfaceless_context not supported\n");
+        error_report("egl: EGL_KHR_surfaceless_context not supported");
         goto err;
     }
     if (!epoxy_has_egl_extension(qemu_egl_display,
                                  "EGL_MESA_image_dma_buf_export")) {
-        fprintf(stderr, "egl: EGL_MESA_image_dma_buf_export not supported\n");
+        error_report("egl: EGL_MESA_image_dma_buf_export not supported");
         goto err;
     }
 
     qemu_egl_rn_ctx = qemu_egl_init_ctx();
     if (!qemu_egl_rn_ctx) {
-        fprintf(stderr, "egl: egl_init_ctx failed\n");
+        error_report("egl: egl_init_ctx failed");
         goto err;
     }
 

@@ -159,13 +160,13 @@ EGLSurface qemu_egl_init_surface_x11(EGLContext ectx, Window win)
                                       qemu_egl_config,
                                       (EGLNativeWindowType)win, NULL);
     if (esurface == EGL_NO_SURFACE) {
-        fprintf(stderr, "egl: eglCreateWindowSurface failed\n");
+        error_report("egl: eglCreateWindowSurface failed");
         return NULL;
     }
 
     b = eglMakeCurrent(qemu_egl_display, esurface, esurface, ectx);
     if (b == EGL_FALSE) {
-        fprintf(stderr, "egl: eglMakeCurrent failed\n");
+        error_report("egl: eglMakeCurrent failed");
         return NULL;
     }
 

@@ -207,21 +208,21 @@ int qemu_egl_init_dpy(EGLNativeDisplayType dpy, bool gles, bool debug)
     egl_dbg("eglGetDisplay (dpy %p) ...\n", dpy);
     qemu_egl_display = eglGetDisplay(dpy);
     if (qemu_egl_display == EGL_NO_DISPLAY) {
-        fprintf(stderr, "egl: eglGetDisplay failed\n");
+        error_report("egl: eglGetDisplay failed");
         return -1;
     }
 
     egl_dbg("eglInitialize ...\n");
     b = eglInitialize(qemu_egl_display, &major, &minor);
     if (b == EGL_FALSE) {
-        fprintf(stderr, "egl: eglInitialize failed\n");
+        error_report("egl: eglInitialize failed");
         return -1;
     }
 
     egl_dbg("eglBindAPI ...\n");
     b = eglBindAPI(gles ? EGL_OPENGL_ES_API : EGL_OPENGL_API);
     if (b == EGL_FALSE) {
-        fprintf(stderr, "egl: eglBindAPI failed\n");
+        error_report("egl: eglBindAPI failed");
         return -1;
     }
 

@@ -230,7 +231,7 @@ int qemu_egl_init_dpy(EGLNativeDisplayType dpy, bool gles, bool debug)
                         gles ? conf_att_gles : conf_att_gl,
                         &qemu_egl_config, 1, &n);
     if (b == EGL_FALSE || n != 1) {
-        fprintf(stderr, "egl: eglChooseConfig failed\n");
+        error_report("egl: eglChooseConfig failed");
         return -1;
     }
 

@@ -255,13 +256,13 @@ EGLContext qemu_egl_init_ctx(void)
     ectx = eglCreateContext(qemu_egl_display, qemu_egl_config, EGL_NO_CONTEXT,
                             egl_gles ? ctx_att_gles : ctx_att_gl);
     if (ectx == EGL_NO_CONTEXT) {
-        fprintf(stderr, "egl: eglCreateContext failed\n");
+        error_report("egl: eglCreateContext failed");
         return NULL;
     }
 
     b = eglMakeCurrent(qemu_egl_display, EGL_NO_SURFACE, EGL_NO_SURFACE, ectx);
     if (b == EGL_FALSE) {
-        fprintf(stderr, "egl: eglMakeCurrent failed\n");
+        error_report("egl: eglMakeCurrent failed");
         return NULL;
     }
 
--

2.7.4

The user explicitly requested spice GL, so if we know it isn't going to work we should exit

Signed-off-by Cole Robinson <[email protected]>

---
For example, trying to use spice GL with libvirt qemu:///system will fail here, since the the VM will lack permissions to access /dev/dir/renderD*

 ui/spice-core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

ui/spice-core.c +4 -3

index 61db3c1..da05054 100644
@@ -833,9 +833,11 @@ void qemu_spice_init(void)
                          "incompatible with -spice port/tls-port");
             exit(1);
         }
-        if (egl_rendernode_init() == 0) {
-            display_opengl = 1;
+        if (egl_rendernode_init() != 0) {
+            error_report("Failed to initialize EGL render node for SPICE GL");
+            exit(1);
         }
+        display_opengl = 1;
     }
 #endif
 }
--

2.7.4

Give users a hint if their config is wrong.

Signed-off-by Cole Robinson <[email protected]>

---
If virgl support is built into qemu, virgl=on is the default, so this could be noisy in cases where people don't even care about virgl. So I won't object if this is dropped.

The message also pops up once via make check, probably from tests/display-vga-test.c , but doesn't cause a failure or anything.

Is there a way to check that user explicitly specified virgl= ?

 hw/display/virtio-gpu.c | 5 +++++
 1 file changed, 5 insertions(+)

hw/display/virtio-gpu.c +5 -1

index c181fb3..d3c567f 100644
@@ -13,6 +13,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu-common.h"
+#include "qemu/error-report.h"
 #include "qemu/iov.h"
 #include "ui/console.h"
 #include "trace.h"

@@ -944,6 +945,10 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
     have_virgl = display_opengl;
 #endif
     if (!have_virgl) {
+        if (virtio_gpu_virgl_enabled(g->conf)) {
+            error_report("Display isn't configured for GL support. "
+                         "Disabling virgl");
+        }
         g->conf.flags &= ~(1 << VIRTIO_GPU_FLAG_VIRGL_ENABLED);
     }
 
--

2.7.4

Give users a hint if their config is wrong.

Signed-off-by Cole Robinson <[email protected]>

---
If virgl support is built into qemu, virgl=on is the default, so this could be noisy in cases where people don't even care about virgl. So I won't object if this is dropped.

Quoted 7 lines · Cole Robinson

Give users a hint if their config is wrong.

Signed-off-by Cole Robinson <[email protected]>

---
If virgl support is built into qemu, virgl=on is the default, so this could be noisy in cases where people don't even care about virgl. So I won't object if this is dropped.

Yes, it's enabled by default, so users have to flip one switch only (-spice gl=on) to enable 3d, not two. So warning here is a bad thing IMO.

We could turn the virgl switch into tristate (OnOffAuto), have it default to Auto, then fail (not warn) in case it is set to On without 3d support being available.

Quoted 4 lines

We could turn the virgl switch into tristate (OnOffAuto), have it default to Auto, then fail (not warn) in case it is set to On without 3d support being available.

Property settings are part of the migration format, right? Is there an easy way to change the 'virgl' property like that in a backwards compatible way? Or decouple the command line handling from the Property value?