[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] build: correctly check for SOICGIFVLAN GET_VLAN_VID_CMD command



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

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]