[libvirt] [PATCH] build: correctly check for SOICGIFVLAN GET_VLAN_VID_CMD command
In order to make a client-only build successful on RHEL4, commit 3ed2e54 modified src/util/virnetdev.c so that the functional version of virNetDevGetVLanID() was only compiled if GET_VLAN_VID_CMD was defined. However, it is *never* defined, but is only an enum value, so the proper version was no longer compiled even on platforms that support it. This resulted in the vlan tag not being properly set for guest traffic on VEPA mode guest macvtap interfaces that were bound to a vlan interface (that's the only place that libvirt currently uses virNetDevGetVLanID) Since there is no way to compile conditionally based on the presence of an enum value, this patch modifies configure.ac to check for said enum value with AC_TRY_COMPILE(), and #defines HAVE_GET_VLAN_VID_CMD if it's successful compiling a test program that uses GET_VLAN_VID_CMD. We can then make the compilation of virNetDevGetVLanID() conditional on that #define instead. --- configure.ac | 10 ++++++++++ src/util/virnetdev.c | 4 ++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 7ecbee9..fcd0170 100644 --- a/configure.ac +++ b/configure.ac @@ -2470,6 +2470,16 @@ if test "$with_virtualport" != "no"; then fi AM_CONDITIONAL([WITH_VIRTUALPORT], [test "$with_virtualport" = "yes"]) +AC_MSG_CHECKING([for GET_VLAN_VID_CMD in /usr/linux/if_vlan.h]) +AC_TRY_COMPILE([ #include <linux/if_vlan.h> ], + [ int x = GET_VLAN_VID_CMD; ], + [ have_get_vlan_vid_cmd=yes ], + [ have_get_vlan_vid_cmd=no ]) +if test "$have_get_vlan_vid_cmd" = "yes"; then + AC_DEFINE_UNQUOTED([HAVE_GET_VLAN_VID_CMD], 1, + [whether the kernel SIOCGIFVLAN ioctl supports GET_VLAN_VID_CMD]) +fi +AC_MSG_RESULT([$have_get_vlan_vid_cmd]) dnl netlink library diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index e74fc5f..72f52c7 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2007-2013 Red Hat, Inc. + * Copyright (C) 2007-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -732,7 +732,7 @@ int virNetDevGetIndex(const char *ifname ATTRIBUTE_UNUSED, #endif /* ! SIOCGIFINDEX */ -#if defined(SIOCGIFVLAN) && defined(HAVE_STRUCT_IFREQ) && defined(GET_VLAN_VID_CMD) +#if defined(SIOCGIFVLAN) && defined(HAVE_STRUCT_IFREQ) && defined(HAVE_GET_VLAN_VID_CMD) int virNetDevGetVLanID(const char *ifname, int *vlanid) { struct vlan_ioctl_args vlanargs = { -- 1.8.5.3
On 02/10/2014 07:17 AM, Laine Stump wrote:
In order to make a client-only build successful on RHEL4, commit 3ed2e54 modified src/util/virnetdev.c so that the functional version
I wonder if that was a typo in the 3ed2e54 commit message - we generally only care about RHEL5, not RHEL4.
of virNetDevGetVLanID() was only compiled if GET_VLAN_VID_CMD was defined. However, it is *never* defined, but is only an enum value, so the proper version was no longer compiled even on platforms that support it. This resulted in the vlan tag not being properly set for guest traffic on VEPA mode guest macvtap interfaces that were bound to a vlan interface (that's the only place that libvirt currently uses virNetDevGetVLanID)
Since there is no way to compile conditionally based on the presence of an enum value, this patch modifies configure.ac to check for said enum value with AC_TRY_COMPILE(), and #defines HAVE_GET_VLAN_VID_CMD if it's successful compiling a test program that uses GET_VLAN_VID_CMD. We can then make the compilation of virNetDevGetVLanID() conditional on that #define instead. --- configure.ac | 10 ++++++++++ src/util/virnetdev.c | 4 ++-- 2 files changed, 12 insertions(+), 2 deletions(-)
+AC_MSG_CHECKING([for GET_VLAN_VID_CMD in /usr/linux/if_vlan.h]) +AC_TRY_COMPILE([ #include <linux/if_vlan.h> ], + [ int x = GET_VLAN_VID_CMD; ], + [ have_get_vlan_vid_cmd=yes ], + [ have_get_vlan_vid_cmd=no ])
Unusual style - most autoconf macro calls don't put spaces between the [] quoting. Also, AC_TRY_COMPILE is marked deprecated in the autoconf manual, with the recommendation to use AC_COMPILE_IFELSE. But see below...
+if test "$have_get_vlan_vid_cmd" = "yes"; then + AC_DEFINE_UNQUOTED([HAVE_GET_VLAN_VID_CMD], 1, + [whether the kernel SIOCGIFVLAN ioctl supports GET_VLAN_VID_CMD]) +fi +AC_MSG_RESULT([$have_get_vlan_vid_cmd])
AC_CHECK_DECLS is more compact; for example, see how we probe for MACVLAN_MODE_PASSTHRU, at which point the rest of the code can use HAVE_DECL_MACVLAN_MODE_PASSTHRU. The idea behind this patch makes sense, but it's probably worth a v2. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Mon, Feb 10, 2014 at 10:42:28AM -0700, Eric Blake wrote:
On 02/10/2014 07:17 AM, Laine Stump wrote:
In order to make a client-only build successful on RHEL4, commit 3ed2e54 modified src/util/virnetdev.c so that the functional version
I wonder if that was a typo in the 3ed2e54 commit message - we generally only care about RHEL5, not RHEL4.
No it isn't a mistake, I explicitly made that change to enable compilation on RHEL4 when doing a client-only build Regards, 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 02/10/2014 07:42 PM, Eric Blake wrote:
On 02/10/2014 07:17 AM, Laine Stump wrote:
In order to make a client-only build successful on RHEL4, commit 3ed2e54 modified src/util/virnetdev.c so that the functional version I wonder if that was a typo in the 3ed2e54 commit message - we generally only care about RHEL5, not RHEL4.
of virNetDevGetVLanID() was only compiled if GET_VLAN_VID_CMD was defined. However, it is *never* defined, but is only an enum value, so the proper version was no longer compiled even on platforms that support it. This resulted in the vlan tag not being properly set for guest traffic on VEPA mode guest macvtap interfaces that were bound to a vlan interface (that's the only place that libvirt currently uses virNetDevGetVLanID)
Since there is no way to compile conditionally based on the presence of an enum value, this patch modifies configure.ac to check for said enum value with AC_TRY_COMPILE(), and #defines HAVE_GET_VLAN_VID_CMD if it's successful compiling a test program that uses GET_VLAN_VID_CMD. We can then make the compilation of virNetDevGetVLanID() conditional on that #define instead. --- configure.ac | 10 ++++++++++ src/util/virnetdev.c | 4 ++-- 2 files changed, 12 insertions(+), 2 deletions(-)
+AC_MSG_CHECKING([for GET_VLAN_VID_CMD in /usr/linux/if_vlan.h]) +AC_TRY_COMPILE([ #include <linux/if_vlan.h> ], + [ int x = GET_VLAN_VID_CMD; ], + [ have_get_vlan_vid_cmd=yes ], + [ have_get_vlan_vid_cmd=no ])
Unusual style - most autoconf macro calls don't put spaces between the [] quoting.
I wondered about the need for those spaces, but I was copying directly from existing code in configure.ac, so...
Also, AC_TRY_COMPILE is marked deprecated in the autoconf manual, with the recommendation to use AC_COMPILE_IFELSE. But see below...
+if test "$have_get_vlan_vid_cmd" = "yes"; then + AC_DEFINE_UNQUOTED([HAVE_GET_VLAN_VID_CMD], 1, + [whether the kernel SIOCGIFVLAN ioctl supports GET_VLAN_VID_CMD]) +fi +AC_MSG_RESULT([$have_get_vlan_vid_cmd])
AC_CHECK_DECLS is more compact;
Yes! That's exactly what I was looking for earlier today when I found AC_TRY_COMPILE() instead. I just sent (much simpler) V2.
for example, see how we probe for MACVLAN_MODE_PASSTHRU, at which point the rest of the code can use HAVE_DECL_MACVLAN_MODE_PASSTHRU.
Yes, so simple that I didn't recognize what it was doing :-)
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Laine Stump