[v2] ui: dbus & misc fixes

[PATCH v2 14/18] ui/sdl: add optional logging when _SDL_DEBUG is set

Posted by marcandre.lureau@redhat.com 2 years, 4 months ago

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Apparently, there is no environment variable you can set for libsdl to
enable logging.

(similar to _VNC_DEBUG)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/ui/sdl2.h | 2 ++
 ui/sdl2.c         | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
index 8fb7e08262..6fea36db82 100644
--- a/include/ui/sdl2.h
+++ b/include/ui/sdl2.h
@@ -6,6 +6,8 @@
 
 #include <SDL.h>
 
+/* #define _SDL_DEBUG 1 */
+
 /* with Alpine / muslc SDL headers pull in directfb headers
  * which in turn trigger warning about redundant decls for
  * direct_waitqueue_deinit.
diff --git a/ui/sdl2.c b/ui/sdl2.c
index f259e4c4d1..592eca3e1c 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -841,6 +841,10 @@ static void sdl2_display_init(DisplayState *ds, DisplayOptions *o)
     }
 #endif
 
+#ifdef _SDL_DEBUG
+    SDL_LogSetAllPriority(SDL_LOG_PRIORITY_VERBOSE);
+#endif
+
     if (SDL_Init(SDL_INIT_VIDEO)) {
         fprintf(stderr, "Could not initialize SDL(%s) - exiting\n",
                 SDL_GetError());
-- 
2.39.2

Re: [PATCH v2 14/18] ui/sdl: add optional logging when _SDL_DEBUG is set

Posted by Philippe Mathieu-Daudé 2 years, 4 months ago

On 7/3/23 12:56, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Apparently, there is no environment variable you can set for libsdl to
> enable logging.

Why not use getenv() in QEMU then?

> (similar to _VNC_DEBUG)
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   include/ui/sdl2.h | 2 ++
>   ui/sdl2.c         | 4 ++++
>   2 files changed, 6 insertions(+)
> 
> diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
> index 8fb7e08262..6fea36db82 100644
> --- a/include/ui/sdl2.h
> +++ b/include/ui/sdl2.h
> @@ -6,6 +6,8 @@
>   
>   #include <SDL.h>
>   
> +/* #define _SDL_DEBUG 1 */
> +
>   /* with Alpine / muslc SDL headers pull in directfb headers
>    * which in turn trigger warning about redundant decls for
>    * direct_waitqueue_deinit.
> diff --git a/ui/sdl2.c b/ui/sdl2.c
> index f259e4c4d1..592eca3e1c 100644
> --- a/ui/sdl2.c
> +++ b/ui/sdl2.c
> @@ -841,6 +841,10 @@ static void sdl2_display_init(DisplayState *ds, DisplayOptions *o)
>       }
>   #endif
>   
> +#ifdef _SDL_DEBUG
> +    SDL_LogSetAllPriority(SDL_LOG_PRIORITY_VERBOSE);
> +#endif
> +
>       if (SDL_Init(SDL_INIT_VIDEO)) {
>           fprintf(stderr, "Could not initialize SDL(%s) - exiting\n",
>                   SDL_GetError());

Re: [PATCH v2 14/18] ui/sdl: add optional logging when _SDL_DEBUG is set

Posted by Marc-André Lureau 2 years, 4 months ago

Hi

(adding Sam Lantinga, SDL maintainer, in CC. I noticed he worked on QEMU
SDL backend yesterday! ;)

On Thu, Mar 9, 2023 at 8:40 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> On 7/3/23 12:56, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Apparently, there is no environment variable you can set for libsdl to
> > enable logging.
>
> Why not use getenv() in QEMU then?
>

QEMU has few environment variables. We generally prefer qmp/cmd arguments.
And that change does not fit with QEMU tracing etc.



> > (similar to _VNC_DEBUG)
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >   include/ui/sdl2.h | 2 ++
> >   ui/sdl2.c         | 4 ++++
> >   2 files changed, 6 insertions(+)
> >
> > diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
> > index 8fb7e08262..6fea36db82 100644
> > --- a/include/ui/sdl2.h
> > +++ b/include/ui/sdl2.h
> > @@ -6,6 +6,8 @@
> >
> >   #include <SDL.h>
> >
> > +/* #define _SDL_DEBUG 1 */
> > +
> >   /* with Alpine / muslc SDL headers pull in directfb headers
> >    * which in turn trigger warning about redundant decls for
> >    * direct_waitqueue_deinit.
> > diff --git a/ui/sdl2.c b/ui/sdl2.c
> > index f259e4c4d1..592eca3e1c 100644
> > --- a/ui/sdl2.c
> > +++ b/ui/sdl2.c
> > @@ -841,6 +841,10 @@ static void sdl2_display_init(DisplayState *ds,
> DisplayOptions *o)
> >       }
> >   #endif
> >
> > +#ifdef _SDL_DEBUG
> > +    SDL_LogSetAllPriority(SDL_LOG_PRIORITY_VERBOSE);
> > +#endif
>

Sam, do you suggest a better way to enable SDL debugging when starting
QEMU? Is there a way to enable it with existing SDL environment variables?

thanks

> +
> >       if (SDL_Init(SDL_INIT_VIDEO)) {
> >           fprintf(stderr, "Could not initialize SDL(%s) - exiting\n",
> >                   SDL_GetError());
>
>

Re: [PATCH v2 14/18] ui/sdl: add optional logging when _SDL_DEBUG is set

Posted by Sam Lantinga 2 years, 4 months ago

If you wanted, you could use the SDL hint system, e.g.

if (SDL_GetHintBoolean("QEMU_ENABLE_SDL_LOGGING", SDL_FALSE)) {
    SDL_LogSetAllPriority(SDL_LOG_PRIORITY_VERBOSE);
}

Then someone can set that environment variable and it would be using the
SDL mechanisms rather than intruding on QEMU... or, if you want tighter
integration, you can set an SDL log handler that feeds into the QEMU
tracing system.

On Thu, Mar 9, 2023 at 9:18 PM Marc-André Lureau <
marcandre.lureau@redhat.com> wrote:

> Hi
>
> (adding Sam Lantinga, SDL maintainer, in CC. I noticed he worked on QEMU
> SDL backend yesterday! ;)
>
> On Thu, Mar 9, 2023 at 8:40 PM Philippe Mathieu-Daudé <philmd@linaro.org>
> wrote:
>
>> On 7/3/23 12:56, marcandre.lureau@redhat.com wrote:
>> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
>> >
>> > Apparently, there is no environment variable you can set for libsdl to
>> > enable logging.
>>
>> Why not use getenv() in QEMU then?
>>
>
> QEMU has few environment variables. We generally prefer qmp/cmd arguments.
> And that change does not fit with QEMU tracing etc.
>
>
>
>> > (similar to _VNC_DEBUG)
>> >
>> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> > ---
>> >   include/ui/sdl2.h | 2 ++
>> >   ui/sdl2.c         | 4 ++++
>> >   2 files changed, 6 insertions(+)
>> >
>> > diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
>> > index 8fb7e08262..6fea36db82 100644
>> > --- a/include/ui/sdl2.h
>> > +++ b/include/ui/sdl2.h
>> > @@ -6,6 +6,8 @@
>> >
>> >   #include <SDL.h>
>> >
>> > +/* #define _SDL_DEBUG 1 */
>> > +
>> >   /* with Alpine / muslc SDL headers pull in directfb headers
>> >    * which in turn trigger warning about redundant decls for
>> >    * direct_waitqueue_deinit.
>> > diff --git a/ui/sdl2.c b/ui/sdl2.c
>> > index f259e4c4d1..592eca3e1c 100644
>> > --- a/ui/sdl2.c
>> > +++ b/ui/sdl2.c
>> > @@ -841,6 +841,10 @@ static void sdl2_display_init(DisplayState *ds,
>> DisplayOptions *o)
>> >       }
>> >   #endif
>> >
>> > +#ifdef _SDL_DEBUG
>> > +    SDL_LogSetAllPriority(SDL_LOG_PRIORITY_VERBOSE);
>> > +#endif
>>
>
> Sam, do you suggest a better way to enable SDL debugging when starting
> QEMU? Is there a way to enable it with existing SDL environment variables?
>
> thanks
>
> > +
>> >       if (SDL_Init(SDL_INIT_VIDEO)) {
>> >           fprintf(stderr, "Could not initialize SDL(%s) - exiting\n",
>> >                   SDL_GetError());
>>
>>