[Qemu-devel] [PATCH] sdl: improve error message on fatal error

[Qemu-devel] [PATCH] sdl: improve error message on fatal error

[Bjørn Mork]

The SDL_SetVideoMode() error condition is easily triggered by a user by
simply configure a guest with a host unsupported display resolution
and attempting to enable fullscreen.  Since the error is fatal, adding
a bit of debugging help can't harm.

Sample output with this change:

 (qemu) Could not open SDL display (1280x1024x32): No video mode large enough for 1280x1024

The width x height might seem redundant as SDL also provides it in
SDL_GetError(), but I believe there are situations where it is
useful.  I.e. if there is some other SDL error.  Anyway, redundant
information in fatal error messages has never harmed a single gerbil.

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 sdl.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/sdl.c b/sdl.c
index f26035c..34061c0 100644
--- a/sdl.c
+++ b/sdl.c
@@ -112,7 +112,8 @@ static void do_sdl_resize(int new_width, int new_height, int bpp)
     height = new_height;
     real_screen = SDL_SetVideoMode(width, height, bpp, flags);
     if (!real_screen) {
-        fprintf(stderr, "Could not open SDL display\n");
+	fprintf(stderr, "Could not open SDL display (%dx%dx%d): %s\n", width, 
+		height, bpp, SDL_GetError());
         exit(1);
     }
 }
-- 
1.5.6.5

Re: [Qemu-devel] [PATCH] sdl: improve error message on fatal error

[Stefan Weil]

Bjørn Mork schrieb:
> The SDL_SetVideoMode() error condition is easily triggered by a user by
> simply configure a guest with a host unsupported display resolution
> and attempting to enable fullscreen. Since the error is fatal, adding
> a bit of debugging help can't harm.
>
> Sample output with this change:
>
> (qemu) Could not open SDL display (1280x1024x32): No video mode large
> enough for 1280x1024
>
> The width x height might seem redundant as SDL also provides it in
> SDL_GetError(), but I believe there are situations where it is
> useful. I.e. if there is some other SDL error. Anyway, redundant
> information in fatal error messages has never harmed a single gerbil.
>
> Signed-off-by: Bjørn Mork <bjorn@mork.no>
> ---
> sdl.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/sdl.c b/sdl.c
> index f26035c..34061c0 100644
> --- a/sdl.c
> +++ b/sdl.c
> @@ -112,7 +112,8 @@ static void do_sdl_resize(int new_width, int
> new_height, int bpp)
> height = new_height;
> real_screen = SDL_SetVideoMode(width, height, bpp, flags);
> if (!real_screen) {
> - fprintf(stderr, "Could not open SDL display\n");
> + fprintf(stderr, "Could not open SDL display (%dx%dx%d): %s\n", width,
> + height, bpp, SDL_GetError());
> exit(1);
> }
> }

Yesterday I stumbled across this problem, too:

QEMU was running a guest with 1024x768 resolution on a netbook
(which has a lower physical resolution).

Switching to fullscreen mode results in "Could not open SDL display\n"
and QEMU terminates which is not good.

The message is wrong because the SDL display was already opened:
"Could not set SDL video mode (%dx%dx%d): %s\n" would be better.

It would be even better if QEMU did not try to switch to an
unsupported video mode. It could either
(a) refuse an unsupported fullscreen mode (stay in window mode) or
(b) scale the display so it fits in the largest supported mode

Of course, as a first step an improved error message would help
a little, too.

Regards,
Stefan

Re: [Qemu-devel] [PATCH] sdl: improve error message on fatal error

[Anthony Liguori]

On 03/08/2010 06:07 AM, Bjørn Mork wrote:
> The SDL_SetVideoMode() error condition is easily triggered by a user by
> simply configure a guest with a host unsupported display resolution
> and attempting to enable fullscreen.  Since the error is fatal, adding
> a bit of debugging help can't harm.
>
> Sample output with this change:
>
>   (qemu) Could not open SDL display (1280x1024x32): No video mode large enough for 1280x1024
>
> The width x height might seem redundant as SDL also provides it in
> SDL_GetError(), but I believe there are situations where it is
> useful.  I.e. if there is some other SDL error.  Anyway, redundant
> information in fatal error messages has never harmed a single gerbil.
>
> Signed-off-by: Bjørn Mork<bjorn@mork.no>
>    
Applied.  Thanks.

Regards,

Anthony Liguori
> ---
>   sdl.c |    3 ++-
>   1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/sdl.c b/sdl.c
> index f26035c..34061c0 100644
> --- a/sdl.c
> +++ b/sdl.c
> @@ -112,7 +112,8 @@ static void do_sdl_resize(int new_width, int new_height, int bpp)
>       height = new_height;
>       real_screen = SDL_SetVideoMode(width, height, bpp, flags);
>       if (!real_screen) {
> -        fprintf(stderr, "Could not open SDL display\n");
> +	fprintf(stderr, "Could not open SDL display (%dx%dx%d): %s\n", width,
> +		height, bpp, SDL_GetError());
>           exit(1);
>       }
>   }
>