Re: [libvirt] [PATCH] portability: handle ifreq differences in virnetdev
Eric Blake wrote:
From: Roman Bogorodskiy bogorods...@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 ebl...@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
pgpR2Dzfv9lEZ.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] portability: handle ifreq differences in virnetdev
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
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] portability: handle ifreq differences in virnetdev
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
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
