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

Re: [libvirt] [PATCH] nss: FreeBSD support



On 25.03.2016 08:43, Roman Bogorodskiy wrote:
>  * tools/nss/libvirt_nss.[ch]: add BSD-comptabile wrappers and
>    register via the nss_module_register() interface
>  * m4/virt-nss.m4: add checks if we're building NSS for FreeBSD
>  * tools/Makefile.am: handle target library name differences, as
>    Linux needs libnss_libvirt.so.2 and FreeBSD needs
>    nss_libvirt.so.1. Also, different syms files have to be used
>    as Linux needs to export all the methods while FreeBSD
>    only needs to have nss_module_register()
>  * libvirt_nss_bsd.syms: FreeBSD syms file
> ---
>  m4/virt-nss.m4                 |  12 +++-
>  tools/Makefile.am              |  16 ++++-
>  tools/nss/libvirt_nss.c        | 140 +++++++++++++++++++++++++++++++++++++++--
>  tools/nss/libvirt_nss.h        |   9 +++
>  tools/nss/libvirt_nss_bsd.syms |   9 +++
>  5 files changed, 179 insertions(+), 7 deletions(-)
>  create mode 100644 tools/nss/libvirt_nss_bsd.syms
> 
> diff --git a/m4/virt-nss.m4 b/m4/virt-nss.m4
> index 3fa4ad3..a8ed8b9 100644
> --- a/m4/virt-nss.m4
> +++ b/m4/virt-nss.m4
> @@ -23,6 +23,7 @@ AC_DEFUN([LIBVIRT_CHECK_NSS],[
>        [enable Name Servie Switch plugin for resolving guest IP addresses])],
>        [], [with_nss_plugin=check])
>  
> +  bsd_nss=no
>    fail=0
>    if test "x$with_nss_plugin" != "xno" ; then
>      AC_CHECK_HEADERS([nss.h], [
> @@ -39,11 +40,20 @@ AC_DEFUN([LIBVIRT_CHECK_NSS],[
>  
>      if test "x$with_nss_plugin" = "xyes" ; then
>        AC_DEFINE_UNQUOTED([NSS], 1, [whether nss plugin is enabled])
> +
> +      AC_CHECK_TYPES([ns_mtab, nss_module_unregister_fn],
> +                     [AC_DEFINE([HAVE_BSD_NSS],
> +                                [1],
> +                                [whether using BSD style NSS])
> +                      bsd_nss=yes
> +                     ],
> +                     [],
> +                     [#include <nsswitch.h>])
>      fi
>    fi
>  
>    AM_CONDITIONAL(WITH_NSS, [test "x$with_nss_plugin" = "xyes"])
> -
> +  AM_CONDITIONAL(WITH_BSD_NSS, [test "x$bsd_nss" = "xyes"])
>  ])
>  
>  AC_DEFUN([LIBVIRT_RESULT_NSS],[
> diff --git a/tools/Makefile.am b/tools/Makefile.am
> index 4320040..6005b8b 100644
> --- a/tools/Makefile.am
> +++ b/tools/Makefile.am
> @@ -417,8 +417,22 @@ CLEANFILES += wireshark/src/plugin.c
>  
>  endif WITH_WIRESHARK_DISSECTOR
>  
> +if WITH_BSD_NSS
> +LIBVIRT_NSS_SYMBOL_FILE = \
> +	$(srcdir)/nss/libvirt_nss_bsd.syms
> +NSS_SO_VER = 1
> +
> +install-exec-hook:
> +	cd $(DESTDIR)$(libdir) && \
> +	  $(LN_S) libnss_libvirt.so.$(NSS_SO_VER) nss_libvirt.so.$(NSS_SO_VER)
> +

Yeah, very similar link exists on my linux machine too (except for the
version at EOL). Maybe one day I'll add the link for Linux too.

> +uninstall-local:
> +	rm $(DESTDIR)$(libdir)/libnss_libvirt.so.$(NSS_SO_VER)
> +else ! WITH_BSD_NSS
>  LIBVIRT_NSS_SYMBOL_FILE = \
>  	$(srcdir)/nss/libvirt_nss.syms
> +NSS_SO_VER = 2
> +endif ! WITH_BSD_NSS
>  
>  LIBVIRT_NSS_SOURCES = \
>  	nss/libvirt_nss.c	\
> @@ -449,7 +463,7 @@ nss_libnss_libvirt_la_LDFLAGS = \
>  	-export-dynamic \
>  	-avoid-version \
>  	-shared \
> -	-shrext .so.2
> +	-shrext .so.$(NSS_SO_VER)
>  
>  nss_libnss_libvirt_la_LIBADD =  \
>  	nss/libnss_libvirt_impl.la
> diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c
> index 218c62a..0ff1348 100644
> --- a/tools/nss/libvirt_nss.c
> +++ b/tools/nss/libvirt_nss.c
> @@ -1,5 +1,4 @@
>  /*
> - * libvirt_nss: Name Service Switch plugin

This is unintended, right?

>   *
>   * The aim is to enable users and applications to translate
>   * domain names into IP addresses. However, this is currently
> @@ -29,11 +28,16 @@
>  
>  #include "libvirt_nss.h"
>  
> +#include <netinet/in.h>
>  #include <resolv.h>
>  #include <sys/types.h>
>  #include <dirent.h>
>  #include <arpa/inet.h>
>  
> +#if defined(HAVE_BSD_NSS)
> +# include <nsswitch.h>
> +#endif
> +
>  #include "virlease.h"
>  #include "viralloc.h"
>  #include "virfile.h"
> @@ -65,7 +69,7 @@ do {                                                            \
>  
>  #define LEASEDIR LOCALSTATEDIR "/lib/libvirt/dnsmasq/"
>  
> -#define ALIGN(x) (((x) + __SIZEOF_POINTER__ - 1) & ~(__SIZEOF_POINTER__ - 1))
> +#define LIBVIRT_ALIGN(x) (((x) + __SIZEOF_POINTER__ - 1) & ~(__SIZEOF_POINTER__ - 1))

Yeah, I've seen the name clash with some header file when trying to
compile on my freebsd machine too.

>  #define FAMILY_ADDRESS_SIZE(family) ((family) == AF_INET6 ? 16 : 4)
>  
>  typedef struct {
> @@ -256,7 +260,7 @@ static inline void *
>  move_and_align(void *buf, size_t len, size_t *idx)
>  {
>      char *buffer = buf;
> -    size_t move = ALIGN(len);
> +    size_t move = LIBVIRT_ALIGN(len);
>  
>      if (!idx)
>          return buffer + move;
> @@ -321,7 +325,7 @@ _nss_libvirt_gethostbyname3_r(const char *name, int af, struct hostent *result,
>       * b) alias
>       * c) addresses
>       * d) NULL stem */
> -    need = ALIGN(nameLen + 1) + naddr * ALIGN(alen) + (naddr + 2) * sizeof(char*);
> +    need = LIBVIRT_ALIGN(nameLen + 1) + naddr * LIBVIRT_ALIGN(alen) + (naddr + 2) * sizeof(char*);
>  
>      if (buflen < need) {
>          *errnop = ENOMEM;
> @@ -383,6 +387,7 @@ _nss_libvirt_gethostbyname3_r(const char *name, int af, struct hostent *result,
>      return ret;
>  }
>  
> +#ifdef HAVE_STRUCT_GAIH_ADDRTUPLE
>  enum nss_status
>  _nss_libvirt_gethostbyname4_r(const char *name, struct gaih_addrtuple **pat,
>                                char *buffer, size_t buflen, int *errnop,
> @@ -426,7 +431,7 @@ _nss_libvirt_gethostbyname4_r(const char *name, struct gaih_addrtuple **pat,
>      /* We need space for:
>       * a) name
>       * b) addresses */
> -    need = ALIGN(nameLen + 1) + naddr * ALIGN(sizeof(struct gaih_addrtuple));
> +    need = LIBVIRT_ALIGN(nameLen + 1) + naddr * LIBVIRT_ALIGN(sizeof(struct gaih_addrtuple));
>  
>      if (buflen < need) {
>          *errnop = ENOMEM;
> @@ -474,3 +479,128 @@ _nss_libvirt_gethostbyname4_r(const char *name, struct gaih_addrtuple **pat,
>   cleanup:
>      return ret;
>  }
> +#endif /* HAVE_STRUCT_GAIH_ADDRTUPLE */

I guess this will not fly. I mean, the symbol is exported so I guess we
need an #else branch providing a dummy implementation, don't we?
Honestly, I don't know. Maybe we don't as long as we don't run
check-symfile whether all symbols are exported I guess we are good. Or
we can just introduce yet another symbol file that would be included
conditionally.

BUT, you forgot to include bit that checks for struct gaih_ddrtuple and
defines HAVE_STRUCT_..?

> +
> +#if defined(HAVE_BSD_NSS)
> +NSS_METHOD_PROTOTYPE(_nss_compat_getaddrinfo);
> +NSS_METHOD_PROTOTYPE(_nss_compat_gethostbyname2_r);
> +
> +ns_mtab methods[] = {
> +    { NSDB_HOSTS, "getaddrinfo", _nss_compat_getaddrinfo, NULL },
> +    { NSDB_HOSTS, "gethostbyname", _nss_compat_gethostbyname2_r, NULL },
> +    { NSDB_HOSTS, "gethostbyname2_r", _nss_compat_gethostbyname2_r, NULL },
> +};
> +
> +static void
> +aiforaf(const char *name, int af, struct addrinfo *pai, struct addrinfo **aip)
> +{
> +    int ret;
> +    struct hostent resolved;
> +    char buf[1024] = { 0 };
> +    int err, herr;
> +    struct addrinfo hints, *res0, *res;
> +    char **addrList;
> +
> +    if ((ret = _nss_libvirt_gethostbyname2_r(name, af, &resolved,
> +                                             buf, sizeof(buf),
> +                                             &err, &herr)) != NS_SUCCESS)
> +        return;
> +
> +    addrList = resolved.h_addr_list;
> +    while (*addrList) {
> +        virSocketAddr sa;
> +        char *ipAddr = NULL;
> +        void *address = *addrList;
> +
> +        memset(&sa, 0, sizeof(sa));
> +        if (resolved.h_addrtype == AF_INET) {
> +            virSocketAddrSetIPv4AddrNetOrder(&sa, *((uint32_t *) address));
> +        } else {
> +            virSocketAddrSetIPv6AddrNetOrder(&sa, address);
> +        }
> +
> +        ipAddr = virSocketAddrFormat(&sa);
> +
> +        hints = *pai;
> +        hints.ai_flags = AI_NUMERICHOST;
> +        hints.ai_family = af;
> +
> +        if (getaddrinfo(ipAddr, NULL, &hints, &res0)) {

Interesting. I don't know how NSS works on *bsd but this function is
called from getaddrinfo() and it is calling getaddrinfo yet again. Is
that okay?

> +            addrList++;
> +            continue;
> +        }
> +
> +        for (res = res0; res; res = res->ai_next)
> +            res->ai_flags = pai->ai_flags;
> +
> +        (*aip)->ai_next = res0;
> +        while ((*aip)->ai_next)
> +           *aip = (*aip)->ai_next;
> +
> +        addrList++;
> +    }
> +}
> +
> +int
> +_nss_compat_getaddrinfo(void *retval, void *mdata ATTRIBUTE_UNUSED, va_list ap)
> +{
> +    struct addrinfo sentinel, *cur, *ai;
> +    const char *name;
> +  

Trailing space.

> +    name  = va_arg(ap, char *);
> +    ai = va_arg(ap, struct addrinfo *);
> +
> +    memset(&sentinel, 0, sizeof(sentinel));
> +    cur = &sentinel;
> +
> +    if ((ai->ai_family == AF_UNSPEC) || (ai->ai_family == AF_INET6))
> +        aiforaf(name, AF_INET6, ai, &cur);
> +    if ((ai->ai_family == AF_UNSPEC) || (ai->ai_family == AF_INET))
> +        aiforaf(name, AF_INET, ai, &cur);
> +
> +    if (sentinel.ai_next == NULL) {
> +        h_errno = HOST_NOT_FOUND;
> +        return NS_NOTFOUND;
> +    }
> +    *((struct addrinfo **)retval) = sentinel.ai_next;
> +
> +    return NS_SUCCESS;
> +}
> +
> +int
> +_nss_compat_gethostbyname2_r(void *retval, void *mdata ATTRIBUTE_UNUSED, va_list ap)
> +{
> +    int ret;
> +
> +    const char *name;
> +    int af;
> +    struct hostent *result;
> +    char *buffer;
> +    size_t buflen;
> +    int *errnop;
> +    int *herrnop;
> +
> +    name = va_arg(ap, const char *);
> +    af = va_arg(ap, int);
> +    result = va_arg(ap, struct hostent *);
> +    buffer = va_arg(ap, char *);
> +    buflen = va_arg(ap, size_t);
> +    errnop = va_arg(ap, int *);
> +    herrnop = va_arg(ap, int *);
> +
> +    ret = _nss_libvirt_gethostbyname2_r(
> +              name, af, result, buffer, buflen, errnop, herrnop);
> +    *(struct hostent **)retval = (ret == NS_SUCCESS) ? result : NULL;
> +
> +    return ret;
> +}
> +
> +ns_mtab*
> +nss_module_register(const char *name ATTRIBUTE_UNUSED, unsigned int *size,
> +                    nss_module_unregister_fn *unregister)
> +{
> +    *size = sizeof(methods) / sizeof(methods[0]);
> +    *unregister = NULL;
> +    return methods;
> +}
> +#endif /* HAVE_BSD_NSS */
> diff --git a/tools/nss/libvirt_nss.h b/tools/nss/libvirt_nss.h
> index 589c1e6..e025e63 100644
> --- a/tools/nss/libvirt_nss.h
> +++ b/tools/nss/libvirt_nss.h
> @@ -45,8 +45,17 @@ enum nss_status
>  _nss_libvirt_gethostbyname3_r(const char *name, int af, struct hostent *result,
>                                char *buffer, size_t buflen, int *errnop,
>                                int *herrnop, int32_t *ttlp, char **canonp);
> +# ifdef HAVE_STRUCT_GAIH_ADDRTUPLE
>  enum nss_status
>  _nss_libvirt_gethostbyname4_r(const char *name, struct gaih_addrtuple **pat,
>                                char *buffer, size_t buflen, int *errnop,
>                                int *herrnop, int32_t *ttlp);
> +# endif /* HAVE_STRUCT_GAIH_ADDRTUPLE */
> +
> +# if defined(HAVE_BSD_NSS)
> +ns_mtab*
> +nss_module_register(const char *name, unsigned int *size,
> +                    nss_module_unregister_fn *unregister);
> +# endif /* HAVE_BSD_NSS */
> +
>  #endif /* __LIBVIRT_NSS_H__ */
> diff --git a/tools/nss/libvirt_nss_bsd.syms b/tools/nss/libvirt_nss_bsd.syms
> new file mode 100644
> index 0000000..7da3926
> --- /dev/null
> +++ b/tools/nss/libvirt_nss_bsd.syms
> @@ -0,0 +1,9 @@
> +#
> +# Officially exported symbols.
> +#
> +
> +{
> +global:
> +    nss_module_register;
> +local: *;
> +};
> 

Michal


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