[PATCH 0/3] virtio-gpu-gl: Improve OpenGL and related display error messages

[PATCH 0/3] virtio-gpu-gl: Improve OpenGL and related display error messages

[Peter Maydell]

If you try to use the virtio-gpu-gl device, the error messages
we produce are not very specific, which can leave users confused
about what they need to do (see the issue
https://gitlab.com/qemu-project/qemu/-/issues/2443 for an example).

This patchset tries to improve those errors:

(1) Trying to use virtio-gpu-gl without enabling GL on the
display backend:

$ qemu-system-aarch64 -M virt -device virtio-gpu-gl

Currently we say
  qemu-system-aarch64: -device virtio-gpu-gl: opengl is not available
With these patches we will say
  qemu-system-aarch64: -device virtio-gpu-gl: The display backend does not have OpenGL support enabled
  It can be enabled with '-display BACKEND,gl=on' where BACKEND is the name of the display backend to use.

(2) Trying to enable GL on a backend that doesn't support it:
$ qemu-system-aarch64 -M virt -device virtio-gpu-gl -display curses,gl=on

Currently we say
  qemu-system-aarch64: OpenGL is not supported by the display
With these patches we will say
  qemu-system-aarch64: OpenGL is not supported by display backend 'curses'

(3) Trying to enable GL when the QEMU binary wasn't built with it:

Currently we say
  qemu-system-aarch64: OpenGL support is disabled
With these patches we will say
  qemu-system-aarch64: OpenGL support was disabled when QEMU was compiled

(4) -display help doesn't mention that display types can have suboptions

Currently -display help just prints the list of backends. With these
patches we will also say
  Some display backends support suboptions, which can be set with
     -display backend,option=value,option=value...
  For a short list of the suboptions for each display, see the top-level -help output; more detail is in the documentation.
               
(The ideal for that last would be to be like -device and let you
say "-display gtk,help", but that doesn't work.)


Peter Maydell (3):
  hw/display/virtio-gpu: Improve "opengl is not available" error message
  system/vl.c: Expand OpenGL related errors
  ui/console: Note in '-display help' that some backends support
    suboptions

 hw/display/virtio-gpu-gl.c | 8 +++++++-
 system/vl.c                | 5 +++--
 ui/console.c               | 5 +++++
 3 files changed, 15 insertions(+), 3 deletions(-)

-- 
2.34.1

[PATCH 1/3] hw/display/virtio-gpu: Improve "opengl is not available" error message

[Peter Maydell]

If the user tries to use the virtio-gpu-gl device but the display
backend doesn't have OpenGL support enabled, we currently print a
rather uninformative error message:

$ qemu-system-aarch64 -M virt -device virtio-gpu-gl
qemu-system-aarch64: -device virtio-gpu-gl: opengl is not available

Since OpenGL is not enabled on display frontends by default, users
are quite likely to run into this. Improve the error message to
be more specific and to suggest to the user a path forward.

Note that the case of "user tried to enable OpenGL but the display
backend doesn't handle it" is caught elsewhere first, so we can
assume that isn't the problem:

$ qemu-system-aarch64 -M virt -device virtio-gpu-gl -display curses,gl=on
qemu-system-aarch64: OpenGL is not supported by the display

(Use of error_append_hint() requires us to add an ERRP_GUARD() to
the function, as noted in include/qapi/error.h.)

With this commit we now produce the hopefully more helpful error:
$ ./build/x86/qemu-system-aarch64 -M virt -device virtio-gpu-gl
qemu-system-aarch64: -device virtio-gpu-gl: The display backend does not have OpenGL support enabled
It can be enabled with '-display BACKEND,gl=on' where BACKEND is the name of the display backend to use.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2443
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/display/virtio-gpu-gl.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 952820a4256..49cb61edd23 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -107,6 +107,7 @@ static void virtio_gpu_gl_reset(VirtIODevice *vdev)
 static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp)
 {
     VirtIOGPU *g = VIRTIO_GPU(qdev);
+    ERRP_GUARD();
 
 #if HOST_BIG_ENDIAN
     error_setg(errp, "virgl is not supported on bigendian platforms");
@@ -119,7 +120,12 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp)
     }
 
     if (!display_opengl) {
-        error_setg(errp, "opengl is not available");
+        error_setg(errp,
+                   "The display backend does not have OpenGL support enabled");
+        error_append_hint(errp,
+                          "It can be enabled with '-display BACKEND,gl=on' "
+                          "where BACKEND is the name of the display backend "
+                          "to use.\n");
         return;
     }
 
-- 
2.34.1

Re: [PATCH 1/3] hw/display/virtio-gpu: Improve "opengl is not available" error message

[Philippe Mathieu-Daudé]

On 31/7/24 17:41, Peter Maydell wrote:
> If the user tries to use the virtio-gpu-gl device but the display
> backend doesn't have OpenGL support enabled, we currently print a
> rather uninformative error message:
> 
> $ qemu-system-aarch64 -M virt -device virtio-gpu-gl
> qemu-system-aarch64: -device virtio-gpu-gl: opengl is not available
> 
> Since OpenGL is not enabled on display frontends by default, users
> are quite likely to run into this. Improve the error message to
> be more specific and to suggest to the user a path forward.
> 
> Note that the case of "user tried to enable OpenGL but the display
> backend doesn't handle it" is caught elsewhere first, so we can
> assume that isn't the problem:
> 
> $ qemu-system-aarch64 -M virt -device virtio-gpu-gl -display curses,gl=on
> qemu-system-aarch64: OpenGL is not supported by the display
> 
> (Use of error_append_hint() requires us to add an ERRP_GUARD() to
> the function, as noted in include/qapi/error.h.)
> 
> With this commit we now produce the hopefully more helpful error:
> $ ./build/x86/qemu-system-aarch64 -M virt -device virtio-gpu-gl
> qemu-system-aarch64: -device virtio-gpu-gl: The display backend does not have OpenGL support enabled
> It can be enabled with '-display BACKEND,gl=on' where BACKEND is the name of the display backend to use.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2443
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/display/virtio-gpu-gl.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 1/3] hw/display/virtio-gpu: Improve "opengl is not available" error message

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> If the user tries to use the virtio-gpu-gl device but the display
> backend doesn't have OpenGL support enabled, we currently print a
> rather uninformative error message:
>
> $ qemu-system-aarch64 -M virt -device virtio-gpu-gl
> qemu-system-aarch64: -device virtio-gpu-gl: opengl is not available
>
> Since OpenGL is not enabled on display frontends by default, users
> are quite likely to run into this. Improve the error message to
> be more specific and to suggest to the user a path forward.
>
> Note that the case of "user tried to enable OpenGL but the display
> backend doesn't handle it" is caught elsewhere first, so we can
> assume that isn't the problem:
>
> $ qemu-system-aarch64 -M virt -device virtio-gpu-gl -display curses,gl=on
> qemu-system-aarch64: OpenGL is not supported by the display
>
> (Use of error_append_hint() requires us to add an ERRP_GUARD() to
> the function, as noted in include/qapi/error.h.)
>
> With this commit we now produce the hopefully more helpful error:
> $ ./build/x86/qemu-system-aarch64 -M virt -device virtio-gpu-gl
> qemu-system-aarch64: -device virtio-gpu-gl: The display backend does not have OpenGL support enabled
> It can be enabled with '-display BACKEND,gl=on' where BACKEND is the name of the display backend to use.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2443
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

[PATCH 2/3] system/vl.c: Expand OpenGL related errors

[Peter Maydell]

Expand the OpenGL related error messages we produce for various
"OpenGL not present/not supported" cases, to hopefully guide the
user towards how to fix things.

Now if the user tries to enable GL on a backend that doesn't
support it the error message is a bit more precise:

$ qemu-system-aarch64 -M virt -device virtio-gpu-gl -display curses,gl=on
qemu-system-aarch64: OpenGL is not supported by display backend 'curses'

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 system/vl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/system/vl.c b/system/vl.c
index 9e8f16f1551..213ee6a6a92 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -1973,9 +1973,10 @@ static void qemu_create_early_backends(void)
 
     if (dpy.has_gl && dpy.gl != DISPLAYGL_MODE_OFF && display_opengl == 0) {
 #if defined(CONFIG_OPENGL)
-        error_report("OpenGL is not supported by the display");
+        error_report("OpenGL is not supported by display backend '%s'",
+                     DisplayType_str(dpy.type));
 #else
-        error_report("OpenGL support is disabled");
+        error_report("OpenGL support was disabled when QEMU was compiled");
 #endif
         exit(1);
     }
-- 
2.34.1

Re: [PATCH 2/3] system/vl.c: Expand OpenGL related errors

[Philippe Mathieu-Daudé]

On 31/7/24 17:41, Peter Maydell wrote:
> Expand the OpenGL related error messages we produce for various
> "OpenGL not present/not supported" cases, to hopefully guide the
> user towards how to fix things.
> 
> Now if the user tries to enable GL on a backend that doesn't
> support it the error message is a bit more precise:
> 
> $ qemu-system-aarch64 -M virt -device virtio-gpu-gl -display curses,gl=on
> qemu-system-aarch64: OpenGL is not supported by display backend 'curses'
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   system/vl.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 2/3] system/vl.c: Expand OpenGL related errors

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> Expand the OpenGL related error messages we produce for various
> "OpenGL not present/not supported" cases, to hopefully guide the
> user towards how to fix things.
>
> Now if the user tries to enable GL on a backend that doesn't
> support it the error message is a bit more precise:
>
> $ qemu-system-aarch64 -M virt -device virtio-gpu-gl -display curses,gl=on
> qemu-system-aarch64: OpenGL is not supported by display backend 'curses'
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  system/vl.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/system/vl.c b/system/vl.c
> index 9e8f16f1551..213ee6a6a92 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -1973,9 +1973,10 @@ static void qemu_create_early_backends(void)
>  
>      if (dpy.has_gl && dpy.gl != DISPLAYGL_MODE_OFF && display_opengl == 0) {
>  #if defined(CONFIG_OPENGL)
> -        error_report("OpenGL is not supported by the display");
> +        error_report("OpenGL is not supported by display backend '%s'",
> +                     DisplayType_str(dpy.type));
>  #else
> -        error_report("OpenGL support is disabled");
> +        error_report("OpenGL support was disabled when QEMU was
>      compiled");

Maybe "OpenGL support was not enabled in this build of QEMU"?

Anyway:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>



>  #endif
>          exit(1);
>      }

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 2/3] system/vl.c: Expand OpenGL related errors

[Antonio Caggiano]

Hi

On 06/08/2024 15:41, Alex Bennée wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
> 
>> Expand the OpenGL related error messages we produce for various
>> "OpenGL not present/not supported" cases, to hopefully guide the
>> user towards how to fix things.
>>
>> Now if the user tries to enable GL on a backend that doesn't
>> support it the error message is a bit more precise:
>>
>> $ qemu-system-aarch64 -M virt -device virtio-gpu-gl -display curses,gl=on
>> qemu-system-aarch64: OpenGL is not supported by display backend 'curses'
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>   system/vl.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/system/vl.c b/system/vl.c
>> index 9e8f16f1551..213ee6a6a92 100644
>> --- a/system/vl.c
>> +++ b/system/vl.c
>> @@ -1973,9 +1973,10 @@ static void qemu_create_early_backends(void)
>>   
>>       if (dpy.has_gl && dpy.gl != DISPLAYGL_MODE_OFF && display_opengl == 0) {
>>   #if defined(CONFIG_OPENGL)
>> -        error_report("OpenGL is not supported by the display");
>> +        error_report("OpenGL is not supported by display backend '%s'",
>> +                     DisplayType_str(dpy.type));
>>   #else
>> -        error_report("OpenGL support is disabled");
>> +        error_report("OpenGL support was disabled when QEMU was
>>       compiled");
> 
> Maybe "OpenGL support was not enabled in this build of QEMU"?

To stay aligned with the other changes, this can provide even more 
details with another hint.

"OpenGL support was disabled at QEMU configuration time."
"It can be enabled by running `./configure --enable-opengl` before 
compilation"


BTW, whole series:
Reviewed-by: Antonio Caggiano <quic_acaggian@quicinc.com>

Cheers,
Antonio

> 
> Anyway:
> 
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> 
> 
> 
>>   #endif
>>           exit(1);
>>       }
> 

Re: [PATCH 2/3] system/vl.c: Expand OpenGL related errors

[Philippe Mathieu-Daudé]

On 6/8/24 17:20, Antonio Caggiano wrote:
> Hi
> 
> On 06/08/2024 15:41, Alex Bennée wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>>> Expand the OpenGL related error messages we produce for various
>>> "OpenGL not present/not supported" cases, to hopefully guide the
>>> user towards how to fix things.
>>>
>>> Now if the user tries to enable GL on a backend that doesn't
>>> support it the error message is a bit more precise:
>>>
>>> $ qemu-system-aarch64 -M virt -device virtio-gpu-gl -display 
>>> curses,gl=on
>>> qemu-system-aarch64: OpenGL is not supported by display backend 'curses'
>>>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>>>   system/vl.c | 5 +++--
>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/system/vl.c b/system/vl.c
>>> index 9e8f16f1551..213ee6a6a92 100644
>>> --- a/system/vl.c
>>> +++ b/system/vl.c
>>> @@ -1973,9 +1973,10 @@ static void qemu_create_early_backends(void)
>>>       if (dpy.has_gl && dpy.gl != DISPLAYGL_MODE_OFF && 
>>> display_opengl == 0) {
>>>   #if defined(CONFIG_OPENGL)
>>> -        error_report("OpenGL is not supported by the display");
>>> +        error_report("OpenGL is not supported by display backend '%s'",
>>> +                     DisplayType_str(dpy.type));
>>>   #else
>>> -        error_report("OpenGL support is disabled");
>>> +        error_report("OpenGL support was disabled when QEMU was
>>>       compiled");
>>
>> Maybe "OpenGL support was not enabled in this build of QEMU"?
> 
> To stay aligned with the other changes, this can provide even more 
> details with another hint.
> 
> "OpenGL support was disabled at QEMU configuration time."
> "It can be enabled by running `./configure --enable-opengl` before 
> compilation"
> 
> 
> BTW, whole series:
> Reviewed-by: Antonio Caggiano <quic_acaggian@quicinc.com>

Thanks Antonio. I already posted the pull request and it is
being tested. I'll include your R-b tag if there is an issue
and I need to repost it.

> 
> Cheers,
> Antonio
> 
>>
>> Anyway:
>>
>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>>
>>
>>
>>>   #endif
>>>           exit(1);
>>>       }
>>
> 

[PATCH 3/3] ui/console: Note in '-display help' that some backends support suboptions

[Peter Maydell]

Currently '-display help' only prints the available backends. Some
of those backends support suboptions (e.g. '-display gtk,gl=on').
Mention that in the help output, and point the user to where they
might be able to find more information about the suboptions.
The new output looks like this:

$ qemu-system-aarch64 -display help
Available display backend types:
none
gtk
sdl
egl-headless
curses
spice-app
dbus

Some display backends support suboptions, which can be set with
   -display backend,option=value,option=value...
For a short list of the suboptions for each display, see the top-level -help output; more detail is in the documentation.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
The ideal here would be to programmatically list all the
suboptions, so you could say '-display gtk,help', but that
seems like a lot of work and I'm not entirely sure how
to do it :-)
---
 ui/console.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/ui/console.c b/ui/console.c
index e8f0083af71..105a0e2c708 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1632,4 +1632,9 @@ void qemu_display_help(void)
             printf("%s\n",  DisplayType_str(dpys[idx]->type));
         }
     }
+    printf("\n"
+           "Some display backends support suboptions, which can be set with\n"
+           "   -display backend,option=value,option=value...\n"
+           "For a short list of the suboptions for each display, see the "
+           "top-level -help output; more detail is in the documentation.\n");
 }
-- 
2.34.1

Re: [PATCH 3/3] ui/console: Note in '-display help' that some backends support suboptions

[Philippe Mathieu-Daudé]

On 31/7/24 17:41, Peter Maydell wrote:
> Currently '-display help' only prints the available backends. Some
> of those backends support suboptions (e.g. '-display gtk,gl=on').
> Mention that in the help output, and point the user to where they
> might be able to find more information about the suboptions.
> The new output looks like this:
> 
> $ qemu-system-aarch64 -display help
> Available display backend types:
> none
> gtk
> sdl
> egl-headless
> curses
> spice-app
> dbus
> 
> Some display backends support suboptions, which can be set with
>     -display backend,option=value,option=value...
> For a short list of the suboptions for each display, see the top-level -help output; more detail is in the documentation.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> The ideal here would be to programmatically list all the
> suboptions, so you could say '-display gtk,help', but that
> seems like a lot of work and I'm not entirely sure how
> to do it :-)
> ---
>   ui/console.c | 5 +++++
>   1 file changed, 5 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 0/3] virtio-gpu-gl: Improve OpenGL and related display error messages

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 2588 bytes --]

On Wed, Jul 31, 2024 at 7:42 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> If you try to use the virtio-gpu-gl device, the error messages
> we produce are not very specific, which can leave users confused
> about what they need to do (see the issue
> https://gitlab.com/qemu-project/qemu/-/issues/2443 for an example).
>
> This patchset tries to improve those errors:
>
> (1) Trying to use virtio-gpu-gl without enabling GL on the
> display backend:
>
> $ qemu-system-aarch64 -M virt -device virtio-gpu-gl
>
> Currently we say
>   qemu-system-aarch64: -device virtio-gpu-gl: opengl is not available
> With these patches we will say
>   qemu-system-aarch64: -device virtio-gpu-gl: The display backend does not
> have OpenGL support enabled
>   It can be enabled with '-display BACKEND,gl=on' where BACKEND is the
> name of the display backend to use.
>
> (2) Trying to enable GL on a backend that doesn't support it:
> $ qemu-system-aarch64 -M virt -device virtio-gpu-gl -display curses,gl=on
>
> Currently we say
>   qemu-system-aarch64: OpenGL is not supported by the display
> With these patches we will say
>   qemu-system-aarch64: OpenGL is not supported by display backend 'curses'
>
> (3) Trying to enable GL when the QEMU binary wasn't built with it:
>
> Currently we say
>   qemu-system-aarch64: OpenGL support is disabled
> With these patches we will say
>   qemu-system-aarch64: OpenGL support was disabled when QEMU was compiled
>
> (4) -display help doesn't mention that display types can have suboptions
>
> Currently -display help just prints the list of backends. With these
> patches we will also say
>   Some display backends support suboptions, which can be set with
>      -display backend,option=value,option=value...
>   For a short list of the suboptions for each display, see the top-level
> -help output; more detail is in the documentation.
>
> (The ideal for that last would be to be like -device and let you
> say "-display gtk,help", but that doesn't work.)
>
>
> Peter Maydell (3):
>   hw/display/virtio-gpu: Improve "opengl is not available" error message
>   system/vl.c: Expand OpenGL related errors
>   ui/console: Note in '-display help' that some backends support
>     suboptions
>
>  hw/display/virtio-gpu-gl.c | 8 +++++++-
>  system/vl.c                | 5 +++--
>  ui/console.c               | 5 +++++
>  3 files changed, 15 insertions(+), 3 deletions(-)
>
> --
> 2.34.1
>
>
Series:
Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>


-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 3414 bytes --]

Re: [PATCH 0/3] virtio-gpu-gl: Improve OpenGL and related display error messages

[Philippe Mathieu-Daudé]

On 31/7/24 17:59, Marc-André Lureau wrote:

> Series:
> Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com 
> <mailto:marcandre.lureau@redhat.com>>

I'll queue this via my hw-misc tree, thanks.