[libvirt] [PATCH 0/3] tests: Include LDADDS in qemu_LDADDS
Finally got around to implement the suggestion I made in https://www.redhat.com/archives/libvir-list/2019-May/msg00894.html Andrea Bolognani (3): tests: Tweak cputest_LDADDS tests: Tweak vircapstest_LDADD tests: Include LDADDS in qemu_LDADDS tests/Makefile.am | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-) -- 2.21.0
We want have all test programs using qemu_LDADDS also use LDADDS, and cputest is the only existing exception. We can't just replace GNULIB_LIBS with LDADDS though, even though the latter is a superset of the former, because that would result in a linking error due to including the same object twice: /usr/bin/ld: ../src/libvirt_probes.o:.../src/libvirt_probes.o.dtrace-temp.c:141: multiple definition of `libvirt_object_new_semaphore'; ../src/libvirt_probes.o:.../src/libvirt_probes.o.dtrace-temp.c:141: first defined here To work around this, we include both qemu_LDADDS and LDADDS when QEMU support is enabled, and just LDADDS otherwise. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- tests/Makefile.am | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 6b17d99501..c34cfba34c 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -962,11 +962,13 @@ interfacexml2xmltest_LDADD = $(LDADDS) cputest_SOURCES = \ cputest.c \ testutils.c testutils.h -cputest_LDADD = $(LDADDS) $(LIBXML_LIBS) +cputest_LDADD = $(LIBXML_LIBS) if WITH_QEMU cputest_SOURCES += testutilsqemu.c testutilsqemu.h -cputest_LDADD += libqemumonitortestutils.la $(qemu_LDADDS) $(GNULIB_LIBS) -endif WITH_QEMU +cputest_LDADD += libqemumonitortestutils.la $(qemu_LDADDS) $(LDADDS) +else ! WITH_QEMU +cputest_LDADD += $(LDADDS) +endif ! WITH_QEMU metadatatest_SOURCES = \ metadatatest.c \ -- 2.21.0
On 6/12/19 5:19 AM, Andrea Bolognani wrote:
We want have all test programs using qemu_LDADDS also use LDADDS,
This part of the sentence seems awkward. How about "We want all test programs using qemu_LDADDS to also use LDADDS," ?
and cputest is the only existing exception.
We can't just replace GNULIB_LIBS with LDADDS though, even though the latter is a superset of the former, because that would result in a linking error due to including the same object twice:
/usr/bin/ld: ../src/libvirt_probes.o:.../src/libvirt_probes.o.dtrace-temp.c:141: multiple definition of `libvirt_object_new_semaphore'; ../src/libvirt_probes.o:.../src/libvirt_probes.o.dtrace-temp.c:141: first defined here
To work around this, we include both qemu_LDADDS and LDADDS when QEMU support is enabled, and just LDADDS otherwise.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- tests/Makefile.am | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tests/Makefile.am b/tests/Makefile.am index 6b17d99501..c34cfba34c 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -962,11 +962,13 @@ interfacexml2xmltest_LDADD = $(LDADDS) cputest_SOURCES = \ cputest.c \ testutils.c testutils.h -cputest_LDADD = $(LDADDS) $(LIBXML_LIBS) +cputest_LDADD = $(LIBXML_LIBS) if WITH_QEMU cputest_SOURCES += testutilsqemu.c testutilsqemu.h -cputest_LDADD += libqemumonitortestutils.la $(qemu_LDADDS) $(GNULIB_LIBS) -endif WITH_QEMU +cputest_LDADD += libqemumonitortestutils.la $(qemu_LDADDS) $(LDADDS) +else ! WITH_QEMU +cputest_LDADD += $(LDADDS) +endif ! WITH_QEMU
metadatatest_SOURCES = \ metadatatest.c \
We optionally include QEMU and LXC support in this test and depending on which is enabled (if either is enabled at all) we need to link in different objects. Right now we implicitly depend on the fact that qemu_LDADDS is empty when QEMU is not enabled to get the correct set of objects, but it's better to be explicit about it. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- tests/Makefile.am | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/Makefile.am b/tests/Makefile.am index c34cfba34c..f9dbf9a0f6 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1029,7 +1029,11 @@ endif WITH_LXC if WITH_QEMU vircapstest_SOURCES += testutilsqemu.c testutilsqemu.h endif WITH_QEMU +if WITH_QEMU vircapstest_LDADD = $(qemu_LDADDS) $(LDADDS) +else ! WITH_QEMU +vircapstest_LDADD = $(LDADDS) +endif ! WITH_QEMU domaincapsmock_la_SOURCES = domaincapsmock.c domaincapsmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) -- 2.21.0
At this point, all test programs that use qemu_LDADDS also use LDADDS, so we can remove a bunch of repetition by simply including the latter in the former. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- tests/Makefile.am | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index f9dbf9a0f6..5ba529124a 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 = $(LIBXML_LIBS) if WITH_QEMU cputest_SOURCES += testutilsqemu.c testutilsqemu.h -cputest_LDADD += libqemumonitortestutils.la $(qemu_LDADDS) $(LDADDS) +cputest_LDADD += libqemumonitortestutils.la $(qemu_LDADDS) else ! WITH_QEMU cputest_LDADD += $(LDADDS) endif ! WITH_QEMU @@ -1030,7 +1030,7 @@ if WITH_QEMU vircapstest_SOURCES += testutilsqemu.c testutilsqemu.h endif WITH_QEMU if WITH_QEMU -vircapstest_LDADD = $(qemu_LDADDS) $(LDADDS) +vircapstest_LDADD = $(qemu_LDADDS) else ! WITH_QEMU vircapstest_LDADD = $(LDADDS) endif ! WITH_QEMU @@ -1404,7 +1404,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
On 6/12/19 5:19 AM, Andrea Bolognani wrote:
Finally got around to implement the suggestion I made in
https://www.redhat.com/archives/libvir-list/2019-May/msg00894.html
Andrea Bolognani (3): tests: Tweak cputest_LDADDS tests: Tweak vircapstest_LDADD tests: Include LDADDS in qemu_LDADDS
tests/Makefile.am | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-)
In addition to reviewing these changes, I also tested them in my LTO build setup. Other than the commit message nit in patch 1, looks good! Reviewed-by: Jim Fehlig <jfehlig@suse.com> Regards, Jim
On Wed, 2019-06-12 at 10:53 -0600, Jim Fehlig wrote:
On 6/12/19 5:19 AM, Andrea Bolognani wrote:
Finally got around to implement the suggestion I made in
https://www.redhat.com/archives/libvir-list/2019-May/msg00894.html
Andrea Bolognani (3): tests: Tweak cputest_LDADDS tests: Tweak vircapstest_LDADD tests: Include LDADDS in qemu_LDADDS
tests/Makefile.am | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-)
In addition to reviewing these changes, I also tested them in my LTO build setup. Other than the commit message nit in patch 1, looks good!
Neat! I've fixed the commit message and pushed the series. Thanks for testing and reviewing :) -- Andrea Bolognani / Red Hat / Virtualization
participants (2)
-
Andrea Bolognani -
Jim Fehlig