On 5/30/19 10:00 AM, Andrea Bolognani wrote:
On Thu, 2019-05-30 at 16:56 +0200, Michal Privoznik wrote:
On 5/30/19 4:44 PM, Jim Fehlig wrote:
On 5/30/19 3:08 AM, Michal Privoznik wrote:
On 5/29/19 7:44 PM, Jim Fehlig wrote:
-libqemutestdriver_la_LIBADD = $(qemu_LDADDS)
+libqemutestdriver_la_LIBADD = $(qemu_LDADDS) $(GNULIB_LIBS)
qemucpumock_la_SOURCES = \
qemucpumock.c testutilshostcpus.h
ACK and safe for freeze to this hunk. Alternatively, we might go with
$(LDADDS) which includes $(GNULIB_LIBS).
Do you have a preference? LDADDS includes some other things which AFAIK
are not needed.
Well, other test libs use LDADDS and I'd say that LTO doesn't link
anything that's not needed. But maybe I'm mistaken. So, no, I don't have
any preference.
I think we don't really care about potentially overlinking when it
comes to test programs, so there's no need to have the kind of
granularity your patch implements.
Actually I'd go one further and adopt what Xen tests are doing:
there's an explicit
libxl_LDADDS += $(LDADDS)
and then most tests include at least $(libxl_LDADDS) in their
_(LD|LIB)ADDs, whereas most QEMU tests need to use
_(LD|LIB)ADD = $(qemu_LDADDS) $(LDADDS)
My suggestion would be to copy that approach, have
qemu_LDADDS += $(LDADDS)
and then drop the extra $(LDADDS) from QEMU tests, which will not
only fix your linkin problem but also clean up Makefile.am pretty
nicely :)
Sorry for not noticing your reply before pushing. As it turns out your
suggestion is not as simple as I expected. The attached patch fails with link
errors such as
/usr/lib64/gcc/x86_64-suse-linux/8/../../../../x86_64-suse-linux/bin/ld:
../src/libvirt_probes.o:/home/jfehlig/virt/upstream/libvirt/src/libvirt_probes.o.dtrace-temp.c:15:
multiple definition of `libvirt_event_poll_add_handle_semaphore';
../src/libvirt_probes.o:/home/jfehlig/virt/upstream/libvirt/src/libvirt_probes.o.dtrace-temp.c:15:
first defined here
/usr/lib64/gcc/x86_64-suse-linux/8/../../../../x86_64-suse-linux/bin/ld:
../src/libvirt_probes.o:/home/jfehlig/virt/upstream/libvirt/src/libvirt_probes.o.dtrace-temp.c:24:
multiple definition of `libvirt_event_poll_update_handle_semaphore';
../src/libvirt_probes.o:/home/jfehlig/virt/upstream/libvirt/src/libvirt_probes.o.dtrace-temp.c:24:
first defined here
This is due to LDADDS including ../src/libvirt_probes.lo (PROBES_O) and
qemu_LDADDS including ../src/libvirt_qemu_probes.lo when WITH_DTRACE_PROBES is
defined. I tried a couple of approaches to fixing this but found nothing
satisfying. (More) Suggestions welcomed :-).
Regards,
Jim
>From 5e96470fc24462083c73b6c7e05616dc9f5806b0 Mon Sep 17 00:00:00 2001
From: Jim Fehlig <jfehlig@xxxxxxxx>
Date: Mon, 3 Jun 2019 14:33:32 -0600
Subject: [PATCH] make: cleanup linking of qemu tests
Commit 76b420d0 fixed a link error in libqemutestdriver by appending
LDADDS to the list of libraries for linking. In a review that was
noticed after pushing, it was suggested to append LDADDS to qemu_LDADDS,
similar to libxl_LDADDS. This change follows up on that suggestion and
removes the now duplicate LDADDS sprinkled throughout the qemu tests.
Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
---
tests/Makefile.am | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 6b17d99501..058e5f9d4e 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -554,10 +554,11 @@ endif WITH_STORAGE
if WITH_DTRACE_PROBES
qemu_LDADDS += ../src/libvirt_qemu_probes.lo
endif WITH_DTRACE_PROBES
+qemu_LDADDS += $(LDADDS)
libqemutestdriver_la_SOURCES =
libqemutestdriver_la_LDFLAGS = $(DRIVERLIB_LDFLAGS)
-libqemutestdriver_la_LIBADD = $(qemu_LDADDS) $(LDADDS)
+libqemutestdriver_la_LIBADD = $(qemu_LDADDS)
qemucpumock_la_SOURCES = \
qemucpumock.c testutilshostcpus.h
@@ -580,7 +581,7 @@ qemuxml2argvmock_la_LIBADD = $(MOCKLIBS_LIBS)
qemuxml2xmltest_SOURCES = \
qemuxml2xmltest.c testutilsqemu.c testutilsqemu.h \
testutils.c testutils.h
-qemuxml2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS)
+qemuxml2xmltest_LDADD = $(qemu_LDADDS)
qemuargv2xmltest_SOURCES = \
qemuargv2xmltest.c testutilsqemu.c testutilsqemu.h \
@@ -594,7 +595,7 @@ qemumonitorjsontest_SOURCES = \
testutilsqemu.c testutilsqemu.h \
$(NULL)
qemumonitorjsontest_LDADD = libqemumonitortestutils.la \
- $(qemu_LDADDS) $(LDADDS)
+ $(qemu_LDADDS)
qemucapabilitiestest_SOURCES = \
qemucapabilitiestest.c \
@@ -602,7 +603,7 @@ qemucapabilitiestest_SOURCES = \
testutilsqemu.c testutilsqemu.h \
$(NULL)
qemucapabilitiestest_LDADD = libqemumonitortestutils.la \
- $(qemu_LDADDS) $(LDADDS)
+ $(qemu_LDADDS)
qemucapsprobe_SOURCES = \
qemucapsprobe.c
@@ -620,14 +621,14 @@ qemucommandutiltest_SOURCES = \
testutilsqemu.c testutilsqemu.h \
$(NULL)
qemucommandutiltest_LDADD = libqemumonitortestutils.la \
- $(qemu_LDADDS) $(LDADDS)
+ $(qemu_LDADDS)
qemucaps2xmltest_SOURCES = \
qemucaps2xmltest.c \
testutils.c testutils.h \
testutilsqemu.c testutilsqemu.h \
$(NULL)
-qemucaps2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS)
+qemucaps2xmltest_LDADD = $(qemu_LDADDS)
qemucaps2xmlmock_la_SOURCES = \
qemucaps2xmlmock.c
@@ -639,14 +640,14 @@ qemuagenttest_SOURCES = \
testutils.c testutils.h \
testutilsqemu.c testutilsqemu.h \
$(NULL)
-qemuagenttest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) $(LDADDS)
+qemuagenttest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS)
qemuhotplugtest_SOURCES = \
qemuhotplugtest.c \
testutils.c testutils.h \
testutilsqemu.c testutilsqemu.h \
$(NULL)
-qemuhotplugtest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) $(LDADDS)
+qemuhotplugtest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS)
qemublocktest_SOURCES = \
qemublocktest.c \
@@ -658,19 +659,18 @@ qemublocktest_LDADD = \
../src/libvirt_conf.la \
../src/libvirt_util.la \
$(qemu_LDADDS) \
- $(LDADDS) \
$(NULL)
domainsnapshotxml2xmltest_SOURCES = \
domainsnapshotxml2xmltest.c testutilsqemu.c testutilsqemu.h \
testutils.c testutils.h
-domainsnapshotxml2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS)
+domainsnapshotxml2xmltest_LDADD = $(qemu_LDADDS)
qemumemlocktest_SOURCES = \
qemumemlocktest.c \
testutilsqemu.c testutilsqemu.h \
testutils.c testutils.h
-qemumemlocktest_LDADD = $(qemu_LDADDS) $(LDADDS)
+qemumemlocktest_LDADD = $(qemu_LDADDS)
qemumigparamstest_SOURCES = \
qemumigparamstest.c \
@@ -678,21 +678,21 @@ qemumigparamstest_SOURCES = \
testutilsqemu.c testutilsqemu.h \
$(NULL)
qemumigparamstest_LDADD = libqemumonitortestutils.la \
- $(qemu_LDADDS) $(LDADDS)
+ $(qemu_LDADDS)
qemusecuritytest_SOURCES = \
qemusecuritytest.c qemusecuritytest.h \
qemusecuritymock.c \
testutils.h testutils.c \
testutilsqemu.h testutilsqemu.c
-qemusecuritytest_LDADD = $(qemu_LDADDS) $(LDADDS)
+qemusecuritytest_LDADD = $(qemu_LDADDS)
qemufirmwaretest_SOURCES = \
qemufirmwaretest.c \
testutils.h testutils.c \
virfilewrapper.c virfilewrapper.h \
$(NULL)
-qemufirmwaretest_LDADD = $(qemu_LDADDS) $(LDADDS)
+qemufirmwaretest_LDADD = $(qemu_LDADDS)
else ! WITH_QEMU
EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c \
@@ -965,7 +965,7 @@ cputest_SOURCES = \
cputest_LDADD = $(LDADDS) $(LIBXML_LIBS)
if WITH_QEMU
cputest_SOURCES += testutilsqemu.c testutilsqemu.h
-cputest_LDADD += libqemumonitortestutils.la $(qemu_LDADDS) $(GNULIB_LIBS)
+cputest_LDADD += libqemumonitortestutils.la $(qemu_LDADDS)
endif WITH_QEMU
metadatatest_SOURCES = \
@@ -1027,7 +1027,7 @@ endif WITH_LXC
if WITH_QEMU
vircapstest_SOURCES += testutilsqemu.c testutilsqemu.h
endif WITH_QEMU
-vircapstest_LDADD = $(qemu_LDADDS) $(LDADDS)
+vircapstest_LDADD = $(qemu_LDADDS)
domaincapsmock_la_SOURCES = domaincapsmock.c
domaincapsmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
@@ -1398,7 +1398,7 @@ if WITH_QEMU
securityselinuxlabeltest_SOURCES = \
securityselinuxlabeltest.c testutils.h testutils.c \
testutilsqemu.h testutilsqemu.c
-securityselinuxlabeltest_LDADD = $(qemu_LDADDS) $(LDADDS) $(SELINUX_LIBS)
+securityselinuxlabeltest_LDADD = $(qemu_LDADDS) $(SELINUX_LIBS)
securityselinuxlabeltest_DEPENDENCIES = libsecurityselinuxhelper.la \
../src/libvirt.la
endif WITH_QEMU
--
2.21.0
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
