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 <address@hidden>
> ---
> 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