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