All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Fam Zheng <famz@redhat.com>
Cc: kwolf@redhat.com, peter.maydell@linaro.org, alex@alex.org.uk,
	mjt@tls.msk.ru, qemu-devel@nongnu.org, vilanova@ac.upc.edu,
	stefanha@redhat.com, xiawenc@linux.vnet.ibm.com, rth@twiddle.net
Subject: Re: [Qemu-devel] [PATCH v12 5/8] module: implement module loading
Date: Thu, 10 Oct 2013 14:16:15 +0200	[thread overview]
Message-ID: <52569A8F.8080501@redhat.com> (raw)
In-Reply-To: <1381404376-28156-6-git-send-email-famz@redhat.com>

Il 10/10/2013 13:26, 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 <famz@redhat.com>
> ---
>  Makefile              |   3 ++
>  configure             |  32 ++++++++++-----
>  include/qemu/module.h |  12 ++++++
>  module-common.c       |  10 +++++
>  rules.mak             |   7 ++--
>  scripts/create_config |  14 +++++++
>  util/module.c         | 107 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  7 files changed, 170 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..dd901ab 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"
> @@ -660,7 +661,8 @@ for opt do
>    ;;
>    --disable-debug-info)
>    ;;
> -  --enable-modules) modules="yes"
> +  --enable-modules)
> +      modules="yes"
>    ;;
>    --cpu=*)
>    ;;
> @@ -685,6 +687,8 @@ for opt do
>    ;;
>    --libdir=*) libdir="$optarg"
>    ;;
> +  --moddir=*) moddir="$optarg"
> +  ;;
>    --libexecdir=*) libexecdir="$optarg"
>    ;;
>    --includedir=*) includedir="$optarg"
> @@ -1084,6 +1088,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"

Is moddir needed?  It should always be LIBDIR/qemu.

Paolo

>  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]"
> @@ -2291,15 +2296,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 +3669,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 +3796,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 +3815,7 @@ echo "libs_softmmu=$libs_softmmu" >> $config_host_mak
>  
>  echo "ARCH=$ARCH" >> $config_host_mak
>  
> +echo "CONFIG_STAMP=$(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..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
> +}
> 

  reply	other threads:[~2013-10-10 12:16 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-10 11:26 [Qemu-devel] [PATCH v12 0/8] Shared Library Module Support Fam Zheng
2013-10-10 11:26 ` [Qemu-devel] [PATCH v12 1/8] ui/Makefile.objs: delete unnecessary cocoa.o dependency Fam Zheng
2013-10-10 11:26 ` [Qemu-devel] [PATCH v12 2/8] make.rule: fix $(obj) to a real relative path Fam Zheng
2013-10-10 11:26 ` [Qemu-devel] [PATCH v12 3/8] rule.mak: allow per object cflags and libs Fam Zheng
2013-10-10 11:26 ` [Qemu-devel] [PATCH v12 4/8] build-sys: introduce common-obj-m and block-obj-m for DSO Fam Zheng
2013-10-10 11:26 ` [Qemu-devel] [PATCH v12 5/8] module: implement module loading Fam Zheng
2013-10-10 12:16   ` Paolo Bonzini [this message]
2013-10-10 12:34     ` Fam Zheng
2013-10-10 13:40       ` Paolo Bonzini
2013-10-11  1:10         ` Fam Zheng
2013-10-10 11:26 ` [Qemu-devel] [PATCH v12 6/8] Makefile: install modules with "make install" Fam Zheng
2013-10-10 11:26 ` [Qemu-devel] [PATCH v12 7/8] .gitignore: ignore module related files (dll, so, mo) Fam Zheng
2013-10-10 11:26 ` [Qemu-devel] [PATCH v12 8/8] block: convert block drivers linked with libs to modules Fam Zheng
2013-10-10 12:19 ` [Qemu-devel] [PATCH v12 0/8] Shared Library Module Support Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52569A8F.8080501@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=alex@alex.org.uk \
    --cc=famz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mjt@tls.msk.ru \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    --cc=stefanha@redhat.com \
    --cc=vilanova@ac.upc.edu \
    --cc=xiawenc@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.