various build flag related fixes

[PATCH 1/9] util: remove useless check for IFLA_VF_MAX

Posted by Laine Stump 4 years, 1 month ago

IFLA_VF_MAX was introduced to the Linux kernel in 2.6.35, and was even
backported to the RHEL*6* 2.6.32 kernel downstream, so it is present
in all supported versions of all Linux distros that libvirt builds
on. Additionally, it can't be conditionally compiled out of a
kernel. There is no reason to conditionalize any piece of code on
presence of IFLA_VF_MAX - if the platform is Linux, it is supported.

Signed-off-by: Laine Stump <laine@redhat.com>
---
 src/util/virnetdev.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index e1a4cc2bef..07208a1876 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1473,7 +1473,7 @@ virNetDevSysfsFile(char **pf_sysfs_device_link G_GNUC_UNUSED,
 
 
 #endif /* !__linux__ */
-#if defined(__linux__) && defined(WITH_LIBNL) && defined(IFLA_VF_MAX)
+#if defined(__linux__) && defined(WITH_LIBNL)
 
 
 static virMacAddr zeroMAC = { .addr = { 0, 0, 0, 0, 0, 0 } };
@@ -2266,7 +2266,7 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
 }
 
 
-#else /* defined(__linux__) && defined(WITH_LIBNL)  && defined(IFLA_VF_MAX) */
+#else /* defined(__linux__) && defined(WITH_LIBNL) */
 
 
 int
@@ -2309,7 +2309,7 @@ virNetDevSetNetConfig(const char *linkdev G_GNUC_UNUSED,
 }
 
 
-#endif /* defined(__linux__) && defined(WITH_LIBNL) && defined(IFLA_VF_MAX) */
+#endif /* defined(__linux__) && defined(WITH_LIBNL) */
 
 VIR_ENUM_IMPL(virNetDevIfState,
               VIR_NETDEV_IF_STATE_LAST,
-- 
2.26.2

Re: [PATCH 1/9] util: remove useless check for IFLA_VF_MAX

Posted by Michal Prívozník 4 years, 1 month ago

On 10/1/20 1:14 AM, Laine Stump wrote:
> IFLA_VF_MAX was introduced to the Linux kernel in 2.6.35, and was even
> backported to the RHEL*6* 2.6.32 kernel downstream, so it is present
> in all supported versions of all Linux distros that libvirt builds
> on. Additionally, it can't be conditionally compiled out of a
> kernel. There is no reason to conditionalize any piece of code on
> presence of IFLA_VF_MAX - if the platform is Linux, it is supported.
> 
> Signed-off-by: Laine Stump <laine@redhat.com>
> ---
>   src/util/virnetdev.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
> index e1a4cc2bef..07208a1876 100644
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -1473,7 +1473,7 @@ virNetDevSysfsFile(char **pf_sysfs_device_link G_GNUC_UNUSED,
>   
>   
>   #endif /* !__linux__ */
> -#if defined(__linux__) && defined(WITH_LIBNL) && defined(IFLA_VF_MAX)
> +#if defined(__linux__) && defined(WITH_LIBNL)
>   
>   
>   static virMacAddr zeroMAC = { .addr = { 0, 0, 0, 0, 0, 0 } };
> @@ -2266,7 +2266,7 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
>   }
>   
>   
> -#else /* defined(__linux__) && defined(WITH_LIBNL)  && defined(IFLA_VF_MAX) */
> +#else /* defined(__linux__) && defined(WITH_LIBNL) */
>   
>   
>   int
> @@ -2309,7 +2309,7 @@ virNetDevSetNetConfig(const char *linkdev G_GNUC_UNUSED,
>   }
>   
>   
> -#endif /* defined(__linux__) && defined(WITH_LIBNL) && defined(IFLA_VF_MAX) */
> +#endif /* defined(__linux__) && defined(WITH_LIBNL) */
>   
>   VIR_ENUM_IMPL(virNetDevIfState,
>                 VIR_NETDEV_IF_STATE_LAST,
> 

This should be squashed in:

diff --git i/meson.build w/meson.build
index 073ea6d49e..592f5882fa 100644
--- i/meson.build
+++ w/meson.build
@@ -1161,8 +1161,7 @@ m_dep = cc.find_library('m', required : false)

  use_macvtap = false
  if not get_option('macvtap').disabled()
-  if (cc.has_header_symbol('linux/if_link.h', 'MACVLAN_MODE_BRIDGE') and
-      cc.has_header_symbol('linux/if_link.h', 'IFLA_VF_MAX'))
+  if cc.has_header_symbol('linux/if_link.h', 'MACVLAN_MODE_BRIDGE') and
      use_macvtap = true
    endif


Ah, you're removing the whole block later in the series. Okay then.

Michal

Re: [PATCH 1/9] util: remove useless check for IFLA_VF_MAX

Posted by Laine Stump 4 years, 1 month ago

On 10/1/20 4:06 AM, Michal Prívozník wrote:
> On 10/1/20 1:14 AM, Laine Stump wrote:
>> IFLA_VF_MAX was introduced to the Linux kernel in 2.6.35, and was even
>> backported to the RHEL*6* 2.6.32 kernel downstream, so it is present
>> in all supported versions of all Linux distros that libvirt builds
>> on. Additionally, it can't be conditionally compiled out of a
>> kernel. There is no reason to conditionalize any piece of code on
>> presence of IFLA_VF_MAX - if the platform is Linux, it is supported.
>>
>> Signed-off-by: Laine Stump <laine@redhat.com>
>> ---
>>   src/util/virnetdev.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
>> index e1a4cc2bef..07208a1876 100644
>> --- a/src/util/virnetdev.c
>> +++ b/src/util/virnetdev.c
>> @@ -1473,7 +1473,7 @@ virNetDevSysfsFile(char **pf_sysfs_device_link 
>> G_GNUC_UNUSED,
>>   #endif /* !__linux__ */
>> -#if defined(__linux__) && defined(WITH_LIBNL) && defined(IFLA_VF_MAX)
>> +#if defined(__linux__) && defined(WITH_LIBNL)
>>   static virMacAddr zeroMAC = { .addr = { 0, 0, 0, 0, 0, 0 } };
>> @@ -2266,7 +2266,7 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
>>   }
>> -#else /* defined(__linux__) && defined(WITH_LIBNL)  && 
>> defined(IFLA_VF_MAX) */
>> +#else /* defined(__linux__) && defined(WITH_LIBNL) */
>>   int
>> @@ -2309,7 +2309,7 @@ virNetDevSetNetConfig(const char *linkdev 
>> G_GNUC_UNUSED,
>>   }
>> -#endif /* defined(__linux__) && defined(WITH_LIBNL) && 
>> defined(IFLA_VF_MAX) */
>> +#endif /* defined(__linux__) && defined(WITH_LIBNL) */
>>   VIR_ENUM_IMPL(virNetDevIfState,
>>                 VIR_NETDEV_IF_STATE_LAST,
>>
> 
> This should be squashed in:
> 
> diff --git i/meson.build w/meson.build
> index 073ea6d49e..592f5882fa 100644
> --- i/meson.build
> +++ w/meson.build
> @@ -1161,8 +1161,7 @@ m_dep = cc.find_library('m', required : false)
> 
>   use_macvtap = false
>   if not get_option('macvtap').disabled()
> -  if (cc.has_header_symbol('linux/if_link.h', 'MACVLAN_MODE_BRIDGE') and
> -      cc.has_header_symbol('linux/if_link.h', 'IFLA_VF_MAX'))
> +  if cc.has_header_symbol('linux/if_link.h', 'MACVLAN_MODE_BRIDGE') and

Well, really I meant to point out in the commit log that 
MACVLAN_MODE_BRIDGE has also been in the kernel since 2.6.33 and so is 
always present for all Linuxes.

>       use_macvtap = true
>     endif
> 
> 
> Ah, you're removing the whole block later in the series. Okay then.

Yeah, that too. I really only left the two patches separate in case 
someone complained about removing the user-controlled flag (although I 
see 0 use for it, since it doesn't remove any library dependency, and 
doesn't reduce the size of any binary by a useful amount).

> 
> Michal