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

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

 



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 :-)

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]