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

[Fam Zheng <famz@redhat.com>]

On Mon, 01/13 22:09, Peter Maydell wrote:
> On 13 January 2014 16:59, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > From: Fam Zheng <famz@redhat.com>
> >
> > 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>
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >  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...)
> 

I think they all usually come with one glib package. But it doean't hurt to
check, it's easy enough to include $i in the error message.

Fam