[libvirt] [PATCH] portability: handle ifreq differences in virnetdev
FreeBSD (and maybe other BSDs) have different member names in struct ifreq when compared to Linux, such as: - uses ifr_data instead of ifr_newname for setting interface names - uses ifr_index instead of ifr_ifindex for interface index Also, add a check for SIOCGIFHWADDR for virNetDevValidateConfig(). Use AF_LOCAL if AF_PACKET is not available. --- configure.ac | 8 ++++++++ src/util/virnetdev.c | 23 +++++++++++++++++------ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/configure.ac b/configure.ac index 23c24d2..4a32f8c 100644 --- a/configure.ac +++ b/configure.ac @@ -2363,6 +2363,14 @@ AM_CONDITIONAL([HAVE_LIBNL], [test "$have_libnl" = "yes"]) AC_SUBST([LIBNL_CFLAGS]) AC_SUBST([LIBNL_LIBS]) +AC_CHECK_MEMBERS([struct ifreq.ifr_newname, + struct ifreq.ifr_ifindex, + struct ifreq.ifr_index], + [], [], + [#include <sys/ioctl.h> + #include <net/if.h> + ]) + # Only COPYING.LIB is under version control, yet COPYING # is included as part of the distribution tarball. # Copy one to the other, but only if this is a srcdir-build. diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 7ffaac1..02c1716 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -38,7 +38,10 @@ #ifdef __linux__ # include <linux/sockios.h> # include <linux/if_vlan.h> -#elif !defined(AF_PACKET) +# define VIR_NETDEV_FAMILY AF_PACKET +#elif defined(HAVE_STRUCT_IFREQ) && defined(AF_LOCAL) +# define VIR_NETDEV_FAMILY AF_LOCAL +#else # undef HAVE_STRUCT_IFREQ #endif @@ -81,7 +84,7 @@ static int virNetDevSetupControlFull(const char *ifname, static int virNetDevSetupControl(const char *ifname, struct ifreq *ifr) { - return virNetDevSetupControlFull(ifname, ifr, AF_PACKET, SOCK_DGRAM); + return virNetDevSetupControlFull(ifname, ifr, VIR_NETDEV_FAMILY, SOCK_DGRAM); } #endif @@ -478,12 +481,16 @@ int virNetDevSetName(const char* ifname, const char *newifname) if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0) return -1; +#if !defined(HAVE_STRUCT_IFREQ_IFR_NEWNAME) + ifr.ifr_data = (caddr_t)newifname; +#else if (virStrcpyStatic(ifr.ifr_newname, newifname) == NULL) { virReportSystemError(ERANGE, _("Network interface name '%s' is too long"), newifname); goto cleanup; } +#endif if (ioctl(fd, SIOCSIFNAME, &ifr)) { virReportSystemError(errno, @@ -630,7 +637,7 @@ int virNetDevGetIndex(const char *ifname, int *ifindex) { int ret = -1; struct ifreq ifreq; - int fd = socket(PF_PACKET, SOCK_DGRAM, 0); + int fd = socket(VIR_NETDEV_FAMILY, SOCK_DGRAM, 0); if (fd < 0) { virReportSystemError(errno, "%s", @@ -654,7 +661,11 @@ int virNetDevGetIndex(const char *ifname, int *ifindex) goto cleanup; } +#if defined(HAVE_STRUCT_IFREQ_IFR_INDEX) + *ifindex = ifreq.ifr_index; +#else *ifindex = ifreq.ifr_ifindex; +#endif ret = 0; cleanup: @@ -870,7 +881,7 @@ int virNetDevGetIPv4Address(const char *ifname ATTRIBUTE_UNUSED, * * Returns 1 if the config matches, 0 if the config does not match, or interface does not exist, -1 on error */ -#if defined(HAVE_STRUCT_IFREQ) +#if defined(SIOCGIFHWADDR) && defined(HAVE_STRUCT_IFREQ) int virNetDevValidateConfig(const char *ifname, const virMacAddrPtr macaddr, int ifindex) { @@ -924,7 +935,7 @@ int virNetDevValidateConfig(const char *ifname, VIR_FORCE_CLOSE(fd); return ret; } -#else /* ! HAVE_STRUCT_IFREQ */ +#else int virNetDevValidateConfig(const char *ifname ATTRIBUTE_UNUSED, const virMacAddrPtr macaddr ATTRIBUTE_UNUSED, int ifindex ATTRIBUTE_UNUSED) @@ -933,7 +944,7 @@ int virNetDevValidateConfig(const char *ifname ATTRIBUTE_UNUSED, _("Unable to check interface config on this platform")); return -1; } -#endif /* ! HAVE_STRUCT_IFREQ */ +#endif #ifdef __linux__ -- 1.8.0
On 04/27/2013 09:50 AM, Roman Bogorodskiy wrote:
FreeBSD (and maybe other BSDs) have different member names in struct ifreq when compared to Linux, such as:
- uses ifr_data instead of ifr_newname for setting interface names - uses ifr_index instead of ifr_ifindex for interface index
Also, add a check for SIOCGIFHWADDR for virNetDevValidateConfig().
Use AF_LOCAL if AF_PACKET is not available. --- configure.ac | 8 ++++++++ src/util/virnetdev.c | 23 +++++++++++++++++------ 2 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/configure.ac b/configure.ac index 23c24d2..4a32f8c 100644 --- a/configure.ac +++ b/configure.ac @@ -2363,6 +2363,14 @@ AM_CONDITIONAL([HAVE_LIBNL], [test "$have_libnl" = "yes"]) AC_SUBST([LIBNL_CFLAGS]) AC_SUBST([LIBNL_LIBS])
+AC_CHECK_MEMBERS([struct ifreq.ifr_newname, + struct ifreq.ifr_ifindex, + struct ifreq.ifr_index], + [], [], + [#include <sys/ioctl.h> + #include <net/if.h> + ])
This is good, although a dnl'd comment explaining why we probe might be nice.
+++ b/src/util/virnetdev.c @@ -38,7 +38,10 @@ #ifdef __linux__ # include <linux/sockios.h> # include <linux/if_vlan.h> -#elif !defined(AF_PACKET) +# define VIR_NETDEV_FAMILY AF_PACKET +#elif defined(HAVE_STRUCT_IFREQ) && defined(AF_LOCAL) +# define VIR_NETDEV_FAMILY AF_LOCAL +#else
I like this one.
@@ -478,12 +481,16 @@ int virNetDevSetName(const char* ifname, const char *newifname) if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0) return -1;
+#if !defined(HAVE_STRUCT_IFREQ_IFR_NEWNAME) + ifr.ifr_data = (caddr_t)newifname; +#else if (virStrcpyStatic(ifr.ifr_newname, newifname) == NULL) { virReportSystemError(ERANGE, _("Network interface name '%s' is too long"), newifname); goto cleanup; } +#endif
This one reads awkwardly. I would have done: #ifdef HAVE_STRUCT_IFREQ_IFR_NEWNAME existing ifr_newname code #else ifr_data code #endif
@@ -654,7 +661,11 @@ int virNetDevGetIndex(const char *ifname, int *ifindex) goto cleanup; }
+#if defined(HAVE_STRUCT_IFREQ_IFR_INDEX)
#ifdef is shorter than #if defined().
+ *ifindex = ifreq.ifr_index; +#else *ifindex = ifreq.ifr_ifindex; +#endif ret = 0;
Overall, looks sane; I'll probably apply the touchups mentioned and push later today after testing on my own FreeBSD VM. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 04/29/2013 11:48 AM, Eric Blake wrote:
On 04/27/2013 09:50 AM, Roman Bogorodskiy wrote:
FreeBSD (and maybe other BSDs) have different member names in struct ifreq when compared to Linux, such as:
- uses ifr_data instead of ifr_newname for setting interface names - uses ifr_index instead of ifr_ifindex for interface index
Also, add a check for SIOCGIFHWADDR for virNetDevValidateConfig().
Use AF_LOCAL if AF_PACKET is not available. ---
Overall, looks sane; I'll probably apply the touchups mentioned and push later today after testing on my own FreeBSD VM.
With this patch, I'm getting a compile failure on FreeBSD 8.2: util/virnetdev.c:667: error: 'struct ifreq' has no member named 'ifr_ifindex' Looking further, I see <net/if.h> has: struct ifreq { ... union { ... short ifru_index; } ifr_ifru; #define ifr_index ifr_ifru.ifru_index }; so it should have picked the ifreq.ifr_index path; next looking at config.log, I see: configure:67334: checking for struct ifreq.ifr_index configure:67334: gcc -std=gnu99 -c -g -D_THREAD_SAFE -D_THREAD_SAFE conftest.c >&5 In file included from conftest.c:540: /usr/include/net/if.h:305: error: field 'ifru_addr' has incomplete type aha - BSD's <net/if.h> is not self-contained. I'm now testing a minor tweak to configure.ac to include further pre-req headers; I'll post the modified version of your patch that finally gets things working for me. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: Roman Bogorodskiy <bogorodskiy@gmail.com> FreeBSD (and maybe other BSDs) have different member names in struct ifreq when compared to Linux, such as: - uses ifr_data instead of ifr_newname for setting interface names - uses ifr_index instead of ifr_ifindex for interface index Also, add a check for SIOCGIFHWADDR for virNetDevValidateConfig(). Use AF_LOCAL if AF_PACKET is not available. Signed-off-by: Eric Blake <eblake@redhat.com> --- Found it: Roman's patch was including <sys/ioctl.h> instead of the intended <sys/socket.h> in configure.ac. Pushing this: configure.ac | 9 +++++++++ src/util/virnetdev.c | 25 ++++++++++++++++++------- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/configure.ac b/configure.ac index 23c24d2..229b3f7 100644 --- a/configure.ac +++ b/configure.ac @@ -2363,6 +2363,15 @@ AM_CONDITIONAL([HAVE_LIBNL], [test "$have_libnl" = "yes"]) AC_SUBST([LIBNL_CFLAGS]) AC_SUBST([LIBNL_LIBS]) +# Check for Linux vs. BSD ifreq members +AC_CHECK_MEMBERS([struct ifreq.ifr_newname, + struct ifreq.ifr_ifindex, + struct ifreq.ifr_index], + [], [], + [#include <sys/socket.h> + #include <net/if.h> + ]) + # Only COPYING.LIB is under version control, yet COPYING # is included as part of the distribution tarball. # Copy one to the other, but only if this is a srcdir-build. diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 7ffaac1..0a3e17d 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2007-2012 Red Hat, Inc. + * Copyright (C) 2007-2013 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -38,7 +38,10 @@ #ifdef __linux__ # include <linux/sockios.h> # include <linux/if_vlan.h> -#elif !defined(AF_PACKET) +# define VIR_NETDEV_FAMILY AF_PACKET +#elif defined(HAVE_STRUCT_IFREQ) && defined(AF_LOCAL) +# define VIR_NETDEV_FAMILY AF_LOCAL +#else # undef HAVE_STRUCT_IFREQ #endif @@ -81,7 +84,7 @@ static int virNetDevSetupControlFull(const char *ifname, static int virNetDevSetupControl(const char *ifname, struct ifreq *ifr) { - return virNetDevSetupControlFull(ifname, ifr, AF_PACKET, SOCK_DGRAM); + return virNetDevSetupControlFull(ifname, ifr, VIR_NETDEV_FAMILY, SOCK_DGRAM); } #endif @@ -478,12 +481,16 @@ int virNetDevSetName(const char* ifname, const char *newifname) if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0) return -1; +# ifdef HAVE_STRUCT_IFREQ_IFR_NEWNAME if (virStrcpyStatic(ifr.ifr_newname, newifname) == NULL) { virReportSystemError(ERANGE, _("Network interface name '%s' is too long"), newifname); goto cleanup; } +# else + ifr.ifr_data = (caddr_t)newifname; +# endif if (ioctl(fd, SIOCSIFNAME, &ifr)) { virReportSystemError(errno, @@ -630,7 +637,7 @@ int virNetDevGetIndex(const char *ifname, int *ifindex) { int ret = -1; struct ifreq ifreq; - int fd = socket(PF_PACKET, SOCK_DGRAM, 0); + int fd = socket(VIR_NETDEV_FAMILY, SOCK_DGRAM, 0); if (fd < 0) { virReportSystemError(errno, "%s", @@ -654,7 +661,11 @@ int virNetDevGetIndex(const char *ifname, int *ifindex) goto cleanup; } +# ifdef HAVE_STRUCT_IFREQ_IFR_INDEX + *ifindex = ifreq.ifr_index; +# else *ifindex = ifreq.ifr_ifindex; +# endif ret = 0; cleanup: @@ -870,7 +881,7 @@ int virNetDevGetIPv4Address(const char *ifname ATTRIBUTE_UNUSED, * * Returns 1 if the config matches, 0 if the config does not match, or interface does not exist, -1 on error */ -#if defined(HAVE_STRUCT_IFREQ) +#if defined(SIOCGIFHWADDR) && defined(HAVE_STRUCT_IFREQ) int virNetDevValidateConfig(const char *ifname, const virMacAddrPtr macaddr, int ifindex) { @@ -924,7 +935,7 @@ int virNetDevValidateConfig(const char *ifname, VIR_FORCE_CLOSE(fd); return ret; } -#else /* ! HAVE_STRUCT_IFREQ */ +#else int virNetDevValidateConfig(const char *ifname ATTRIBUTE_UNUSED, const virMacAddrPtr macaddr ATTRIBUTE_UNUSED, int ifindex ATTRIBUTE_UNUSED) @@ -933,7 +944,7 @@ int virNetDevValidateConfig(const char *ifname ATTRIBUTE_UNUSED, _("Unable to check interface config on this platform")); return -1; } -#endif /* ! HAVE_STRUCT_IFREQ */ +#endif #ifdef __linux__ -- 1.8.1.4
Eric Blake wrote:
From: Roman Bogorodskiy <bogorodskiy@gmail.com>
FreeBSD (and maybe other BSDs) have different member names in struct ifreq when compared to Linux, such as:
- uses ifr_data instead of ifr_newname for setting interface names - uses ifr_index instead of ifr_ifindex for interface index
Also, add a check for SIOCGIFHWADDR for virNetDevValidateConfig().
Use AF_LOCAL if AF_PACKET is not available.
Signed-off-by: Eric Blake <eblake@redhat.com> ---
Found it: Roman's patch was including <sys/ioctl.h> instead of the intended <sys/socket.h> in configure.ac. Pushing this:
Thanks for catching that!
configure.ac | 9 +++++++++ src/util/virnetdev.c | 25 ++++++++++++++++++------- 2 files changed, 27 insertions(+), 7 deletions(-)
diff --git a/configure.ac b/configure.ac index 23c24d2..229b3f7 100644 --- a/configure.ac +++ b/configure.ac @@ -2363,6 +2363,15 @@ AM_CONDITIONAL([HAVE_LIBNL], [test "$have_libnl" = "yes"]) AC_SUBST([LIBNL_CFLAGS]) AC_SUBST([LIBNL_LIBS])
+# Check for Linux vs. BSD ifreq members +AC_CHECK_MEMBERS([struct ifreq.ifr_newname, + struct ifreq.ifr_ifindex, + struct ifreq.ifr_index], + [], [], + [#include <sys/socket.h> + #include <net/if.h> + ]) + # Only COPYING.LIB is under version control, yet COPYING # is included as part of the distribution tarball. # Copy one to the other, but only if this is a srcdir-build. diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 7ffaac1..0a3e17d 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2007-2012 Red Hat, Inc. + * Copyright (C) 2007-2013 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -38,7 +38,10 @@ #ifdef __linux__ # include <linux/sockios.h> # include <linux/if_vlan.h> -#elif !defined(AF_PACKET) +# define VIR_NETDEV_FAMILY AF_PACKET +#elif defined(HAVE_STRUCT_IFREQ) && defined(AF_LOCAL) +# define VIR_NETDEV_FAMILY AF_LOCAL +#else # undef HAVE_STRUCT_IFREQ #endif
@@ -81,7 +84,7 @@ static int virNetDevSetupControlFull(const char *ifname, static int virNetDevSetupControl(const char *ifname, struct ifreq *ifr) { - return virNetDevSetupControlFull(ifname, ifr, AF_PACKET, SOCK_DGRAM); + return virNetDevSetupControlFull(ifname, ifr, VIR_NETDEV_FAMILY, SOCK_DGRAM); } #endif
@@ -478,12 +481,16 @@ int virNetDevSetName(const char* ifname, const char *newifname) if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0) return -1;
+# ifdef HAVE_STRUCT_IFREQ_IFR_NEWNAME if (virStrcpyStatic(ifr.ifr_newname, newifname) == NULL) { virReportSystemError(ERANGE, _("Network interface name '%s' is too long"), newifname); goto cleanup; } +# else + ifr.ifr_data = (caddr_t)newifname; +# endif
if (ioctl(fd, SIOCSIFNAME, &ifr)) { virReportSystemError(errno, @@ -630,7 +637,7 @@ int virNetDevGetIndex(const char *ifname, int *ifindex) { int ret = -1; struct ifreq ifreq; - int fd = socket(PF_PACKET, SOCK_DGRAM, 0); + int fd = socket(VIR_NETDEV_FAMILY, SOCK_DGRAM, 0);
if (fd < 0) { virReportSystemError(errno, "%s", @@ -654,7 +661,11 @@ int virNetDevGetIndex(const char *ifname, int *ifindex) goto cleanup; }
+# ifdef HAVE_STRUCT_IFREQ_IFR_INDEX + *ifindex = ifreq.ifr_index; +# else *ifindex = ifreq.ifr_ifindex; +# endif ret = 0;
cleanup: @@ -870,7 +881,7 @@ int virNetDevGetIPv4Address(const char *ifname ATTRIBUTE_UNUSED, * * Returns 1 if the config matches, 0 if the config does not match, or interface does not exist, -1 on error */ -#if defined(HAVE_STRUCT_IFREQ) +#if defined(SIOCGIFHWADDR) && defined(HAVE_STRUCT_IFREQ) int virNetDevValidateConfig(const char *ifname, const virMacAddrPtr macaddr, int ifindex) { @@ -924,7 +935,7 @@ int virNetDevValidateConfig(const char *ifname, VIR_FORCE_CLOSE(fd); return ret; } -#else /* ! HAVE_STRUCT_IFREQ */ +#else int virNetDevValidateConfig(const char *ifname ATTRIBUTE_UNUSED, const virMacAddrPtr macaddr ATTRIBUTE_UNUSED, int ifindex ATTRIBUTE_UNUSED) @@ -933,7 +944,7 @@ int virNetDevValidateConfig(const char *ifname ATTRIBUTE_UNUSED, _("Unable to check interface config on this platform")); return -1; } -#endif /* ! HAVE_STRUCT_IFREQ */ +#endif
#ifdef __linux__ -- 1.8.1.4
Roman Bogorodskiy
participants (2)
-
Eric Blake -
Roman Bogorodskiy