Re: [Qemu-devel] [PATCH v13 5/8] module: implement module loading

[Paolo Bonzini]

Il 11/10/2013 03:16, Fam Zheng ha scritto:
> This patch adds loading, stamp checking and initialization of modules.
> 
> The init function of dynamic module is no longer directly called as
> __attribute__((constructor)) in static linked version, it is called
> only after passed the checking of presense of stamp symbol:
> 
>     qemu_stamp_$(date +%s$$$RANDOM)
> 
> 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 <address@hidden>
> ---
>  Makefile              |   3 ++
>  configure             |  29 +++++++++-----
>  include/qemu/module.h |  12 ++++++
>  module-common.c       |  10 +++++
>  rules.mak             |   7 ++--
>  scripts/create_config |  14 +++++++
>  util/module.c         | 107 
> +++++++++++++++++++++++++++++++++++++++++++++++++-
>  7 files changed, 167 insertions(+), 15 deletions(-)
>  create mode 100644 module-common.c
> 
> diff --git a/Makefile b/Makefile
> index a8488d6..51de298 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -196,6 +196,9 @@ Makefile: $(version-obj-y) $(version-lobj-y)
>  libqemustub.a: $(stub-obj-y)
>  libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o
>  
> +block-modules = $(foreach o,$(block-obj-m),"$(basename $(subst /,-,$o))",) 
> NULL
> +util/module.o-cflags = -D'CONFIG_BLOCK_MODULES=$(block-modules)'
> +
>  ######################################################################
>  
>  qemu-img.o: qemu-img-cmds.h
> diff --git a/configure b/configure
> index 7b8771a..4bc981e 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"

This should be \${libdir}/qemu.  I can fix it when applying.

>  libexecdir="\${prefix}/libexec"
>  includedir="\${prefix}/include"
>  sysconfdir="\${prefix}/etc"
> @@ -660,7 +661,8 @@ for opt do
>    ;;
>    --disable-debug-info)
>    ;;
> -  --enable-modules) modules="yes"
> +  --enable-modules)
> +      modules="yes"
>    ;;
>    --cpu=*)
>    ;;
> @@ -2291,15 +2293,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
> @@ -3660,6 +3666,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`"
> @@ -3786,6 +3793,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
> @@ -3804,6 +3812,7 @@ echo "libs_softmmu=$libs_softmmu" >> $config_host_mak
>  
>  echo "ARCH=$ARCH" >> $config_host_mak
>  
> +echo "CONFIG_STAMP=$(date +%s$$$RANDOM)" >> $config_host_mak

Adding an underscore between the "fields" is a bit nicer.  Can also be
done when applying.

Paolo

>  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..47b7f1d 100644
> --- a/include/qemu/module.h
> +++ b/include/qemu/module.h
> @@ -14,11 +14,22 @@
>  #ifndef QEMU_MODULE_H
>  #define QEMU_MODULE_H
>  
> +#ifdef BUILD_DSO
> +void DSO_STAMP_FUN(void);
> +/* For error message, this function is an identification of qemu module */
> +void qemu_module_dummy(void);
> +
> +#define module_init(function, type)                                         \
> +static void __attribute__((constructor)) do_qemu_init_ ## function(void) {  \
> +    register_dso_module_init(function, type);                               \
> +}
> +#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,
> @@ -34,6 +45,7 @@ typedef enum {
>  #define type_init(function) module_init(function, MODULE_INIT_QOM)
>  
>  void register_module_init(void (*fn)(void), module_init_type type);
> +void register_dso_module_init(void (*fn)(void), module_init_type type);
>  
>  void module_call_init(module_init_type type);
>  
> diff --git a/module-common.c b/module-common.c
> new file mode 100644
> index 0000000..50c6750
> --- /dev/null
> +++ b/module-common.c
> @@ -0,0 +1,10 @@
> +#include "config-host.h"
> +#include "qemu/module.h"
> +
> +void qemu_module_dummy(void)
> +{
> +}
> +
> +void DSO_STAMP_FUN(void)
> +{
> +}
> diff --git a/rules.mak b/rules.mak
> index 2ff2f16..b0ccfcd 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -69,9 +69,9 @@ 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
> +%$(DSOSUF): %.mo libqemustub.a module-common.o
>       $(call LINK,$^)
>  
>  .PHONY: modules
> @@ -178,7 +178,8 @@ $(foreach o,$(filter %.o,$($1)),
>       $(eval $(patsubst %.o,%.mo,$o)-objs := $o))
>  $(foreach o,$(filter %.mo,$($1)),$(eval \
>      $o: $($o-objs)))
> -$(eval modules-m += $(patsubst %.o,%.mo,$($1)))
> +$(eval t := $(patsubst %.o,%.mo,$($1)))
> +$(foreach o,$t,$(eval modules-m += $o)))
>  endef
>  
>  define unnest-vars
> diff --git a/scripts/create_config b/scripts/create_config
> index b1adbf5..d7ba61d 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_STAMP=*)
> +    echo "#define DSO_STAMP_FUN qemu_stamp_${line#*=}"
> +    echo "#define DSO_STAMP_FUN_STR \"qemu_stamp_${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..c4115be 100644
> --- a/util/module.c
> +++ b/util/module.c
> @@ -13,6 +13,7 @@
>   * GNU GPL, version 2 or (at your option) any later version.
>   */
>  
> +#include <gmodule.h>
>  #include "qemu-common.h"
>  #include "qemu/queue.h"
>  #include "qemu/module.h"
> @@ -21,13 +22,16 @@ typedef struct ModuleEntry
>  {
>      void (*init)(void);
>      QTAILQ_ENTRY(ModuleEntry) node;
> +    module_init_type type;
>  } ModuleEntry;
>  
>  typedef QTAILQ_HEAD(, ModuleEntry) ModuleTypeList;
>  
>  static ModuleTypeList init_type_list[MODULE_INIT_MAX];
>  
> -static void init_types(void)
> +static ModuleTypeList dso_init_list;
> +
> +static void init_lists(void)
>  {
>      static int inited;
>      int i;
> @@ -40,6 +44,8 @@ static void init_types(void)
>          QTAILQ_INIT(&init_type_list[i]);
>      }
>  
> +    QTAILQ_INIT(&dso_init_list);
> +
>      inited = 1;
>  }
>  
> @@ -48,7 +54,7 @@ static ModuleTypeList *find_type(module_init_type type)
>  {
>      ModuleTypeList *l;
>  
> -    init_types();
> +    init_lists();
>  
>      l = &init_type_list[type];
>  
> @@ -62,20 +68,117 @@ void register_module_init(void (*fn)(void), 
> module_init_type type)
>  
>      e = g_malloc0(sizeof(*e));
>      e->init = fn;
> +    e->type = type;
>  
>      l = find_type(type);
>  
>      QTAILQ_INSERT_TAIL(l, e, node);
>  }
>  
> +void register_dso_module_init(void (*fn)(void), module_init_type type)
> +{
> +    ModuleEntry *e;
> +
> +    init_lists();
> +
> +    e = g_malloc0(sizeof(*e));
> +    e->init = fn;
> +    e->type = type;
> +
> +    QTAILQ_INSERT_TAIL(&dso_init_list, e, node);
> +}
> +
> +static void module_load(module_init_type type);
> +
>  void module_call_init(module_init_type type)
>  {
>      ModuleTypeList *l;
>      ModuleEntry *e;
>  
> +    module_load(type);
>      l = find_type(type);
>  
>      QTAILQ_FOREACH(e, l, node) {
>          e->init();
>      }
>  }
> +
> +#ifdef CONFIG_MODULES
> +static void module_load_file(const char *fname)
> +{
> +    GModule *g_module;
> +    void (*sym)(void);
> +    const char *dsosuf = HOST_DSOSUF;
> +    int len = strlen(fname);
> +    int suf_len = strlen(dsosuf);
> +    ModuleEntry *e, *next;
> +
> +    if (len <= suf_len || strcmp(&fname[len - suf_len], dsosuf)) {
> +        /* wrong suffix */
> +        return;
> +    }
> +    if (access(fname, F_OK)) {
> +        return;
> +    }
> +
> +    assert(QTAILQ_EMPTY(&dso_init_list));
> +
> +    g_module = g_module_open(fname, G_MODULE_BIND_LAZY | 
> G_MODULE_BIND_LOCAL);
> +    if (!g_module) {
> +        fprintf(stderr, "Failed to load module: %s\n",
> +                g_module_error());
> +        return;
> +    }
> +    if (!g_module_symbol(g_module, DSO_STAMP_FUN_STR, (gpointer *)&sym)) {
> +        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_dummy", (gpointer 
> *)&sym)) {
> +            fprintf(stderr,
> +                    "Note: only modules from the same build can be 
> loaded.\n");
> +        }
> +        g_module_close(g_module);
> +    } else {
> +        QTAILQ_FOREACH(e, &dso_init_list, node) {
> +            register_module_init(e->init, e->type);
> +        }
> +    }
> +
> +    QTAILQ_FOREACH_SAFE(e, &dso_init_list, node, next) {
> +        QTAILQ_REMOVE(&dso_init_list, e, node);
> +        g_free(e);
> +    }
> +}
> +#endif
> +
> +void module_load(module_init_type type)
> +{
> +#ifdef CONFIG_MODULES
> +    char *fname = NULL;
> +    const char **mp;
> +    static const char *block_modules[] = {
> +        CONFIG_BLOCK_MODULES
> +    };
> +
> +    if (!g_module_supported()) {
> +        return;
> +    }
> +
> +    switch (type) {
> +    case MODULE_INIT_BLOCK:
> +        mp = block_modules;
> +        break;
> +    /* no other types have dynamic modules for now*/
> +    default:
> +        return;
> +    }
> +
> +    for ( ; *mp; mp++) {
> +        fname = g_strdup_printf("%s/%s%s", CONFIG_MODDIR, *mp, HOST_DSOSUF);
> +        module_load_file(fname);
> +        g_free(fname);
> +    }
> +
> +#endif
> +}
>