1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 00/41] Split the libvirtd daemon into per-driver daemons
  4. View patch

[libvirt] [PATCH 13/41] remote: refactor & rename variables for building libvirtd

Daniel P. Berrangé posted 41 patches 6 years, 6 months ago
There is a newer version of this series
[libvirt] [PATCH 13/41] remote: refactor & rename variables for building libvirtd
Posted by Daniel P. Berrangé 6 years, 6 months ago 
The same make variables will be useful for building both libvirtd and
the split daemons, so refactor & rename variables to facilitate reuse.

Automake gets annoyed if you define a variable ending LDFLAGS:

src/remote/Makefile.inc.am:53: warning: variable 'REMOTE_DAEMON_LDFLAGS' is defined but no program or
src/remote/Makefile.inc.am:53: library has 'REMOTE_DAEMON' as canonical name (possible typo)

So we trick it by using an LD_FLAGS or LD_ADD suffix instead.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/remote/Makefile.inc.am | 95 ++++++++++++++++++++------------------
 1 file changed, 51 insertions(+), 44 deletions(-)

diff --git a/src/remote/Makefile.inc.am b/src/remote/Makefile.inc.am
index a11b2ff9b9..e45c0a6ce7 100644
--- a/src/remote/Makefile.inc.am
+++ b/src/remote/Makefile.inc.am
@@ -18,13 +18,13 @@ REMOTE_DRIVER_SOURCES = \
 	$(REMOTE_DRIVER_GENERATED) \
 	$(NULL)
 
-LIBVIRTD_GENERATED = \
+REMOTE_DAEMON_GENERATED = \
 	remote/remote_daemon_dispatch_stubs.h \
 	remote/remote_daemon_dispatch_lxc_stubs.h \
 	remote/remote_daemon_dispatch_qemu_stubs.h \
 	$(NULL)
 
-LIBVIRTD_SOURCES = \
+REMOTE_DAEMON_SOURCES = \
 	remote/remote_daemon.c \
 	remote/remote_daemon.h \
 	remote/remote_daemon_config.c \
@@ -33,9 +33,50 @@ LIBVIRTD_SOURCES = \
 	remote/remote_daemon_dispatch.h \
 	remote/remote_daemon_stream.c \
 	remote/remote_daemon_stream.h \
-	$(LIBVIRTD_GENERATED) \
+	$(REMOTE_DAEMON_GENERATED) \
 	$(NULL)
 
+REMOTE_DAEMON_CFLAGS = \
+	$(LIBXML_CFLAGS) \
+	$(GNUTLS_CFLAGS) \
+	$(SASL_CFLAGS) \
+	$(XDR_CFLAGS) \
+	$(DBUS_CFLAGS) \
+	$(LIBNL_CFLAGS) \
+	$(WARN_CFLAGS) \
+	$(PIE_CFLAGS) \
+	-I$(srcdir)/access \
+	-I$(srcdir)/conf \
+	-I$(srcdir)/rpc \
+	$(NULL)
+
+REMOTE_DAEMON_LD_FLAGS = \
+	$(RELRO_LDFLAGS) \
+	$(PIE_LDFLAGS) \
+	$(NO_INDIRECT_LDFLAGS) \
+	$(NO_UNDEFINED_LDFLAGS) \
+	$(NULL)
+
+REMOTE_DAEMON_LD_ADD = \
+	libvirt_driver_admin.la \
+	libvirt-lxc.la \
+	libvirt-qemu.la \
+	libvirt.la \
+	$(LIBXML_LIBS) \
+	$(GNUTLS_LIBS) \
+	$(SASL_LIBS) \
+	$(DBUS_LIBS) \
+	$(LIBNL_LIBS) \
+	$(NULL)
+
+if WITH_DTRACE_PROBES
+REMOTE_DAEMON_LD_ADD += ../src/libvirt_probes.lo
+endif WITH_DTRACE_PROBES
+
+REMOTE_DAEMON_LD_ADD += \
+	../gnulib/lib/libgnu.la \
+	$(LIBSOCKET) \
+	$(NULL)
 
 LOGROTATE_FILES_IN += \
 	remote/libvirtd.qemu.logrotate.in \
@@ -73,7 +114,7 @@ DRIVER_SOURCE_FILES += $(REMOTE_DRIVER_SOURCES)
 EXTRA_DIST += \
 	$(REMOTE_DRIVER_PROTOCOL) \
 	$(REMOTE_DRIVER_SOURCES) \
-	$(LIBVIRTD_SOURCES) \
+	$(REMOTE_DAEMON_SOURCES) \
 	remote/test_libvirtd.aug.in \
 	remote/libvirtd.aug.in \
 	remote/libvirtd.conf.in \
@@ -87,11 +128,11 @@ EXTRA_DIST += \
 # the WITH_REMOTE/WITH_LIBVIRTD conditionals
 BUILT_SOURCES += \
 	$(REMOTE_DRIVER_GENERATED) \
-	$(LIBVIRTD_GENERATED) \
+	$(REMOTE_DAEMON_GENERATED) \
 	$(NULL)
 MAINTAINERCLEANFILES += \
 	$(REMOTE_DRIVER_GENERATED) \
-	$(LIBVIRTD_GENERATED) \
+	$(REMOTE_DAEMON_GENERATED) \
 	$(NULL)
 CLEANFILES += \
 	remote/libvirtd.conf \
@@ -137,52 +178,18 @@ CLEANFILES += remote/libvirtd.aug
 
 man8_MANS += libvirtd.8
 
-libvirtd_SOURCES = $(LIBVIRTD_SOURCES)
+libvirtd_SOURCES = $(REMOTE_DAEMON_SOURCES)
 
 libvirtd_CFLAGS = \
-	$(LIBXML_CFLAGS) \
-	$(GNUTLS_CFLAGS) \
-	$(SASL_CFLAGS) \
-	$(XDR_CFLAGS) \
-	$(DBUS_CFLAGS) \
-	$(LIBNL_CFLAGS) \
-	$(WARN_CFLAGS) \
-	$(PIE_CFLAGS) \
-	-I$(srcdir)/access \
-	-I$(srcdir)/conf \
-	-I$(srcdir)/rpc \
+	$(REMOTE_DAEMON_CFLAGS) \
 	-DSOCK_PREFIX="\"libvirt\"" \
 	-DDAEMON_NAME="\"libvirtd\"" \
 	-DENABLE_IP \
 	$(NULL)
 
-libvirtd_LDFLAGS = \
-	$(RELRO_LDFLAGS) \
-	$(PIE_LDFLAGS) \
-	$(NO_INDIRECT_LDFLAGS) \
-	$(NO_UNDEFINED_LDFLAGS) \
-	$(NULL)
-
-libvirtd_LDADD = \
-	libvirt_driver_admin.la \
-	libvirt-lxc.la \
-	libvirt-qemu.la \
-	libvirt.la \
-	$(LIBXML_LIBS) \
-	$(GNUTLS_LIBS) \
-	$(SASL_LIBS) \
-	$(DBUS_LIBS) \
-	$(LIBNL_LIBS) \
-	$(NULL)
-
-if WITH_DTRACE_PROBES
-libvirtd_LDADD += ../src/libvirt_probes.lo
-endif WITH_DTRACE_PROBES
+libvirtd_LDFLAGS = $(REMOTE_DAEMON_LD_FLAGS)
 
-libvirtd_LDADD += \
-	../gnulib/lib/libgnu.la \
-	$(LIBSOCKET) \
-	$(NULL)
+libvirtd_LDADD = $(REMOTE_DAEMON_LD_ADD)
 
 remote/libvirtd.conf: remote/libvirtd.conf.in
 	$(AM_V_GEN)sed \
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 13/41] remote: refactor & rename variables for building libvirtd
Posted by Andrea Bolognani 6 years, 6 months ago 
On Tue, 2019-07-23 at 17:02 +0100, Daniel P. Berrangé wrote:
> The same make variables will be useful for building both libvirtd and
> the split daemons, so refactor & rename variables to facilitate reuse.
> 
> Automake gets annoyed if you define a variable ending LDFLAGS:
> 
> src/remote/Makefile.inc.am:53: warning: variable 'REMOTE_DAEMON_LDFLAGS' is defined but no program or
> src/remote/Makefile.inc.am:53: library has 'REMOTE_DAEMON' as canonical name (possible typo)

I'd indent this by two spaces for readabilty.

You're recreating the pre-existing setup faithfully, so

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

[...]
> +REMOTE_DAEMON_LD_ADD += \
> +	../gnulib/lib/libgnu.la \
> +	$(LIBSOCKET) \
> +	$(NULL)

As an aside, it looks like $(LIBSOCKET) is a leftover of days long
gone and no longer used for anything.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 13/41] remote: refactor & rename variables for building libvirtd
Posted by Daniel P. Berrangé 6 years, 6 months ago 
On Fri, Jul 26, 2019 at 05:19:56PM +0200, Andrea Bolognani wrote:
> On Tue, 2019-07-23 at 17:02 +0100, Daniel P. Berrangé wrote:
> > The same make variables will be useful for building both libvirtd and
> > the split daemons, so refactor & rename variables to facilitate reuse.
> > 
> > Automake gets annoyed if you define a variable ending LDFLAGS:
> > 
> > src/remote/Makefile.inc.am:53: warning: variable 'REMOTE_DAEMON_LDFLAGS' is defined but no program or
> > src/remote/Makefile.inc.am:53: library has 'REMOTE_DAEMON' as canonical name (possible typo)
> 
> I'd indent this by two spaces for readabilty.
> 
> You're recreating the pre-existing setup faithfully, so
> 
>   Reviewed-by: Andrea Bolognani <abologna@redhat.com>
> 
> [...]
> > +REMOTE_DAEMON_LD_ADD += \
> > +	../gnulib/lib/libgnu.la \
> > +	$(LIBSOCKET) \
> > +	$(NULL)
> 
> As an aside, it looks like $(LIBSOCKET) is a leftover of days long
> gone and no longer used for anything.

It is something that gnulib defines. Whether it expands to a non-empty
string on any of our supported build platforms though, I don't know.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 13/41] remote: refactor & rename variables for building libvirtd
Posted by Andrea Bolognani 6 years, 6 months ago 
On Fri, 2019-07-26 at 16:21 +0100, Daniel P. Berrangé wrote:
> On Fri, Jul 26, 2019 at 05:19:56PM +0200, Andrea Bolognani wrote:
> > [...]
> > > +REMOTE_DAEMON_LD_ADD += \
> > > +	../gnulib/lib/libgnu.la \
> > > +	$(LIBSOCKET) \
> > > +	$(NULL)
> > 
> > As an aside, it looks like $(LIBSOCKET) is a leftover of days long
> > gone and no longer used for anything.
> 
> It is something that gnulib defines. Whether it expands to a non-empty
> string on any of our supported build platforms though, I don't know.

I tried removing it and ran it through the full gauntlet without
getting any failures, so I'm pretty confident we don't need it. It'd
be pretty weird if we did, since we have at least two other daemons
already and neither of those is using it...

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 13/41] remote: refactor & rename variables for building libvirtd
Posted by Daniel P. Berrangé 6 years, 6 months ago 
On Fri, Jul 26, 2019 at 05:46:38PM +0200, Andrea Bolognani wrote:
> On Fri, 2019-07-26 at 16:21 +0100, Daniel P. Berrangé wrote:
> > On Fri, Jul 26, 2019 at 05:19:56PM +0200, Andrea Bolognani wrote:
> > > [...]
> > > > +REMOTE_DAEMON_LD_ADD += \
> > > > +	../gnulib/lib/libgnu.la \
> > > > +	$(LIBSOCKET) \
> > > > +	$(NULL)
> > > 
> > > As an aside, it looks like $(LIBSOCKET) is a leftover of days long
> > > gone and no longer used for anything.
> > 
> > It is something that gnulib defines. Whether it expands to a non-empty
> > string on any of our supported build platforms though, I don't know.
> 
> I tried removing it and ran it through the full gauntlet without
> getting any failures, so I'm pretty confident we don't need it. It'd
> be pretty weird if we did, since we have at least two other daemons
> already and neither of those is using it...

LIBSOCKET is set to -lws2_32 on Windows builds.

We don't build libvirtd on mingw though which is why you don't see
a problem. Previously we would have needed this for cygwin I expect
but that's not a supported build target

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 13/41] remote: refactor & rename variables for building libvirtd
Posted by Andrea Bolognani 6 years, 6 months ago 
On Mon, 2019-07-29 at 12:39 +0100, Daniel P. Berrangé wrote:
> On Fri, Jul 26, 2019 at 05:46:38PM +0200, Andrea Bolognani wrote:
> > On Fri, 2019-07-26 at 16:21 +0100, Daniel P. Berrangé wrote:
> > > On Fri, Jul 26, 2019 at 05:19:56PM +0200, Andrea Bolognani wrote:
> > > > [...]
> > > > > +REMOTE_DAEMON_LD_ADD += \
> > > > > +	../gnulib/lib/libgnu.la \
> > > > > +	$(LIBSOCKET) \
> > > > > +	$(NULL)
> > > > 
> > > > As an aside, it looks like $(LIBSOCKET) is a leftover of days long
> > > > gone and no longer used for anything.
> > > 
> > > It is something that gnulib defines. Whether it expands to a non-empty
> > > string on any of our supported build platforms though, I don't know.
> > 
> > I tried removing it and ran it through the full gauntlet without
> > getting any failures, so I'm pretty confident we don't need it. It'd
> > be pretty weird if we did, since we have at least two other daemons
> > already and neither of those is using it...
> 
> LIBSOCKET is set to -lws2_32 on Windows builds.
> 
> We don't build libvirtd on mingw though which is why you don't see
> a problem. Previously we would have needed this for cygwin I expect
> but that's not a supported build target

I guess anyone who wanted to (re?)introduce support for non-MinGW
builds will have to address many more issues, so as long as it's not
needed on any of our target build platforms we can safely drop it.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.