[Qemu-devel] [RFC PATCH v5 0/6] Shared Library Module Support

[Qemu-devel] [RFC PATCH v5 0/6] Shared Library Module Support

[Fam Zheng]

This series implements feature of shared object building as described in:

http://wiki.qemu.org/Features/Modules

It's achieved in three steps, with extra bonus to change curl and qed to shared
library modules in the end (only to demonstrate the usage, no "make install"
support of .so files yet).

v5: Keep foo.mo-objs idea for module objects.
    Unnest block-obj-m and common-obj-m in Makefile.target.
    Move add-modules to unnest-vars to be reused in Makefile.target.
    Use /dev/null to replace realpath for expand-objs.

v4: Added --enable-modules in the end of series.
    Make nested-vars and obj-base as arguemnts to unnest-vars.
    Take Paolo's idea in comments for v2 and switch back module objects syntax
    to:
        $(obj)/foo.mo : $(addprefix $(obj)/, bar.o biz.o qux.o)

    because this needs less duplication among Makefiles.

Fam Zheng (6):
  make.rule: fix $(obj) to a real relative path
  rule.mak: allow per object cflags and libs
  Makefile: introduce common-obj-m and block-obj-m for DSO
  module: implement module loading function
  configure: introduce --enable-modules
  block: build qed and curl as shared library

 Makefile              | 22 ++++++++++++++-
 Makefile.objs         | 18 ++----------
 Makefile.target       | 20 +++++++++++--
 block.c               |  1 +
 block/Makefile.objs   |  7 +++--
 bsd-user/main.c       |  3 ++
 configure             | 40 ++++++++++++++++++--------
 include/qemu/module.h |  9 ++++++
 linux-user/main.c     |  3 ++
 rules.mak             | 77 ++++++++++++++++++++++++++++++++++++++++++---------
 scripts/create_config |  4 +++
 tests/Makefile        |  2 ++
 util/module.c         | 53 +++++++++++++++++++++++++++++++++++
 vl.c                  |  2 ++
 14 files changed, 215 insertions(+), 46 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [RFC PATCH v5 1/6] make.rule: fix $(obj) to a real relative path

[Fam Zheng]

Makefile.target includes rule.mak and unnested common-obj-y, then prefix
them with '../', this will ignore object specific QEMU_CFLAGS in subdir
Makefile.objs:

    $(obj)/curl.o: QEMU_CFLAGS += $(CURL_CFLAGS)

Because $(obj) here is './block', instead of '../block'. This doesn't
hurt compiling because we basically build all .o from top Makefile,
before entering Makefile.target, but it will affact arriving per-object
libs support.

The starting point of $(obj) is passed in as argument of unnest-vars, as
well as nested variables, so that different Makefiles can pass in a
right value.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 Makefile        | 16 +++++++++++++++-
 Makefile.objs   | 16 +---------------
 Makefile.target | 16 +++++++++++++---
 configure       |  1 +
 rules.mak       | 12 +++++++-----
 tests/Makefile  |  2 ++
 6 files changed, 39 insertions(+), 24 deletions(-)

diff --git a/Makefile b/Makefile
index 806946e..9e603c6 100644
--- a/Makefile
+++ b/Makefile
@@ -115,14 +115,28 @@ defconfig:
 
 ifneq ($(wildcard config-host.mak),)
 include $(SRC_PATH)/Makefile.objs
-include $(SRC_PATH)/tests/Makefile
 endif
 ifeq ($(CONFIG_SMARTCARD_NSS),y)
 include $(SRC_PATH)/libcacard/Makefile
 endif
 
+dummy := $(call unnest-vars,, \
+                stub-obj-y \
+                util-obj-y \
+                qga-obj-y \
+                block-obj-y \
+                common-obj-y)
+
+ifneq ($(wildcard config-host.mak),)
+include $(SRC_PATH)/tests/Makefile
+endif
+
 all: $(DOCS) $(TOOLS) $(HELPERS-y) recurse-all
 
+vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
+
+vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS)
+
 config-host.h: config-host.h-timestamp
 config-host.h-timestamp: config-host.mak
 qemu-options.def: $(SRC_PATH)/qemu-options.hx
diff --git a/Makefile.objs b/Makefile.objs
index f46a4cd..4f7a364 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -41,7 +41,7 @@ libcacard-y += libcacard/vcardt.o
 # single QEMU executable should support all CPUs and machines.
 
 ifeq ($(CONFIG_SOFTMMU),y)
-common-obj-y = $(block-obj-y) blockdev.o blockdev-nbd.o block/
+common-obj-y = blockdev.o blockdev-nbd.o block/
 common-obj-y += net/
 common-obj-y += readline.o
 common-obj-y += qdev-monitor.o device-hotplug.o
@@ -109,17 +109,3 @@ version-lobj-$(CONFIG_WIN32) += $(BUILD_DIR)/version.lo
 # FIXME: a few definitions from qapi-types.o/qapi-visit.o are needed
 # by libqemuutil.a.  These should be moved to a separate .json schema.
 qga-obj-y = qga/ qapi-types.o qapi-visit.o
-
-vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
-
-vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS)
-
-QEMU_CFLAGS+=$(GLIB_CFLAGS)
-
-nested-vars += \
-	stub-obj-y \
-	util-obj-y \
-	qga-obj-y \
-	block-obj-y \
-	common-obj-y
-dummy := $(call unnest-vars)
diff --git a/Makefile.target b/Makefile.target
index 9a49852..1d92523 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -143,13 +143,23 @@ endif # CONFIG_SOFTMMU
 # Workaround for http://gcc.gnu.org/PR55489, see configure.
 %/translate.o: QEMU_CFLAGS += $(TRANSLATE_OPT_CFLAGS)
 
-nested-vars += obj-y
+dummy := $(call unnest-vars,,obj-y)
 
-# This resolves all nested paths, so it must come last
+# we are making another call to unnest-vars with different vars, protect obj-y,
+# it can be overriden in subdir Makefile.objs
+obj-y-save := $(obj-y)
+
+block-obj-y :=
+common-obj-y :=
 include $(SRC_PATH)/Makefile.objs
+dummy := $(call unnest-vars,..,block-obj-y common-obj-y)
+
+# Now restore obj-y
+obj-y := $(obj-y-save)
 
 all-obj-y = $(obj-y)
-all-obj-y += $(addprefix ../, $(common-obj-y))
+all-obj-y += $(addprefix ../, $(common-obj-y) $(block-obj-y))
+
 
 ifndef CONFIG_HAIKU
 LIBS+=-lm
diff --git a/configure b/configure
index e989609..cc3cd4d 100755
--- a/configure
+++ b/configure
@@ -2251,6 +2251,7 @@ 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
diff --git a/rules.mak b/rules.mak
index 4499745..c08b356 100644
--- a/rules.mak
+++ b/rules.mak
@@ -103,9 +103,6 @@ clean: clean-timestamp
 
 # magic to descend into other directories
 
-obj := .
-old-nested-dirs :=
-
 define push-var
 $(eval save-$2-$1 = $(value $1))
 $(eval $1 :=)
@@ -119,9 +116,11 @@ endef
 
 define unnest-dir
 $(foreach var,$(nested-vars),$(call push-var,$(var),$1/))
-$(eval obj := $(obj)/$1)
+$(eval obj-parent-$1 := $(obj))
+$(eval obj := $(if $(obj),$(obj)/$1,$1))
 $(eval include $(SRC_PATH)/$1/Makefile.objs)
-$(eval obj := $(patsubst %/$1,%,$(obj)))
+$(eval obj := $(obj-parent-$1))
+$(eval obj-parent-$1 := )
 $(foreach var,$(nested-vars),$(call pop-var,$(var),$1/))
 endef
 
@@ -136,6 +135,9 @@ $(if $(nested-dirs),
 endef
 
 define unnest-vars
+$(eval obj := $1)
+$(eval nested-vars := $2)
+$(eval old-nested-dirs := )
 $(call unnest-vars-1)
 $(foreach var,$(nested-vars),$(eval $(var) := $(filter-out %/, $($(var)))))
 $(shell mkdir -p $(sort $(foreach var,$(nested-vars),$(dir $($(var))))))
diff --git a/tests/Makefile b/tests/Makefile
index baba9e9..1b4048c 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -110,6 +110,8 @@ test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o
 $(test-obj-y): QEMU_INCLUDES += -Itests
 QEMU_CFLAGS += -I$(SRC_PATH)/tests
 
+dummy := $(call unnest-vars,..,block-obj-y)
+
 tests/test-x86-cpuid.o: QEMU_INCLUDES += -I$(SRC_PATH)/target-i386
 
 tests/check-qint$(EXESUF): tests/check-qint.o libqemuutil.a
-- 
1.8.3.1

[Qemu-devel] [RFC PATCH v5 2/6] rule.mak: allow per object cflags and libs

[Fam Zheng]

Adds extract-libs in LINK to expand any "per object libs", the syntax to define
such a libs options is like:

        foo.o-libs := $(CURL_LIBS)

in block/Makefile.objs.

Similarly,

        foo.o-cflags := $(FOO_CFLAGS)

is also supported.

"foo.o" must be listed a nested var (e.g. common-obj-y) to make the
option variables effective.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 rules.mak | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/rules.mak b/rules.mak
index c08b356..6342d60 100644
--- a/rules.mak
+++ b/rules.mak
@@ -17,15 +17,17 @@ QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(*D)/$(*F).d
 # Same as -I$(SRC_PATH) -I., but for the nested source/object directories
 QEMU_INCLUDES += -I$(<D) -I$(@D)
 
+extract-libs = $(strip $(foreach o,$1,$($o-libs)))
+
 %.o: %.c
-	$(call quiet-command,$(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $<,"  CC    $(TARGET_DIR)$@")
+	$(call quiet-command,$(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) -c -o $@ $<,"  CC    $(TARGET_DIR)$@")
 %.o: %.rc
 	$(call quiet-command,$(WINDRES) -I. -o $@ $<,"  RC    $(TARGET_DIR)$@")
 
 ifeq ($(LIBTOOL),)
 LINK = $(call quiet-command,$(CC) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ \
        $(sort $(filter %.o, $1)) $(filter-out %.o, $1) $(version-obj-y) \
-       $(LIBS),"  LINK  $(TARGET_DIR)$@")
+       $(call extract-libs,$^) $(LIBS),"  LINK  $(TARGET_DIR)$@")
 else
 LIBTOOL += $(if $(V),,--quiet)
 %.lo: %.c
@@ -41,7 +43,7 @@ LINK = $(call quiet-command,\
        $(sort $(filter %.o, $1)) $(filter-out %.o, $1) \
        $(if $(filter %.lo %.la,$^),$(version-lobj-y),$(version-obj-y)) \
        $(if $(filter %.lo %.la,$^),$(LIBTOOLFLAGS)) \
-       $(LIBS),$(if $(filter %.lo %.la,$^),"lt LINK ", "  LINK  ")"$(TARGET_DIR)$@")
+       $(call extract-libs,$^) $(LIBS),$(if $(filter %.lo %.la,$^),"lt LINK ", "  LINK  ")"$(TARGET_DIR)$@")
 endif
 
 %.asm: %.S
@@ -114,11 +116,22 @@ $(eval $1 = $(value save-$2-$1) $$(subdir-$2-$1))
 $(eval save-$2-$1 :=)
 endef
 
+define fix-obj-vars
+$(foreach v,$($1), \
+	$(if $($v-cflags), \
+		$(eval $2$v-cflags := $($v-cflags)) \
+		$(eval $v-cflags := )) \
+	$(if $($v-libs), \
+		$(eval $2$v-libs := $($v-libs)) \
+		$(eval $v-libs := )))
+endef
+
 define unnest-dir
 $(foreach var,$(nested-vars),$(call push-var,$(var),$1/))
 $(eval obj-parent-$1 := $(obj))
 $(eval obj := $(if $(obj),$(obj)/$1,$1))
 $(eval include $(SRC_PATH)/$1/Makefile.objs)
+$(foreach v,$(nested-vars),$(call fix-obj-vars,$v,$(if $(obj),$(obj)/)))
 $(eval obj := $(obj-parent-$1))
 $(eval obj-parent-$1 := )
 $(foreach var,$(nested-vars),$(call pop-var,$(var),$1/))
-- 
1.8.3.1

[Qemu-devel] [RFC PATCH v5 3/6] Makefile: introduce common-obj-m and block-obj-m for DSO

[Fam Zheng]

Add necessary rules and flags for shared object generation.
$(common-obj-m) will include $(block-obj-m), like $(common-obj-y) does
for $(block-obj-y). The new rules introduced here are:

0) For all %.so compiling:

    QEMU_CFLAGS += -fPIC

1) %.o in $(common-obj-m) is compiled to %.o, then linked to %.so.

2) %.mo in $(common-obj-m) is the placeholder for %.so for pattern
matching in Makefile. It's linked to "-shared" with all its dependencies
(multiple *.o) as input. Which means the list of depended objects must
be ruled out in each sub-Makefile.objs with an variable:

    foo.mo-objs := bar.o baz.o qux.o

in the same style with foo.o-cflags and foo.o-libs.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 Makefile      |  8 +++++++-
 Makefile.objs |  2 ++
 configure     |  6 ++++++
 rules.mak     | 48 ++++++++++++++++++++++++++++++++++++++++--------
 4 files changed, 55 insertions(+), 9 deletions(-)

diff --git a/Makefile b/Makefile
index 9e603c6..db1d091 100644
--- a/Makefile
+++ b/Makefile
@@ -125,7 +125,9 @@ dummy := $(call unnest-vars,, \
                 util-obj-y \
                 qga-obj-y \
                 block-obj-y \
-                common-obj-y)
+                block-obj-m \
+                common-obj-y \
+                common-obj-m)
 
 ifneq ($(wildcard config-host.mak),)
 include $(SRC_PATH)/tests/Makefile
@@ -249,6 +251,10 @@ clean:
 	rm -f qemu-options.def
 	find . -name '*.[oda]' -type f -exec rm -f {} +
 	find . -name '*.l[oa]' -type f -exec rm -f {} +
+	find . -name '*.so' -type f -exec rm -f {} +
+	find . -name '*.mo' -type f -exec rm -f {} +
+	find . -name '*.dll' -type f -exec rm -f {} +
+
 	rm -f $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~
 	rm -Rf .libs
 	rm -f qemu-img-cmds.h
diff --git a/Makefile.objs b/Makefile.objs
index 4f7a364..023166b 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -19,6 +19,8 @@ block-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o
 block-obj-y += qemu-coroutine-sleep.o
 block-obj-y += coroutine-$(CONFIG_COROUTINE_BACKEND).o
 
+block-obj-m = block/
+
 ifeq ($(CONFIG_VIRTIO)$(CONFIG_VIRTFS)$(CONFIG_PCI),yyy)
 # Lots of the fsdev/9pcode is pulled in by vl.c via qemu_fsdev_add.
 # only pull in the actual virtio-9p device if we also enabled virtio.
diff --git a/configure b/configure
index cc3cd4d..c6d4a62 100755
--- a/configure
+++ b/configure
@@ -190,6 +190,8 @@ mingw32="no"
 gcov="no"
 gcov_tool="gcov"
 EXESUF=""
+DSOSUF=".so"
+LDFLAGS_SHARED="-shared"
 prefix="/usr/local"
 mandir="\${prefix}/share/man"
 datadir="\${prefix}/share"
@@ -485,6 +487,7 @@ OpenBSD)
 Darwin)
   bsd="yes"
   darwin="yes"
+  LDFLAGS_SHARED="-bundle"
   if [ "$cpu" = "x86_64" ] ; then
     QEMU_CFLAGS="-arch x86_64 $QEMU_CFLAGS"
     LDFLAGS="-arch x86_64 $LDFLAGS"
@@ -584,6 +587,7 @@ fi
 
 if test "$mingw32" = "yes" ; then
   EXESUF=".exe"
+  DSOSUF=".dll"
   QEMU_CFLAGS="-DWIN32_LEAN_AND_MEAN -DWINVER=0x501 $QEMU_CFLAGS"
   # enable C99/POSIX format strings (needs mingw32-runtime 3.15 or later)
   QEMU_CFLAGS="-D__USE_MINGW_ANSI_STDIO=1 $QEMU_CFLAGS"
@@ -4175,6 +4179,8 @@ echo "LIBTOOLFLAGS=$LIBTOOLFLAGS" >> $config_host_mak
 echo "LIBS+=$LIBS" >> $config_host_mak
 echo "LIBS_TOOLS+=$libs_tools" >> $config_host_mak
 echo "EXESUF=$EXESUF" >> $config_host_mak
+echo "DSOSUF=$DSOSUF" >> $config_host_mak
+echo "LDFLAGS_SHARED=$LDFLAGS_SHARED" >> $config_host_mak
 echo "LIBS_QGA+=$libs_qga" >> $config_host_mak
 echo "POD2MAN=$POD2MAN" >> $config_host_mak
 echo "TRANSLATE_OPT_CFLAGS=$TRANSLATE_OPT_CFLAGS" >> $config_host_mak
diff --git a/rules.mak b/rules.mak
index 6342d60..ea97888 100644
--- a/rules.mak
+++ b/rules.mak
@@ -18,6 +18,9 @@ QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(*D)/$(*F).d
 QEMU_INCLUDES += -I$(<D) -I$(@D)
 
 extract-libs = $(strip $(foreach o,$1,$($o-libs)))
+expand-objs = $(strip $(sort $(filter %.o,$1)) \
+                  $(shell cat /dev/null $(filter %.mo,$1)) \
+                  $(filter-out %.o %.mo,$1))
 
 %.o: %.c
 	$(call quiet-command,$(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) -c -o $@ $<,"  CC    $(TARGET_DIR)$@")
@@ -26,8 +29,8 @@ extract-libs = $(strip $(foreach o,$1,$($o-libs)))
 
 ifeq ($(LIBTOOL),)
 LINK = $(call quiet-command,$(CC) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ \
-       $(sort $(filter %.o, $1)) $(filter-out %.o, $1) $(version-obj-y) \
-       $(call extract-libs,$^) $(LIBS),"  LINK  $(TARGET_DIR)$@")
+       $(call expand-objs $1) $(version-obj-y) \
+       $(call extract-libs,$1) $(LIBS),"  LINK  $(TARGET_DIR)$@")
 else
 LIBTOOL += $(if $(V),,--quiet)
 %.lo: %.c
@@ -38,12 +41,12 @@ LIBTOOL += $(if $(V),,--quiet)
 	$(call quiet-command,$(LIBTOOL) --mode=compile --tag=CC dtrace -o $@ -G -s $<, " lt GEN $(TARGET_DIR)$@")
 
 LINK = $(call quiet-command,\
-       $(if $(filter %.lo %.la,$^),$(LIBTOOL) --mode=link --tag=CC \
+       $(if $(filter %.lo %.la,$1),$(LIBTOOL) --mode=link --tag=CC \
        )$(CC) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ \
-       $(sort $(filter %.o, $1)) $(filter-out %.o, $1) \
-       $(if $(filter %.lo %.la,$^),$(version-lobj-y),$(version-obj-y)) \
-       $(if $(filter %.lo %.la,$^),$(LIBTOOLFLAGS)) \
-       $(call extract-libs,$^) $(LIBS),$(if $(filter %.lo %.la,$^),"lt LINK ", "  LINK  ")"$(TARGET_DIR)$@")
+       $(call expand-objs,$1) \
+       $(if $(filter %.lo %.la,$1),$(version-lobj-y),$(version-obj-y)) \
+       $(if $(filter %.lo %.la,$1),$(LIBTOOLFLAGS)) \
+       $(call extract-libs,$1) $(LIBS),$(if $(filter %.lo %.la,$1),"lt LINK ", "  LINK  ")"$(TARGET_DIR)$@")
 endif
 
 %.asm: %.S
@@ -58,6 +61,17 @@ endif
 %.o: %.dtrace
 	$(call quiet-command,dtrace -o $@ -G -s $<, "  GEN   $(TARGET_DIR)$@")
 
+%$(DSOSUF): QEMU_CFLAGS += -fPIC
+%$(DSOSUF): LDFLAGS += $(LDFLAGS_SHARED)
+%$(DSOSUF): %.mo
+	$(call LINK,$^)
+
+.PHONY: FORCE
+%.mo: FORCE
+	$(call quiet-command,echo $(sort $(filter-out FORCE,$^)) > $@-new,"  GEN   $(TARGET_DIR)$@")
+	$(call quiet-command,diff $@-new $@ &>/dev/null || mv $@-new $@)
+	@rm $@-new &>/dev/null || true
+
 %$(EXESUF): %.o
 	$(call LINK,$^)
 
@@ -123,7 +137,10 @@ $(foreach v,$($1), \
 		$(eval $v-cflags := )) \
 	$(if $($v-libs), \
 		$(eval $2$v-libs := $($v-libs)) \
-		$(eval $v-libs := )))
+		$(eval $v-libs := )) \
+	$(if $($v-objs), \
+		$(eval $2$v-objs := $(addprefix $2,$($v-objs))) \
+		$(eval $v-objs := )))
 endef
 
 define unnest-dir
@@ -147,6 +164,14 @@ $(if $(nested-dirs),
   $(call unnest-vars-1))
 endef
 
+define add-modules
+$(foreach o,$(filter %.o,$($1)),$(eval \
+    $(patsubst %.o,%.mo,$o): $o))
+$(foreach o,$(filter %.mo,$($1)),$(eval \
+    $o: $($o-objs)))
+$(eval modules-m += $(patsubst %.o,%.mo,$($1)))
+endef
+
 define unnest-vars
 $(eval obj := $1)
 $(eval nested-vars := $2)
@@ -156,4 +181,11 @@ $(foreach var,$(nested-vars),$(eval $(var) := $(filter-out %/, $($(var)))))
 $(shell mkdir -p $(sort $(foreach var,$(nested-vars),$(dir $($(var))))))
 $(foreach var,$(nested-vars), $(eval \
   -include $(addsuffix *.d, $(sort $(dir $($(var)))))))
+
+$(foreach v,$(filter %-m,$(nested-vars)), \
+    $(call add-modules,$v))
+
+$(eval modules: $(patsubst %.mo,%$(DSOSUF),$(modules-m)))
+$(eval all: modules)
+
 endef
-- 
1.8.3.1

[Qemu-devel] [RFC PATCH v5 4/6] module: implement module loading function

[Fam Zheng]

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 "${PREFIX}/qemu/", e.g.
"/usr/lib/qemu/block". Modules of each type should be loaded before
respective subsystem initialization code.

Requires gmodule-2.0 from glib.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c               |  1 +
 bsd-user/main.c       |  3 +++
 configure             | 22 ++++++++++++---------
 include/qemu/module.h |  9 +++++++++
 linux-user/main.c     |  3 +++
 scripts/create_config |  4 ++++
 util/module.c         | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++
 vl.c                  |  2 ++
 8 files changed, 88 insertions(+), 9 deletions(-)

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/bsd-user/main.c b/bsd-user/main.c
index f9246aa..6cb9e35 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -33,6 +33,7 @@
 #include "tcg.h"
 #include "qemu/timer.h"
 #include "qemu/envlist.h"
+#include "qemu/module.h"
 
 int singlestep;
 #if defined(CONFIG_USE_GUEST_BASE)
@@ -749,6 +750,8 @@ int main(int argc, char **argv)
     if (argc <= 1)
         usage();
 
+    module_load(MODULE_LOAD_UI);
+    module_load(MODULE_LOAD_NET);
     module_call_init(MODULE_INIT_QOM);
 
     if ((envlist = envlist_create()) == NULL) {
diff --git a/configure b/configure
index c6d4a62..a2858c2 100755
--- a/configure
+++ b/configure
@@ -2252,15 +2252,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
diff --git a/include/qemu/module.h b/include/qemu/module.h
index c4ccd57..f00bc25 100644
--- a/include/qemu/module.h
+++ b/include/qemu/module.h
@@ -37,4 +37,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/linux-user/main.c b/linux-user/main.c
index 5c2f7b2..db08c23 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -34,6 +34,7 @@
 #include "qemu/timer.h"
 #include "qemu/envlist.h"
 #include "elf.h"
+#include <qemu/module.h>
 
 char *exec_path;
 
@@ -3551,6 +3552,8 @@ int main(int argc, char **argv, char **envp)
     int i;
     int ret;
 
+    module_load(MODULE_LOAD_UI);
+    module_load(MODULE_LOAD_NET);
     module_call_init(MODULE_INIT_QOM);
 
     qemu_cache_utils_init(envp);
diff --git a/scripts/create_config b/scripts/create_config
index b1adbf5..7a54f2d 100755
--- a/scripts/create_config
+++ b/scripts/create_config
@@ -25,6 +25,7 @@ case $line in
  prefix=*)
     # save for the next definitions
     prefix=${line#*=}
+    echo "#define CONFIG_PREFIX \"$prefix\""
     ;;
  CONFIG_AUDIO_DRIVERS=*)
     drivers=${line#*=}
@@ -104,6 +105,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..ef75f8e 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,54 @@ void module_call_init(module_init_type type)
         e->init();
     }
 }
+
+void module_load(module_load_type type)
+{
+    const char *path;
+    const char *dsosuf = HOST_DSOSUF;
+    char *fname;
+    int suf_len = strlen(dsosuf);
+    DIR *dp;
+    struct dirent *ep = NULL;
+    GModule *g_module;
+
+    if (!g_module_supported()) {
+        return;
+    }
+
+    switch (type) {
+    case MODULE_LOAD_BLOCK:
+        path = CONFIG_PREFIX "/qemu/block/";
+        break;
+    case MODULE_LOAD_UI:
+        path = CONFIG_PREFIX "/qemu/ui/";
+        break;
+    case MODULE_LOAD_NET:
+        path = CONFIG_PREFIX "/qemu/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)) {
+        int len = strlen(ep->d_name);
+        if (len > suf_len &&
+                !strcmp(&ep->d_name[len - suf_len], dsosuf)) {
+            fname = g_strdup_printf("%s%s", path, ep->d_name);
+            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());
+                g_free(fname);
+                continue;
+            }
+            g_free(fname);
+        }
+    }
+}
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);
-- 
1.8.3.1

[Qemu-devel] [RFC PATCH v5 5/6] configure: introduce --enable-modules

[Fam Zheng]

The new option will enable support of shared object build. Otherwise
objects are static linked to executables.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 Makefile.target | 6 +++++-
 configure       | 8 ++++++++
 rules.mak       | 8 ++++++--
 3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index 1d92523..beab0f9 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -152,7 +152,11 @@ obj-y-save := $(obj-y)
 block-obj-y :=
 common-obj-y :=
 include $(SRC_PATH)/Makefile.objs
-dummy := $(call unnest-vars,..,block-obj-y common-obj-y)
+dummy := $(call unnest-vars,.., \
+               block-obj-y \
+               block-obj-m \
+               common-obj-y \
+               common-obj-m)
 
 # Now restore obj-y
 obj-y := $(obj-y-save)
diff --git a/configure b/configure
index a2858c2..f1d7fa7 100755
--- a/configure
+++ b/configure
@@ -192,6 +192,7 @@ gcov_tool="gcov"
 EXESUF=""
 DSOSUF=".so"
 LDFLAGS_SHARED="-shared"
+modules="no"
 prefix="/usr/local"
 mandir="\${prefix}/share/man"
 datadir="\${prefix}/share"
@@ -650,6 +651,8 @@ for opt do
   ;;
   --disable-debug-info)
   ;;
+  --enable-modules) modules="yes"
+  ;;
   --cpu=*)
   ;;
   --target-list=*) target_list="$optarg"
@@ -1052,6 +1055,7 @@ echo "  --libdir=PATH            install libraries 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]"
+echo "  --enable-modules         enable modules support"
 echo "  --enable-debug-tcg       enable TCG debugging"
 echo "  --disable-debug-tcg      disable TCG debugging (default)"
 echo "  --enable-debug-info       enable debugging information (default)"
@@ -3580,6 +3584,7 @@ echo "python            $python"
 if test "$slirp" = "yes" ; then
     echo "smbd              $smbd"
 fi
+echo "module support    $modules"
 echo "host CPU          $cpu"
 echo "host big endian   $bigendian"
 echo "target list       $target_list"
@@ -3697,6 +3702,9 @@ echo "libs_softmmu=$libs_softmmu" >> $config_host_mak
 
 echo "ARCH=$ARCH" >> $config_host_mak
 
+if test "$modules" = "yes"; then
+  echo "CONFIG_MODULES=y" >> $config_host_mak
+fi
 case "$cpu" in
   arm|i386|x86_64|x32|ppc|aarch64)
     # The TCG interpreter currently does not support ld/st optimization.
diff --git a/rules.mak b/rules.mak
index ea97888..860b8ac 100644
--- a/rules.mak
+++ b/rules.mak
@@ -185,7 +185,11 @@ $(foreach var,$(nested-vars), $(eval \
 $(foreach v,$(filter %-m,$(nested-vars)), \
     $(call add-modules,$v))
 
-$(eval modules: $(patsubst %.mo,%$(DSOSUF),$(modules-m)))
-$(eval all: modules)
+$(if $(CONFIG_MODULES), \
+    $(eval modules: $(patsubst %.mo,%$(DSOSUF),$(modules-m))) \
+    $(eval all: modules), \
+    $(foreach v,$(filter %-m,$(nested-vars)), \
+        $(eval $(patsubst %-m,%-y,$v) += $($v)) \
+        $(eval $v := )))
 
 endef
-- 
1.8.3.1

[Qemu-devel] [RFC PATCH v5 6/6] block: build qed and curl as shared library

[Fam Zheng]

Curl and qed block drivers are built as shared object module.  We have
per object cflags and libs support now, move CURL_CFLAGS and CURL_LIBS
from global option variables to a per object basis.

"make install" is not installing them yet, manually copy it to
${prefix}/qemu/block/ to make it loaded.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/Makefile.objs | 7 ++++---
 configure           | 5 ++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 3bb85b5..741b92f 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -1,7 +1,6 @@
 block-obj-y += raw_bsd.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o vvfat.o
 block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o
-block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
-block-obj-y += qed-check.o
+block-obj-m += qed.mo
 block-obj-y += vhdx.o
 block-obj-y += parallels.o blkdebug.o blkverify.o
 block-obj-y += snapshot.o qapi.o
@@ -23,4 +22,6 @@ common-obj-y += commit.o
 common-obj-y += mirror.o
 common-obj-y += backup.o
 
-$(obj)/curl.o: QEMU_CFLAGS+=$(CURL_CFLAGS)
+curl.o-cflags := $(CURL_CFLAGS)
+curl.o-libs := $(CURL_LIBS)
+qed.mo-objs := qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o qed-check.o
diff --git a/configure b/configure
index f1d7fa7..f8be093 100755
--- a/configure
+++ b/configure
@@ -2217,8 +2217,6 @@ EOF
   curl_libs=`$curlconfig --libs 2>/dev/null`
   if compile_prog "$curl_cflags" "$curl_libs" ; then
     curl=yes
-    libs_tools="$curl_libs $libs_tools"
-    libs_softmmu="$curl_libs $libs_softmmu"
   else
     if test "$curl" = "yes" ; then
       feature_not_found "curl"
@@ -3901,8 +3899,9 @@ if test "$bswap_h" = "yes" ; then
   echo "CONFIG_MACHINE_BSWAP_H=y" >> $config_host_mak
 fi
 if test "$curl" = "yes" ; then
-  echo "CONFIG_CURL=y" >> $config_host_mak
+  echo "CONFIG_CURL=m" >> $config_host_mak
   echo "CURL_CFLAGS=$curl_cflags" >> $config_host_mak
+  echo "CURL_LIBS=$curl_libs" >> $config_host_mak
 fi
 if test "$brlapi" = "yes" ; then
   echo "CONFIG_BRLAPI=y" >> $config_host_mak
-- 
1.8.3.1

Re: [Qemu-devel] [RFC PATCH v5 1/6] make.rule: fix $(obj) to a real relative path

[Paolo Bonzini]

Il 11/09/2013 07:38, Fam Zheng ha scritto:
> Makefile.target includes rule.mak and unnested common-obj-y, then prefix
> them with '../', this will ignore object specific QEMU_CFLAGS in subdir
> Makefile.objs:
> 
>     $(obj)/curl.o: QEMU_CFLAGS += $(CURL_CFLAGS)
> 
> Because $(obj) here is './block', instead of '../block'. This doesn't
> hurt compiling because we basically build all .o from top Makefile,
> before entering Makefile.target, but it will affact arriving per-object
> libs support.
> 
> The starting point of $(obj) is passed in as argument of unnest-vars, as
> well as nested variables, so that different Makefiles can pass in a
> right value.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  Makefile        | 16 +++++++++++++++-
>  Makefile.objs   | 16 +---------------
>  Makefile.target | 16 +++++++++++++---
>  configure       |  1 +
>  rules.mak       | 12 +++++++-----
>  tests/Makefile  |  2 ++
>  6 files changed, 39 insertions(+), 24 deletions(-)

Just two questions...

> diff --git a/Makefile.target b/Makefile.target
> index 9a49852..1d92523 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -143,13 +143,23 @@ endif # CONFIG_SOFTMMU
>  # Workaround for http://gcc.gnu.org/PR55489, see configure.
>  %/translate.o: QEMU_CFLAGS += $(TRANSLATE_OPT_CFLAGS)
>  
> -nested-vars += obj-y
> +dummy := $(call unnest-vars,,obj-y)
>  
> -# This resolves all nested paths, so it must come last
> +# we are making another call to unnest-vars with different vars, protect obj-y,
> +# it can be overriden in subdir Makefile.objs
> +obj-y-save := $(obj-y)
> +
> +block-obj-y :=
> +common-obj-y :=
>  include $(SRC_PATH)/Makefile.objs
> +dummy := $(call unnest-vars,..,block-obj-y common-obj-y)
> +
> +# Now restore obj-y
> +obj-y := $(obj-y-save)
>  
>  all-obj-y = $(obj-y)
> -all-obj-y += $(addprefix ../, $(common-obj-y))
> +all-obj-y += $(addprefix ../, $(common-obj-y) $(block-obj-y))

Why is addprefix still needed?

>  
>  ifndef CONFIG_HAIKU
>  LIBS+=-lm
> diff --git a/configure b/configure
> index e989609..cc3cd4d 100755
> --- a/configure
> +++ b/configure
> @@ -2251,6 +2251,7 @@ 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
> diff --git a/tests/Makefile b/tests/Makefile
> index baba9e9..1b4048c 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -110,6 +110,8 @@ test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o
>  $(test-obj-y): QEMU_INCLUDES += -Itests
>  QEMU_CFLAGS += -I$(SRC_PATH)/tests
>  
> +dummy := $(call unnest-vars,..,block-obj-y)
> +

And why is this needed, since tests/Makefile is included from the top
directory?

Paolo

>  tests/test-x86-cpuid.o: QEMU_INCLUDES += -I$(SRC_PATH)/target-i386
>  
>  tests/check-qint$(EXESUF): tests/check-qint.o libqemuutil.a
> 

Re: [Qemu-devel] [RFC PATCH v5 1/6] make.rule: fix $(obj) to a real relative path

[Fam Zheng]

On Wed, 09/11 08:30, Paolo Bonzini wrote:
> Il 11/09/2013 07:38, Fam Zheng ha scritto:
> > Makefile.target includes rule.mak and unnested common-obj-y, then prefix
> > them with '../', this will ignore object specific QEMU_CFLAGS in subdir
> > Makefile.objs:
> > 
> >     $(obj)/curl.o: QEMU_CFLAGS += $(CURL_CFLAGS)
> > 
> > Because $(obj) here is './block', instead of '../block'. This doesn't
> > hurt compiling because we basically build all .o from top Makefile,
> > before entering Makefile.target, but it will affact arriving per-object
> > libs support.
> > 
> > The starting point of $(obj) is passed in as argument of unnest-vars, as
> > well as nested variables, so that different Makefiles can pass in a
> > right value.
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  Makefile        | 16 +++++++++++++++-
> >  Makefile.objs   | 16 +---------------
> >  Makefile.target | 16 +++++++++++++---
> >  configure       |  1 +
> >  rules.mak       | 12 +++++++-----
> >  tests/Makefile  |  2 ++
> >  6 files changed, 39 insertions(+), 24 deletions(-)
> 
> Just two questions...
> 
> > diff --git a/Makefile.target b/Makefile.target
> > index 9a49852..1d92523 100644
> > --- a/Makefile.target
> > +++ b/Makefile.target
> > @@ -143,13 +143,23 @@ endif # CONFIG_SOFTMMU
> >  # Workaround for http://gcc.gnu.org/PR55489, see configure.
> >  %/translate.o: QEMU_CFLAGS += $(TRANSLATE_OPT_CFLAGS)
> >  
> > -nested-vars += obj-y
> > +dummy := $(call unnest-vars,,obj-y)
> >  
> > -# This resolves all nested paths, so it must come last
> > +# we are making another call to unnest-vars with different vars, protect obj-y,
> > +# it can be overriden in subdir Makefile.objs
> > +obj-y-save := $(obj-y)
> > +
> > +block-obj-y :=
> > +common-obj-y :=
> >  include $(SRC_PATH)/Makefile.objs
> > +dummy := $(call unnest-vars,..,block-obj-y common-obj-y)
> > +
> > +# Now restore obj-y
> > +obj-y := $(obj-y-save)
> >  
> >  all-obj-y = $(obj-y)
> > -all-obj-y += $(addprefix ../, $(common-obj-y))
> > +all-obj-y += $(addprefix ../, $(common-obj-y) $(block-obj-y))
> 
> Why is addprefix still needed?
> 
OK, should move to unnest-vars as well, it's here because I overlooked the line
in your next quesion, which makes that impossible.

> >  
> >  ifndef CONFIG_HAIKU
> >  LIBS+=-lm
> > diff --git a/configure b/configure
> > index e989609..cc3cd4d 100755
> > --- a/configure
> > +++ b/configure
> > @@ -2251,6 +2251,7 @@ 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
> > diff --git a/tests/Makefile b/tests/Makefile
> > index baba9e9..1b4048c 100644
> > --- a/tests/Makefile
> > +++ b/tests/Makefile
> > @@ -110,6 +110,8 @@ test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o
> >  $(test-obj-y): QEMU_INCLUDES += -Itests
> >  QEMU_CFLAGS += -I$(SRC_PATH)/tests
> >  
> > +dummy := $(call unnest-vars,..,block-obj-y)
> > +
> 
> And why is this needed, since tests/Makefile is included from the top
> directory?
> 
Legacy. Seems OK to drop.

You are really suggesting towards the direction to clean code!

Thanks,

Fam
> 
> >  tests/test-x86-cpuid.o: QEMU_INCLUDES += -I$(SRC_PATH)/target-i386
> >  
> >  tests/check-qint$(EXESUF): tests/check-qint.o libqemuutil.a
> > 
> 

Re: [Qemu-devel] [RFC PATCH v5 5/6] configure: introduce --enable-modules

[Paolo Bonzini]

Il 11/09/2013 07:38, Fam Zheng ha scritto:
> The new option will enable support of shared object build. Otherwise
> objects are static linked to executables.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  Makefile.target | 6 +++++-
>  configure       | 8 ++++++++
>  rules.mak       | 8 ++++++--
>  3 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile.target b/Makefile.target
> index 1d92523..beab0f9 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -152,7 +152,11 @@ obj-y-save := $(obj-y)
>  block-obj-y :=
>  common-obj-y :=
>  include $(SRC_PATH)/Makefile.objs
> -dummy := $(call unnest-vars,..,block-obj-y common-obj-y)
> +dummy := $(call unnest-vars,.., \
> +               block-obj-y \
> +               block-obj-m \
> +               common-obj-y \
> +               common-obj-m)
>  
>  # Now restore obj-y
>  obj-y := $(obj-y-save)
> diff --git a/configure b/configure
> index a2858c2..f1d7fa7 100755
> --- a/configure
> +++ b/configure
> @@ -192,6 +192,7 @@ gcov_tool="gcov"
>  EXESUF=""
>  DSOSUF=".so"
>  LDFLAGS_SHARED="-shared"
> +modules="no"
>  prefix="/usr/local"
>  mandir="\${prefix}/share/man"
>  datadir="\${prefix}/share"
> @@ -650,6 +651,8 @@ for opt do
>    ;;
>    --disable-debug-info)
>    ;;
> +  --enable-modules) modules="yes"
> +  ;;
>    --cpu=*)
>    ;;
>    --target-list=*) target_list="$optarg"
> @@ -1052,6 +1055,7 @@ echo "  --libdir=PATH            install libraries 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]"
> +echo "  --enable-modules         enable modules support"
>  echo "  --enable-debug-tcg       enable TCG debugging"
>  echo "  --disable-debug-tcg      disable TCG debugging (default)"
>  echo "  --enable-debug-info       enable debugging information (default)"
> @@ -3580,6 +3584,7 @@ echo "python            $python"
>  if test "$slirp" = "yes" ; then
>      echo "smbd              $smbd"
>  fi
> +echo "module support    $modules"
>  echo "host CPU          $cpu"
>  echo "host big endian   $bigendian"
>  echo "target list       $target_list"
> @@ -3697,6 +3702,9 @@ echo "libs_softmmu=$libs_softmmu" >> $config_host_mak
>  
>  echo "ARCH=$ARCH" >> $config_host_mak
>  
> +if test "$modules" = "yes"; then
> +  echo "CONFIG_MODULES=y" >> $config_host_mak
> +fi
>  case "$cpu" in
>    arm|i386|x86_64|x32|ppc|aarch64)
>      # The TCG interpreter currently does not support ld/st optimization.
> diff --git a/rules.mak b/rules.mak
> index ea97888..860b8ac 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -185,7 +185,11 @@ $(foreach var,$(nested-vars), $(eval \
>  $(foreach v,$(filter %-m,$(nested-vars)), \
>      $(call add-modules,$v))
>  
> -$(eval modules: $(patsubst %.mo,%$(DSOSUF),$(modules-m)))
> -$(eval all: modules)
> +$(if $(CONFIG_MODULES), \
> +    $(eval modules: $(patsubst %.mo,%$(DSOSUF),$(modules-m))) \
> +    $(eval all: modules), \

Since you'll have a v6, please move "all: modules" to Makefile, and in
rules.mak:

.PHONY: modules
modules:

There are a couple of things that can be improved still (I don't like
obj-save-y for example), but things are taking shape and all of this
looks like something that can be fixed on top.  If you look at
converting more parts to modules (e.g. rbd or spice), you can drop that
RFC! :)

Thanks,

Paolo

> +    $(foreach v,$(filter %-m,$(nested-vars)), \
> +        $(eval $(patsubst %-m,%-y,$v) += $($v)) \
> +        $(eval $v := )))
>  
>  endef
> 

Re: [Qemu-devel] [RFC PATCH v5 6/6] block: build qed and curl as shared library

[Paolo Bonzini]

Il 11/09/2013 07:38, Fam Zheng ha scritto:
> Curl and qed block drivers are built as shared object module.  We have
> per object cflags and libs support now, move CURL_CFLAGS and CURL_LIBS
> from global option variables to a per object basis.
> 
> "make install" is not installing them yet, manually copy it to
> ${prefix}/qemu/block/ to make it loaded.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block/Makefile.objs | 7 ++++---
>  configure           | 5 ++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index 3bb85b5..741b92f 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -1,7 +1,6 @@
>  block-obj-y += raw_bsd.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o vvfat.o
>  block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o
> -block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
> -block-obj-y += qed-check.o
> +block-obj-m += qed.mo

I suppose this is just an example of multifile module, as there's not
really any benefit in modularizing qed, right?

Paolo

>  block-obj-y += vhdx.o
>  block-obj-y += parallels.o blkdebug.o blkverify.o
>  block-obj-y += snapshot.o qapi.o
> @@ -23,4 +22,6 @@ common-obj-y += commit.o
>  common-obj-y += mirror.o
>  common-obj-y += backup.o
>  
> -$(obj)/curl.o: QEMU_CFLAGS+=$(CURL_CFLAGS)
> +curl.o-cflags := $(CURL_CFLAGS)
> +curl.o-libs := $(CURL_LIBS)
> +qed.mo-objs := qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o qed-check.o
> diff --git a/configure b/configure
> index f1d7fa7..f8be093 100755
> --- a/configure
> +++ b/configure
> @@ -2217,8 +2217,6 @@ EOF
>    curl_libs=`$curlconfig --libs 2>/dev/null`
>    if compile_prog "$curl_cflags" "$curl_libs" ; then
>      curl=yes
> -    libs_tools="$curl_libs $libs_tools"
> -    libs_softmmu="$curl_libs $libs_softmmu"
>    else
>      if test "$curl" = "yes" ; then
>        feature_not_found "curl"
> @@ -3901,8 +3899,9 @@ if test "$bswap_h" = "yes" ; then
>    echo "CONFIG_MACHINE_BSWAP_H=y" >> $config_host_mak
>  fi
>  if test "$curl" = "yes" ; then
> -  echo "CONFIG_CURL=y" >> $config_host_mak
> +  echo "CONFIG_CURL=m" >> $config_host_mak
>    echo "CURL_CFLAGS=$curl_cflags" >> $config_host_mak
> +  echo "CURL_LIBS=$curl_libs" >> $config_host_mak
>  fi
>  if test "$brlapi" = "yes" ; then
>    echo "CONFIG_BRLAPI=y" >> $config_host_mak
> 

Re: [Qemu-devel] [RFC PATCH v5 6/6] block: build qed and curl as shared library

[Fam Zheng]

On Wed, 09/11 09:28, Paolo Bonzini wrote:
> Il 11/09/2013 07:38, Fam Zheng ha scritto:
> > Curl and qed block drivers are built as shared object module.  We have
> > per object cflags and libs support now, move CURL_CFLAGS and CURL_LIBS
> > from global option variables to a per object basis.
> > 
> > "make install" is not installing them yet, manually copy it to
> > ${prefix}/qemu/block/ to make it loaded.
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  block/Makefile.objs | 7 ++++---
> >  configure           | 5 ++---
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/block/Makefile.objs b/block/Makefile.objs
> > index 3bb85b5..741b92f 100644
> > --- a/block/Makefile.objs
> > +++ b/block/Makefile.objs
> > @@ -1,7 +1,6 @@
> >  block-obj-y += raw_bsd.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o vvfat.o
> >  block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o
> > -block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
> > -block-obj-y += qed-check.o
> > +block-obj-m += qed.mo
> 
> I suppose this is just an example of multifile module, as there's not
> really any benefit in modularizing qed, right?
> 

Right, the main reason for modules is the libs dependencies, curl, rbd, etc...
I'll try to convert such block drivers, but keep qed static, in v6.

Fam
> 
> >  block-obj-y += vhdx.o
> >  block-obj-y += parallels.o blkdebug.o blkverify.o
> >  block-obj-y += snapshot.o qapi.o
> > @@ -23,4 +22,6 @@ common-obj-y += commit.o
> >  common-obj-y += mirror.o
> >  common-obj-y += backup.o
> >  
> > -$(obj)/curl.o: QEMU_CFLAGS+=$(CURL_CFLAGS)
> > +curl.o-cflags := $(CURL_CFLAGS)
> > +curl.o-libs := $(CURL_LIBS)
> > +qed.mo-objs := qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o qed-check.o
> > diff --git a/configure b/configure
> > index f1d7fa7..f8be093 100755
> > --- a/configure
> > +++ b/configure
> > @@ -2217,8 +2217,6 @@ EOF
> >    curl_libs=`$curlconfig --libs 2>/dev/null`
> >    if compile_prog "$curl_cflags" "$curl_libs" ; then
> >      curl=yes
> > -    libs_tools="$curl_libs $libs_tools"
> > -    libs_softmmu="$curl_libs $libs_softmmu"
> >    else
> >      if test "$curl" = "yes" ; then
> >        feature_not_found "curl"
> > @@ -3901,8 +3899,9 @@ if test "$bswap_h" = "yes" ; then
> >    echo "CONFIG_MACHINE_BSWAP_H=y" >> $config_host_mak
> >  fi
> >  if test "$curl" = "yes" ; then
> > -  echo "CONFIG_CURL=y" >> $config_host_mak
> > +  echo "CONFIG_CURL=m" >> $config_host_mak
> >    echo "CURL_CFLAGS=$curl_cflags" >> $config_host_mak
> > +  echo "CURL_LIBS=$curl_libs" >> $config_host_mak
> >  fi
> >  if test "$brlapi" = "yes" ; then
> >    echo "CONFIG_BRLAPI=y" >> $config_host_mak
> > 
> 

Re: [Qemu-devel] [RFC PATCH v5 4/6] module: implement module loading function

[Paolo Bonzini]

Il 11/09/2013 07:38, Fam Zheng ha scritto:
> Added three types of modules:
> 
>     typedef enum {
>         MODULE_LOAD_BLOCK = 0,
>         MODULE_LOAD_UI,
>         MODULE_LOAD_NET,
>         MODULE_LOAD_MAX,
>     } module_load_type;

If you want to make spice into a module, you probably need also audio,
char and hw modules.

Paolo

> and their loading function:
> 
>     void module_load(module_load_type).
> 
> which loads all ".so" files in a subdir under "${PREFIX}/qemu/", e.g.
> "/usr/lib/qemu/block". Modules of each type should be loaded before
> respective subsystem initialization code.
> 
> Requires gmodule-2.0 from glib.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block.c               |  1 +
>  bsd-user/main.c       |  3 +++
>  configure             | 22 ++++++++++++---------
>  include/qemu/module.h |  9 +++++++++
>  linux-user/main.c     |  3 +++
>  scripts/create_config |  4 ++++
>  util/module.c         | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  vl.c                  |  2 ++
>  8 files changed, 88 insertions(+), 9 deletions(-)
> 
> 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/bsd-user/main.c b/bsd-user/main.c
> index f9246aa..6cb9e35 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -33,6 +33,7 @@
>  #include "tcg.h"
>  #include "qemu/timer.h"
>  #include "qemu/envlist.h"
> +#include "qemu/module.h"
>  
>  int singlestep;
>  #if defined(CONFIG_USE_GUEST_BASE)
> @@ -749,6 +750,8 @@ int main(int argc, char **argv)
>      if (argc <= 1)
>          usage();
>  
> +    module_load(MODULE_LOAD_UI);
> +    module_load(MODULE_LOAD_NET);
>      module_call_init(MODULE_INIT_QOM);
>  
>      if ((envlist = envlist_create()) == NULL) {
> diff --git a/configure b/configure
> index c6d4a62..a2858c2 100755
> --- a/configure
> +++ b/configure
> @@ -2252,15 +2252,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
> diff --git a/include/qemu/module.h b/include/qemu/module.h
> index c4ccd57..f00bc25 100644
> --- a/include/qemu/module.h
> +++ b/include/qemu/module.h
> @@ -37,4 +37,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/linux-user/main.c b/linux-user/main.c
> index 5c2f7b2..db08c23 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -34,6 +34,7 @@
>  #include "qemu/timer.h"
>  #include "qemu/envlist.h"
>  #include "elf.h"
> +#include <qemu/module.h>
>  
>  char *exec_path;
>  
> @@ -3551,6 +3552,8 @@ int main(int argc, char **argv, char **envp)
>      int i;
>      int ret;
>  
> +    module_load(MODULE_LOAD_UI);
> +    module_load(MODULE_LOAD_NET);
>      module_call_init(MODULE_INIT_QOM);
>  
>      qemu_cache_utils_init(envp);
> diff --git a/scripts/create_config b/scripts/create_config
> index b1adbf5..7a54f2d 100755
> --- a/scripts/create_config
> +++ b/scripts/create_config
> @@ -25,6 +25,7 @@ case $line in
>   prefix=*)
>      # save for the next definitions
>      prefix=${line#*=}
> +    echo "#define CONFIG_PREFIX \"$prefix\""
>      ;;
>   CONFIG_AUDIO_DRIVERS=*)
>      drivers=${line#*=}
> @@ -104,6 +105,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..ef75f8e 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,54 @@ void module_call_init(module_init_type type)
>          e->init();
>      }
>  }
> +
> +void module_load(module_load_type type)
> +{
> +    const char *path;
> +    const char *dsosuf = HOST_DSOSUF;
> +    char *fname;
> +    int suf_len = strlen(dsosuf);
> +    DIR *dp;
> +    struct dirent *ep = NULL;
> +    GModule *g_module;
> +
> +    if (!g_module_supported()) {
> +        return;
> +    }
> +
> +    switch (type) {
> +    case MODULE_LOAD_BLOCK:
> +        path = CONFIG_PREFIX "/qemu/block/";
> +        break;
> +    case MODULE_LOAD_UI:
> +        path = CONFIG_PREFIX "/qemu/ui/";
> +        break;
> +    case MODULE_LOAD_NET:
> +        path = CONFIG_PREFIX "/qemu/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)) {
> +        int len = strlen(ep->d_name);
> +        if (len > suf_len &&
> +                !strcmp(&ep->d_name[len - suf_len], dsosuf)) {
> +            fname = g_strdup_printf("%s%s", path, ep->d_name);
> +            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());
> +                g_free(fname);
> +                continue;
> +            }
> +            g_free(fname);
> +        }
> +    }
> +}
> 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);
> 

Re: [Qemu-devel] [RFC PATCH v5 5/6] configure: introduce --enable-modules

[Paolo Bonzini]

Il 11/09/2013 09:27, Paolo Bonzini ha scritto:
> 
> There are a couple of things that can be improved still (I don't like
> obj-save-y for example), but things are taking shape and all of this
> looks like something that can be fixed on top.  If you look at
> converting more parts to modules (e.g. rbd or spice), you can drop that
> RFC! :)

Talking about spice, a question for Gerd.

With Fam's work to enable shared modules, hw/display/qxl* would have to
be placed in a module as well because they depend on ui/spice-core.c.
Right now, modularization is limited to files that are built once for
all of QEMU, which is not the case for qxl.

It looks like TARGET_PAGE_SIZE is the only reason why qxl is built
per-target, and in qxl_ram_set_dirty it should be enough to do

    qxl_set_dirty(&qxl->vga.vram, offset, offset + 1);

Would it be fine to use a generic 4096 constant everywhere else?

Paolo

Re: [Qemu-devel] [RFC PATCH v5 5/6] configure: introduce --enable-modules

[Gerd Hoffmann]

On Mi, 2013-09-11 at 09:41 +0200, Paolo Bonzini wrote:
> Il 11/09/2013 09:27, Paolo Bonzini ha scritto:
> > 
> > There are a couple of things that can be improved still (I don't like
> > obj-save-y for example), but things are taking shape and all of this
> > looks like something that can be fixed on top.  If you look at
> > converting more parts to modules (e.g. rbd or spice), you can drop that
> > RFC! :)
> 
> Talking about spice, a question for Gerd.
> 
> With Fam's work to enable shared modules, hw/display/qxl* would have to
> be placed in a module as well because they depend on ui/spice-core.c.

Yes.  Can modules depend on modules?  Or would we have to create a
single, big spice module with core, qxl, audio, chardev etc?

> Right now, modularization is limited to files that are built once for
> all of QEMU, which is not the case for qxl.
> 
> It looks like TARGET_PAGE_SIZE is the only reason why qxl is built
> per-target, and in qxl_ram_set_dirty it should be enough to do
> 
>     qxl_set_dirty(&qxl->vga.vram, offset, offset + 1);
> 
> Would it be fine to use a generic 4096 constant everywhere else?

Yes.  Maybe s/TARGET_PAGE_SIZE/QXL_PAGE_SIZE/ to make clear that qxl
operates on 4k pages.

cheers,
  Gerd

Re: [Qemu-devel] [RFC PATCH v5 5/6] configure: introduce --enable-modules

[Fam Zheng]

On Wed, 09/11 10:01, Gerd Hoffmann wrote:
> On Mi, 2013-09-11 at 09:41 +0200, Paolo Bonzini wrote:
> > Il 11/09/2013 09:27, Paolo Bonzini ha scritto:
> > > 
> > > There are a couple of things that can be improved still (I don't like
> > > obj-save-y for example), but things are taking shape and all of this
> > > looks like something that can be fixed on top.  If you look at
> > > converting more parts to modules (e.g. rbd or spice), you can drop that
> > > RFC! :)
> > 
> > Talking about spice, a question for Gerd.
> > 
> > With Fam's work to enable shared modules, hw/display/qxl* would have to
> > be placed in a module as well because they depend on ui/spice-core.c.
> 
> Yes.  Can modules depend on modules?  Or would we have to create a
> single, big spice module with core, qxl, audio, chardev etc?
> 
No dependence support yet.

Fam

> > Right now, modularization is limited to files that are built once for
> > all of QEMU, which is not the case for qxl.
> > 
> > It looks like TARGET_PAGE_SIZE is the only reason why qxl is built
> > per-target, and in qxl_ram_set_dirty it should be enough to do
> > 
> >     qxl_set_dirty(&qxl->vga.vram, offset, offset + 1);
> > 
> > Would it be fine to use a generic 4096 constant everywhere else?
> 
> Yes.  Maybe s/TARGET_PAGE_SIZE/QXL_PAGE_SIZE/ to make clear that qxl
> operates on 4k pages.
> 
> cheers,
>   Gerd
> 
> 

Re: [Qemu-devel] [RFC PATCH v5 5/6] configure: introduce --enable-modules

[Peter Maydell]

On 11 September 2013 09:01, Gerd Hoffmann <kraxel@redhat.com> wrote:
> On Mi, 2013-09-11 at 09:41 +0200, Paolo Bonzini wrote:
>> It looks like TARGET_PAGE_SIZE is the only reason why qxl is built
>> per-target, and in qxl_ram_set_dirty it should be enough to do
>>
>>     qxl_set_dirty(&qxl->vga.vram, offset, offset + 1);
>>
>> Would it be fine to use a generic 4096 constant everywhere else?
>
> Yes.  Maybe s/TARGET_PAGE_SIZE/QXL_PAGE_SIZE/ to make clear that qxl
> operates on 4k pages.

Does this mean that the code was previously wrong for targets
which didn't have 4K pages, or would we just have been a bit
inefficient? I ask because ARM's TARGET_PAGE_SIZE is 1K...

-- PMM

Re: [Qemu-devel] [RFC PATCH v5 5/6] configure: introduce --enable-modules

[Fam Zheng]

On Wed, 09/11 09:27, Paolo Bonzini wrote:
> Il 11/09/2013 07:38, Fam Zheng ha scritto:
> > The new option will enable support of shared object build. Otherwise
> > objects are static linked to executables.
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  Makefile.target | 6 +++++-
> >  configure       | 8 ++++++++
> >  rules.mak       | 8 ++++++--
> >  3 files changed, 19 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Makefile.target b/Makefile.target
> > index 1d92523..beab0f9 100644
> > --- a/Makefile.target
> > +++ b/Makefile.target
> > @@ -152,7 +152,11 @@ obj-y-save := $(obj-y)
> >  block-obj-y :=
> >  common-obj-y :=
> >  include $(SRC_PATH)/Makefile.objs
> > -dummy := $(call unnest-vars,..,block-obj-y common-obj-y)
> > +dummy := $(call unnest-vars,.., \
> > +               block-obj-y \
> > +               block-obj-m \
> > +               common-obj-y \
> > +               common-obj-m)
> >  
> >  # Now restore obj-y
> >  obj-y := $(obj-y-save)
> > diff --git a/configure b/configure
> > index a2858c2..f1d7fa7 100755
> > --- a/configure
> > +++ b/configure
> > @@ -192,6 +192,7 @@ gcov_tool="gcov"
> >  EXESUF=""
> >  DSOSUF=".so"
> >  LDFLAGS_SHARED="-shared"
> > +modules="no"
> >  prefix="/usr/local"
> >  mandir="\${prefix}/share/man"
> >  datadir="\${prefix}/share"
> > @@ -650,6 +651,8 @@ for opt do
> >    ;;
> >    --disable-debug-info)
> >    ;;
> > +  --enable-modules) modules="yes"
> > +  ;;
> >    --cpu=*)
> >    ;;
> >    --target-list=*) target_list="$optarg"
> > @@ -1052,6 +1055,7 @@ echo "  --libdir=PATH            install libraries 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]"
> > +echo "  --enable-modules         enable modules support"
> >  echo "  --enable-debug-tcg       enable TCG debugging"
> >  echo "  --disable-debug-tcg      disable TCG debugging (default)"
> >  echo "  --enable-debug-info       enable debugging information (default)"
> > @@ -3580,6 +3584,7 @@ echo "python            $python"
> >  if test "$slirp" = "yes" ; then
> >      echo "smbd              $smbd"
> >  fi
> > +echo "module support    $modules"
> >  echo "host CPU          $cpu"
> >  echo "host big endian   $bigendian"
> >  echo "target list       $target_list"
> > @@ -3697,6 +3702,9 @@ echo "libs_softmmu=$libs_softmmu" >> $config_host_mak
> >  
> >  echo "ARCH=$ARCH" >> $config_host_mak
> >  
> > +if test "$modules" = "yes"; then
> > +  echo "CONFIG_MODULES=y" >> $config_host_mak
> > +fi
> >  case "$cpu" in
> >    arm|i386|x86_64|x32|ppc|aarch64)
> >      # The TCG interpreter currently does not support ld/st optimization.
> > diff --git a/rules.mak b/rules.mak
> > index ea97888..860b8ac 100644
> > --- a/rules.mak
> > +++ b/rules.mak
> > @@ -185,7 +185,11 @@ $(foreach var,$(nested-vars), $(eval \
> >  $(foreach v,$(filter %-m,$(nested-vars)), \
> >      $(call add-modules,$v))
> >  
> > -$(eval modules: $(patsubst %.mo,%$(DSOSUF),$(modules-m)))
> > -$(eval all: modules)
> > +$(if $(CONFIG_MODULES), \
> > +    $(eval modules: $(patsubst %.mo,%$(DSOSUF),$(modules-m))) \
> > +    $(eval all: modules), \
> 
> Since you'll have a v6, please move "all: modules" to Makefile, and in
> rules.mak:
> 
> .PHONY: modules
> modules:
> 
Why is ".PHONY: modules" not in Makefile too? And why do we need blank
"modules:", with the real one generated above?

Fam

> There are a couple of things that can be improved still (I don't like
> obj-save-y for example), but things are taking shape and all of this
> looks like something that can be fixed on top.  If you look at
> converting more parts to modules (e.g. rbd or spice), you can drop that
> RFC! :)
> 
> Thanks,
> 
> Paolo
> 
> > +    $(foreach v,$(filter %-m,$(nested-vars)), \
> > +        $(eval $(patsubst %-m,%-y,$v) += $($v)) \
> > +        $(eval $v := )))
> >  
> >  endef
> > 
> 

Re: [Qemu-devel] [RFC PATCH v5 5/6] configure: introduce --enable-modules

[Paolo Bonzini]

Il 11/09/2013 10:35, Fam Zheng ha scritto:
>> > Since you'll have a v6, please move "all: modules" to Makefile, and in
>> > rules.mak:
>> > 
>> > .PHONY: modules
>> > modules:
>> > 
> Why is ".PHONY: modules" not in Makefile too?

I think you would need it even in Makefile.target, so I suggested rules.mak.

> And why do we need blank
> "modules:", with the real one generated above?

Just for clarity, since the target is declared only in a somewhat hidden
manner through $(eval).

Paolo

Re: [Qemu-devel] [RFC PATCH v5 5/6] configure: introduce --enable-modules

[Gerd Hoffmann]

On Mi, 2013-09-11 at 09:20 +0100, Peter Maydell wrote:
> On 11 September 2013 09:01, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > On Mi, 2013-09-11 at 09:41 +0200, Paolo Bonzini wrote:
> >> It looks like TARGET_PAGE_SIZE is the only reason why qxl is built
> >> per-target, and in qxl_ram_set_dirty it should be enough to do
> >>
> >>     qxl_set_dirty(&qxl->vga.vram, offset, offset + 1);
> >>
> >> Would it be fine to use a generic 4096 constant everywhere else?
> >
> > Yes.  Maybe s/TARGET_PAGE_SIZE/QXL_PAGE_SIZE/ to make clear that qxl
> > operates on 4k pages.
> 
> Does this mean that the code was previously wrong for targets
> which didn't have 4K pages, or would we just have been a bit
> inefficient? I ask because ARM's TARGET_PAGE_SIZE is 1K...

Hmm.  There are three places where TARGET_PAGE_SIZE is used.

 (1) Dirtying.  The page dirtying would be just a bit inefficient
     (range is larger than it needs to be).
 (2) rom_size.  Just needs be cleaned up, is hard-coded to 8192 anyway,
     no need at all to look at the page size.
 (3) rom->num_pages field.  That one will change for arm.  The linux
     kernel qxl kms driver seems not to care at all.  Not surprising,
     it is more convenient to use the offsets in the rom to figure how
     the qxl memory layout looks like.

I still think it is better to go for a fixed page size.  Real hardware
doesn't adapt to the target page size too.  ehci for example operates on
4k pages no matter what, and arm then has to care to allocate 4 1k pages
in a row for usb xfers.

cheers,
  Gerd

Re: [Qemu-devel] [RFC PATCH v5 5/6] configure: introduce --enable-modules

[Peter Maydell]

On 11 September 2013 09:56, Gerd Hoffmann <kraxel@redhat.com> wrote:
> On Mi, 2013-09-11 at 09:20 +0100, Peter Maydell wrote:
>> Does this mean that the code was previously wrong for targets
>> which didn't have 4K pages, or would we just have been a bit
>> inefficient? I ask because ARM's TARGET_PAGE_SIZE is 1K...
>
> Hmm.  There are three places where TARGET_PAGE_SIZE is used.
>
>  (1) Dirtying.  The page dirtying would be just a bit inefficient
>      (range is larger than it needs to be).
>  (2) rom_size.  Just needs be cleaned up, is hard-coded to 8192 anyway,
>      no need at all to look at the page size.
>  (3) rom->num_pages field.  That one will change for arm.  The linux
>      kernel qxl kms driver seems not to care at all.  Not surprising,
>      it is more convenient to use the offsets in the rom to figure how
>      the qxl memory layout looks like.

Note that the ARM Linux *kernel* will (probably) be using 4K pages
anyway. It's just that QEMU's TARGET_PAGE_SIZE means "smallest
page size this CPU family could possibly support", which for ARM
is 1K, even if 99.9% of guests won't use 1K pages. This is one
of the reasons it's not very useful for devices -- it's almost just
an internal implementation detail of QEMU's TLB/memory system.

What is the num_pages field supposed to mean, given that
"page size" isn't a well defined platform independent value
(for hardware or for QEMU)?

> I still think it is better to go for a fixed page size.  Real hardware
> doesn't adapt to the target page size too.  ehci for example operates on
> 4k pages no matter what, and arm then has to care to allocate 4 1k pages
> in a row for usb xfers.

I agree that devices should not (in general) be using
TARGET_PAGE_SIZE, not least because it has an odd and
not entirely obvious meaning (see above).

-- PMM

Re: [Qemu-devel] [RFC PATCH v5 5/6] configure: introduce --enable-modules

[Gerd Hoffmann]

> >  (3) rom->num_pages field.  That one will change for arm.  The linux
> >      kernel qxl kms driver seems not to care at all.  Not surprising,
> >      it is more convenient to use the offsets in the rom to figure how
> >      the qxl memory layout looks like.
> 
> Note that the ARM Linux *kernel* will (probably) be using 4K pages
> anyway. It's just that QEMU's TARGET_PAGE_SIZE means "smallest
> page size this CPU family could possibly support", which for ARM
> is 1K, even if 99.9% of guests won't use 1K pages. This is one
> of the reasons it's not very useful for devices -- it's almost just
> an internal implementation detail of QEMU's TLB/memory system.
> 
> What is the num_pages field supposed to mean, given that
> "page size" isn't a well defined platform independent value
> (for hardware or for QEMU)?

It's 4k on x86 and not really defined on !x86 (with the !x86 installed
base being pretty close to zero).

I think we should just define it to be 4k everywhere.

cheers,
  Gerd

Re: [Qemu-devel] [RFC PATCH v5 5/6] configure: introduce --enable-modules

[Peter Maydell]

On 11 September 2013 11:37, Gerd Hoffmann <kraxel@redhat.com> wrote:
>> What is the num_pages field supposed to mean, given that
>> "page size" isn't a well defined platform independent value
>> (for hardware or for QEMU)?
>
> It's 4k on x86 and not really defined on !x86 (with the !x86 installed
> base being pretty close to zero).
>
> I think we should just define it to be 4k everywhere.

Yeah, if you're just using it as a "we didn't want a 64 bit
field so we could make this a simple byte count" then
saying "it's in units of 4K" is the simplest fix.

-- PMM

Re: [Qemu-devel] [RFC PATCH v5 4/6] module: implement module loading function

[Alex Bligh]

--On 11 September 2013 13:38:27 +0800 Fam Zheng <famz@redhat.com> wrote:

> +    switch (type) {
> +    case MODULE_LOAD_BLOCK:
> +        path = CONFIG_PREFIX "/qemu/block/";
> +        break;
> +    case MODULE_LOAD_UI:
> +        path = CONFIG_PREFIX "/qemu/ui/";
> +        break;
> +    case MODULE_LOAD_NET:
> +        path = CONFIG_PREFIX "/qemu/net/";
> +        break;
> +    default:
> +        return;
> +    }
> +

I appreciate I am coming in late into this discussion, and am only scanning
the code quickly, so apologies if I have the wrong end of the stick.

This APPEARS to load modules from
a) a fixed path determined at compile time
b) a path which is not dependent on qemu version

This would make it hard to have 2 versions of qemu installed on a
system at once, or even develop one version of qemu with another
version installed. I suspect this will be hard not only for developers,
but also for distributions, particularly if the idea is to keep vms
running during upgrades. Consider the case where packages A and B
both depend on qemu module package C, then you wish to upgrade to
A', B' and C'. At some point you are likely to want both C and C'
installed. Is the idea here that QEMU is always built with CONFIG_PREFIX
having versioning inside it (in a distro environment)?

Can I suggest that at the very least, it should be possible to specify
an alternate path to the module directory via the CLI?

-- 
Alex Bligh

Re: [Qemu-devel] [RFC PATCH v5 4/6] module: implement module loading function

[Peter Maydell]

On 11 September 2013 06:38, Fam Zheng <famz@redhat.com> wrote:
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -34,6 +34,7 @@
>  #include "qemu/timer.h"
>  #include "qemu/envlist.h"
>  #include "elf.h"
> +#include <qemu/module.h>
>
>  char *exec_path;
>
> @@ -3551,6 +3552,8 @@ int main(int argc, char **argv, char **envp)
>      int i;
>      int ret;
>
> +    module_load(MODULE_LOAD_UI);
> +    module_load(MODULE_LOAD_NET);
>      module_call_init(MODULE_INIT_QOM);

This looks kind of fishy. The *-user binaries don't even
have any UI, and they shouldn't be using the networking
either. For that matter it's really unclear to me that they
should have any kind of loadable modules at all.

-- PMM

Re: [Qemu-devel] [RFC PATCH v5 4/6] module: implement module loading function

[Paolo Bonzini]

Il 11/09/2013 16:10, Alex Bligh ha scritto:
> 
> 
> --On 11 September 2013 13:38:27 +0800 Fam Zheng <famz@redhat.com> wrote:
> 
>> +    switch (type) {
>> +    case MODULE_LOAD_BLOCK:
>> +        path = CONFIG_PREFIX "/qemu/block/";
>> +        break;
>> +    case MODULE_LOAD_UI:
>> +        path = CONFIG_PREFIX "/qemu/ui/";
>> +        break;
>> +    case MODULE_LOAD_NET:
>> +        path = CONFIG_PREFIX "/qemu/net/";
>> +        break;
>> +    default:
>> +        return;
>> +    }
>> +
> 
> I appreciate I am coming in late into this discussion, and am only scanning
> the code quickly, so apologies if I have the wrong end of the stick.

You're absolutely not coming in late!  So far we really just discussed
the build side of the implementation, and I didn't review this patch at all.

> This APPEARS to load modules from
> a) a fixed path determined at compile time
> b) a path which is not dependent on qemu version

c) a path that is not under the normal /usr/lib or similar path.

> This would make it hard to have 2 versions of qemu installed on a
> system at once, or even develop one version of qemu with another
> version installed.

This is hard anyway because the firmware files are not necessarily
compatible with different QEMU versions.  With 2 versions of QEMU
installed on a system, I would suggest putting both of them in different
subdirectories under /opt.

However, (c) is a problem and...

> I suspect this will be hard not only for developers,
> but also for distributions, particularly if the idea is to keep vms
> running during upgrades. Consider the case where packages A and B
> both depend on qemu module package C, then you wish to upgrade to
> A', B' and C'. At some point you are likely to want both C and C'
> installed. Is the idea here that QEMU is always built with CONFIG_PREFIX
> having versioning inside it (in a distro environment)?
> 
> Can I suggest that at the very least, it should be possible to specify
> an alternate path to the module directory via the CLI?

... this is also a good idea.  Probably it should use an algorithm
similar to that used for data_dir.

Paolo

Re: [Qemu-devel] [RFC PATCH v5 4/6] module: implement module loading function

[Richard Henderson]

On 09/11/2013 07:10 AM, Alex Bligh wrote:
> 
> 
> --On 11 September 2013 13:38:27 +0800 Fam Zheng <famz@redhat.com> wrote:
> 
>> +    switch (type) {
>> +    case MODULE_LOAD_BLOCK:
>> +        path = CONFIG_PREFIX "/qemu/block/";
>> +        break;
>> +    case MODULE_LOAD_UI:
>> +        path = CONFIG_PREFIX "/qemu/ui/";
>> +        break;
>> +    case MODULE_LOAD_NET:
>> +        path = CONFIG_PREFIX "/qemu/net/";
>> +        break;
>> +    default:
>> +        return;
>> +    }
>> +
> 
> I appreciate I am coming in late into this discussion, and am only scanning
> the code quickly, so apologies if I have the wrong end of the stick.
> 
> This APPEARS to load modules from
> a) a fixed path determined at compile time
> b) a path which is not dependent on qemu version
> 
> This would make it hard to have 2 versions of qemu installed on a
> system at once, or even develop one version of qemu with another
> version installed. I suspect this will be hard not only for developers,
> but also for distributions, particularly if the idea is to keep vms
> running during upgrades. Consider the case where packages A and B
> both depend on qemu module package C, then you wish to upgrade to
> A', B' and C'. At some point you are likely to want both C and C'
> installed. Is the idea here that QEMU is always built with CONFIG_PREFIX
> having versioning inside it (in a distro environment)?
> 
> Can I suggest that at the very least, it should be possible to specify
> an alternate path to the module directory via the CLI?
> 

If we want dependencies between modules, we may well need to get the
dynamic linker involved with the search directory.  This would rule
out any command-line, or monitor-line altering of the path.

But it does suggest the default being DT_RUNPATH, overridable "near"
the command-line via LD_RUN_PATH.

I also wonder about the utility of the subdirectories above, as
opposed to filename prefixes.


r~

Re: [Qemu-devel] [RFC PATCH v5 4/6] module: implement module loading function

[Alex Bligh]

--On 11 September 2013 08:06:20 -0700 Richard Henderson <rth@twiddle.net> 
wrote:


> If we want dependencies between modules, we may well need to get the
> dynamic linker involved with the search directory.  This would rule
> out any command-line, or monitor-line altering of the path.
>
> But it does suggest the default being DT_RUNPATH, overridable "near"
> the command-line via LD_RUN_PATH.

Not quite sure what you are suggesting here. DT_RUNPATH is an elf
tag IIRC. Not sure what LD_RUN_PATH does. LD_LIBRARY_PATH affects
more than just this.

If we are using the dynamic linker and DT_RUNPATH (which I believe
is in the executable) or the normal paths we WILL NEED proper API
versioning etc.; in this case (in the distro example I pulled out),
an upgrade to qemu making an API change will have different API
versioned libs and these can happily coexist, so no need for directories.

But are we really sufficiently stable for that? Any internal change
in API (as modules communicate using an internal API, not something
wrapped with opaque struct *) is going to result in a library major
version number change as far as I can see. That's going to play merry
hell with distributions.

Assuming we restrict this to load dependencies (as opposed to version
dependencies), I would have thought the way to go is
a) ensure that each build of qemu has ALL the modules available.
b) qemu and all its modules always look in the same place
c) that place can be changed at load time (as well as at compile
   time) to permit different versions of qemu to be around.

I think this can be done with dlopen() as follows:

  If filename contains a slash ("/"), then it is interpreted as a
  (relative or absolute) pathname.

This means no searching is done with DT_RPATH, DT_RUNPATH etc.

It later says:

  If the library has dependencies on other shared libraries, then these are
  also automatically loaded by the dynamic linker using the  same  rules.
  (This process may occur recursively, if those libraries in turn have
  dependencies, and so on.)

My dlopen() foo is insufficiently strong to know what happens if you
have dependencies AND specify an absolute or relative path (as opposed
to just a module name). In an ideal world it would just look in the
same directory and not elsewhere, in which case we can use the
scheme I suggested AND use dlopen() AND not worry about API versioning
(yet).

> I also wonder about the utility of the subdirectories above, as
> opposed to filename prefixes.

+1

-- 
Alex Bligh

Re: [Qemu-devel] [RFC PATCH v5 4/6] module: implement module loading function

[Richard Henderson]

On 09/11/2013 08:23 AM, Alex Bligh wrote:
>> But it does suggest the default being DT_RUNPATH, overridable "near"
>> the command-line via LD_RUN_PATH.
> 
> Not quite sure what you are suggesting here. DT_RUNPATH is an elf
> tag IIRC. Not sure what LD_RUN_PATH does. LD_LIBRARY_PATH affects
> more than just this.

LD_RUN_PATH was my thinko -- LD_LIBRARY_PATH was what I was thinking.

And while it does affect more than this, if set specifically for the
qemu executable e.g. by a script then it need not really affect much.

> If we are using the dynamic linker and DT_RUNPATH (which I believe
> is in the executable) or the normal paths we WILL NEED proper API
> versioning etc.; in this case (in the distro example I pulled out),
> an upgrade to qemu making an API change will have different API
> versioned libs and these can happily coexist, so no need for directories.

I don't believe I ever suggested we're stable, or should even pretend
to be so at this point.

Install multiple qemu to different trees, and use different paths.

Or even use the $ORIGIN prefix, which allows paths relative to the
main executable, allowing the installed qemu tree to be moved at
its root.


r~

Re: [Qemu-devel] [RFC PATCH v5 4/6] module: implement module loading function

[Fam Zheng]

On Wed, 09/11 15:24, Peter Maydell wrote:
> On 11 September 2013 06:38, Fam Zheng <famz@redhat.com> wrote:
> > --- a/linux-user/main.c
> > +++ b/linux-user/main.c
> > @@ -34,6 +34,7 @@
> >  #include "qemu/timer.h"
> >  #include "qemu/envlist.h"
> >  #include "elf.h"
> > +#include <qemu/module.h>
> >
> >  char *exec_path;
> >
> > @@ -3551,6 +3552,8 @@ int main(int argc, char **argv, char **envp)
> >      int i;
> >      int ret;
> >
> > +    module_load(MODULE_LOAD_UI);
> > +    module_load(MODULE_LOAD_NET);
> >      module_call_init(MODULE_INIT_QOM);
> 
> This looks kind of fishy. The *-user binaries don't even
> have any UI, and they shouldn't be using the networking
> either. For that matter it's really unclear to me that they
> should have any kind of loadable modules at all.
> 
I see, dropping this change.