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

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



On 02/11/2014 12:49 AM, Eric Blake wrote:
> On 02/10/2014 03:38 PM, 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
>> 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_CHECK_DECLS(), which #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.
>> ---
>>
>> Difference from V1:
>>
>> Use the amazingly compact AC_CHECK_DECLS() instead
>> of deprecated AC_TRY_COMPILE() + extra trappings.
>>
>>  configure.ac         | 2 ++
>>  src/util/virnetdev.c | 4 ++--
>>  2 files changed, 4 insertions(+), 2 deletions(-)
> Almost!
>
>>  
>> -#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)
> s/defined(HAVE_GET_VLAN_VID_CMD)/HAVE_GET_VLAN_VID_CMD/

As a matter of fact, "HAVE_DECL_GET_VLAN_VID_CMD". I fixed that and
pushed. Thanks for the tutorial :-)


>
> AC_CHECK_DECLS unconditionally defines the variable to either 0 or 1, so
> checking if it is defined will still fail to compile on older headers
> that lack the enum; 

Well, it would have compiled successfully, but compiled the wrong code :-)

> what you want is to check that it has a non-zero value.

Now that I've seen this, it bugs me that all HAVE_* #defines aren't
consistent about this - some are undefined if the capability is missing,
and some are #defined, but to 0.


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