On 09/18/2017 01:21 PM, John Ferlan wrote: > 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. > > 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. > > This is further complicated by a change between kernel 4.8 and 4.11 > where the originally defined name DEVLINK_CMD_ESWITCH_MODE_GET was > changed to DEVLINK_CMD_ESWITCH_GET with the comment/caveat that > the old format is obsolete should never be used. Therefore, even > though some kernels will have a couple of the same symbols that > were added at the same time (DEVLINK_ATTR_ESWITCH_MODE and > DEVLINK_ESWITCH_MODE_SWITCHDEV), they won't have the newer name > and thus cause a build failure. > > So even though multiple DEVLINK_CMD_SWITCH* symbols are used to > determine whether the set HAVE_DECL_DEVLINK, this should cover > those kernels before 4.11 with the old definition. > > Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> > --- > > I'd push this as a build breaker, but I don't want to need to go through > the trouble again if i got this wrong, so hopefully someone who's seeing > this can try out the patch... It's also present on a couple of the CI > infrastructure environments (f23, centos-7): > > https://ci.centos.org/view/libvirt/job/libvirt-master-build/ > > configure.ac | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > UGH! While not a total SNAK, I realized sometime early this morning (who knows why) - this won't work because it has the same problem as the current code, but with far less AC_CHECK_DECLS options. I think though what could be done is add: # ifndef DEVLINK_CMD_ESWITCH_GET # define DEVLINK_CMD_ESWITCH_GET DEVLINK_CMD_ESWITCH_MODE_GET # endif to src/util/virnetdev.c after it includes <linux/devlink.h> for some reason I have a recollection we've done something like that before for a similar issue. In the end what the kernel does in 4.11 by adding a #define DEVLINK_CMD_ESWITCH_MODE_GET DEVLINK_CMD_ESWITCH_GET is the opposite mechanism. If that doesn't work, then I think just go with DEVLINK_CMD_ESWITCH_GET in the AC_CHECK_DECLS below and let someone else figure out how to make it work right! John > 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 > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
