[PATCH v2] util: Fix configure.ac check for DEVLINK_CMD_ESWITCH_GET

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

 



Turns out the mechanism of providing multiple definitions to check via
AC_CHECK_DECLS in order to determine whether HAVE_DECL_DEVLINK should
be set resulted in a false positive since as long as one of the defs
was true, then the HAVE_DECL_DEVLINK got defined.

But we cannot just check for the symbol we most care about due to
a complication in a kernel 4.11 change that altered the name of the
symbol from DEVLINK_CMD_ESWITCH_MODE_GET to DEVLINK_CMD_ESWITCH_GET
in the newer kernel and added a #define for the former name with an
ominous comment/caveat that the old format is obsolete should never
be used. However, if we only looked for the new symbol, that would
leave kernels 4.8 through 4.10 unable to use the command even though
it exists under an old name.

So, rather than checking for whether the devlink.h on the system has
multiple symbols, let's only check for whether the command we want
is defined. This patch checks for both symbol names in order to
allow usage and then adds a check to see if the new name doesn't
exist after linux/devlink.h is include, then to use #define of
the new name to the old name - essentially the reverse logic that
the 4.11 header file has.

Note that we do not need to check for other symbols as well since
they already existed prior to when the various ESWITCH symbols were
added. Other defintions include DEVLINK_GENL_VERSION, DEVLINK_GENL_NAME,
DEVLINK_ATTR_MAX, DEVLINK_ATTR_BUS_NAME, and DEVLINK_ATTR_DEV_NAME.
The DEVLINK_ATTR_ESWITCH_MODE and DEVLINK_ESWITCH_MODE_SWITCHDEV
were added at the same time as DEVLINK_CMD_ESWITCH_MODE_GET, but
it's only DEVLINK_CMD_ESWITCH_MODE_GET that changed in 4.11.

Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---

 v1: https://www.redhat.com/archives/libvir-list/2017-September/msg00543.html

 Just to make it official - here's what I described in the v1 followup
 which got the ACK if comment from Andrea:

  https://www.redhat.com/archives/libvir-list/2017-September/msg00591.html

 Still, I won't push this yet until it too gets an official blessing.

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

diff --git a/configure.ac b/configure.ac
index c9509c7f9..73ae7fdd5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -630,12 +630,16 @@ fi
 dnl
 dnl check for kernel headers required by devlink
 dnl
+dnl kernel 4.8 introduced DEVLINK_CMD_ESWITCH_MODE_GET, but that was
+dnl later changed in kernel 4.11 to be just DEVLINK_CMD_ESWITCH_GET, so
+dnl for "completeness" let's allow HAVE_DECL_DEVLINK to be define if
+dnl either is defined.
 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_CHECK_DECLS([DEVLINK_CMD_ESWITCH_GET, DEVLINK_CMD_ESWITCH_MODE_GET],
                    [AC_DEFINE([HAVE_DECL_DEVLINK],
                               [1],
-                              [whether devlink declarations are available])],
+                              [whether devlink CMD_ESWITCH_GET is available])],
                    [],
                    [[#include <linux/devlink.h>]])
 fi
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 040693925..435ca5562 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -61,6 +61,9 @@
 
 #if HAVE_DECL_DEVLINK
 # include <linux/devlink.h>
+# ifndef DEVLINK_CMD_ESWITCH_GET
+#  define DEVLINK_CMD_ESWITCH_GET DEVLINK_CMD_ESWITCH_MODE_GET
+# endif
 #endif
 
 #ifndef IFNAMSIZ
-- 
2.13.5

--
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]
  Powered by Linux