[libvirt] [PATCH] configure: fix check for DEVLINK_CMD_ESWITCH_GET

Ján Tomko posted 1 patch 6 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/6d0adc50897c699713550f9c22553406874b045f.1505822676.git.jtomko@redhat.com
configure.ac         | 9 +++------
src/util/virnetdev.c | 4 ++--
2 files changed, 5 insertions(+), 8 deletions(-)
[libvirt] [PATCH] configure: fix check for DEVLINK_CMD_ESWITCH_GET
Posted by Ján Tomko 6 years, 6 months ago
Instead of checking for all possible constants that every
kernel header with devlink support should have (and defining
HAVE_DECL_DEVLINK as 1 if any of them is present due to the
way AC_CHECK_DECLS works), only check for DEVLINK_CMD_ESWITCH_GET.

This is the name of the constant since kernel 4.11. Between 4.8
and 4.11, the now deprecated spelling DEVLINK_CMD_ESWITCH_MODE_GET
was used.
---
Yet another version.

 configure.ac         | 9 +++------
 src/util/virnetdev.c | 4 ++--
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/configure.ac b/configure.ac
index c9509c7f9..f542359a6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -628,15 +628,12 @@ if test "$with_linux" = "yes"; then
 fi
 
 dnl
-dnl check for kernel headers required by devlink
+dnl check for DEVLINK_CMD_ESWITCH_GET, required for checking
+dnl the DEVLINK_ESWITCH_MODE_SWITCHDEV capability
 dnl
 if test "$with_linux" = "yes"; then
     AC_CHECK_HEADERS([linux/devlink.h])
-    AC_CHECK_DECLS([DEVLINK_GENL_VERSION, DEVLINK_GENL_NAME, DEVLINK_ATTR_MAX, DEVLINK_CMD_ESWITCH_GET, DEVLINK_ATTR_BUS_NAME, DEVLINK_ATTR_DEV_NAME, DEVLINK_ATTR_ESWITCH_MODE, DEVLINK_ESWITCH_MODE_SWITCHDEV],
-                   [AC_DEFINE([HAVE_DECL_DEVLINK],
-                              [1],
-                              [whether devlink declarations are available])],
-                   [],
+    AC_CHECK_DECLS([DEVLINK_CMD_ESWITCH_GET], [], [],
                    [[#include <linux/devlink.h>]])
 fi
 
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 040693925..41a659732 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -59,7 +59,7 @@
 # include <net/if_dl.h>
 #endif
 
-#if HAVE_DECL_DEVLINK
+#if HAVE_LINUX_DEVLINK_H
 # include <linux/devlink.h>
 #endif
 
@@ -3120,7 +3120,7 @@ virNetDevGetEthtoolFeatures(virBitmapPtr bitmap,
 }
 
 
-# if HAVE_DECL_DEVLINK
+# if HAVE_DECL_DEVLINK_CMD_ESWITCH_GET
 /**
  * virNetDevPutExtraHeader
  * reserve and prepare room for an extra header
-- 
2.13.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] configure: fix check for DEVLINK_CMD_ESWITCH_GET
Posted by John Ferlan 6 years, 6 months ago

On 09/19/2017 08:04 AM, Ján Tomko wrote:
> Instead of checking for all possible constants that every
> kernel header with devlink support should have (and defining
> HAVE_DECL_DEVLINK as 1 if any of them is present due to the
> way AC_CHECK_DECLS works), only check for DEVLINK_CMD_ESWITCH_GET.
> 
> This is the name of the constant since kernel 4.11. Between 4.8
> and 4.11, the now deprecated spelling DEVLINK_CMD_ESWITCH_MODE_GET
> was used.
> ---
> Yet another version.
> 
>  configure.ac         | 9 +++------
>  src/util/virnetdev.c | 4 ++--
>  2 files changed, 5 insertions(+), 8 deletions(-)
> 

The one drawback for this is that hosts with kernels between 4.8 and
4.11 won't be able to use the functionality only because the name
changes in 4.11.  It's not that the command isn't available and the
other symbols aren't available, it's just a name change.

Still, I think this is a lot cleaner and avoids the name change
completely which I'm perfectly fine with; however, if the "claim" from
the committed patch is that this works as of kernel 4.8, then a
docs/news.xml update would also be necessary.

Unless of course the original author can come up with a better way to
add support between 4.8 and 4.10 inclusive.

John


> diff --git a/configure.ac b/configure.ac
> index c9509c7f9..f542359a6 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -628,15 +628,12 @@ if test "$with_linux" = "yes"; then
>  fi
>  
>  dnl
> -dnl check for kernel headers required by devlink
> +dnl check for DEVLINK_CMD_ESWITCH_GET, required for checking
> +dnl the DEVLINK_ESWITCH_MODE_SWITCHDEV capability
>  dnl
>  if test "$with_linux" = "yes"; then
>      AC_CHECK_HEADERS([linux/devlink.h])
> -    AC_CHECK_DECLS([DEVLINK_GENL_VERSION, DEVLINK_GENL_NAME, DEVLINK_ATTR_MAX, DEVLINK_CMD_ESWITCH_GET, DEVLINK_ATTR_BUS_NAME, DEVLINK_ATTR_DEV_NAME, DEVLINK_ATTR_ESWITCH_MODE, DEVLINK_ESWITCH_MODE_SWITCHDEV],
> -                   [AC_DEFINE([HAVE_DECL_DEVLINK],
> -                              [1],
> -                              [whether devlink declarations are available])],
> -                   [],
> +    AC_CHECK_DECLS([DEVLINK_CMD_ESWITCH_GET], [], [],
>                     [[#include <linux/devlink.h>]])
>  fi
>  
> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
> index 040693925..41a659732 100644
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -59,7 +59,7 @@
>  # include <net/if_dl.h>
>  #endif
>  
> -#if HAVE_DECL_DEVLINK
> +#if HAVE_LINUX_DEVLINK_H
>  # include <linux/devlink.h>
>  #endif
>  
> @@ -3120,7 +3120,7 @@ virNetDevGetEthtoolFeatures(virBitmapPtr bitmap,
>  }
>  
>  
> -# if HAVE_DECL_DEVLINK
> +# if HAVE_DECL_DEVLINK_CMD_ESWITCH_GET
>  /**
>   * virNetDevPutExtraHeader
>   * reserve and prepare room for an extra header
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] configure: fix check for DEVLINK_CMD_ESWITCH_GET
Posted by Andrea Bolognani 6 years, 6 months ago
On Tue, 2017-09-19 at 14:04 +0200, Ján Tomko wrote:
> +    AC_CHECK_DECLS([DEVLINK_CMD_ESWITCH_GET], [], [],
>                     [[#include <linux/devlink.h>]])
>  fi

Your argument in the other thread convinced me, so ACK if you
add a short comment explaining the fact that we only need to
check the availability of this one symbol because all other
devlink symbols we need were introduced before it.

-- 
Andrea Bolognani / Red Hat / Virtualization

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