[Qemu-devel] [PATCH v2 0/7] build-sys: Fix iscsi module loading failure

[Qemu-devel] [PATCH v2 0/7] build-sys: Fix iscsi module loading failure

[Fam Zheng]

v2: 
[01/07] build-sys: Move fifo8.c to hw/misc
        Move to hw/misc/ instead of hw/. (Peter)

[02/07] configure: Add -lutil to libs_qga if necessary
        To replace "stubs: Add openpty.c" in v1. (Paolo)

[04/07] stubs: Merge set-fd-handler.c into iohandler.c
        New. (Paolo)

[05/07] util: Move throttle.c out to top level
        To replace "stubs: Add timer.c" in v1. (Paolo)

[06/07] build-sys: Change libqemuutil.a to qemuutil.o and link whole object
        LD_REL. (Paolo)

[07/07] iscsi: Move iqn generation code to util
        Drop parameter of iqn_generate(). (Paolo)

The iscsi driver doesn't work if built with --enable-modules:

$ ~/build/last/qemu-img
Failed to open module: /home/fam/build/master/block-iscsi.so: undefined symbol: qmp_query_uuid
qemu-img: Not enough arguments
Try 'qemu-img --help' for more information

This fixes it by completely linking libqemuutil.a (now qemuutil.o) rather than
on demand.

A few stub functions are added into libqemustub to make linker happy.

Lastly, iqn generation code is moved from iscsi.c to util, so that
qmp_query_uuid or its stub is not missed.

Fam


Fam Zheng (7):
  build-sys: Move fifo8.c to hw/misc
  configure: Add -lutil to libs_qga if necessary
  stubs: Add iohandler.c
  stubs: Merge set-fd-handler.c into iohandler.c
  util: Move throttle.c out to top level
  build-sys: Change libqemuutil.a to qemuutil.o and link whole object
  iscsi: Move iqn generation code to util

 Makefile                      | 17 +++++++-----
 Makefile.objs                 |  4 +--
 Makefile.target               |  2 +-
 block/iscsi.c                 | 15 +----------
 configure                     |  1 +
 hw/misc/Makefile.objs         |  1 +
 {util => hw/misc}/fifo8.c     |  0
 include/qemu-common.h         |  3 +++
 stubs/Makefile.objs           |  2 +-
 stubs/iohandler.c             | 29 +++++++++++++++++++++
 stubs/set-fd-handler.c        | 11 --------
 tests/Makefile                | 60 +++++++++++++++++++++----------------------
 util/throttle.c => throttle.c |  0
 util/Makefile.objs            |  3 +--
 util/iqn.c                    | 37 ++++++++++++++++++++++++++
 15 files changed, 117 insertions(+), 68 deletions(-)
 rename {util => hw/misc}/fifo8.c (100%)
 create mode 100644 stubs/iohandler.c
 delete mode 100644 stubs/set-fd-handler.c
 rename util/throttle.c => throttle.c (100%)
 create mode 100644 util/iqn.c

-- 
2.0.3

[Qemu-devel] [PATCH v2 1/7] build-sys: Move fifo8.c to hw/misc

[Fam Zheng]

Since it has a dependency on vmstate and is only used by device
emulation, moving out from util will make the archive more independent.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/misc/Makefile.objs     | 1 +
 {util => hw/misc}/fifo8.c | 0
 util/Makefile.objs        | 1 -
 3 files changed, 1 insertion(+), 1 deletion(-)
 rename {util => hw/misc}/fifo8.c (100%)

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 979e532..a4a5ad7 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -41,3 +41,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
 obj-$(CONFIG_ZYNQ) += zynq_slcr.o
 
 obj-$(CONFIG_PVPANIC) += pvpanic.o
+obj-y += fifo8.o
diff --git a/util/fifo8.c b/hw/misc/fifo8.c
similarity index 100%
rename from util/fifo8.c
rename to hw/misc/fifo8.c
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 6b3c83b..65a36f6 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -3,7 +3,6 @@ util-obj-$(CONFIG_WIN32) += oslib-win32.o qemu-thread-win32.o event_notifier-win
 util-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o event_notifier-posix.o qemu-openpty.o
 util-obj-y += envlist.o path.o host-utils.o module.o
 util-obj-y += bitmap.o bitops.o hbitmap.o
-util-obj-y += fifo8.o
 util-obj-y += acl.o
 util-obj-y += error.o qemu-error.o
 util-obj-$(CONFIG_POSIX) += compatfd.o
-- 
2.0.3

Re: [Qemu-devel] [PATCH v2 1/7] build-sys: Move fifo8.c to hw/misc

[Peter Crosthwaite]

On Fri, Aug 22, 2014 at 8:54 PM, Fam Zheng <famz@redhat.com> wrote:
> Since it has a dependency on vmstate and is only used by device
> emulation, moving out from util will make the archive more independent.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>

We probably also want to move the corresponding header at some stage too.

but

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

> ---
>  hw/misc/Makefile.objs     | 1 +
>  {util => hw/misc}/fifo8.c | 0
>  util/Makefile.objs        | 1 -
>  3 files changed, 1 insertion(+), 1 deletion(-)
>  rename {util => hw/misc}/fifo8.c (100%)
>
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 979e532..a4a5ad7 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -41,3 +41,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
>  obj-$(CONFIG_ZYNQ) += zynq_slcr.o
>
>  obj-$(CONFIG_PVPANIC) += pvpanic.o
> +obj-y += fifo8.o
> diff --git a/util/fifo8.c b/hw/misc/fifo8.c
> similarity index 100%
> rename from util/fifo8.c
> rename to hw/misc/fifo8.c
> diff --git a/util/Makefile.objs b/util/Makefile.objs
> index 6b3c83b..65a36f6 100644
> --- a/util/Makefile.objs
> +++ b/util/Makefile.objs
> @@ -3,7 +3,6 @@ util-obj-$(CONFIG_WIN32) += oslib-win32.o qemu-thread-win32.o event_notifier-win
>  util-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o event_notifier-posix.o qemu-openpty.o
>  util-obj-y += envlist.o path.o host-utils.o module.o
>  util-obj-y += bitmap.o bitops.o hbitmap.o
> -util-obj-y += fifo8.o
>  util-obj-y += acl.o
>  util-obj-y += error.o qemu-error.o
>  util-obj-$(CONFIG_POSIX) += compatfd.o
> --
> 2.0.3
>
>

[Qemu-devel] [PATCH v2 2/7] configure: Add -lutil to libs_qga if necessary

[Fam Zheng]

Since we will soon need to resolve openpty (3) when linking.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 configure | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configure b/configure
index 283c71c..5c35ecf 100755
--- a/configure
+++ b/configure
@@ -3546,6 +3546,7 @@ fi
 if test "$darwin" != "yes" -a "$mingw32" != "yes" -a "$solaris" != yes -a \
         "$aix" != "yes" -a "$haiku" != "yes" ; then
     libs_softmmu="-lutil $libs_softmmu"
+    libs_qga="-lutil $libs_qga"
 fi
 
 ##########################################
-- 
2.0.3

[Qemu-devel] [PATCH v2 3/7] stubs: Add iohandler.c

[Fam Zheng]

Add stub function "qemu_set_fd_handler" which is used by
util/event_notifier-posix.c.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 stubs/Makefile.objs |  1 +
 stubs/iohandler.c   | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)
 create mode 100644 stubs/iohandler.c

diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 5e347d0..d9cad1b 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -40,3 +40,4 @@ stub-obj-$(CONFIG_WIN32) += fd-register.o
 stub-obj-y += cpus.o
 stub-obj-y += kvm.o
 stub-obj-y += qmp_pc_dimm_device_list.o
+stub-obj-y += iohandler.o
diff --git a/stubs/iohandler.c b/stubs/iohandler.c
new file mode 100644
index 0000000..97c0ce5
--- /dev/null
+++ b/stubs/iohandler.c
@@ -0,0 +1,20 @@
+/*
+ * iohandler stub functions
+ *
+ * Copyright Red Hat, Inc., 2014
+ *
+ * Author: Fam Zheng <famz@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/main-loop.h"
+
+int qemu_set_fd_handler(int fd,
+                        IOHandler *fd_read,
+                        IOHandler *fd_write,
+                        void *opaque)
+{
+    abort();
+}
-- 
2.0.3

[Qemu-devel] [PATCH v2 4/7] stubs: Merge set-fd-handler.c into iohandler.c

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 stubs/Makefile.objs    |  1 -
 stubs/iohandler.c      |  9 +++++++++
 stubs/set-fd-handler.c | 11 -----------
 3 files changed, 9 insertions(+), 12 deletions(-)
 delete mode 100644 stubs/set-fd-handler.c

diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index d9cad1b..2489c17 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -29,7 +29,6 @@ stub-obj-$(CONFIG_SPICE) += qemu-chr-open-spice.o
 stub-obj-y += qtest.o
 stub-obj-y += reset.o
 stub-obj-y += runstate-check.o
-stub-obj-y += set-fd-handler.o
 stub-obj-y += slirp.o
 stub-obj-y += sysbus.o
 stub-obj-y += uuid.o
diff --git a/stubs/iohandler.c b/stubs/iohandler.c
index 97c0ce5..31b8b0f 100644
--- a/stubs/iohandler.c
+++ b/stubs/iohandler.c
@@ -18,3 +18,12 @@ int qemu_set_fd_handler(int fd,
 {
     abort();
 }
+
+int qemu_set_fd_handler2(int fd,
+                         IOCanReadHandler *fd_read_poll,
+                         IOHandler *fd_read,
+                         IOHandler *fd_write,
+                         void *opaque)
+{
+    abort();
+}
diff --git a/stubs/set-fd-handler.c b/stubs/set-fd-handler.c
deleted file mode 100644
index fc874d3..0000000
--- a/stubs/set-fd-handler.c
+++ /dev/null
@@ -1,11 +0,0 @@
-#include "qemu-common.h"
-#include "qemu/main-loop.h"
-
-int qemu_set_fd_handler2(int fd,
-                         IOCanReadHandler *fd_read_poll,
-                         IOHandler *fd_read,
-                         IOHandler *fd_write,
-                         void *opaque)
-{
-    abort();
-}
-- 
2.0.3

[Qemu-devel] [PATCH v2 5/7] util: Move throttle.c out to top level

[Fam Zheng]

It is only used by block code and has dependency on timers, so move it
out to allow util to be linked unconditionally.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 Makefile.objs                 | 2 +-
 util/throttle.c => throttle.c | 0
 util/Makefile.objs            | 1 -
 3 files changed, 1 insertion(+), 2 deletions(-)
 rename util/throttle.c => throttle.c (100%)

diff --git a/Makefile.objs b/Makefile.objs
index 97db978..6445ce9 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -7,7 +7,7 @@ util-obj-y = util/ qobject/ qapi/ qapi-types.o qapi-visit.o qapi-event.o
 # block-obj-y is code used by both qemu system emulation and qemu-img
 
 block-obj-y = async.o thread-pool.o
-block-obj-y += nbd.o block.o blockjob.o
+block-obj-y += nbd.o block.o blockjob.o throttle.o
 block-obj-y += main-loop.o iohandler.o qemu-timer.o
 block-obj-$(CONFIG_POSIX) += aio-posix.o
 block-obj-$(CONFIG_WIN32) += aio-win32.o
diff --git a/util/throttle.c b/throttle.c
similarity index 100%
rename from util/throttle.c
rename to throttle.c
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 65a36f6..5940acc 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -10,7 +10,6 @@ util-obj-y += iov.o aes.o qemu-config.o qemu-sockets.o uri.o notify.o
 util-obj-y += qemu-option.o qemu-progress.o
 util-obj-y += hexdump.o
 util-obj-y += crc32c.o
-util-obj-y += throttle.o
 util-obj-y += getauxval.o
 util-obj-y += readline.o
 util-obj-y += rfifolock.o
-- 
2.0.3

Re: [Qemu-devel] [PATCH v2 5/7] util: Move throttle.c out to top level

[Paolo Bonzini]

Il 22/08/2014 12:54, Fam Zheng ha scritto:
> It is only used by block code and has dependency on timers, so move it
> out to allow util to be linked unconditionally.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  Makefile.objs                 | 2 +-
>  util/throttle.c => throttle.c | 0
>  util/Makefile.objs            | 1 -
>  3 files changed, 1 insertion(+), 2 deletions(-)
>  rename util/throttle.c => throttle.c (100%)
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index 97db978..6445ce9 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -7,7 +7,7 @@ util-obj-y = util/ qobject/ qapi/ qapi-types.o qapi-visit.o qapi-event.o
>  # block-obj-y is code used by both qemu system emulation and qemu-img
>  
>  block-obj-y = async.o thread-pool.o
> -block-obj-y += nbd.o block.o blockjob.o
> +block-obj-y += nbd.o block.o blockjob.o throttle.o
>  block-obj-y += main-loop.o iohandler.o qemu-timer.o
>  block-obj-$(CONFIG_POSIX) += aio-posix.o
>  block-obj-$(CONFIG_WIN32) += aio-win32.o
> diff --git a/util/throttle.c b/throttle.c
> similarity index 100%
> rename from util/throttle.c
> rename to throttle.c
> diff --git a/util/Makefile.objs b/util/Makefile.objs
> index 65a36f6..5940acc 100644
> --- a/util/Makefile.objs
> +++ b/util/Makefile.objs
> @@ -10,7 +10,6 @@ util-obj-y += iov.o aes.o qemu-config.o qemu-sockets.o uri.o notify.o
>  util-obj-y += qemu-option.o qemu-progress.o
>  util-obj-y += hexdump.o
>  util-obj-y += crc32c.o
> -util-obj-y += throttle.o
>  util-obj-y += getauxval.o
>  util-obj-y += readline.o
>  util-obj-y += rfifolock.o
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Stefan, would you pick this up in the block branch already?

Paolo

[Qemu-devel] [PATCH v2 6/7] build-sys: Change libqemuutil.a to qemuutil.o and link whole object

[Fam Zheng]

When an executable is being generated, unused functions from
libqemuutil.a are not linked. This is the linker's convention on
archives (libqemuutil.a).

Now that we have dynamically loaded modules, which may reference
function from libqemuutil.a but not linked in the executable, because
the executable itself didn't reference this symbol. That is a problem
for module build.

We can't link both executable and the dynamic shared object to
libqemuutil.a, because of the risk of inconsistent views of program
variables: DSO module sees a copy of some data because it is linked
against libqemuutil.a, whereas the executable sees another copy. In
other words, they each maintains a copy but with a same name. In this
case, it can be very tricky to notice such a duplication, and make a bug
hard to reason. So it's good to avoid it from the beginning.

This patch solves the above issue by fully linking. Specifically, it
fixes block-iscsi.mo: in block/iscsi.c, util/bitmap.c functions are
used, but qemu-img doesn't link it.

The solution is to link everything in libqemuutil.a. We do this by
changing it to qemuutil.o, which includes all the util objects. This is
easier and also expected to be more portable than "--whole-archive".

Because qemuutil.o is now fully linked and hence make executables
references more symbols than before, some test executables now need
libqemustub.a, so add them as necessary too.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 Makefile        | 17 +++++++++-------
 Makefile.objs   |  2 +-
 Makefile.target |  2 +-
 tests/Makefile  | 60 ++++++++++++++++++++++++++++-----------------------------
 4 files changed, 42 insertions(+), 39 deletions(-)

diff --git a/Makefile b/Makefile
index b33aaac..773e5eb 100644
--- a/Makefile
+++ b/Makefile
@@ -187,7 +187,7 @@ subdir-dtc:dtc/libfdt dtc/tests
 dtc/%:
 	mkdir -p $@
 
-$(SUBDIR_RULES): libqemuutil.a libqemustub.a $(common-obj-y)
+$(SUBDIR_RULES): qemuutil.o libqemustub.a $(common-obj-y)
 
 ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS))
 romsubdir-%:
@@ -208,7 +208,10 @@ Makefile: $(version-obj-y) $(version-lobj-y)
 # Build libraries
 
 libqemustub.a: $(stub-obj-y)
-libqemuutil.a: $(util-obj-y)
+
+LD_REL := $(CC) -nostdlib -Wl,-r
+qemuutil.o: $(util-obj-y)
+	$(call quiet-command,$(LD_REL) -o $@ $^,"  LD -r $(TARGET_DIR)$@")
 
 block-modules = $(foreach o,$(block-obj-m),"$(basename $(subst /,-,$o))",) NULL
 util/module.o-cflags = -D'CONFIG_BLOCK_MODULES=$(block-modules)'
@@ -217,13 +220,13 @@ util/module.o-cflags = -D'CONFIG_BLOCK_MODULES=$(block-modules)'
 
 qemu-img.o: qemu-img-cmds.h
 
-qemu-img$(EXESUF): qemu-img.o $(block-obj-y) libqemuutil.a libqemustub.a
-qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) libqemuutil.a libqemustub.a
-qemu-io$(EXESUF): qemu-io.o $(block-obj-y) libqemuutil.a libqemustub.a
+qemu-img$(EXESUF): qemu-img.o $(block-obj-y) qemuutil.o libqemustub.a
+qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) qemuutil.o libqemustub.a
+qemu-io$(EXESUF): qemu-io.o $(block-obj-y) qemuutil.o libqemustub.a
 
 qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
 
-fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o fsdev/virtio-9p-marshal.o libqemuutil.a libqemustub.a
+fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o fsdev/virtio-9p-marshal.o qemuutil.o libqemustub.a
 fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap
 
 qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
@@ -280,7 +283,7 @@ $(qapi-modules) $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
 QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h)
 $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
 
-qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a
+qemu-ga$(EXESUF): $(qga-obj-y) qemuutil.o libqemustub.a
 	$(call LINK, $^)
 
 clean:
diff --git a/Makefile.objs b/Makefile.objs
index 6445ce9..bbb2ea4 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -109,6 +109,6 @@ target-obj-y += trace/
 # guest agent
 
 # 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.
+# by qemuutil.o.  These should be moved to a separate .json schema.
 qga-obj-y = qga/
 qga-vss-dll-obj-y = qga/
diff --git a/Makefile.target b/Makefile.target
index 1e8d7ab..48a3089 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -176,7 +176,7 @@ all-obj-y += $(target-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y)
 
 # build either PROG or PROGW
-$(QEMU_PROG_BUILD): $(all-obj-y) ../libqemuutil.a ../libqemustub.a
+$(QEMU_PROG_BUILD): $(all-obj-y) ../qemuutil.o ../libqemustub.a
 	$(call LINK,$^)
 
 gdbstub-xml.c: $(TARGET_XML_FILES) $(SRC_PATH)/scripts/feature_to_c.sh
diff --git a/tests/Makefile b/tests/Makefile
index 837e9c8..b79a299 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -226,22 +226,22 @@ qom-core-obj = qom/object.o qom/qom-qobject.o qom/container.o
 
 tests/test-x86-cpuid.o: QEMU_INCLUDES += -I$(SRC_PATH)/target-i386
 
-tests/check-qint$(EXESUF): tests/check-qint.o libqemuutil.a
-tests/check-qstring$(EXESUF): tests/check-qstring.o libqemuutil.a
-tests/check-qdict$(EXESUF): tests/check-qdict.o libqemuutil.a
-tests/check-qlist$(EXESUF): tests/check-qlist.o libqemuutil.a
-tests/check-qfloat$(EXESUF): tests/check-qfloat.o libqemuutil.a
-tests/check-qjson$(EXESUF): tests/check-qjson.o libqemuutil.a libqemustub.a
-tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o $(qom-core-obj) libqemuutil.a libqemustub.a
-tests/test-coroutine$(EXESUF): tests/test-coroutine.o $(block-obj-y) libqemuutil.a libqemustub.a
-tests/test-aio$(EXESUF): tests/test-aio.o $(block-obj-y) libqemuutil.a libqemustub.a
-tests/test-rfifolock$(EXESUF): tests/test-rfifolock.o libqemuutil.a libqemustub.a
-tests/test-throttle$(EXESUF): tests/test-throttle.o $(block-obj-y) libqemuutil.a libqemustub.a
-tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o $(block-obj-y) libqemuutil.a libqemustub.a
-tests/test-iov$(EXESUF): tests/test-iov.o libqemuutil.a
-tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o libqemuutil.a libqemustub.a
+tests/check-qint$(EXESUF): tests/check-qint.o qemuutil.o libqemustub.a
+tests/check-qstring$(EXESUF): tests/check-qstring.o qemuutil.o libqemustub.a
+tests/check-qdict$(EXESUF): tests/check-qdict.o qemuutil.o libqemustub.a
+tests/check-qlist$(EXESUF): tests/check-qlist.o qemuutil.o libqemustub.a
+tests/check-qfloat$(EXESUF): tests/check-qfloat.o qemuutil.o libqemustub.a
+tests/check-qjson$(EXESUF): tests/check-qjson.o qemuutil.o libqemustub.a
+tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o $(qom-core-obj) qemuutil.o libqemustub.a
+tests/test-coroutine$(EXESUF): tests/test-coroutine.o $(block-obj-y) qemuutil.o libqemustub.a
+tests/test-aio$(EXESUF): tests/test-aio.o $(block-obj-y) qemuutil.o libqemustub.a
+tests/test-rfifolock$(EXESUF): tests/test-rfifolock.o qemuutil.o libqemustub.a
+tests/test-throttle$(EXESUF): tests/test-throttle.o $(block-obj-y) qemuutil.o libqemustub.a
+tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o $(block-obj-y) qemuutil.o libqemustub.a
+tests/test-iov$(EXESUF): tests/test-iov.o qemuutil.o libqemustub.a
+tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o qemuutil.o libqemustub.a
 tests/test-x86-cpuid$(EXESUF): tests/test-x86-cpuid.o
-tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o xbzrle.o page_cache.o libqemuutil.a
+tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o xbzrle.o page_cache.o qemuutil.o libqemustub.a
 tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o
 tests/test-int128$(EXESUF): tests/test-int128.o
 tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
@@ -250,10 +250,10 @@ tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
 	hw/core/fw-path-provider.o \
 	$(qom-core-obj) \
 	$(test-qapi-obj-y) \
-	libqemuutil.a libqemustub.a
+	qemuutil.o libqemustub.a
 tests/test-vmstate$(EXESUF): tests/test-vmstate.o \
 	vmstate.o qemu-file.o \
-	libqemuutil.a
+	qemuutil.o libqemustub.a
 
 tests/test-qapi-types.c tests/test-qapi-types.h :\
 $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py
@@ -276,18 +276,18 @@ $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-eve
 		$(gen-out-type) -o tests -p "test-" -i $<, \
 		"  GEN   $@")
 
-tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
-tests/test-string-input-visitor$(EXESUF): tests/test-string-input-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
-tests/test-qmp-event$(EXESUF): tests/test-qmp-event.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
-tests/test-qmp-output-visitor$(EXESUF): tests/test-qmp-output-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
-tests/test-qmp-input-visitor$(EXESUF): tests/test-qmp-input-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
-tests/test-qmp-input-strict$(EXESUF): tests/test-qmp-input-strict.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
-tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o tests/test-qmp-marshal.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
-tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
-tests/test-opts-visitor$(EXESUF): tests/test-opts-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
+tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o $(test-qapi-obj-y) qemuutil.o libqemustub.a
+tests/test-string-input-visitor$(EXESUF): tests/test-string-input-visitor.o $(test-qapi-obj-y) qemuutil.o libqemustub.a
+tests/test-qmp-event$(EXESUF): tests/test-qmp-event.o $(test-qapi-obj-y) qemuutil.o libqemustub.a
+tests/test-qmp-output-visitor$(EXESUF): tests/test-qmp-output-visitor.o $(test-qapi-obj-y) qemuutil.o libqemustub.a
+tests/test-qmp-input-visitor$(EXESUF): tests/test-qmp-input-visitor.o $(test-qapi-obj-y) qemuutil.o libqemustub.a
+tests/test-qmp-input-strict$(EXESUF): tests/test-qmp-input-strict.o $(test-qapi-obj-y) qemuutil.o libqemustub.a
+tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o tests/test-qmp-marshal.o $(test-qapi-obj-y) qemuutil.o libqemustub.a
+tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(test-qapi-obj-y) qemuutil.o libqemustub.a
+tests/test-opts-visitor$(EXESUF): tests/test-opts-visitor.o $(test-qapi-obj-y) qemuutil.o libqemustub.a
 
-tests/test-mul64$(EXESUF): tests/test-mul64.o libqemuutil.a
-tests/test-bitops$(EXESUF): tests/test-bitops.o libqemuutil.a
+tests/test-mul64$(EXESUF): tests/test-mul64.o qemuutil.o libqemustub.a
+tests/test-bitops$(EXESUF): tests/test-bitops.o qemuutil.o libqemustub.a
 
 libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o
 libqos-obj-y += tests/libqos/i2c.o
@@ -338,7 +338,7 @@ tests/ioh3420-test$(EXESUF): tests/ioh3420-test.o
 tests/usb-hcd-ehci-test$(EXESUF): tests/usb-hcd-ehci-test.o $(libqos-pc-obj-y)
 tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o qemu-char.o qemu-timer.o $(qtest-obj-y)
 tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o
-tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o libqemuutil.a libqemustub.a
+tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o qemuutil.o libqemustub.a
 
 ifeq ($(CONFIG_POSIX),y)
 LIBS += -lutil
@@ -352,7 +352,7 @@ QTEST_TARGETS=$(foreach TARGET,$(TARGETS), $(if $(check-qtest-$(TARGET)-y), $(TA
 check-qtest-y=$(foreach TARGET,$(TARGETS), $(check-qtest-$(TARGET)-y))
 endif
 
-qtest-obj-y = tests/libqtest.o libqemuutil.a libqemustub.a
+qtest-obj-y = tests/libqtest.o qemuutil.o libqemustub.a
 $(check-qtest-y): $(qtest-obj-y)
 
 .PHONY: check-help
-- 
2.0.3

[Qemu-devel] [PATCH v2 7/7] iscsi: Move iqn generation code to util

[Fam Zheng]

Function qmp_query_uuid, even with a version in libqemustub.a, is not
linked in qemu-img, unless we use it in somewhere that is linked into
qemu-img. For example util-obj-y - since iqn generation a general
function, not iscsi specific, moving it to util makes some sense, and
more importantly this will allow us to build iscsi as a dynamic module.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/iscsi.c         | 15 +--------------
 include/qemu-common.h |  3 +++
 util/Makefile.objs    |  1 +
 util/iqn.c            | 37 +++++++++++++++++++++++++++++++++++++
 4 files changed, 42 insertions(+), 14 deletions(-)
 create mode 100644 util/iqn.c

diff --git a/block/iscsi.c b/block/iscsi.c
index 2c9cfc1..4436156 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -37,8 +37,6 @@
 #include "trace.h"
 #include "block/scsi.h"
 #include "qemu/iov.h"
-#include "sysemu/sysemu.h"
-#include "qmp-commands.h"
 
 #include <iscsi/iscsi.h>
 #include <iscsi/scsi-lowlevel.h>
@@ -1034,8 +1032,6 @@ static char *parse_initiator_name(const char *target)
     QemuOptsList *list;
     QemuOpts *opts;
     const char *name;
-    char *iscsi_name;
-    UuidInfo *uuid_info;
 
     list = qemu_find_opts("iscsi");
     if (list) {
@@ -1051,16 +1047,7 @@ static char *parse_initiator_name(const char *target)
         }
     }
 
-    uuid_info = qmp_query_uuid(NULL);
-    if (strcmp(uuid_info->UUID, UUID_NONE) == 0) {
-        name = qemu_get_vm_name();
-    } else {
-        name = uuid_info->UUID;
-    }
-    iscsi_name = g_strdup_printf("iqn.2008-11.org.linux-kvm%s%s",
-                                 name ? ":" : "", name ? name : "");
-    qapi_free_UuidInfo(uuid_info);
-    return iscsi_name;
+    return iqn_generate();
 }
 
 static void iscsi_nop_timed_event(void *opaque)
diff --git a/include/qemu-common.h b/include/qemu-common.h
index bcf7a6a..69505b5 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -420,6 +420,9 @@ int uleb128_decode_small(const uint8_t *in, uint32_t *n);
 /* unicode.c */
 int mod_utf8_codepoint(const char *s, size_t n, char **end);
 
+/* iqn.c */
+char *iqn_generate(void);
+
 /*
  * Hexdump a buffer to a file. An optional string prefix is added to every line
  */
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 5940acc..30a3c77 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -13,3 +13,4 @@ util-obj-y += crc32c.o
 util-obj-y += getauxval.o
 util-obj-y += readline.o
 util-obj-y += rfifolock.o
+util-obj-y += iqn.o
diff --git a/util/iqn.c b/util/iqn.c
new file mode 100644
index 0000000..ed30f0f
--- /dev/null
+++ b/util/iqn.c
@@ -0,0 +1,37 @@
+/*
+ * iqn generate function
+ *
+ * Copyright Red Hat, Inc., 2014
+ *
+ * Author: Paolo Bonzini <pbonzini@redhat.com>
+ *         Fam Zheng <famz@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#include <glib.h>
+#include "qemu/error-report.h"
+#include "qemu-common.h"
+#include "sysemu/sysemu.h"
+#include "qmp-commands.h"
+
+char *iqn_generate(void)
+{
+    const char *name;
+    char *iqn;
+    UuidInfo *uuid_info;
+
+    uuid_info = qmp_query_uuid(NULL);
+    if (strcmp(uuid_info->UUID, UUID_NONE) == 0) {
+        name = qemu_get_vm_name();
+    } else {
+        name = uuid_info->UUID;
+    }
+    iqn = g_strdup_printf("iqn.2008-11.org.linux-kvm%s%s",
+                          name ? ":" : "",
+                          name ? : "");
+    qapi_free_UuidInfo(uuid_info);
+
+    return iqn;
+}
-- 
2.0.3

Re: [Qemu-devel] [PATCH v2 0/7] build-sys: Fix iscsi module loading failure

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1206 bytes --]

On Fri, Aug 22, 2014 at 06:54:16PM +0800, Fam Zheng wrote:
> The iscsi driver doesn't work if built with --enable-modules:
> 
> $ ~/build/last/qemu-img
> Failed to open module: /home/fam/build/master/block-iscsi.so: undefined symbol: qmp_query_uuid
> qemu-img: Not enough arguments
> Try 'qemu-img --help' for more information
> 
> This fixes it by completely linking libqemuutil.a (now qemuutil.o) rather than
> on demand.
> 
> A few stub functions are added into libqemustub to make linker happy.
> 
> Lastly, iqn generation code is moved from iscsi.c to util, so that
> qmp_query_uuid or its stub is not missed.

Did you try ld --just-symbols=filename to include module symbol
dependencies when linking the QEMU binary?

The advantage is that the QEMU binary stays smaller.  And (depending on
whether you consider this a feature or not) it discourages people from
building out-of-tree modules.

Did you compare the before/after binary size with your patch?  Please
use size(1).

It's unfortunate to bloat the binary, not just from a code size
perspective, but also from a security perspective less code is better
(cannot be abused in return-oriented-programming).

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v2 0/7] build-sys: Fix iscsi module loading failure

[Paolo Bonzini]

Il 22/08/2014 15:35, Stefan Hajnoczi ha scritto:
> The advantage is that the QEMU binary stays smaller.  And (depending on
> whether you consider this a feature or not) it discourages people from
> building out-of-tree modules.

I agree, though I'm not sure which platforms support --just-symbols.

Also, patches 2 and 5 are useful anyway.

Paolo

Re: [Qemu-devel] [PATCH v2 0/7] build-sys: Fix iscsi module loading failure

[Fam Zheng]

On Fri, 08/22 14:35, Stefan Hajnoczi wrote:
> On Fri, Aug 22, 2014 at 06:54:16PM +0800, Fam Zheng wrote:
> > The iscsi driver doesn't work if built with --enable-modules:
> > 
> > $ ~/build/last/qemu-img
> > Failed to open module: /home/fam/build/master/block-iscsi.so: undefined symbol: qmp_query_uuid
> > qemu-img: Not enough arguments
> > Try 'qemu-img --help' for more information
> > 
> > This fixes it by completely linking libqemuutil.a (now qemuutil.o) rather than
> > on demand.
> > 
> > A few stub functions are added into libqemustub to make linker happy.
> > 
> > Lastly, iqn generation code is moved from iscsi.c to util, so that
> > qmp_query_uuid or its stub is not missed.
> 
> Did you try ld --just-symbols=filename to include module symbol
> dependencies when linking the QEMU binary?

Yes.

I didn't use it because I couldn't find a counterpart option for Mac OS X.

Anyway I tested that at least on Linux it works, except that on my laptop there
seems to be an ld 2.24 bug with --just-symbols:

"""
/usr/bin/ld: BFD (GNU Binutils) 2.24 internal error, aborting at
/build/binutils/src/binutils-2.24/bfd/elf64-x86-64.c line 3641 in
elf_x86_64_relocate_section

/usr/bin/ld: Please report this bug.

collect2: error: ld returned 1 exit status
"""

But on RHEL it works very well.

> 
> The advantage is that the QEMU binary stays smaller.  And (depending on
> whether you consider this a feature or not) it discourages people from
> building out-of-tree modules.

We already block out-of-tree modules. There is a configure time stamp symbol
that is checked before loading a module.

> 
> Did you compare the before/after binary size with your patch?  Please
> use size(1).

Before:

text     data     bss      dec       hex     filename
24264    3016     8        27288     6a98    /home/fam/build/master/block-iscsi.so
24264    3016     8        27288     6a98    /home/fam/build/master/block/iscsi.so
35356    2272     4440     42068     a454    /home/fam/build/master/fsdev/virtfs-proxy-helper
392541   7568     4672     404781    62d2d   /home/fam/build/master/qemu-ga
878979   33168    4204568  5116715   4e132b  /home/fam/build/master/qemu-img
904801   57784    4208664  5171249   4ee831  /home/fam/build/master/qemu-io
854255   32872    4204504  5091631   4db12f  /home/fam/build/master/qemu-nbd
4896094  1293960  4727496  10917550  a696ae  /home/fam/build/master/x86_64-softmmu/qemu-system-x86_64

After:

text     data     bss      dec       hex     filename
23852    2992     8        26852     68e4    /home/fam/build/iscsi-modules/block-iscsi.so
23852    2992     8        26852     68e4    /home/fam/build/iscsi-modules/block/iscsi.so
509345   32488    5192     547025    858d1   /home/fam/build/iscsi-modules/fsdev/virtfs-proxy-helper
563169   33312    5192     601673    92e49   /home/fam/build/iscsi-modules/qemu-ga
966709   58304    4204632  5229645   4fcc4d  /home/fam/build/iscsi-modules/qemu-img
940145   58088    4208792  5207025   4f73f1  /home/fam/build/iscsi-modules/qemu-io
943434   58048    4204632  5206114   4f7062  /home/fam/build/iscsi-modules/qemu-nbd
4904197  1293944  4727560  10925701  a6b685  /home/fam/build/iscsi-modules/x86_64-softmmu/qemu-system-x86_64

>
> It's unfortunate to bloat the binary, not just from a code size
> perspective, but also from a security perspective less code is better
> (cannot be abused in return-oriented-programming).
> 

So would it be a good idea to share code between executables with a
qemu-common.so? (The same as existing modules, we also check the stamp symbol
so that it must be built in the same source tree.)

Fam

Re: [Qemu-devel] [PATCH v2 0/7] build-sys: Fix iscsi module loading failure

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2532 bytes --]

On Mon, Aug 25, 2014 at 10:27:07AM +0800, Fam Zheng wrote:
> On Fri, 08/22 14:35, Stefan Hajnoczi wrote:
> > Did you compare the before/after binary size with your patch?  Please
> > use size(1).
> 
> Before:
> 
> text     data     bss      dec       hex     filename
> 24264    3016     8        27288     6a98    /home/fam/build/master/block-iscsi.so
> 24264    3016     8        27288     6a98    /home/fam/build/master/block/iscsi.so
> 35356    2272     4440     42068     a454    /home/fam/build/master/fsdev/virtfs-proxy-helper
> 392541   7568     4672     404781    62d2d   /home/fam/build/master/qemu-ga
> 878979   33168    4204568  5116715   4e132b  /home/fam/build/master/qemu-img
> 904801   57784    4208664  5171249   4ee831  /home/fam/build/master/qemu-io
> 854255   32872    4204504  5091631   4db12f  /home/fam/build/master/qemu-nbd
> 4896094  1293960  4727496  10917550  a696ae  /home/fam/build/master/x86_64-softmmu/qemu-system-x86_64
> 
> After:
> 
> text     data     bss      dec       hex     filename
> 23852    2992     8        26852     68e4    /home/fam/build/iscsi-modules/block-iscsi.so
> 23852    2992     8        26852     68e4    /home/fam/build/iscsi-modules/block/iscsi.so
> 509345   32488    5192     547025    858d1   /home/fam/build/iscsi-modules/fsdev/virtfs-proxy-helper
> 563169   33312    5192     601673    92e49   /home/fam/build/iscsi-modules/qemu-ga
> 966709   58304    4204632  5229645   4fcc4d  /home/fam/build/iscsi-modules/qemu-img
> 940145   58088    4208792  5207025   4f73f1  /home/fam/build/iscsi-modules/qemu-io
> 943434   58048    4204632  5206114   4f7062  /home/fam/build/iscsi-modules/qemu-nbd
> 4904197  1293944  4727560  10925701  a6b685  /home/fam/build/iscsi-modules/x86_64-softmmu/qemu-system-x86_64

It doesn't have a big impact on the QEMU binary.  The QEMU tools do get
a bit bloated though.

I guess I'm happy with this approach.

> >
> > It's unfortunate to bloat the binary, not just from a code size
> > perspective, but also from a security perspective less code is better
> > (cannot be abused in return-oriented-programming).
> > 
> 
> So would it be a good idea to share code between executables with a
> qemu-common.so? (The same as existing modules, we also check the stamp symbol
> so that it must be built in the same source tree.)

No, I don't think there is any code size advantage to putting things in
a common shared library.  The whole library will be mmapped so it
doesn't reduce code size.

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v2 0/7] build-sys: Fix iscsi module loading failure

[Fam Zheng]

On Thu, 08/28 12:17, Stefan Hajnoczi wrote:
> On Mon, Aug 25, 2014 at 10:27:07AM +0800, Fam Zheng wrote:
> > On Fri, 08/22 14:35, Stefan Hajnoczi wrote:
> > > Did you compare the before/after binary size with your patch?  Please
> > > use size(1).
> > 
> > Before:
> > 
> > text     data     bss      dec       hex     filename
> > 24264    3016     8        27288     6a98    /home/fam/build/master/block-iscsi.so
> > 24264    3016     8        27288     6a98    /home/fam/build/master/block/iscsi.so
> > 35356    2272     4440     42068     a454    /home/fam/build/master/fsdev/virtfs-proxy-helper
> > 392541   7568     4672     404781    62d2d   /home/fam/build/master/qemu-ga
> > 878979   33168    4204568  5116715   4e132b  /home/fam/build/master/qemu-img
> > 904801   57784    4208664  5171249   4ee831  /home/fam/build/master/qemu-io
> > 854255   32872    4204504  5091631   4db12f  /home/fam/build/master/qemu-nbd
> > 4896094  1293960  4727496  10917550  a696ae  /home/fam/build/master/x86_64-softmmu/qemu-system-x86_64
> > 
> > After:
> > 
> > text     data     bss      dec       hex     filename
> > 23852    2992     8        26852     68e4    /home/fam/build/iscsi-modules/block-iscsi.so
> > 23852    2992     8        26852     68e4    /home/fam/build/iscsi-modules/block/iscsi.so
> > 509345   32488    5192     547025    858d1   /home/fam/build/iscsi-modules/fsdev/virtfs-proxy-helper
> > 563169   33312    5192     601673    92e49   /home/fam/build/iscsi-modules/qemu-ga
> > 966709   58304    4204632  5229645   4fcc4d  /home/fam/build/iscsi-modules/qemu-img
> > 940145   58088    4208792  5207025   4f73f1  /home/fam/build/iscsi-modules/qemu-io
> > 943434   58048    4204632  5206114   4f7062  /home/fam/build/iscsi-modules/qemu-nbd
> > 4904197  1293944  4727560  10925701  a6b685  /home/fam/build/iscsi-modules/x86_64-softmmu/qemu-system-x86_64
> 
> It doesn't have a big impact on the QEMU binary.  The QEMU tools do get
> a bit bloated though.
> 
> I guess I'm happy with this approach.
> 

OK. I'm working on an alternative approach (-Wl,-u,SYMBOL) as suggested by the
binutils developer:

https://sourceware.org/bugzilla/show_bug.cgi?id=17306#c5

(Also as said in the comment, --just-symbols may not be a right way to do this.)

I'll post it later for comparison, and we can choose one between them.

Thanks,
Fam

Re: [Qemu-devel] [PATCH v2 0/7] build-sys: Fix iscsi module loading failure

[Paolo Bonzini]

Il 28/08/2014 14:12, Fam Zheng ha scritto:
> OK. I'm working on an alternative approach (-Wl,-u,SYMBOL) as suggested by the
> binutils developer:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=17306#c5
> 
> (Also as said in the comment, --just-symbols may not be a right way to do this.)
> 
> I'll post it later for comparison, and we can choose one between them.

I like -Wl,-u since unlike --whole-archive it can work just as well for
libqemustub!

Paolo