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

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

Attachment: signature.asc
Description: OpenPGP digital signature


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