Patchwork [v8,5/9] module: implement module loading function

login
register
mail settings
Submitter Fam Zheng
Date Sept. 13, 2013, 8:38 a.m.
Message ID <1379061534-19171-6-git-send-email-famz@redhat.com>
Download mbox | patch
Permalink /patch/274687/
State New
Headers show

Comments

Fam Zheng - Sept. 13, 2013, 8:38 a.m.
Added three types of modules:

    typedef enum {
        MODULE_LOAD_BLOCK = 0,
        MODULE_LOAD_UI,
        MODULE_LOAD_NET,
        MODULE_LOAD_MAX,
    } module_load_type;

and their loading function:

    void module_load(module_load_type).

which loads all ".so" files in a subdir under configured ${MODDIR}, e.g.
"/usr/lib/qemu/block". Modules of each type should be loaded before
respective subsystem initialization code.

The init function of dynamic module is no longer with
__attribute__((constructor)) and need to be explicitly called once
loaded. The function name is mangled with per configure fingerprint as:

    init_$(date +%s$$$RANDOM)

Which is known to module_load function: the loading fails if this symbol
is not there. With this, modules built from a different
tree/version/configure will not be loaded.

The module loading code requires gmodule-2.0.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c               |  1 +
 configure             | 29 +++++++++++++------
 include/qemu/module.h | 23 +++++++++++++++
 rules.mak             |  2 +-
 scripts/create_config | 14 ++++++++++
 util/module.c         | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++
 vl.c                  |  2 ++
 7 files changed, 138 insertions(+), 10 deletions(-)
Alex Bligh - Sept. 13, 2013, 9:56 a.m.
On 13 Sep 2013, at 09:38, Fam Zheng wrote:

> Added three types of modules:
> 
>    typedef enum {
>        MODULE_LOAD_BLOCK = 0,
>        MODULE_LOAD_UI,
>        MODULE_LOAD_NET,
>        MODULE_LOAD_MAX,
>    } module_load_type;
> 
> and their loading function:

What is the advantage of this enum and having
different types of module at all? If they are
all built together, why can't they all live
together in the same directory?

Seems like an overcomplication.
Paolo Bonzini - Sept. 13, 2013, 10:02 a.m.
Il 13/09/2013 11:56, Alex Bligh ha scritto:
> 
> On 13 Sep 2013, at 09:38, Fam Zheng wrote:
> 
>> Added three types of modules:
>>
>>    typedef enum {
>>        MODULE_LOAD_BLOCK = 0,
>>        MODULE_LOAD_UI,
>>        MODULE_LOAD_NET,
>>        MODULE_LOAD_MAX,
>>    } module_load_type;
>>
>> and their loading function:
> 
> What is the advantage of this enum and having
> different types of module at all? If they are
> all built together, why can't they all live
> together in the same directory?
> 
> Seems like an overcomplication.

At least block have to be separate, since qemu-img and friends do not
have the symbols needed by ui, network and hardware modules.

Paolo
Fam Zheng - Sept. 13, 2013, 10:05 a.m.
On Fri, 09/13 10:56, Alex Bligh wrote:
> 
> On 13 Sep 2013, at 09:38, Fam Zheng wrote:
> 
> > Added three types of modules:
> > 
> >    typedef enum {
> >        MODULE_LOAD_BLOCK = 0,
> >        MODULE_LOAD_UI,
> >        MODULE_LOAD_NET,
> >        MODULE_LOAD_MAX,
> >    } module_load_type;
> > 
> > and their loading function:
> 
> What is the advantage of this enum and having
> different types of module at all? If they are
> all built together, why can't they all live
> together in the same directory?
> 
> Seems like an overcomplication.
> 

They anyway need to be grouped: block drivers are almost always loaded, but not
ui, or net (although they're not modulized in this series), and they shouldn't
be loaded, from example by qemu-img, because of missing necessary functions.
For this reason, I don't think subdirecotry is an overcomplication over file
name prefix/postfix, or arrays of file name lists.

Fam
Alex Bligh - Sept. 13, 2013, 10:38 a.m.
On 13 Sep 2013, at 11:02, Paolo Bonzini wrote:

>> 
>> Seems like an overcomplication.
> 
> At least block have to be separate, since qemu-img and friends do not
> have the symbols needed by ui, network and hardware modules.

But don't we know what the available modules are at load time? So
qemu-img would never attempt to load (e.g.) the spice module.

Patch

diff --git a/block.c b/block.c
index 26639e8..16ceaaf 100644
--- a/block.c
+++ b/block.c
@@ -4008,6 +4008,7 @@  BlockDriverAIOCB *bdrv_aio_discard(BlockDriverState *bs,
 
 void bdrv_init(void)
 {
+    module_load(MODULE_LOAD_BLOCK);
     module_call_init(MODULE_INIT_BLOCK);
 }
 
diff --git a/configure b/configure
index 5bc7256..3043059 100755
--- a/configure
+++ b/configure
@@ -199,6 +199,7 @@  datadir="\${prefix}/share"
 qemu_docdir="\${prefix}/share/doc/qemu"
 bindir="\${prefix}/bin"
 libdir="\${prefix}/lib"
+moddir="\${prefix}/lib/qemu"
 libexecdir="\${prefix}/libexec"
 includedir="\${prefix}/include"
 sysconfdir="\${prefix}/etc"
@@ -676,6 +677,8 @@  for opt do
   ;;
   --libdir=*) libdir="$optarg"
   ;;
+  --moddir=*) moddir="$optarg"
+  ;;
   --libexecdir=*) libexecdir="$optarg"
   ;;
   --includedir=*) includedir="$optarg"
@@ -1052,6 +1055,7 @@  echo "  --datadir=PATH           install firmware in PATH$confsuffix"
 echo "  --docdir=PATH            install documentation in PATH$confsuffix"
 echo "  --bindir=PATH            install binaries in PATH"
 echo "  --libdir=PATH            install libraries in PATH"
+echo "  --moddir=PATH            install modules in PATH"
 echo "  --sysconfdir=PATH        install config in PATH$confsuffix"
 echo "  --localstatedir=PATH     install local state in PATH (set at runtime on win32)"
 echo "  --with-confsuffix=SUFFIX suffix for QEMU data inside datadir and sysconfdir [$confsuffix]"
@@ -2256,15 +2260,19 @@  if test "$mingw32" = yes; then
 else
     glib_req_ver=2.12
 fi
-if $pkg_config --atleast-version=$glib_req_ver gthread-2.0; then
-    glib_cflags=`$pkg_config --cflags gthread-2.0`
-    glib_libs=`$pkg_config --libs gthread-2.0`
-    CFLAGS="$glib_cflags $CFLAGS"
-    LIBS="$glib_libs $LIBS"
-    libs_qga="$glib_libs $libs_qga"
-else
-    error_exit "glib-$glib_req_ver required to compile QEMU"
-fi
+
+for i in gthread-2.0 gmodule-2.0; do
+    if $pkg_config --atleast-version=$glib_req_ver $i; then
+        glib_cflags=`$pkg_config --cflags $i`
+        glib_libs=`$pkg_config --libs $i`
+        CFLAGS="$glib_cflags $CFLAGS"
+        LIBS="$glib_libs $LIBS"
+        libs_qga="$glib_libs $libs_qga"
+    else
+        error_exit "glib-$glib_req_ver required to compile QEMU"
+    fi
+done
+
 
 ##########################################
 # pixman support probe
@@ -3557,6 +3565,7 @@  echo "Install prefix    $prefix"
 echo "BIOS directory    `eval echo $qemu_datadir`"
 echo "binary directory  `eval echo $bindir`"
 echo "library directory `eval echo $libdir`"
+echo "module directory  `eval echo $moddir`"
 echo "libexec directory `eval echo $libexecdir`"
 echo "include directory `eval echo $includedir`"
 echo "config directory  `eval echo $sysconfdir`"
@@ -3680,6 +3689,7 @@  echo all: >> $config_host_mak
 echo "prefix=$prefix" >> $config_host_mak
 echo "bindir=$bindir" >> $config_host_mak
 echo "libdir=$libdir" >> $config_host_mak
+echo "moddir=$moddir" >> $config_host_mak
 echo "libexecdir=$libexecdir" >> $config_host_mak
 echo "includedir=$includedir" >> $config_host_mak
 echo "mandir=$mandir" >> $config_host_mak
@@ -3698,6 +3708,7 @@  echo "libs_softmmu=$libs_softmmu" >> $config_host_mak
 
 echo "ARCH=$ARCH" >> $config_host_mak
 
+echo "CONFIG_FINGERPRINT=$(date +%s$$$RANDOM)" >> $config_host_mak
 if test "$modules" = "yes"; then
   echo "CONFIG_MODULES=y" >> $config_host_mak
 fi
diff --git a/include/qemu/module.h b/include/qemu/module.h
index c4ccd57..6458d8f 100644
--- a/include/qemu/module.h
+++ b/include/qemu/module.h
@@ -14,11 +14,25 @@ 
 #ifndef QEMU_MODULE_H
 #define QEMU_MODULE_H
 
+#ifdef BUILD_DSO
+
+/* For error message, this function is an identification of qemu module */
+void qemu_module_do_init(void (*init)(void));
+
+/* To restrict loading of arbitrary DSO, The init function name is changed per
+ * "./configure" to refuse unknown DSO file */
+void DSO_INIT_FUN(void);
+
+#define module_init(function, type)                                         \
+void qemu_module_do_init(void (*init)(void)) { init(); }                    \
+void DSO_INIT_FUN(void) { qemu_module_do_init(function); }
+#else
 /* This should not be used directly.  Use block_init etc. instead.  */
 #define module_init(function, type)                                         \
 static void __attribute__((constructor)) do_qemu_init_ ## function(void) {  \
     register_module_init(function, type);                                   \
 }
+#endif
 
 typedef enum {
     MODULE_INIT_BLOCK,
@@ -37,4 +51,13 @@  void register_module_init(void (*fn)(void), module_init_type type);
 
 void module_call_init(module_init_type type);
 
+typedef enum {
+    MODULE_LOAD_BLOCK = 0,
+    MODULE_LOAD_UI,
+    MODULE_LOAD_NET,
+    MODULE_LOAD_MAX,
+} module_load_type;
+
+void module_load(module_load_type type);
+
 #endif
diff --git a/rules.mak b/rules.mak
index 43a502e..0670366 100644
--- a/rules.mak
+++ b/rules.mak
@@ -62,7 +62,7 @@  endif
 %.o: %.dtrace
 	$(call quiet-command,dtrace -o $@ -G -s $<, "  GEN   $(TARGET_DIR)$@")
 
-%$(DSOSUF): QEMU_CFLAGS += -fPIC
+%$(DSOSUF): QEMU_CFLAGS += -fPIC -DBUILD_DSO
 %$(DSOSUF): LDFLAGS += $(LDFLAGS_SHARED)
 %$(DSOSUF): %.mo libqemustub.a
 	$(call LINK,$^)
diff --git a/scripts/create_config b/scripts/create_config
index b1adbf5..ecc5d4d 100755
--- a/scripts/create_config
+++ b/scripts/create_config
@@ -26,6 +26,17 @@  case $line in
     # save for the next definitions
     prefix=${line#*=}
     ;;
+ moddir=*)
+    eval "moddir=\"${line#*=}\""
+    echo "#define CONFIG_MODDIR \"$moddir\""
+    ;;
+ CONFIG_FINGERPRINT=*)
+    echo "#define DSO_INIT_FUN init_${line#*=}"
+    echo "#define DSO_INIT_FUN_STR \"init_${line#*=}\""
+    ;;
+ CONFIG_MODULES=*)
+    echo "#define CONFIG_MODULES \"${line#*=}\""
+    ;;
  CONFIG_AUDIO_DRIVERS=*)
     drivers=${line#*=}
     echo "#define CONFIG_AUDIO_DRIVERS \\"
@@ -104,6 +115,9 @@  case $line in
     value=${line#*=}
     echo "#define $name $value"
     ;;
+ DSOSUF=*)
+    echo "#define HOST_DSOSUF \"${line#*=}\""
+    ;;
 esac
 
 done # read
diff --git a/util/module.c b/util/module.c
index 7acc33d..9135c14 100644
--- a/util/module.c
+++ b/util/module.c
@@ -13,6 +13,8 @@ 
  * GNU GPL, version 2 or (at your option) any later version.
  */
 
+#include <gmodule.h>
+#include <dirent.h>
 #include "qemu-common.h"
 #include "qemu/queue.h"
 #include "qemu/module.h"
@@ -79,3 +81,78 @@  void module_call_init(module_init_type type)
         e->init();
     }
 }
+
+#ifdef CONFIG_MODULES
+static void module_load_file(const char *fname)
+{
+    GModule *g_module;
+    void (*init_fun)(void);
+    const char *dsosuf = HOST_DSOSUF;
+    int len = strlen(fname);
+    int suf_len = strlen(dsosuf);
+
+    if (len <= suf_len || strcmp(&fname[len - suf_len], dsosuf)) {
+        /* wrong suffix */
+        return;
+    }
+    g_module = g_module_open(fname, G_MODULE_BIND_LAZY | G_MODULE_BIND_LOCAL);
+    if (!g_module) {
+        fprintf(stderr, "Failed to open module file %s\n",
+                g_module_error());
+        return;
+    }
+    if (!g_module_symbol(g_module, DSO_INIT_FUN_STR, (gpointer *)&init_fun)) {
+        fprintf(stderr, "Failed to initialize module: %s\n",
+                fname);
+        /* Print some info if this is a QEMU module (but from different build),
+         * this will make debugging user problems easier. */
+        if (g_module_symbol(g_module, "qemu_module_do_init",
+                            (gpointer *)&init_fun)) {
+            fprintf(stderr,
+                    "Note: only modules from the same build can be loaded.\n");
+        }
+        g_module_close(g_module);
+        return;
+    }
+    init_fun();
+}
+#endif
+
+void module_load(module_load_type type)
+{
+#ifdef CONFIG_MODULES
+    const char *path;
+    char *fname = NULL;
+    DIR *dp;
+    struct dirent *ep = NULL;
+
+    if (!g_module_supported()) {
+        return;
+    }
+
+    switch (type) {
+    case MODULE_LOAD_BLOCK:
+        path = CONFIG_MODDIR "/block/";
+        break;
+    case MODULE_LOAD_UI:
+        path = CONFIG_MODDIR "/ui/";
+        break;
+    case MODULE_LOAD_NET:
+        path = CONFIG_MODDIR "/net/";
+        break;
+    default:
+        return;
+    }
+
+    dp = opendir(path);
+    if (!dp) {
+        fprintf(stderr, "Failed to open dir %s\n", path);
+        return;
+    }
+    for (ep = readdir(dp); ep; ep = readdir(dp)) {
+        fname = g_strdup_printf("%s%s", path, ep->d_name);
+        module_load_file(fname);
+        g_free(fname);
+    }
+#endif
+}
diff --git a/vl.c b/vl.c
index b4b119a..659e53a 100644
--- a/vl.c
+++ b/vl.c
@@ -2940,6 +2940,8 @@  int main(int argc, char **argv, char **envp)
 #endif
     }
 
+    module_load(MODULE_LOAD_UI);
+    module_load(MODULE_LOAD_NET);
     module_call_init(MODULE_INIT_QOM);
 
     qemu_add_opts(&qemu_drive_opts);