| Message ID | 20190321124624.26131-5-lvivier@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | build: cleanup in Makefile.objs | expand |
On Thu, Mar 21, 2019 at 01:46:24PM +0100, Laurent Vivier wrote: > Some objects are only needed for system emulation and tools. > We can ignore them for the user mode case > > Update tests to run accordingly. > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > Makefile.objs | 33 ++++++++++++++++----------------- > tests/Makefile.include | 26 +++++++++++++++----------- > 2 files changed, 31 insertions(+), 28 deletions(-) > > diff --git a/Makefile.objs b/Makefile.objs > index 3538789808af..5d4585c8e2f5 100644 > --- a/Makefile.objs > +++ b/Makefile.objs > @@ -5,6 +5,12 @@ util-obj-y = util/ qobject/ qapi/ > > chardev-obj-y = chardev/ > > +qom-obj-y = qom/ > + > +crypto-obj-y = crypto/ > +crypto-aes-obj-y = crypto/ FYI, this is going to conflict with https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg05060.html Regards, Daniel
Le jeu. 21 mars 2019 13:53, Laurent Vivier <lvivier@redhat.com> a écrit : > Some objects are only needed for system emulation and tools. > We can ignore them for the user mode case > Finally! > Update tests to run accordingly. > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > Makefile.objs | 33 ++++++++++++++++----------------- > tests/Makefile.include | 26 +++++++++++++++----------- > 2 files changed, 31 insertions(+), 28 deletions(-) > > diff --git a/Makefile.objs b/Makefile.objs > index 3538789808af..5d4585c8e2f5 100644 > --- a/Makefile.objs > +++ b/Makefile.objs > @@ -5,6 +5,12 @@ util-obj-y = util/ qobject/ qapi/ > > chardev-obj-y = chardev/ > > +qom-obj-y = qom/ > + > +crypto-obj-y = crypto/ > +crypto-aes-obj-y = crypto/ > + > +ifneq ($(CONFIG_USER_ONLY),y) > ####################################################################### > # authz-obj-y is code used by both qemu system emulation and qemu-img > > @@ -21,21 +27,11 @@ block-obj-$(CONFIG_REPLICATION) += replication.o > > block-obj-m = block/ > > -####################################################################### > -# crypto-obj-y is code used by both qemu system emulation and qemu-img > - > -crypto-obj-y = crypto/ > -crypto-aes-obj-y = crypto/ > - > -####################################################################### > -# qom-obj-y is code used by both qemu system emulation and qemu-img > - > -qom-obj-y = qom/ > - > ####################################################################### > # io-obj-y is code used by both qemu system emulation and qemu-img > > io-obj-y = io/ > +endif > > ###################################################################### > # Target independent part of system emulation. The long term path is to > @@ -132,10 +128,18 @@ rdmacm-mux-obj-y = contrib/rdmacm-mux/ > trace-events-subdirs = > trace-events-subdirs += accel/kvm > trace-events-subdirs += accel/tcg > +ifeq ($(CONFIG_USER_ONLY),y) > +trace-events-subdirs += linux-user > +else > trace-events-subdirs += authz > +trace-events-subdirs += nbd > trace-events-subdirs += block > -trace-events-subdirs += chardev > +trace-events-subdirs += scsi > +trace-events-subdirs += io > +trace-events-subdirs += hw/display # needed by qemu-edid > +endif > trace-events-subdirs += crypto > +trace-events-subdirs += chardev > ifeq ($(CONFIG_SOFTMMU),y) > trace-events-subdirs += audio > trace-events-subdirs += hw/9pfs > @@ -146,7 +150,6 @@ trace-events-subdirs += hw/audio > trace-events-subdirs += hw/block > trace-events-subdirs += hw/block/dataplane > trace-events-subdirs += hw/char > -trace-events-subdirs += hw/display > trace-events-subdirs += hw/dma > trace-events-subdirs += hw/hppa > trace-events-subdirs += hw/i2c > @@ -183,12 +186,8 @@ trace-events-subdirs += migration > trace-events-subdirs += net > trace-events-subdirs += ui > endif > -trace-events-subdirs += io > -trace-events-subdirs += linux-user > -trace-events-subdirs += nbd > trace-events-subdirs += qapi > trace-events-subdirs += qom > -trace-events-subdirs += scsi > trace-events-subdirs += target/arm > trace-events-subdirs += target/hppa > trace-events-subdirs += target/i386 > diff --git a/tests/Makefile.include b/tests/Makefile.include > index 852f17b8c79c..c147182fb052 100644 > --- a/tests/Makefile.include > +++ b/tests/Makefile.include > @@ -45,7 +45,6 @@ SYSEMU_TARGET_LIST := $(subst -softmmu.mak,,$(notdir \ > > check-unit-y += tests/check-qdict$(EXESUF) > check-unit-y += tests/check-block-qdict$(EXESUF) > -check-unit-y += tests/test-char$(EXESUF) > check-unit-y += tests/check-qnum$(EXESUF) > check-unit-y += tests/check-qstring$(EXESUF) > check-unit-y += tests/check-qlist$(EXESUF) > @@ -61,9 +60,12 @@ check-unit-y += tests/test-string-input-visitor$(EXESUF) > check-unit-y += tests/test-string-output-visitor$(EXESUF) > check-unit-y += tests/test-qmp-event$(EXESUF) > check-unit-y += tests/test-opts-visitor$(EXESUF) > -check-unit-y += tests/test-coroutine$(EXESUF) > check-unit-y += tests/test-visitor-serialization$(EXESUF) > check-unit-y += tests/test-iov$(EXESUF) > +check-unit-y += tests/test-x86-cpuid$(EXESUF) > +ifneq ($(CONFIG_USER_ONLY),y) > +check-unit-y += tests/test-char$(EXESUF) > +check-unit-y += tests/test-coroutine$(EXESUF) > check-unit-y += tests/test-aio$(EXESUF) > check-unit-y += tests/test-aio-multithread$(EXESUF) > check-unit-y += tests/test-throttle$(EXESUF) > @@ -76,7 +78,7 @@ check-unit-y += tests/test-blockjob-txn$(EXESUF) > check-unit-y += tests/test-block-backend$(EXESUF) > check-unit-y += tests/test-block-iothread$(EXESUF) > check-unit-y += tests/test-image-locking$(EXESUF) > -check-unit-y += tests/test-x86-cpuid$(EXESUF) > +endif > # all code tested by test-x86-cpuid is inside topology.h > ifeq ($(CONFIG_SOFTMMU),y) > check-unit-y += tests/test-xbzrle$(EXESUF) > @@ -101,6 +103,14 @@ check-unit-y += tests/check-qom-interface$(EXESUF) > check-unit-y += tests/check-qom-proplist$(EXESUF) > check-unit-y += tests/test-qemu-opts$(EXESUF) > check-unit-y += tests/test-keyval$(EXESUF) > +ifneq (,$(findstring qemu-ga,$(TOOLS))) > +check-unit-$(call land,$(CONFIG_LINUX),$(CONFIG_VIRTIO_SERIAL)) += > tests/test-qga$(EXESUF) > +endif > +check-unit-y += tests/test-timed-average$(EXESUF) > +check-unit-$(CONFIG_INOTIFY1) += tests/test-util-filemonitor$(EXESUF) > +check-unit-y += tests/test-util-sockets$(EXESUF) > +check-unit-y += tests/test-base64$(EXESUF) > +ifneq ($(CONFIG_USER_ONLY),y) > check-unit-y += tests/test-write-threshold$(EXESUF) > check-unit-y += tests/test-crypto-hash$(EXESUF) > check-speed-y += tests/benchmark-crypto-hash$(EXESUF) > @@ -111,12 +121,6 @@ check-speed-y += > tests/benchmark-crypto-cipher$(EXESUF) > check-unit-y += tests/test-crypto-secret$(EXESUF) > check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlscredsx509$(EXESUF) > check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlssession$(EXESUF) > -ifneq (,$(findstring qemu-ga,$(TOOLS))) > -check-unit-$(call land,$(CONFIG_LINUX),$(CONFIG_VIRTIO_SERIAL)) += > tests/test-qga$(EXESUF) > -endif > -check-unit-y += tests/test-timed-average$(EXESUF) > -check-unit-$(CONFIG_INOTIFY1) += tests/test-util-filemonitor$(EXESUF) > -check-unit-y += tests/test-util-sockets$(EXESUF) > check-unit-y += tests/test-authz-simple$(EXESUF) > check-unit-y += tests/test-authz-list$(EXESUF) > check-unit-y += tests/test-authz-listfile$(EXESUF) > @@ -127,14 +131,14 @@ check-unit-y += tests/test-io-channel-file$(EXESUF) > check-unit-$(CONFIG_GNUTLS) += tests/test-io-channel-tls$(EXESUF) > check-unit-y += tests/test-io-channel-command$(EXESUF) > check-unit-y += tests/test-io-channel-buffer$(EXESUF) > -check-unit-y += tests/test-base64$(EXESUF) > check-unit-$(if $(CONFIG_NETTLE),y,$(CONFIG_GCRYPT)) += > tests/test-crypto-pbkdf$(EXESUF) > check-unit-y += tests/test-crypto-ivgen$(EXESUF) > check-unit-y += tests/test-crypto-afsplit$(EXESUF) > check-unit-y += tests/test-crypto-xts$(EXESUF) > check-unit-y += tests/test-crypto-block$(EXESUF) > -check-unit-y += tests/test-logging$(EXESUF) > check-unit-$(CONFIG_REPLICATION) += tests/test-replication$(EXESUF) > +endif > +check-unit-y += tests/test-logging$(EXESUF) > check-unit-y += tests/test-bufferiszero$(EXESUF) > check-unit-y += tests/test-uuid$(EXESUF) > check-unit-y += tests/ptimer-test$(EXESUF) > -- > 2.20.1 > > >
On 21/03/19 13:46, Laurent Vivier wrote: > + > +ifneq ($(CONFIG_USER_ONLY),y) > ####################################################################### > # authz-obj-y is code used by both qemu system emulation and qemu-img > > @@ -21,21 +27,11 @@ block-obj-$(CONFIG_REPLICATION) += replication.o > > block-obj-m = block/ Two remarks: 1) Isn't CONFIG_USER_ONLY set to y even if tools are built? 2) Notwithstanding the answer to (1), why is this part needed at all? The only things that have dependencies on crypto-obj-y, io-obj-y etc. are $(SOFTMMU_SUBDIR_RULES), the tools, and the tests that you are disabling already. Therefore, why are they built at all for user-only builds? 3) I don't really see the need or advantage in conditionalizing tests of libqemuutil.a, which is built even for linux-user builds (even though e.g. the coroutines code does not end up in the binary). Since most of the tests you are removing are related to the block layer, perhaps you can add CONFIG_BLOCK := $(call lor, $(CONFIG_SOFTMMU), $(call notempty,$(TOOLS)) and use it to conditionalize the testcases that depend on block-obj-y? Paolo
On 26/03/2019 10:00, Paolo Bonzini wrote: > On 21/03/19 13:46, Laurent Vivier wrote: >> + >> +ifneq ($(CONFIG_USER_ONLY),y) >> ####################################################################### >> # authz-obj-y is code used by both qemu system emulation and qemu-img >> >> @@ -21,21 +27,11 @@ block-obj-$(CONFIG_REPLICATION) += replication.o >> >> block-obj-m = block/ > > Two remarks: > > 1) Isn't CONFIG_USER_ONLY set to y even if tools are built? Yes, you right. I didn't test combination "--enable-user --enable-tools --disable-system" > > 2) Notwithstanding the answer to (1), why is this part needed at all? > The only things that have dependencies on crypto-obj-y, io-obj-y etc. > are $(SOFTMMU_SUBDIR_RULES), the tools, and the tests that you are > disabling already. Therefore, why are they built at all for user-only > builds? The change was based on the comments in the file. And for the crypto part we need it and this could explain: commit fb37726db77b21f3731b90693d2c93ade1777528 Author: Daniel P. Berrange <berrange@redhat.com> Date: Wed Sep 2 10:57:27 2015 +0100 crypto: move crypto objects out of libqemuutil.la Future patches will be adding more crypto related APIs which rely on QOM infrastructure. This creates a problem, because QOM relies on library constructors to register objects. When you have a file in a static .a library though which is only referenced by a constructor the linker is dumb and will drop that file when linking to the final executable :-( The only workaround for this is to link the .a library to the executable using the -Wl,--whole-archive flag, but this creates its own set of problems because QEMU is relying on lazy linking for libqemuutil.a. Using --whole-archive majorly increases the size of final executables as they now contain a bunch of object code they don't actually use. The least bad option is to thus not include the crypto objects in libqemuutil.la, and instead define a crypto-obj-y variable that is referenced directly by all the executables that need this code (tools + softmmu, but not qemu-ga). We avoid pulling entire of crypto-obj-y into the userspace emulators as that would force them to link to gnutls too, which is not required. > 3) I don't really see the need or advantage in conditionalizing tests of > libqemuutil.a, which is built even for linux-user builds (even though > e.g. the coroutines code does not end up in the binary). Since most of > the tests you are removing are related to the block layer, perhaps you > can add > > CONFIG_BLOCK := $(call lor, $(CONFIG_SOFTMMU), $(call notempty,$(TOOLS)) > > and use it to conditionalize the testcases that depend on block-obj-y? > I agree. Thanks, Laurent
On 26/03/19 10:35, Laurent Vivier wrote:
> The change was based on the comments in the file.
Right, but the comments do not imply that crypto is being built in the
user-only case.
In fact, only char/ is being built for --disable-system --disable-tools,
and even that is because:
1) --disable-system is not disabling qemu-ga. Something like this would be
a useful improvement:
diff --git a/configure b/configure
index 1c563a7..4473c4b 100755
--- a/configure
+++ b/configure
@@ -6083,7 +6083,9 @@ fi
# Probe for guest agent support/options
if [ "$guest_agent" != "no" ]; then
- if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" -o "$mingw32" = "yes" ] ; then
+ if [ "$softmmu" = no ] ; then
+ guest_agent=no
+ elif [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" -o "$mingw32" = "yes" ] ; then
tools="qemu-ga $tools"
guest_agent=yes
elif [ "$guest_agent" != yes ]; then
2) There is one overzealous dependency:
diff --git a/Makefile b/Makefile
index d8dad39..6db517a 100644
--- a/Makefile
+++ b/Makefile
@@ -441,6 +441,7 @@ SOFTMMU_SUBDIR_RULES=$(filter %-softmmu,$(SUBDIR_RULES))
$(SOFTMMU_SUBDIR_RULES): $(authz-obj-y)
$(SOFTMMU_SUBDIR_RULES): $(block-obj-y)
+$(SOFTMMU_SUBDIR_RULES): $(chardev-obj-y)
$(SOFTMMU_SUBDIR_RULES): $(crypto-obj-y)
$(SOFTMMU_SUBDIR_RULES): $(io-obj-y)
$(SOFTMMU_SUBDIR_RULES): config-all-devices.mak
@@ -476,7 +477,7 @@ subdir-capstone: .git-submodule-status
subdir-slirp: .git-submodule-status
$(call quiet-command,$(MAKE) -C $(SRC_PATH)/slirp BUILD_DIR="$(BUILD_DIR)/slirp" CC="$(CC)" AR="$(AR)" LD="$(LD)" RANLIB="$(RANLIB)" CFLAGS="$(QEMU_CFLAGS)")
-$(SUBDIR_RULES): libqemuutil.a $(common-obj-y) $(chardev-obj-y) \
+$(SUBDIR_RULES): libqemuutil.a $(common-obj-y) \
$(qom-obj-y) $(crypto-aes-obj-$(CONFIG_USER_ONLY))
ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS))
Feel free to include both in your series.
Paolo
diff --git a/Makefile.objs b/Makefile.objs index 3538789808af..5d4585c8e2f5 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -5,6 +5,12 @@ util-obj-y = util/ qobject/ qapi/ chardev-obj-y = chardev/ +qom-obj-y = qom/ + +crypto-obj-y = crypto/ +crypto-aes-obj-y = crypto/ + +ifneq ($(CONFIG_USER_ONLY),y) ####################################################################### # authz-obj-y is code used by both qemu system emulation and qemu-img @@ -21,21 +27,11 @@ block-obj-$(CONFIG_REPLICATION) += replication.o block-obj-m = block/ -####################################################################### -# crypto-obj-y is code used by both qemu system emulation and qemu-img - -crypto-obj-y = crypto/ -crypto-aes-obj-y = crypto/ - -####################################################################### -# qom-obj-y is code used by both qemu system emulation and qemu-img - -qom-obj-y = qom/ - ####################################################################### # io-obj-y is code used by both qemu system emulation and qemu-img io-obj-y = io/ +endif ###################################################################### # Target independent part of system emulation. The long term path is to @@ -132,10 +128,18 @@ rdmacm-mux-obj-y = contrib/rdmacm-mux/ trace-events-subdirs = trace-events-subdirs += accel/kvm trace-events-subdirs += accel/tcg +ifeq ($(CONFIG_USER_ONLY),y) +trace-events-subdirs += linux-user +else trace-events-subdirs += authz +trace-events-subdirs += nbd trace-events-subdirs += block -trace-events-subdirs += chardev +trace-events-subdirs += scsi +trace-events-subdirs += io +trace-events-subdirs += hw/display # needed by qemu-edid +endif trace-events-subdirs += crypto +trace-events-subdirs += chardev ifeq ($(CONFIG_SOFTMMU),y) trace-events-subdirs += audio trace-events-subdirs += hw/9pfs @@ -146,7 +150,6 @@ trace-events-subdirs += hw/audio trace-events-subdirs += hw/block trace-events-subdirs += hw/block/dataplane trace-events-subdirs += hw/char -trace-events-subdirs += hw/display trace-events-subdirs += hw/dma trace-events-subdirs += hw/hppa trace-events-subdirs += hw/i2c @@ -183,12 +186,8 @@ trace-events-subdirs += migration trace-events-subdirs += net trace-events-subdirs += ui endif -trace-events-subdirs += io -trace-events-subdirs += linux-user -trace-events-subdirs += nbd trace-events-subdirs += qapi trace-events-subdirs += qom -trace-events-subdirs += scsi trace-events-subdirs += target/arm trace-events-subdirs += target/hppa trace-events-subdirs += target/i386 diff --git a/tests/Makefile.include b/tests/Makefile.include index 852f17b8c79c..c147182fb052 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -45,7 +45,6 @@ SYSEMU_TARGET_LIST := $(subst -softmmu.mak,,$(notdir \ check-unit-y += tests/check-qdict$(EXESUF) check-unit-y += tests/check-block-qdict$(EXESUF) -check-unit-y += tests/test-char$(EXESUF) check-unit-y += tests/check-qnum$(EXESUF) check-unit-y += tests/check-qstring$(EXESUF) check-unit-y += tests/check-qlist$(EXESUF) @@ -61,9 +60,12 @@ check-unit-y += tests/test-string-input-visitor$(EXESUF) check-unit-y += tests/test-string-output-visitor$(EXESUF) check-unit-y += tests/test-qmp-event$(EXESUF) check-unit-y += tests/test-opts-visitor$(EXESUF) -check-unit-y += tests/test-coroutine$(EXESUF) check-unit-y += tests/test-visitor-serialization$(EXESUF) check-unit-y += tests/test-iov$(EXESUF) +check-unit-y += tests/test-x86-cpuid$(EXESUF) +ifneq ($(CONFIG_USER_ONLY),y) +check-unit-y += tests/test-char$(EXESUF) +check-unit-y += tests/test-coroutine$(EXESUF) check-unit-y += tests/test-aio$(EXESUF) check-unit-y += tests/test-aio-multithread$(EXESUF) check-unit-y += tests/test-throttle$(EXESUF) @@ -76,7 +78,7 @@ check-unit-y += tests/test-blockjob-txn$(EXESUF) check-unit-y += tests/test-block-backend$(EXESUF) check-unit-y += tests/test-block-iothread$(EXESUF) check-unit-y += tests/test-image-locking$(EXESUF) -check-unit-y += tests/test-x86-cpuid$(EXESUF) +endif # all code tested by test-x86-cpuid is inside topology.h ifeq ($(CONFIG_SOFTMMU),y) check-unit-y += tests/test-xbzrle$(EXESUF) @@ -101,6 +103,14 @@ check-unit-y += tests/check-qom-interface$(EXESUF) check-unit-y += tests/check-qom-proplist$(EXESUF) check-unit-y += tests/test-qemu-opts$(EXESUF) check-unit-y += tests/test-keyval$(EXESUF) +ifneq (,$(findstring qemu-ga,$(TOOLS))) +check-unit-$(call land,$(CONFIG_LINUX),$(CONFIG_VIRTIO_SERIAL)) += tests/test-qga$(EXESUF) +endif +check-unit-y += tests/test-timed-average$(EXESUF) +check-unit-$(CONFIG_INOTIFY1) += tests/test-util-filemonitor$(EXESUF) +check-unit-y += tests/test-util-sockets$(EXESUF) +check-unit-y += tests/test-base64$(EXESUF) +ifneq ($(CONFIG_USER_ONLY),y) check-unit-y += tests/test-write-threshold$(EXESUF) check-unit-y += tests/test-crypto-hash$(EXESUF) check-speed-y += tests/benchmark-crypto-hash$(EXESUF) @@ -111,12 +121,6 @@ check-speed-y += tests/benchmark-crypto-cipher$(EXESUF) check-unit-y += tests/test-crypto-secret$(EXESUF) check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlscredsx509$(EXESUF) check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlssession$(EXESUF) -ifneq (,$(findstring qemu-ga,$(TOOLS))) -check-unit-$(call land,$(CONFIG_LINUX),$(CONFIG_VIRTIO_SERIAL)) += tests/test-qga$(EXESUF) -endif -check-unit-y += tests/test-timed-average$(EXESUF) -check-unit-$(CONFIG_INOTIFY1) += tests/test-util-filemonitor$(EXESUF) -check-unit-y += tests/test-util-sockets$(EXESUF) check-unit-y += tests/test-authz-simple$(EXESUF) check-unit-y += tests/test-authz-list$(EXESUF) check-unit-y += tests/test-authz-listfile$(EXESUF) @@ -127,14 +131,14 @@ check-unit-y += tests/test-io-channel-file$(EXESUF) check-unit-$(CONFIG_GNUTLS) += tests/test-io-channel-tls$(EXESUF) check-unit-y += tests/test-io-channel-command$(EXESUF) check-unit-y += tests/test-io-channel-buffer$(EXESUF) -check-unit-y += tests/test-base64$(EXESUF) check-unit-$(if $(CONFIG_NETTLE),y,$(CONFIG_GCRYPT)) += tests/test-crypto-pbkdf$(EXESUF) check-unit-y += tests/test-crypto-ivgen$(EXESUF) check-unit-y += tests/test-crypto-afsplit$(EXESUF) check-unit-y += tests/test-crypto-xts$(EXESUF) check-unit-y += tests/test-crypto-block$(EXESUF) -check-unit-y += tests/test-logging$(EXESUF) check-unit-$(CONFIG_REPLICATION) += tests/test-replication$(EXESUF) +endif +check-unit-y += tests/test-logging$(EXESUF) check-unit-y += tests/test-bufferiszero$(EXESUF) check-unit-y += tests/test-uuid$(EXESUF) check-unit-y += tests/ptimer-test$(EXESUF)
Some objects are only needed for system emulation and tools. We can ignore them for the user mode case Update tests to run accordingly. Signed-off-by: Laurent Vivier <lvivier@redhat.com> --- Makefile.objs | 33 ++++++++++++++++----------------- tests/Makefile.include | 26 +++++++++++++++----------- 2 files changed, 31 insertions(+), 28 deletions(-)