Re: [Qemu-devel] [PATCH v15 6/9] module: implement module loading

[Peter Maydell]

On 13 January 2014 16:59, Paolo Bonzini <address@hidden> wrote:
> From: Fam Zheng <address@hidden>
>
> 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>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  Makefile              |    3 +
>  configure             |   31 +++++++++-----
>  include/qemu/module.h |   12 +++++
>  rules.mak             |    7 ++-
>  scripts/create_config |   14 ++++++
>  util/module.c         |  107 
> ++++++++++++++++++++++++++++++++++++++++++++++++-
>  6 files changed, 158 insertions(+), 16 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 9de66cb..670ce44 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -203,6 +203,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 6b46c66..c382044 100755
> --- a/configure
> +++ b/configure
> @@ -677,7 +677,8 @@ for opt do
>    ;;
>    --disable-debug-info)
>    ;;
> -  --enable-modules) modules="yes"
> +  --enable-modules)
> +      modules="yes"
>    ;;
>    --cpu=*)
>    ;;
> @@ -1130,7 +1131,7 @@ Advanced options (experts only):
>    --libdir=PATH            install libraries in PATH
>    --sysconfdir=PATH        install config in PATH$confsuffix
>    --localstatedir=PATH     install local state in PATH (set at runtime on 
> win32)
> -  --with-confsuffix=SUFFIX suffix for QEMU data inside datadir and 
> sysconfdir [$confsuffix]
> +  --with-confsuffix=SUFFIX suffix for QEMU data inside 
> datadir/libdir/sysconfdir [$confsuffix]
>    --enable-modules         enable modules support
>    --enable-debug-tcg       enable TCG debugging
>    --disable-debug-tcg      disable TCG debugging (default)
> @@ -2346,15 +2347,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

Also, isn't this going to give rather unhelpful error messages if
the system has glib and gthread but not gmodule? (Or is it
guaranteed that they come as a set? If so then we can either
not bother checking or print a "your system's glib seems to be
busted" message...)

thanks
-- PMM