[libvirt] build: fix linking virt-login-shell

After commit 3e2f27e1, I've noticed build failures of virt-login-shell when libapparmor-devel is installed on the build host CCLD virt-login-shell ../src/.libs/libvirt-setuid-rpc-client.a(libvirt_setuid_rpc_client_la-vircommand.o): In function `virExec': /home/jfehlig/virt/upstream/libvirt/src/util/vircommand.c:653: undefined reference to `aa_change_profile' collect2: error: ld returned 1 exit status I was about to commit an easy fix under the build-breaker rule (build-fix-1.patch), but thought to extend the notion of SECDRIVER_LIBS to SECDRIVER_CFLAGS, and use both throughout src/Makefile.am where it makes sense (build-fix-2.patch). Should I just stick with the simple fix, or is something along the lines of patch 2 preferred? Regards, Jim

On Mon, Oct 21, 2013 at 03:36:11PM -0600, Jim Fehlig wrote:
From a0f35945f3127ab70d051101037e821b1759b4bb Mon Sep 17 00:00:00 2001 From: Jim Fehlig <jfehlig@suse.com> Date: Mon, 21 Oct 2013 15:30:02 -0600 Subject: [PATCH] build: fix virt-login-shell build with apparmor
With libapparmor-devel installed, virt-login-shell fails to link
CCLD virt-login-shell ../src/.libs/libvirt-setuid-rpc-client.a(libvirt_setuid_rpc_client_la-vircommand.o): In function `virExec': /home/jfehlig/virt/upstream/libvirt/src/util/vircommand.c:653: undefined reference to `aa_change_profile' collect2: error: ld returned 1 exit status
Fix by linking libvirt_setuid_rpc_client with previously determined SECDRIVER_LIBS in src/Makefile.am. While at it, introduce SECDRIVER_CFLAGS and use both throughout src/Makefile.am where it makes sense.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/Makefile.am | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am index 9dab4df..21b9caf 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -49,11 +49,14 @@ nodist_conf_DATA =
THREAD_LIBS = $(LIB_PTHREAD) $(LTLIBMULTITHREAD)
+SECDRIVER_CFLAGS = SECDRIVER_LIBS = if WITH_SECDRIVER_SELINUX +SECDRIVER_CFLAGS += $(SELINUX_CFLAGS) SECDRIVER_LIBS += $(SELINUX_LIBS) endif WITH_SECDRIVER_SELINUX if WITH_SECDRIVER_APPARMOR +SECDRIVER_CFLAGS += $(APPARMOR_CFLAGS) SECDRIVER_LIBS += $(APPARMOR_LIBS) endif WITH_SECDRIVER_APPARMOR
@@ -2027,14 +2030,14 @@ libvirt_setuid_rpc_client_la_SOURCES = \ libvirt_setuid_rpc_client_la_LDFLAGS = \ $(AM_LDFLAGS) \ $(LIBXML_LIBS) \ - $(SELINUX_LIBS) \ + $(SECDRIVER_LIBS) \ $(NULL) libvirt_setuid_rpc_client_la_CFLAGS = \ -DLIBVIRT_SETUID_RPC_CLIENT \ -I$(top_srcdir)/src/conf \ -I$(top_srcdir)/src/rpc \ $(AM_CFLAGS) \ - $(SELINUX_CFLAGS) \ + $(SECDRIVER_CFLAGS) \ $(NULL) endif WITH_LXC
@@ -2316,7 +2319,7 @@ libvirt_net_rpc_la_LDFLAGS = \ $(GNUTLS_LIBS) \ $(SASL_LIBS) \ $(SSH2_LIBS)\ - $(SELINUX_LIBS) \ + $(SECDRIVER_LIBS) \ $(AM_LDFLAGS) \ $(CYGWIN_EXTRA_LDFLAGS) \ $(MINGW_EXTRA_LDFLAGS) @@ -2460,12 +2463,7 @@ if WITH_BLKID libvirt_lxc_CFLAGS += $(BLKID_CFLAGS) libvirt_lxc_LDADD += $(BLKID_LIBS) endif WITH_BLKID -if WITH_SECDRIVER_SELINUX -libvirt_lxc_CFLAGS += $(SELINUX_CFLAGS) -endif WITH_SECDRIVER_SELINUX -if WITH_SECDRIVER_APPARMOR -libvirt_lxc_CFLAGS += $(APPARMOR_CFLAGS) -endif WITH_SECDRIVER_APPARMOR +libvirt_lxc_CFLAGS += $(SECDRIVER_CFLAGS) endif WITH_LIBVIRTD endif WITH_LXC EXTRA_DIST += $(LXC_CONTROLLER_SOURCES)
ACK to this version of the patch Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 22.10.2013 09:29, Daniel P. Berrange wrote:
On Mon, Oct 21, 2013 at 03:36:11PM -0600, Jim Fehlig wrote:
From a0f35945f3127ab70d051101037e821b1759b4bb Mon Sep 17 00:00:00 2001 From: Jim Fehlig <jfehlig@suse.com> Date: Mon, 21 Oct 2013 15:30:02 -0600 Subject: [PATCH] build: fix virt-login-shell build with apparmor
With libapparmor-devel installed, virt-login-shell fails to link
CCLD virt-login-shell ../src/.libs/libvirt-setuid-rpc-client.a(libvirt_setuid_rpc_client_la-vircommand.o): In function `virExec': /home/jfehlig/virt/upstream/libvirt/src/util/vircommand.c:653: undefined reference to `aa_change_profile' collect2: error: ld returned 1 exit status
Fix by linking libvirt_setuid_rpc_client with previously determined SECDRIVER_LIBS in src/Makefile.am. While at it, introduce SECDRIVER_CFLAGS and use both throughout src/Makefile.am where it makes sense.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/Makefile.am | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
ACK to this version of the patch
Daniel
I went ahead and pushed this as it's build breaker which I'm experiencing everytime I try to build (yes, I do build with AppArmor). Michal

At Tue, 22 Oct 2013 16:27:02 +0100, Michal Privoznik wrote:
On 22.10.2013 09:29, Daniel P. Berrange wrote:
On Mon, Oct 21, 2013 at 03:36:11PM -0600, Jim Fehlig wrote:
From a0f35945f3127ab70d051101037e821b1759b4bb Mon Sep 17 00:00:00 2001 From: Jim Fehlig <jfehlig@suse.com> Date: Mon, 21 Oct 2013 15:30:02 -0600 Subject: [PATCH] build: fix virt-login-shell build with apparmor
With libapparmor-devel installed, virt-login-shell fails to link
CCLD virt-login-shell ../src/.libs/libvirt-setuid-rpc-client.a(libvirt_setuid_rpc_client_la-vircommand.o): In function `virExec': /home/jfehlig/virt/upstream/libvirt/src/util/vircommand.c:653: undefined reference to `aa_change_profile' collect2: error: ld returned 1 exit status
Fix by linking libvirt_setuid_rpc_client with previously determined SECDRIVER_LIBS in src/Makefile.am. While at it, introduce SECDRIVER_CFLAGS and use both throughout src/Makefile.am where it makes sense.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/Makefile.am | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
ACK to this version of the patch
Daniel
I went ahead and pushed this as it's build breaker which I'm experiencing everytime I try to build (yes, I do build with AppArmor).
[too bad you didn't clean up the commit message beforehand...] Could someone cherrypick this to the maint branches as well, please?! And, while at it, maybe also e350826c6 and 843bdb2f8a3. Thanks, Claudio -- AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany Phone: +49 341 265 310 19 Web:<http://www.av-test.org> Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076) Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern

On 23.10.2013 07:17, Claudio Bley wrote:
At Tue, 22 Oct 2013 16:27:02 +0100, Michal Privoznik wrote:
On 22.10.2013 09:29, Daniel P. Berrange wrote:
On Mon, Oct 21, 2013 at 03:36:11PM -0600, Jim Fehlig wrote:
From a0f35945f3127ab70d051101037e821b1759b4bb Mon Sep 17 00:00:00 2001 From: Jim Fehlig <jfehlig@suse.com> Date: Mon, 21 Oct 2013 15:30:02 -0600 Subject: [PATCH] build: fix virt-login-shell build with apparmor
With libapparmor-devel installed, virt-login-shell fails to link
CCLD virt-login-shell ../src/.libs/libvirt-setuid-rpc-client.a(libvirt_setuid_rpc_client_la-vircommand.o): In function `virExec': /home/jfehlig/virt/upstream/libvirt/src/util/vircommand.c:653: undefined reference to `aa_change_profile' collect2: error: ld returned 1 exit status
Fix by linking libvirt_setuid_rpc_client with previously determined SECDRIVER_LIBS in src/Makefile.am. While at it, introduce SECDRIVER_CFLAGS and use both throughout src/Makefile.am where it makes sense.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/Makefile.am | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
ACK to this version of the patch
Daniel
I went ahead and pushed this as it's build breaker which I'm experiencing everytime I try to build (yes, I do build with AppArmor).
[too bad you didn't clean up the commit message beforehand...]
Oh my, I didn't? Sorry, I've just took the patch, applied it and checked it fixes the issue I'm seeing.
Could someone cherrypick this to the maint branches as well, please?!
And, while at it, maybe also e350826c6 and 843bdb2f8a3.
I've backported all three patches onto v1.1.3-maint and v1.1.2-maint branches. Michal

At Wed, 23 Oct 2013 09:57:40 +0100, Michal Privoznik wrote:
On 23.10.2013 07:17, Claudio Bley wrote:
Could someone cherrypick this to the maint branches as well, please?!
And, while at it, maybe also e350826c6 and 843bdb2f8a3.
I've backported all three patches onto v1.1.3-maint and v1.1.2-maint branches.
Thanks! BTW, what's the policy of pushing to maint branches? I remember that there was a message on this list regarding permissions on those branches, but can't find it right now. Is it prohibited / uncalled for to push to those branches? Claudio -- AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany Phone: +49 341 265 310 19 Web:<http://www.av-test.org> Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076) Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern

On 10/23/2013 10:45 AM, Claudio Bley wrote:
At Wed, 23 Oct 2013 09:57:40 +0100, Michal Privoznik wrote:
On 23.10.2013 07:17, Claudio Bley wrote:
Could someone cherrypick this to the maint branches as well, please?!
And, while at it, maybe also e350826c6 and 843bdb2f8a3.
I've backported all three patches onto v1.1.3-maint and v1.1.2-maint branches.
Thanks!
BTW, what's the policy of pushing to maint branches? I remember that there was a message on this list regarding permissions on those branches, but can't find it right now. Is it prohibited / uncalled for to push to those branches?
If it's a bug fix (not a feature) and you have push rights, you're more than welcome to push it to maint branches. If the backport is not trivial, it's probably best to post it for review, but if it backports cleanly, it's fine to just state that you already pushed the backport without repeating the contents of the patch. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

At Wed, 23 Oct 2013 10:51:29 +0100, Eric Blake wrote:
BTW, what's the policy of pushing to maint branches? I remember that there was a message on this list regarding permissions on those branches, but can't find it right now. Is it prohibited / uncalled for to push to those branches?
If it's a bug fix (not a feature) and you have push rights, you're more than welcome to push it to maint branches. If the backport is not trivial, it's probably best to post it for review, but if it backports cleanly, it's fine to just state that you already pushed the backport without repeating the contents of the patch.
Alright, I was a bit hesitant going forth and just push to those branches. Thanks for clearing that up. Claudio -- AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany Phone: +49 341 265 310 19 Web:<http://www.av-test.org> Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076) Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern
participants (5)
-
Claudio Bley
-
Daniel P. Berrange
-
Eric Blake
-
Jim Fehlig
-
Michal Privoznik