[Qemu-devel] [PATCH for 2.8 0/3] Modularize SDL

[Qemu-devel] [PATCH for 2.8 0/3] Modularize SDL

[Fam Zheng]

The new module is named ui-sdl.so, following the name scheme of existing
block-iscsi.so, etc..

Fam Zheng (3):
  rules.mak: Don't extract libs from .mo-libs in link command
  configure: Add CONFIG_SDL2
  sdl: Modularize

 Makefile.objs         |  1 +
 configure             |  8 ++++--
 include/qemu/module.h |  2 ++
 include/ui/console.h  |  5 ++--
 rules.mak             |  2 +-
 ui/Makefile.objs      |  2 ++
 ui/sdl-init.c         | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++
 ui/sdl.c              | 19 ++++++-------
 ui/sdl2.c             | 26 ++++++------------
 util/module.c         |  6 +++++
 vl.c                  |  5 +++-
 11 files changed, 116 insertions(+), 35 deletions(-)
 create mode 100644 ui/sdl-init.c

-- 
2.7.4

[Qemu-devel] [PATCH for 2.8 1/3] rules.mak: Don't extract libs from .mo-libs in link command

[Fam Zheng]

For module build, .mo objects are passed to LINK and consumed in
process-archive-undefs. The reason behind that is documented in the
comment above process-archive-undefs.

Similarly, extract-libs should be called with .mo filtered out too.
Otherwise, the .mo-libs are added to the link command incorrectly,
spoiling the purpose of modularization.

Currently we don't have any .mo-libs usage, but it will be used soon
when we modularize more multi-source objects, like sdl and gtk.

Reported-by: Colin Lord <clord@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 rules.mak | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rules.mak b/rules.mak
index ed8e482..80705a7 100644
--- a/rules.mak
+++ b/rules.mak
@@ -50,7 +50,7 @@ process-archive-undefs = $(filter-out %.a %.mo,$1) \
                               $(call undefined-symbols,$(filter %.mo,$1)))) \
                 $(filter %.a,$1)
 
-extract-libs = $(strip $(foreach o,$1,$($o-libs)))
+extract-libs = $(strip $(foreach o,$(filter-out %.mo,$1),$($o-libs)))
 expand-objs = $(strip $(sort $(filter %.o,$1)) \
                   $(foreach o,$(filter %.mo,$1),$($o-objs)) \
                   $(filter-out %.o %.mo,$1))
-- 
2.7.4

[Qemu-devel] [PATCH for 2.8 2/3] configure: Add CONFIG_SDL2

[Fam Zheng]

C code can test ABI version with this macro.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 configure | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/configure b/configure
index 879324b..e04e59f 100755
--- a/configure
+++ b/configure
@@ -5069,6 +5069,10 @@ if test "$sdl" = "yes" ; then
   echo "CONFIG_SDLABI=$sdlabi" >> $config_host_mak
   echo "SDL_CFLAGS=$sdl_cflags" >> $config_host_mak
 fi
+if test "$sdlabi" = "2.0"; then
+  echo "CONFIG_SDL2=y" >> $config_host_mak
+fi
+
 if test "$cocoa" = "yes" ; then
   echo "CONFIG_COCOA=y" >> $config_host_mak
 fi
-- 
2.7.4

[Qemu-devel] [PATCH for 2.8 3/3] sdl: Modularize

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 Makefile.objs         |  1 +
 configure             |  4 +--
 include/qemu/module.h |  2 ++
 include/ui/console.h  |  5 ++--
 ui/Makefile.objs      |  2 ++
 ui/sdl-init.c         | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++
 ui/sdl.c              | 19 ++++++-------
 ui/sdl2.c             | 26 ++++++------------
 util/module.c         |  6 +++++
 vl.c                  |  5 +++-
 10 files changed, 111 insertions(+), 34 deletions(-)
 create mode 100644 ui/sdl-init.c

diff --git a/Makefile.objs b/Makefile.objs
index 6d5ddcf..08c5746 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -62,6 +62,7 @@ common-obj-y += accel.o
 common-obj-y += replay/
 
 common-obj-y += ui/
+common-obj-m += ui/
 common-obj-y += bt-host.o bt-vhci.o
 bt-host.o-cflags := $(BLUEZ_CFLAGS)
 
diff --git a/configure b/configure
index e04e59f..1b9be8f 100755
--- a/configure
+++ b/configure
@@ -2533,7 +2533,6 @@ EOF
     sdl_cflags="$sdl_cflags $x11_cflags"
     sdl_libs="$sdl_libs $x11_libs"
   fi
-  libs_softmmu="$sdl_libs $libs_softmmu"
 fi
 
 ##########################################
@@ -5065,9 +5064,10 @@ if test "$modules" = "yes"; then
   echo "CONFIG_MODULES=y" >> $config_host_mak
 fi
 if test "$sdl" = "yes" ; then
-  echo "CONFIG_SDL=y" >> $config_host_mak
+  echo "CONFIG_SDL=m" >> $config_host_mak
   echo "CONFIG_SDLABI=$sdlabi" >> $config_host_mak
   echo "SDL_CFLAGS=$sdl_cflags" >> $config_host_mak
+  echo "SDL_LIBS=$sdl_libs" >> $config_host_mak
 fi
 if test "$sdlabi" = "2.0"; then
   echo "CONFIG_SDL2=y" >> $config_host_mak
diff --git a/include/qemu/module.h b/include/qemu/module.h
index 2370708..f5e012b 100644
--- a/include/qemu/module.h
+++ b/include/qemu/module.h
@@ -44,6 +44,7 @@ typedef enum {
     MODULE_INIT_OPTS,
     MODULE_INIT_QAPI,
     MODULE_INIT_QOM,
+    MODULE_INIT_SDL,
     MODULE_INIT_MAX
 } module_init_type;
 
@@ -51,6 +52,7 @@ typedef enum {
 #define opts_init(function) module_init(function, MODULE_INIT_OPTS)
 #define qapi_init(function) module_init(function, MODULE_INIT_QAPI)
 #define type_init(function) module_init(function, MODULE_INIT_QOM)
+#define sdl_init(function) module_init(function, MODULE_INIT_SDL)
 
 void register_module_init(void (*fn)(void), module_init_type type);
 void register_dso_module_init(void (*fn)(void), module_init_type type);
diff --git a/include/ui/console.h b/include/ui/console.h
index 2703a3a..1d9b0bb 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -425,10 +425,11 @@ void surface_gl_setup_viewport(ConsoleGLState *gls,
 
 /* sdl.c */
 #ifdef CONFIG_SDL
-void sdl_display_early_init(int opengl);
+bool sdl_display_early_init(int opengl);
 void sdl_display_init(DisplayState *ds, int full_screen, int no_frame);
+void sdl_register_init_fun(void *fn);
 #else
-static inline void sdl_display_early_init(int opengl)
+static inline bool sdl_display_early_init(int opengl)
 {
     /* This must never be called if CONFIG_SDL is disabled */
     error_report("SDL support is disabled");
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index dc936f1..0b82650 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -26,7 +26,9 @@ ifeq ($(CONFIG_OPENGL),y)
 sdl.mo-objs += sdl2-gl.o
 endif
 endif
+common-obj-y += sdl-init.o
 sdl.mo-cflags := $(SDL_CFLAGS)
+sdl.mo-libs := $(SDL_LIBS)
 
 ifeq ($(CONFIG_OPENGL),y)
 common-obj-y += shader.o
diff --git a/ui/sdl-init.c b/ui/sdl-init.c
new file mode 100644
index 0000000..6a780a4
--- /dev/null
+++ b/ui/sdl-init.c
@@ -0,0 +1,75 @@
+/*
+ * QEMU SDL display driver init function
+ *
+ * Copyright (c) 2003 Fabrice Bellard
+ * Copyright (c) 2016 Red Hat, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "ui/console.h"
+#include "qemu/module.h"
+
+static void (*init_fn)(DisplayState *ds, int full_screen, int no_frame);
+void sdl_register_init_fun(void *fn)
+{
+    assert(!init_fn);
+    init_fn = fn;
+}
+
+bool sdl_display_early_init(int opengl)
+{
+
+#ifdef CONFIG_SDL2
+    switch (opengl) {
+    case -1: /* default */
+    case 0:  /* off */
+        break;
+    case 1: /* on */
+#ifdef CONFIG_OPENGL
+        display_opengl = 1;
+#endif
+        break;
+    default:
+        g_assert_not_reached();
+        break;
+    }
+#else
+    if (opengl == 1 /* on */) {
+        fprintf(stderr,
+                "SDL1 display code has no opengl support.\n"
+                "Please recompile qemu with SDL2, using\n"
+                "./configure --enable-sdl --with-sdlabi=2.0\n");
+        /* XXX: Should we return false here? */
+    }
+#endif
+
+    module_call_init(MODULE_INIT_SDL);
+    if (!init_fn) {
+        return false;
+    }
+    return true;
+}
+
+void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
+{
+    assert(init_fn);
+    init_fn(ds, full_screen, no_frame);
+}
diff --git a/ui/sdl.c b/ui/sdl.c
index d8cf5bc..a30f442 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -932,17 +932,7 @@ static const DisplayChangeListenerOps dcl_ops = {
     .dpy_cursor_define    = sdl_mouse_define,
 };
 
-void sdl_display_early_init(int opengl)
-{
-    if (opengl == 1 /* on */) {
-        fprintf(stderr,
-                "SDL1 display code has no opengl support.\n"
-                "Please recompile qemu with SDL2, using\n"
-                "./configure --enable-sdl --with-sdlabi=2.0\n");
-    }
-}
-
-void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
+static void sdl_display_init_do(DisplayState *ds, int full_screen, int no_frame)
 {
     int flags;
     uint8_t data = 0;
@@ -1025,3 +1015,10 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
 
     atexit(sdl_cleanup);
 }
+
+static void sdl_init_fn(void)
+{
+    sdl_register_init_fun(sdl_display_init_do);
+}
+
+sdl_init(sdl_init_fn);
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 30d2a3c..c2b4049 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -738,24 +738,7 @@ static const DisplayChangeListenerOps dcl_gl_ops = {
 };
 #endif
 
-void sdl_display_early_init(int opengl)
-{
-    switch (opengl) {
-    case -1: /* default */
-    case 0:  /* off */
-        break;
-    case 1: /* on */
-#ifdef CONFIG_OPENGL
-        display_opengl = 1;
-#endif
-        break;
-    default:
-        g_assert_not_reached();
-        break;
-    }
-}
-
-void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
+static void sdl_display_init_do(DisplayState *ds, int full_screen, int no_frame)
 {
     int flags;
     uint8_t data = 0;
@@ -842,3 +825,10 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
 
     atexit(sdl_cleanup);
 }
+
+static void sdl_init_fn(void)
+{
+    sdl_register_init_fun(sdl_display_init_do);
+}
+
+sdl_init(sdl_init_fn);
diff --git a/util/module.c b/util/module.c
index 86e3f7a..b95d4fa 100644
--- a/util/module.c
+++ b/util/module.c
@@ -167,6 +167,9 @@ static void module_load(module_init_type type)
     static const char *block_modules[] = {
         CONFIG_BLOCK_MODULES
     };
+    static const char *sdl_modules[] = {
+        "ui-sdl",
+    };
     char *exec_dir;
     char *dirs[3];
     int i = 0;
@@ -181,6 +184,9 @@ static void module_load(module_init_type type)
     case MODULE_INIT_BLOCK:
         mp = block_modules;
         break;
+    case MODULE_INIT_SDL:
+        mp = sdl_modules;
+        break;
     default:
         /* no other types have dynamic modules for now*/
         return;
diff --git a/vl.c b/vl.c
index a455947..fdbbe47 100644
--- a/vl.c
+++ b/vl.c
@@ -4221,7 +4221,10 @@ int main(int argc, char **argv, char **envp)
     }
 
     if (display_type == DT_SDL) {
-        sdl_display_early_init(request_opengl);
+        if (!sdl_display_early_init(request_opengl)) {
+            error_report("Failed to initialize SDL");
+            exit(1);
+        }
     }
 
     if (request_opengl == 1 && display_opengl == 0) {
-- 
2.7.4

Re: [Qemu-devel] [PATCH for 2.8 3/3] sdl: Modularize

[Colin Lord]

On 07/27/2016 02:26 AM, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  Makefile.objs         |  1 +
>  configure             |  4 +--
>  include/qemu/module.h |  2 ++
>  include/ui/console.h  |  5 ++--
>  ui/Makefile.objs      |  2 ++
>  ui/sdl-init.c         | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  ui/sdl.c              | 19 ++++++-------
>  ui/sdl2.c             | 26 ++++++------------
>  util/module.c         |  6 +++++
>  vl.c                  |  5 +++-
>  10 files changed, 111 insertions(+), 34 deletions(-)
>  create mode 100644 ui/sdl-init.c
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index 6d5ddcf..08c5746 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -62,6 +62,7 @@ common-obj-y += accel.o
>  common-obj-y += replay/
>  
>  common-obj-y += ui/
> +common-obj-m += ui/
>  common-obj-y += bt-host.o bt-vhci.o
>  bt-host.o-cflags := $(BLUEZ_CFLAGS)
>  
> diff --git a/configure b/configure
> index e04e59f..1b9be8f 100755
> --- a/configure
> +++ b/configure
> @@ -2533,7 +2533,6 @@ EOF
>      sdl_cflags="$sdl_cflags $x11_cflags"
>      sdl_libs="$sdl_libs $x11_libs"
>    fi
> -  libs_softmmu="$sdl_libs $libs_softmmu"
>  fi
>  
>  ##########################################
> @@ -5065,9 +5064,10 @@ if test "$modules" = "yes"; then
>    echo "CONFIG_MODULES=y" >> $config_host_mak
>  fi
>  if test "$sdl" = "yes" ; then
> -  echo "CONFIG_SDL=y" >> $config_host_mak
> +  echo "CONFIG_SDL=m" >> $config_host_mak
>    echo "CONFIG_SDLABI=$sdlabi" >> $config_host_mak
>    echo "SDL_CFLAGS=$sdl_cflags" >> $config_host_mak
> +  echo "SDL_LIBS=$sdl_libs" >> $config_host_mak
>  fi
>  if test "$sdlabi" = "2.0"; then
>    echo "CONFIG_SDL2=y" >> $config_host_mak
> diff --git a/include/qemu/module.h b/include/qemu/module.h
> index 2370708..f5e012b 100644
> --- a/include/qemu/module.h
> +++ b/include/qemu/module.h
> @@ -44,6 +44,7 @@ typedef enum {
>      MODULE_INIT_OPTS,
>      MODULE_INIT_QAPI,
>      MODULE_INIT_QOM,
> +    MODULE_INIT_SDL,
>      MODULE_INIT_MAX
>  } module_init_type;
>  
> @@ -51,6 +52,7 @@ typedef enum {
>  #define opts_init(function) module_init(function, MODULE_INIT_OPTS)
>  #define qapi_init(function) module_init(function, MODULE_INIT_QAPI)
>  #define type_init(function) module_init(function, MODULE_INIT_QOM)
> +#define sdl_init(function) module_init(function, MODULE_INIT_SDL)
>  
>  void register_module_init(void (*fn)(void), module_init_type type);
>  void register_dso_module_init(void (*fn)(void), module_init_type type);
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 2703a3a..1d9b0bb 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -425,10 +425,11 @@ void surface_gl_setup_viewport(ConsoleGLState *gls,
>  
>  /* sdl.c */
>  #ifdef CONFIG_SDL
> -void sdl_display_early_init(int opengl);
> +bool sdl_display_early_init(int opengl);
>  void sdl_display_init(DisplayState *ds, int full_screen, int no_frame);
> +void sdl_register_init_fun(void *fn);
>  #else
> -static inline void sdl_display_early_init(int opengl)
> +static inline bool sdl_display_early_init(int opengl)
>  {
>      /* This must never be called if CONFIG_SDL is disabled */
>      error_report("SDL support is disabled");
> diff --git a/ui/Makefile.objs b/ui/Makefile.objs
> index dc936f1..0b82650 100644
> --- a/ui/Makefile.objs
> +++ b/ui/Makefile.objs
> @@ -26,7 +26,9 @@ ifeq ($(CONFIG_OPENGL),y)
>  sdl.mo-objs += sdl2-gl.o
>  endif
>  endif
> +common-obj-y += sdl-init.o
>  sdl.mo-cflags := $(SDL_CFLAGS)
> +sdl.mo-libs := $(SDL_LIBS)
>  
>  ifeq ($(CONFIG_OPENGL),y)
>  common-obj-y += shader.o
> diff --git a/ui/sdl-init.c b/ui/sdl-init.c
> new file mode 100644
> index 0000000..6a780a4
> --- /dev/null
> +++ b/ui/sdl-init.c
> @@ -0,0 +1,75 @@
> +/*
> + * QEMU SDL display driver init function
> + *
> + * Copyright (c) 2003 Fabrice Bellard
> + * Copyright (c) 2016 Red Hat, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "ui/console.h"
> +#include "qemu/module.h"
> +
> +static void (*init_fn)(DisplayState *ds, int full_screen, int no_frame);
> +void sdl_register_init_fun(void *fn)
> +{
> +    assert(!init_fn);
> +    init_fn = fn;
> +}
> +
> +bool sdl_display_early_init(int opengl)
> +{
> +
> +#ifdef CONFIG_SDL2
> +    switch (opengl) {
> +    case -1: /* default */
> +    case 0:  /* off */
> +        break;
> +    case 1: /* on */
> +#ifdef CONFIG_OPENGL
> +        display_opengl = 1;
> +#endif
> +        break;
> +    default:
> +        g_assert_not_reached();
> +        break;
> +    }
> +#else
> +    if (opengl == 1 /* on */) {
> +        fprintf(stderr,
> +                "SDL1 display code has no opengl support.\n"
> +                "Please recompile qemu with SDL2, using\n"
> +                "./configure --enable-sdl --with-sdlabi=2.0\n");
> +        /* XXX: Should we return false here? */
> +    }
> +#endif
> +
> +    module_call_init(MODULE_INIT_SDL);
> +    if (!init_fn) {
> +        return false;
> +    }
> +    return true;
> +}
> +
> +void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
> +{
> +    assert(init_fn);
> +    init_fn(ds, full_screen, no_frame);
> +}
> diff --git a/ui/sdl.c b/ui/sdl.c
> index d8cf5bc..a30f442 100644
> --- a/ui/sdl.c
> +++ b/ui/sdl.c
> @@ -932,17 +932,7 @@ static const DisplayChangeListenerOps dcl_ops = {
>      .dpy_cursor_define    = sdl_mouse_define,
>  };
>  
> -void sdl_display_early_init(int opengl)
> -{
> -    if (opengl == 1 /* on */) {
> -        fprintf(stderr,
> -                "SDL1 display code has no opengl support.\n"
> -                "Please recompile qemu with SDL2, using\n"
> -                "./configure --enable-sdl --with-sdlabi=2.0\n");
> -    }
> -}
> -
> -void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
> +static void sdl_display_init_do(DisplayState *ds, int full_screen, int no_frame)
>  {
>      int flags;
>      uint8_t data = 0;
> @@ -1025,3 +1015,10 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
>  
>      atexit(sdl_cleanup);
>  }
> +
> +static void sdl_init_fn(void)
> +{
> +    sdl_register_init_fun(sdl_display_init_do);
> +}
> +
> +sdl_init(sdl_init_fn);
> diff --git a/ui/sdl2.c b/ui/sdl2.c
> index 30d2a3c..c2b4049 100644
> --- a/ui/sdl2.c
> +++ b/ui/sdl2.c
> @@ -738,24 +738,7 @@ static const DisplayChangeListenerOps dcl_gl_ops = {
>  };
>  #endif
>  
> -void sdl_display_early_init(int opengl)
> -{
> -    switch (opengl) {
> -    case -1: /* default */
> -    case 0:  /* off */
> -        break;
> -    case 1: /* on */
> -#ifdef CONFIG_OPENGL
> -        display_opengl = 1;
> -#endif
> -        break;
> -    default:
> -        g_assert_not_reached();
> -        break;
> -    }
> -}
> -
> -void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
> +static void sdl_display_init_do(DisplayState *ds, int full_screen, int no_frame)
>  {
>      int flags;
>      uint8_t data = 0;
> @@ -842,3 +825,10 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
>  
>      atexit(sdl_cleanup);
>  }
> +
> +static void sdl_init_fn(void)
> +{
> +    sdl_register_init_fun(sdl_display_init_do);
> +}
> +
> +sdl_init(sdl_init_fn);
> diff --git a/util/module.c b/util/module.c
> index 86e3f7a..b95d4fa 100644
> --- a/util/module.c
> +++ b/util/module.c
> @@ -167,6 +167,9 @@ static void module_load(module_init_type type)
>      static const char *block_modules[] = {
>          CONFIG_BLOCK_MODULES
>      };
> +    static const char *sdl_modules[] = {
> +        "ui-sdl",
> +    };
>      char *exec_dir;
>      char *dirs[3];
>      int i = 0;
> @@ -181,6 +184,9 @@ static void module_load(module_init_type type)
>      case MODULE_INIT_BLOCK:
>          mp = block_modules;
>          break;
> +    case MODULE_INIT_SDL:
> +        mp = sdl_modules;
> +        break;
>      default:
>          /* no other types have dynamic modules for now*/
>          return;
> diff --git a/vl.c b/vl.c
> index a455947..fdbbe47 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4221,7 +4221,10 @@ int main(int argc, char **argv, char **envp)
>      }
>  
>      if (display_type == DT_SDL) {
> -        sdl_display_early_init(request_opengl);
> +        if (!sdl_display_early_init(request_opengl)) {
> +            error_report("Failed to initialize SDL");
> +            exit(1);
> +        }
>      }
>  
>      if (request_opengl == 1 && display_opengl == 0) {
> 
Maybe I'm doing something wrong, but when I apply this third patch
(along with the first two) to master it doesn't seem to build when
modules are enabled in the configuration:

  LINK  x86_64-softmmu/qemu-system-x86_64
../backends/baum.o: In function `chr_baum_init':
/home/bos/clord/Documents/qemu/backends/baum.c:616: undefined reference
to `SDL_GetWMInfo'
collect2: error: ld returned 1 exit status
Makefile:197: recipe for target 'qemu-system-x86_64' failed
make[1]: *** [qemu-system-x86_64] Error 1
Makefile:204: recipe for target 'subdir-x86_64-softmmu' failed
make: *** [subdir-x86_64-softmmu] Error 2

Colin

Re: [Qemu-devel] [PATCH for 2.8 3/3] sdl: Modularize

[Fam Zheng]

On Wed, 07/27 19:23, Colin Lord wrote:
> On 07/27/2016 02:26 AM, Fam Zheng wrote:
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  Makefile.objs         |  1 +
> >  configure             |  4 +--
> >  include/qemu/module.h |  2 ++
> >  include/ui/console.h  |  5 ++--
> >  ui/Makefile.objs      |  2 ++
> >  ui/sdl-init.c         | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  ui/sdl.c              | 19 ++++++-------
> >  ui/sdl2.c             | 26 ++++++------------
> >  util/module.c         |  6 +++++
> >  vl.c                  |  5 +++-
> >  10 files changed, 111 insertions(+), 34 deletions(-)
> >  create mode 100644 ui/sdl-init.c
> > 
> > diff --git a/Makefile.objs b/Makefile.objs
> > index 6d5ddcf..08c5746 100644
> > --- a/Makefile.objs
> > +++ b/Makefile.objs
> > @@ -62,6 +62,7 @@ common-obj-y += accel.o
> >  common-obj-y += replay/
> >  
> >  common-obj-y += ui/
> > +common-obj-m += ui/
> >  common-obj-y += bt-host.o bt-vhci.o
> >  bt-host.o-cflags := $(BLUEZ_CFLAGS)
> >  
> > diff --git a/configure b/configure
> > index e04e59f..1b9be8f 100755
> > --- a/configure
> > +++ b/configure
> > @@ -2533,7 +2533,6 @@ EOF
> >      sdl_cflags="$sdl_cflags $x11_cflags"
> >      sdl_libs="$sdl_libs $x11_libs"
> >    fi
> > -  libs_softmmu="$sdl_libs $libs_softmmu"
> >  fi
> >  
> >  ##########################################
> > @@ -5065,9 +5064,10 @@ if test "$modules" = "yes"; then
> >    echo "CONFIG_MODULES=y" >> $config_host_mak
> >  fi
> >  if test "$sdl" = "yes" ; then
> > -  echo "CONFIG_SDL=y" >> $config_host_mak
> > +  echo "CONFIG_SDL=m" >> $config_host_mak
> >    echo "CONFIG_SDLABI=$sdlabi" >> $config_host_mak
> >    echo "SDL_CFLAGS=$sdl_cflags" >> $config_host_mak
> > +  echo "SDL_LIBS=$sdl_libs" >> $config_host_mak
> >  fi
> >  if test "$sdlabi" = "2.0"; then
> >    echo "CONFIG_SDL2=y" >> $config_host_mak
> > diff --git a/include/qemu/module.h b/include/qemu/module.h
> > index 2370708..f5e012b 100644
> > --- a/include/qemu/module.h
> > +++ b/include/qemu/module.h
> > @@ -44,6 +44,7 @@ typedef enum {
> >      MODULE_INIT_OPTS,
> >      MODULE_INIT_QAPI,
> >      MODULE_INIT_QOM,
> > +    MODULE_INIT_SDL,
> >      MODULE_INIT_MAX
> >  } module_init_type;
> >  
> > @@ -51,6 +52,7 @@ typedef enum {
> >  #define opts_init(function) module_init(function, MODULE_INIT_OPTS)
> >  #define qapi_init(function) module_init(function, MODULE_INIT_QAPI)
> >  #define type_init(function) module_init(function, MODULE_INIT_QOM)
> > +#define sdl_init(function) module_init(function, MODULE_INIT_SDL)
> >  
> >  void register_module_init(void (*fn)(void), module_init_type type);
> >  void register_dso_module_init(void (*fn)(void), module_init_type type);
> > diff --git a/include/ui/console.h b/include/ui/console.h
> > index 2703a3a..1d9b0bb 100644
> > --- a/include/ui/console.h
> > +++ b/include/ui/console.h
> > @@ -425,10 +425,11 @@ void surface_gl_setup_viewport(ConsoleGLState *gls,
> >  
> >  /* sdl.c */
> >  #ifdef CONFIG_SDL
> > -void sdl_display_early_init(int opengl);
> > +bool sdl_display_early_init(int opengl);
> >  void sdl_display_init(DisplayState *ds, int full_screen, int no_frame);
> > +void sdl_register_init_fun(void *fn);
> >  #else
> > -static inline void sdl_display_early_init(int opengl)
> > +static inline bool sdl_display_early_init(int opengl)
> >  {
> >      /* This must never be called if CONFIG_SDL is disabled */
> >      error_report("SDL support is disabled");
> > diff --git a/ui/Makefile.objs b/ui/Makefile.objs
> > index dc936f1..0b82650 100644
> > --- a/ui/Makefile.objs
> > +++ b/ui/Makefile.objs
> > @@ -26,7 +26,9 @@ ifeq ($(CONFIG_OPENGL),y)
> >  sdl.mo-objs += sdl2-gl.o
> >  endif
> >  endif
> > +common-obj-y += sdl-init.o
> >  sdl.mo-cflags := $(SDL_CFLAGS)
> > +sdl.mo-libs := $(SDL_LIBS)
> >  
> >  ifeq ($(CONFIG_OPENGL),y)
> >  common-obj-y += shader.o
> > diff --git a/ui/sdl-init.c b/ui/sdl-init.c
> > new file mode 100644
> > index 0000000..6a780a4
> > --- /dev/null
> > +++ b/ui/sdl-init.c
> > @@ -0,0 +1,75 @@
> > +/*
> > + * QEMU SDL display driver init function
> > + *
> > + * Copyright (c) 2003 Fabrice Bellard
> > + * Copyright (c) 2016 Red Hat, Inc.
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a copy
> > + * of this software and associated documentation files (the "Software"), to deal
> > + * in the Software without restriction, including without limitation the rights
> > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> > + * copies of the Software, and to permit persons to whom the Software is
> > + * furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be included in
> > + * all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> > + * THE SOFTWARE.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "ui/console.h"
> > +#include "qemu/module.h"
> > +
> > +static void (*init_fn)(DisplayState *ds, int full_screen, int no_frame);
> > +void sdl_register_init_fun(void *fn)
> > +{
> > +    assert(!init_fn);
> > +    init_fn = fn;
> > +}
> > +
> > +bool sdl_display_early_init(int opengl)
> > +{
> > +
> > +#ifdef CONFIG_SDL2
> > +    switch (opengl) {
> > +    case -1: /* default */
> > +    case 0:  /* off */
> > +        break;
> > +    case 1: /* on */
> > +#ifdef CONFIG_OPENGL
> > +        display_opengl = 1;
> > +#endif
> > +        break;
> > +    default:
> > +        g_assert_not_reached();
> > +        break;
> > +    }
> > +#else
> > +    if (opengl == 1 /* on */) {
> > +        fprintf(stderr,
> > +                "SDL1 display code has no opengl support.\n"
> > +                "Please recompile qemu with SDL2, using\n"
> > +                "./configure --enable-sdl --with-sdlabi=2.0\n");
> > +        /* XXX: Should we return false here? */
> > +    }
> > +#endif
> > +
> > +    module_call_init(MODULE_INIT_SDL);
> > +    if (!init_fn) {
> > +        return false;
> > +    }
> > +    return true;
> > +}
> > +
> > +void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
> > +{
> > +    assert(init_fn);
> > +    init_fn(ds, full_screen, no_frame);
> > +}
> > diff --git a/ui/sdl.c b/ui/sdl.c
> > index d8cf5bc..a30f442 100644
> > --- a/ui/sdl.c
> > +++ b/ui/sdl.c
> > @@ -932,17 +932,7 @@ static const DisplayChangeListenerOps dcl_ops = {
> >      .dpy_cursor_define    = sdl_mouse_define,
> >  };
> >  
> > -void sdl_display_early_init(int opengl)
> > -{
> > -    if (opengl == 1 /* on */) {
> > -        fprintf(stderr,
> > -                "SDL1 display code has no opengl support.\n"
> > -                "Please recompile qemu with SDL2, using\n"
> > -                "./configure --enable-sdl --with-sdlabi=2.0\n");
> > -    }
> > -}
> > -
> > -void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
> > +static void sdl_display_init_do(DisplayState *ds, int full_screen, int no_frame)
> >  {
> >      int flags;
> >      uint8_t data = 0;
> > @@ -1025,3 +1015,10 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
> >  
> >      atexit(sdl_cleanup);
> >  }
> > +
> > +static void sdl_init_fn(void)
> > +{
> > +    sdl_register_init_fun(sdl_display_init_do);
> > +}
> > +
> > +sdl_init(sdl_init_fn);
> > diff --git a/ui/sdl2.c b/ui/sdl2.c
> > index 30d2a3c..c2b4049 100644
> > --- a/ui/sdl2.c
> > +++ b/ui/sdl2.c
> > @@ -738,24 +738,7 @@ static const DisplayChangeListenerOps dcl_gl_ops = {
> >  };
> >  #endif
> >  
> > -void sdl_display_early_init(int opengl)
> > -{
> > -    switch (opengl) {
> > -    case -1: /* default */
> > -    case 0:  /* off */
> > -        break;
> > -    case 1: /* on */
> > -#ifdef CONFIG_OPENGL
> > -        display_opengl = 1;
> > -#endif
> > -        break;
> > -    default:
> > -        g_assert_not_reached();
> > -        break;
> > -    }
> > -}
> > -
> > -void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
> > +static void sdl_display_init_do(DisplayState *ds, int full_screen, int no_frame)
> >  {
> >      int flags;
> >      uint8_t data = 0;
> > @@ -842,3 +825,10 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
> >  
> >      atexit(sdl_cleanup);
> >  }
> > +
> > +static void sdl_init_fn(void)
> > +{
> > +    sdl_register_init_fun(sdl_display_init_do);
> > +}
> > +
> > +sdl_init(sdl_init_fn);
> > diff --git a/util/module.c b/util/module.c
> > index 86e3f7a..b95d4fa 100644
> > --- a/util/module.c
> > +++ b/util/module.c
> > @@ -167,6 +167,9 @@ static void module_load(module_init_type type)
> >      static const char *block_modules[] = {
> >          CONFIG_BLOCK_MODULES
> >      };
> > +    static const char *sdl_modules[] = {
> > +        "ui-sdl",
> > +    };
> >      char *exec_dir;
> >      char *dirs[3];
> >      int i = 0;
> > @@ -181,6 +184,9 @@ static void module_load(module_init_type type)
> >      case MODULE_INIT_BLOCK:
> >          mp = block_modules;
> >          break;
> > +    case MODULE_INIT_SDL:
> > +        mp = sdl_modules;
> > +        break;
> >      default:
> >          /* no other types have dynamic modules for now*/
> >          return;
> > diff --git a/vl.c b/vl.c
> > index a455947..fdbbe47 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -4221,7 +4221,10 @@ int main(int argc, char **argv, char **envp)
> >      }
> >  
> >      if (display_type == DT_SDL) {
> > -        sdl_display_early_init(request_opengl);
> > +        if (!sdl_display_early_init(request_opengl)) {
> > +            error_report("Failed to initialize SDL");
> > +            exit(1);
> > +        }
> >      }
> >  
> >      if (request_opengl == 1 && display_opengl == 0) {
> > 
> Maybe I'm doing something wrong, but when I apply this third patch
> (along with the first two) to master it doesn't seem to build when
> modules are enabled in the configuration:
> 
>   LINK  x86_64-softmmu/qemu-system-x86_64
> ../backends/baum.o: In function `chr_baum_init':
> /home/bos/clord/Documents/qemu/backends/baum.c:616: undefined reference
> to `SDL_GetWMInfo'
> collect2: error: ld returned 1 exit status
> Makefile:197: recipe for target 'qemu-system-x86_64' failed
> make[1]: *** [qemu-system-x86_64] Error 1
> Makefile:204: recipe for target 'subdir-x86_64-softmmu' failed
> make: *** [subdir-x86_64-softmmu] Error 2

You are right, looks like audio and baum both want SDL library. I need to take
another look.

Fam

Re: [Qemu-devel] [PATCH for 2.8 3/3] sdl: Modularize

[Fam Zheng]

On Thu, 07/28 13:17, Fam Zheng wrote:
> > Maybe I'm doing something wrong, but when I apply this third patch
> > (along with the first two) to master it doesn't seem to build when
> > modules are enabled in the configuration:
> > 
> >   LINK  x86_64-softmmu/qemu-system-x86_64
> > ../backends/baum.o: In function `chr_baum_init':
> > /home/bos/clord/Documents/qemu/backends/baum.c:616: undefined reference
> > to `SDL_GetWMInfo'
> > collect2: error: ld returned 1 exit status
> > Makefile:197: recipe for target 'qemu-system-x86_64' failed
> > make[1]: *** [qemu-system-x86_64] Error 1
> > Makefile:204: recipe for target 'subdir-x86_64-softmmu' failed
> > make: *** [subdir-x86_64-softmmu] Error 2
> 
> You are right, looks like audio and baum both want SDL library. I need to take
> another look.

We need to modularize backends/baum.c and add appropriate loading code
somewhere (probably when it is specifically selected).

In addition, audio/sdl.o should probably to be bundled together with ui/sdl.mo.

I'll leave this series for now and you can pick up patch 1 into your series if
it helps. Also feel free to pick up modularizing SDL and baum, if you want.
Otherwise, I'll revisit these when your "load on demand" work settles down.

Fam

Re: [Qemu-devel] [PATCH for 2.8 1/3] rules.mak: Don't extract libs from .mo-libs in link command

[Paolo Bonzini]

On 27/07/2016 08:26, Fam Zheng wrote:
> For module build, .mo objects are passed to LINK and consumed in
> process-archive-undefs. The reason behind that is documented in the
> comment above process-archive-undefs.
> 
> Similarly, extract-libs should be called with .mo filtered out too.
> Otherwise, the .mo-libs are added to the link command incorrectly,
> spoiling the purpose of modularization.
> 
> Currently we don't have any .mo-libs usage, but it will be used soon
> when we modularize more multi-source objects, like sdl and gtk.
> 
> Reported-by: Colin Lord <clord@redhat.com>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  rules.mak | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/rules.mak b/rules.mak
> index ed8e482..80705a7 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -50,7 +50,7 @@ process-archive-undefs = $(filter-out %.a %.mo,$1) \
>                                $(call undefined-symbols,$(filter %.mo,$1)))) \
>                  $(filter %.a,$1)
>  
> -extract-libs = $(strip $(foreach o,$1,$($o-libs)))
> +extract-libs = $(strip $(foreach o,$(filter-out %.mo,$1),$($o-libs)))
>  expand-objs = $(strip $(sort $(filter %.o,$1)) \
>                    $(foreach o,$(filter %.mo,$1),$($o-objs)) \
>                    $(filter-out %.o %.mo,$1))
> 

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

Re: [Qemu-devel] [PATCH for 2.8 3/3] sdl: Modularize

[Paolo Bonzini]

On 27/07/2016 08:26, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  Makefile.objs         |  1 +
>  configure             |  4 +--
>  include/qemu/module.h |  2 ++
>  include/ui/console.h  |  5 ++--
>  ui/Makefile.objs      |  2 ++
>  ui/sdl-init.c         | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  ui/sdl.c              | 19 ++++++-------
>  ui/sdl2.c             | 26 ++++++------------
>  util/module.c         |  6 +++++
>  vl.c                  |  5 +++-
>  10 files changed, 111 insertions(+), 34 deletions(-)
>  create mode 100644 ui/sdl-init.c
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index 6d5ddcf..08c5746 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -62,6 +62,7 @@ common-obj-y += accel.o
>  common-obj-y += replay/
>  
>  common-obj-y += ui/
> +common-obj-m += ui/
>  common-obj-y += bt-host.o bt-vhci.o
>  bt-host.o-cflags := $(BLUEZ_CFLAGS)
>  
> diff --git a/configure b/configure
> index e04e59f..1b9be8f 100755
> --- a/configure
> +++ b/configure
> @@ -2533,7 +2533,6 @@ EOF
>      sdl_cflags="$sdl_cflags $x11_cflags"
>      sdl_libs="$sdl_libs $x11_libs"
>    fi
> -  libs_softmmu="$sdl_libs $libs_softmmu"
>  fi
>  
>  ##########################################
> @@ -5065,9 +5064,10 @@ if test "$modules" = "yes"; then
>    echo "CONFIG_MODULES=y" >> $config_host_mak
>  fi
>  if test "$sdl" = "yes" ; then
> -  echo "CONFIG_SDL=y" >> $config_host_mak
> +  echo "CONFIG_SDL=m" >> $config_host_mak
>    echo "CONFIG_SDLABI=$sdlabi" >> $config_host_mak
>    echo "SDL_CFLAGS=$sdl_cflags" >> $config_host_mak
> +  echo "SDL_LIBS=$sdl_libs" >> $config_host_mak
>  fi
>  if test "$sdlabi" = "2.0"; then
>    echo "CONFIG_SDL2=y" >> $config_host_mak
> diff --git a/include/qemu/module.h b/include/qemu/module.h
> index 2370708..f5e012b 100644
> --- a/include/qemu/module.h
> +++ b/include/qemu/module.h
> @@ -44,6 +44,7 @@ typedef enum {
>      MODULE_INIT_OPTS,
>      MODULE_INIT_QAPI,
>      MODULE_INIT_QOM,
> +    MODULE_INIT_SDL,
>      MODULE_INIT_MAX
>  } module_init_type;
>  
> @@ -51,6 +52,7 @@ typedef enum {
>  #define opts_init(function) module_init(function, MODULE_INIT_OPTS)
>  #define qapi_init(function) module_init(function, MODULE_INIT_QAPI)
>  #define type_init(function) module_init(function, MODULE_INIT_QOM)
> +#define sdl_init(function) module_init(function, MODULE_INIT_SDL)
>  
>  void register_module_init(void (*fn)(void), module_init_type type);
>  void register_dso_module_init(void (*fn)(void), module_init_type type);
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 2703a3a..1d9b0bb 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -425,10 +425,11 @@ void surface_gl_setup_viewport(ConsoleGLState *gls,
>  
>  /* sdl.c */
>  #ifdef CONFIG_SDL
> -void sdl_display_early_init(int opengl);
> +bool sdl_display_early_init(int opengl);
>  void sdl_display_init(DisplayState *ds, int full_screen, int no_frame);
> +void sdl_register_init_fun(void *fn);
>  #else
> -static inline void sdl_display_early_init(int opengl)
> +static inline bool sdl_display_early_init(int opengl)
>  {
>      /* This must never be called if CONFIG_SDL is disabled */
>      error_report("SDL support is disabled");
> diff --git a/ui/Makefile.objs b/ui/Makefile.objs
> index dc936f1..0b82650 100644
> --- a/ui/Makefile.objs
> +++ b/ui/Makefile.objs
> @@ -26,7 +26,9 @@ ifeq ($(CONFIG_OPENGL),y)
>  sdl.mo-objs += sdl2-gl.o
>  endif
>  endif
> +common-obj-y += sdl-init.o
>  sdl.mo-cflags := $(SDL_CFLAGS)
> +sdl.mo-libs := $(SDL_LIBS)
>  
>  ifeq ($(CONFIG_OPENGL),y)
>  common-obj-y += shader.o
> diff --git a/ui/sdl-init.c b/ui/sdl-init.c
> new file mode 100644
> index 0000000..6a780a4
> --- /dev/null
> +++ b/ui/sdl-init.c
> @@ -0,0 +1,75 @@
> +/*
> + * QEMU SDL display driver init function
> + *
> + * Copyright (c) 2003 Fabrice Bellard
> + * Copyright (c) 2016 Red Hat, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "ui/console.h"
> +#include "qemu/module.h"
> +
> +static void (*init_fn)(DisplayState *ds, int full_screen, int no_frame);
> +void sdl_register_init_fun(void *fn)
> +{
> +    assert(!init_fn);
> +    init_fn = fn;
> +}
> +
> +bool sdl_display_early_init(int opengl)
> +{
> +
> +#ifdef CONFIG_SDL2
> +    switch (opengl) {
> +    case -1: /* default */
> +    case 0:  /* off */
> +        break;
> +    case 1: /* on */
> +#ifdef CONFIG_OPENGL
> +        display_opengl = 1;
> +#endif
> +        break;
> +    default:
> +        g_assert_not_reached();
> +        break;
> +    }
> +#else
> +    if (opengl == 1 /* on */) {
> +        fprintf(stderr,
> +                "SDL1 display code has no opengl support.\n"
> +                "Please recompile qemu with SDL2, using\n"
> +                "./configure --enable-sdl --with-sdlabi=2.0\n");
> +        /* XXX: Should we return false here? */
> +    }
> +#endif
> +
> +    module_call_init(MODULE_INIT_SDL);
> +    if (!init_fn) {
> +        return false;
> +    }
> +    return true;
> +}
> +
> +void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
> +{
> +    assert(init_fn);
> +    init_fn(ds, full_screen, no_frame);
> +}
> diff --git a/ui/sdl.c b/ui/sdl.c
> index d8cf5bc..a30f442 100644
> --- a/ui/sdl.c
> +++ b/ui/sdl.c
> @@ -932,17 +932,7 @@ static const DisplayChangeListenerOps dcl_ops = {
>      .dpy_cursor_define    = sdl_mouse_define,
>  };
>  
> -void sdl_display_early_init(int opengl)
> -{
> -    if (opengl == 1 /* on */) {
> -        fprintf(stderr,
> -                "SDL1 display code has no opengl support.\n"
> -                "Please recompile qemu with SDL2, using\n"
> -                "./configure --enable-sdl --with-sdlabi=2.0\n");
> -    }
> -}
> -
> -void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
> +static void sdl_display_init_do(DisplayState *ds, int full_screen, int no_frame)
>  {
>      int flags;
>      uint8_t data = 0;
> @@ -1025,3 +1015,10 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
>  
>      atexit(sdl_cleanup);
>  }
> +
> +static void sdl_init_fn(void)
> +{
> +    sdl_register_init_fun(sdl_display_init_do);
> +}
> +
> +sdl_init(sdl_init_fn);

Can you use __attribute__((constructor)) instead of going through the
burden of defining a new type?  The registration function only does a
single assignment, so it doesn't really have ordering dependencies.

Thanks,

Paolo

> diff --git a/ui/sdl2.c b/ui/sdl2.c
> index 30d2a3c..c2b4049 100644
> --- a/ui/sdl2.c
> +++ b/ui/sdl2.c
> @@ -738,24 +738,7 @@ static const DisplayChangeListenerOps dcl_gl_ops = {
>  };
>  #endif
>  
> -void sdl_display_early_init(int opengl)
> -{
> -    switch (opengl) {
> -    case -1: /* default */
> -    case 0:  /* off */
> -        break;
> -    case 1: /* on */
> -#ifdef CONFIG_OPENGL
> -        display_opengl = 1;
> -#endif
> -        break;
> -    default:
> -        g_assert_not_reached();
> -        break;
> -    }
> -}
> -
> -void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
> +static void sdl_display_init_do(DisplayState *ds, int full_screen, int no_frame)
>  {
>      int flags;
>      uint8_t data = 0;
> @@ -842,3 +825,10 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
>  
>      atexit(sdl_cleanup);
>  }
> +
> +static void sdl_init_fn(void)
> +{
> +    sdl_register_init_fun(sdl_display_init_do);
> +}
> +
> +sdl_init(sdl_init_fn);
> diff --git a/util/module.c b/util/module.c
> index 86e3f7a..b95d4fa 100644
> --- a/util/module.c
> +++ b/util/module.c
> @@ -167,6 +167,9 @@ static void module_load(module_init_type type)
>      static const char *block_modules[] = {
>          CONFIG_BLOCK_MODULES
>      };
> +    static const char *sdl_modules[] = {
> +        "ui-sdl",
> +    };
>      char *exec_dir;
>      char *dirs[3];
>      int i = 0;
> @@ -181,6 +184,9 @@ static void module_load(module_init_type type)
>      case MODULE_INIT_BLOCK:
>          mp = block_modules;
>          break;
> +    case MODULE_INIT_SDL:
> +        mp = sdl_modules;
> +        break;
>      default:
>          /* no other types have dynamic modules for now*/
>          return;
> diff --git a/vl.c b/vl.c
> index a455947..fdbbe47 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4221,7 +4221,10 @@ int main(int argc, char **argv, char **envp)
>      }
>  
>      if (display_type == DT_SDL) {
> -        sdl_display_early_init(request_opengl);
> +        if (!sdl_display_early_init(request_opengl)) {
> +            error_report("Failed to initialize SDL");
> +            exit(1);
> +        }
>      }
>  
>      if (request_opengl == 1 && display_opengl == 0) {
> 

Re: [Qemu-devel] [PATCH for 2.8 3/3] sdl: Modularize

[Paolo Bonzini]

On 28/07/2016 07:17, Fam Zheng wrote:
> On Wed, 07/27 19:23, Colin Lord wrote:
>> On 07/27/2016 02:26 AM, Fam Zheng wrote:
>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>> ---
>>>  Makefile.objs         |  1 +
>>>  configure             |  4 +--
>>>  include/qemu/module.h |  2 ++
>>>  include/ui/console.h  |  5 ++--
>>>  ui/Makefile.objs      |  2 ++
>>>  ui/sdl-init.c         | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  ui/sdl.c              | 19 ++++++-------
>>>  ui/sdl2.c             | 26 ++++++------------
>>>  util/module.c         |  6 +++++
>>>  vl.c                  |  5 +++-
>>>  10 files changed, 111 insertions(+), 34 deletions(-)
>>>  create mode 100644 ui/sdl-init.c
>>>
>>> diff --git a/Makefile.objs b/Makefile.objs
>>> index 6d5ddcf..08c5746 100644
>>> --- a/Makefile.objs
>>> +++ b/Makefile.objs
>>> @@ -62,6 +62,7 @@ common-obj-y += accel.o
>>>  common-obj-y += replay/
>>>  
>>>  common-obj-y += ui/
>>> +common-obj-m += ui/
>>>  common-obj-y += bt-host.o bt-vhci.o
>>>  bt-host.o-cflags := $(BLUEZ_CFLAGS)
>>>  
>>> diff --git a/configure b/configure
>>> index e04e59f..1b9be8f 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -2533,7 +2533,6 @@ EOF
>>>      sdl_cflags="$sdl_cflags $x11_cflags"
>>>      sdl_libs="$sdl_libs $x11_libs"
>>>    fi
>>> -  libs_softmmu="$sdl_libs $libs_softmmu"
>>>  fi
>>>  
>>>  ##########################################
>>> @@ -5065,9 +5064,10 @@ if test "$modules" = "yes"; then
>>>    echo "CONFIG_MODULES=y" >> $config_host_mak
>>>  fi
>>>  if test "$sdl" = "yes" ; then
>>> -  echo "CONFIG_SDL=y" >> $config_host_mak
>>> +  echo "CONFIG_SDL=m" >> $config_host_mak
>>>    echo "CONFIG_SDLABI=$sdlabi" >> $config_host_mak
>>>    echo "SDL_CFLAGS=$sdl_cflags" >> $config_host_mak
>>> +  echo "SDL_LIBS=$sdl_libs" >> $config_host_mak
>>>  fi
>>>  if test "$sdlabi" = "2.0"; then
>>>    echo "CONFIG_SDL2=y" >> $config_host_mak
>>> diff --git a/include/qemu/module.h b/include/qemu/module.h
>>> index 2370708..f5e012b 100644
>>> --- a/include/qemu/module.h
>>> +++ b/include/qemu/module.h
>>> @@ -44,6 +44,7 @@ typedef enum {
>>>      MODULE_INIT_OPTS,
>>>      MODULE_INIT_QAPI,
>>>      MODULE_INIT_QOM,
>>> +    MODULE_INIT_SDL,
>>>      MODULE_INIT_MAX
>>>  } module_init_type;
>>>  
>>> @@ -51,6 +52,7 @@ typedef enum {
>>>  #define opts_init(function) module_init(function, MODULE_INIT_OPTS)
>>>  #define qapi_init(function) module_init(function, MODULE_INIT_QAPI)
>>>  #define type_init(function) module_init(function, MODULE_INIT_QOM)
>>> +#define sdl_init(function) module_init(function, MODULE_INIT_SDL)
>>>  
>>>  void register_module_init(void (*fn)(void), module_init_type type);
>>>  void register_dso_module_init(void (*fn)(void), module_init_type type);
>>> diff --git a/include/ui/console.h b/include/ui/console.h
>>> index 2703a3a..1d9b0bb 100644
>>> --- a/include/ui/console.h
>>> +++ b/include/ui/console.h
>>> @@ -425,10 +425,11 @@ void surface_gl_setup_viewport(ConsoleGLState *gls,
>>>  
>>>  /* sdl.c */
>>>  #ifdef CONFIG_SDL
>>> -void sdl_display_early_init(int opengl);
>>> +bool sdl_display_early_init(int opengl);
>>>  void sdl_display_init(DisplayState *ds, int full_screen, int no_frame);
>>> +void sdl_register_init_fun(void *fn);
>>>  #else
>>> -static inline void sdl_display_early_init(int opengl)
>>> +static inline bool sdl_display_early_init(int opengl)
>>>  {
>>>      /* This must never be called if CONFIG_SDL is disabled */
>>>      error_report("SDL support is disabled");
>>> diff --git a/ui/Makefile.objs b/ui/Makefile.objs
>>> index dc936f1..0b82650 100644
>>> --- a/ui/Makefile.objs
>>> +++ b/ui/Makefile.objs
>>> @@ -26,7 +26,9 @@ ifeq ($(CONFIG_OPENGL),y)
>>>  sdl.mo-objs += sdl2-gl.o
>>>  endif
>>>  endif
>>> +common-obj-y += sdl-init.o
>>>  sdl.mo-cflags := $(SDL_CFLAGS)
>>> +sdl.mo-libs := $(SDL_LIBS)
>>>  
>>>  ifeq ($(CONFIG_OPENGL),y)
>>>  common-obj-y += shader.o
>>> diff --git a/ui/sdl-init.c b/ui/sdl-init.c
>>> new file mode 100644
>>> index 0000000..6a780a4
>>> --- /dev/null
>>> +++ b/ui/sdl-init.c
>>> @@ -0,0 +1,75 @@
>>> +/*
>>> + * QEMU SDL display driver init function
>>> + *
>>> + * Copyright (c) 2003 Fabrice Bellard
>>> + * Copyright (c) 2016 Red Hat, Inc.
>>> + *
>>> + * Permission is hereby granted, free of charge, to any person obtaining a copy
>>> + * of this software and associated documentation files (the "Software"), to deal
>>> + * in the Software without restriction, including without limitation the rights
>>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>>> + * copies of the Software, and to permit persons to whom the Software is
>>> + * furnished to do so, subject to the following conditions:
>>> + *
>>> + * The above copyright notice and this permission notice shall be included in
>>> + * all copies or substantial portions of the Software.
>>> + *
>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>>> + * THE SOFTWARE.
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "ui/console.h"
>>> +#include "qemu/module.h"
>>> +
>>> +static void (*init_fn)(DisplayState *ds, int full_screen, int no_frame);
>>> +void sdl_register_init_fun(void *fn)
>>> +{
>>> +    assert(!init_fn);
>>> +    init_fn = fn;
>>> +}
>>> +
>>> +bool sdl_display_early_init(int opengl)
>>> +{
>>> +
>>> +#ifdef CONFIG_SDL2
>>> +    switch (opengl) {
>>> +    case -1: /* default */
>>> +    case 0:  /* off */
>>> +        break;
>>> +    case 1: /* on */
>>> +#ifdef CONFIG_OPENGL
>>> +        display_opengl = 1;
>>> +#endif
>>> +        break;
>>> +    default:
>>> +        g_assert_not_reached();
>>> +        break;
>>> +    }
>>> +#else
>>> +    if (opengl == 1 /* on */) {
>>> +        fprintf(stderr,
>>> +                "SDL1 display code has no opengl support.\n"
>>> +                "Please recompile qemu with SDL2, using\n"
>>> +                "./configure --enable-sdl --with-sdlabi=2.0\n");
>>> +        /* XXX: Should we return false here? */
>>> +    }
>>> +#endif
>>> +
>>> +    module_call_init(MODULE_INIT_SDL);
>>> +    if (!init_fn) {
>>> +        return false;
>>> +    }
>>> +    return true;
>>> +}
>>> +
>>> +void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
>>> +{
>>> +    assert(init_fn);
>>> +    init_fn(ds, full_screen, no_frame);
>>> +}
>>> diff --git a/ui/sdl.c b/ui/sdl.c
>>> index d8cf5bc..a30f442 100644
>>> --- a/ui/sdl.c
>>> +++ b/ui/sdl.c
>>> @@ -932,17 +932,7 @@ static const DisplayChangeListenerOps dcl_ops = {
>>>      .dpy_cursor_define    = sdl_mouse_define,
>>>  };
>>>  
>>> -void sdl_display_early_init(int opengl)
>>> -{
>>> -    if (opengl == 1 /* on */) {
>>> -        fprintf(stderr,
>>> -                "SDL1 display code has no opengl support.\n"
>>> -                "Please recompile qemu with SDL2, using\n"
>>> -                "./configure --enable-sdl --with-sdlabi=2.0\n");
>>> -    }
>>> -}
>>> -
>>> -void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
>>> +static void sdl_display_init_do(DisplayState *ds, int full_screen, int no_frame)
>>>  {
>>>      int flags;
>>>      uint8_t data = 0;
>>> @@ -1025,3 +1015,10 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
>>>  
>>>      atexit(sdl_cleanup);
>>>  }
>>> +
>>> +static void sdl_init_fn(void)
>>> +{
>>> +    sdl_register_init_fun(sdl_display_init_do);
>>> +}
>>> +
>>> +sdl_init(sdl_init_fn);
>>> diff --git a/ui/sdl2.c b/ui/sdl2.c
>>> index 30d2a3c..c2b4049 100644
>>> --- a/ui/sdl2.c
>>> +++ b/ui/sdl2.c
>>> @@ -738,24 +738,7 @@ static const DisplayChangeListenerOps dcl_gl_ops = {
>>>  };
>>>  #endif
>>>  
>>> -void sdl_display_early_init(int opengl)
>>> -{
>>> -    switch (opengl) {
>>> -    case -1: /* default */
>>> -    case 0:  /* off */
>>> -        break;
>>> -    case 1: /* on */
>>> -#ifdef CONFIG_OPENGL
>>> -        display_opengl = 1;
>>> -#endif
>>> -        break;
>>> -    default:
>>> -        g_assert_not_reached();
>>> -        break;
>>> -    }
>>> -}
>>> -
>>> -void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
>>> +static void sdl_display_init_do(DisplayState *ds, int full_screen, int no_frame)
>>>  {
>>>      int flags;
>>>      uint8_t data = 0;
>>> @@ -842,3 +825,10 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
>>>  
>>>      atexit(sdl_cleanup);
>>>  }
>>> +
>>> +static void sdl_init_fn(void)
>>> +{
>>> +    sdl_register_init_fun(sdl_display_init_do);
>>> +}
>>> +
>>> +sdl_init(sdl_init_fn);
>>> diff --git a/util/module.c b/util/module.c
>>> index 86e3f7a..b95d4fa 100644
>>> --- a/util/module.c
>>> +++ b/util/module.c
>>> @@ -167,6 +167,9 @@ static void module_load(module_init_type type)
>>>      static const char *block_modules[] = {
>>>          CONFIG_BLOCK_MODULES
>>>      };
>>> +    static const char *sdl_modules[] = {
>>> +        "ui-sdl",
>>> +    };
>>>      char *exec_dir;
>>>      char *dirs[3];
>>>      int i = 0;
>>> @@ -181,6 +184,9 @@ static void module_load(module_init_type type)
>>>      case MODULE_INIT_BLOCK:
>>>          mp = block_modules;
>>>          break;
>>> +    case MODULE_INIT_SDL:
>>> +        mp = sdl_modules;
>>> +        break;
>>>      default:
>>>          /* no other types have dynamic modules for now*/
>>>          return;
>>> diff --git a/vl.c b/vl.c
>>> index a455947..fdbbe47 100644
>>> --- a/vl.c
>>> +++ b/vl.c
>>> @@ -4221,7 +4221,10 @@ int main(int argc, char **argv, char **envp)
>>>      }
>>>  
>>>      if (display_type == DT_SDL) {
>>> -        sdl_display_early_init(request_opengl);
>>> +        if (!sdl_display_early_init(request_opengl)) {
>>> +            error_report("Failed to initialize SDL");
>>> +            exit(1);
>>> +        }
>>>      }
>>>  
>>>      if (request_opengl == 1 && display_opengl == 0) {
>>>
>> Maybe I'm doing something wrong, but when I apply this third patch
>> (along with the first two) to master it doesn't seem to build when
>> modules are enabled in the configuration:
>>
>>   LINK  x86_64-softmmu/qemu-system-x86_64
>> ../backends/baum.o: In function `chr_baum_init':
>> /home/bos/clord/Documents/qemu/backends/baum.c:616: undefined reference
>> to `SDL_GetWMInfo'
>> collect2: error: ld returned 1 exit status
>> Makefile:197: recipe for target 'qemu-system-x86_64' failed
>> make[1]: *** [qemu-system-x86_64] Error 1
>> Makefile:204: recipe for target 'subdir-x86_64-softmmu' failed
>> make: *** [subdir-x86_64-softmmu] Error 2
> 
> You are right, looks like audio and baum both want SDL library. I need to take
> another look.

SDL Audio wants SDL, but it was reported to be broken last year.

Baum can place $(SDL_LIBS) in baum.o-libs, I guess.

Paolo

Re: [Qemu-devel] [PATCH for 2.8 3/3] sdl: Modularize

[Daniel P. Berrange]

On Wed, Jul 27, 2016 at 02:26:17PM +0800, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  Makefile.objs         |  1 +
>  configure             |  4 +--
>  include/qemu/module.h |  2 ++
>  include/ui/console.h  |  5 ++--
>  ui/Makefile.objs      |  2 ++
>  ui/sdl-init.c         | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  ui/sdl.c              | 19 ++++++-------
>  ui/sdl2.c             | 26 ++++++------------
>  util/module.c         |  6 +++++
>  vl.c                  |  5 +++-
>  10 files changed, 111 insertions(+), 34 deletions(-)
>  create mode 100644 ui/sdl-init.c

[snip]

> diff --git a/vl.c b/vl.c
> index a455947..fdbbe47 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4221,7 +4221,10 @@ int main(int argc, char **argv, char **envp)
>      }
>  
>      if (display_type == DT_SDL) {
> -        sdl_display_early_init(request_opengl);
> +        if (!sdl_display_early_init(request_opengl)) {
> +            error_report("Failed to initialize SDL");
> +            exit(1);
> +        }
>      }

SDL has some nasty stuff where it tries to replace main() with
a call to SDL_main on certain platforms. Merely #include'ing
SDL.h in vl.c causes this to happen on Win32 at least, perhaps
other platforms too. I'm unclear if your modularization here
will apply to all platforms or just Linux - g_module at least
can work on Win32.

Anyway, to fully support modularization without nasty surprises,
we probably want to stop this main() replacement.

I sent a patch for that last year

  https://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg01631.html

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH for 2.8 3/3] sdl: Modularize

[Gerd Hoffmann]

On Mi, 2016-07-27 at 14:26 +0800, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>

more verbose commit message please, especially for those (like me) who
have not worked yet with modules.

Will sdl be loaded unconditionally?
Or only with -display sdl?
What happens if the module is not present?

The main benefit I see in modularizing the ui is that we can move the ui
modules (and the UI libs dependency chain) to a separate rpm subpackage,
can we start that with this series applied?

Can modules have dependencies on other modules?  When it comes to
modularizing spice we will need that as qxl has a spice dependency ...

cheers,
  Gerd

Re: [Qemu-devel] [PATCH for 2.8 3/3] sdl: Modularize

[Fam Zheng]

On Mon, 08/01 13:53, Gerd Hoffmann wrote:
> On Mi, 2016-07-27 at 14:26 +0800, Fam Zheng wrote:
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> 
> more verbose commit message please, especially for those (like me) who
> have not worked yet with modules.

Yeah, my bad. Maybe I should have put RFC to the subject too.

> 
> Will sdl be loaded unconditionally?
> Or only with -display sdl?
> What happens if the module is not present?

Only with -display sdl, and report error + exit if the module is not present.

> 
> The main benefit I see in modularizing the ui is that we can move the ui
> modules (and the UI libs dependency chain) to a separate rpm subpackage,
> can we start that with this series applied?

Yes, that is the intention, but we need v2 to fix SDL audio and baum to achieve
that.

Colin also tested that not loading gui shared libraries (gtk?) speeds up
starting up.

> 
> Can modules have dependencies on other modules?  When it comes to
> modularizing spice we will need that as qxl has a spice dependency ...

Yes. spice's modules init function can call module_call_init(MODULE_INIT_QXL),
then check if QXL is available after that.

Fam

Re: [Qemu-devel] [PATCH for 2.8 3/3] sdl: Modularize

[Fam Zheng]

On Mon, 08/01 12:31, Paolo Bonzini wrote:
> > +
> > +static void sdl_init_fn(void)
> > +{
> > +    sdl_register_init_fun(sdl_display_init_do);
> > +}
> > +
> > +sdl_init(sdl_init_fn);
> 
> Can you use __attribute__((constructor)) instead of going through the
> burden of defining a new type?  The registration function only does a
> single assignment, so it doesn't really have ordering dependencies.

Yeah, it makes sense! Thanks.

Fam