On Thu, Mar 03, 2016 at 06:11:43PM +0100, Michal Privoznik wrote:
The implementation is pretty straightforward. Moreover, because of the nature of things, gethostbyname_r and gethostbyname2_r can be implemented at the same time too. Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> --- config-post.h | 24 ++++ src/Makefile.am | 59 ++++++++ src/util/virfile.c | 2 +- tests/Makefile.am | 2 +- tools/Makefile.am | 8 +- tools/nss/libvirt_nss.c | 341 ++++++++++++++++++++++++++++++++++++++++++++- tools/nss/libvirt_nss.h | 14 +- tools/nss/libvirt_nss.syms | 4 +- 8 files changed, 446 insertions(+), 8 deletions(-) diff --git a/config-post.h b/config-post.h index 8367200..2398d3d 100644 --- a/config-post.h +++ b/config-post.h @@ -43,3 +43,27 @@ # undef WITH_YAJL # undef WITH_YAJL2 #endif + +/* + * With the NSS module it's the same story as virt-login-shell. See the + * explanation above. + */ +#ifdef LIBVIRT_NSS +# undef HAVE_LIBDEVMAPPER_H +# undef HAVE_LIBNL +# undef HAVE_LIBNL3 +# undef HAVE_LIBSASL2 +# undef WITH_CAPNG +# undef WITH_CURL +# undef WITH_DTRACE_PROBES +# undef WITH_GNUTLS +# undef WITH_GNUTLS_GCRYPT +# undef WITH_MACVTAP +# undef WITH_NUMACTL +# undef WITH_SASL +# undef WITH_SSH2 +# undef WITH_VIRTUALPORT +# undef WITH_SECDRIVER_SELINUX +# undef WITH_SECDRIVER_APPARMOR +# undef WITH_CAPNG +#endif /* LIBVIRT_NSS */ diff --git a/src/Makefile.am b/src/Makefile.am index 2ba397f..8ea5422 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -2946,6 +2946,65 @@ endif WITH_LIBVIRTD endif WITH_SECDRIVER_APPARMOR EXTRA_DIST += $(SECURITY_DRIVER_APPARMOR_HELPER_SOURCES) +if WITH_NSS
Remove this condition, otherwise you won't be able to build tests without nss.
+noinst_LTLIBRARIES += libvirt-nss.la + +libvirt_nss_la_SOURCES = \ + util/viralloc.c \ + util/viralloc.h \ + util/virbitmap.c \ + util/virbitmap.h \ + util/virbuffer.c \ + util/virbuffer.h \ + util/vircommand.c \ + util/vircommand.h \ + util/virerror.c \ + util/virerror.h \ + util/virfile.c \ + util/virfile.h \ + util/virjson.c \ + util/virjson.h \ + util/virkmod.c \ + util/virkmod.h \ + util/virlease.c \ + util/virlease.h \ + util/virlog.c \ + util/virlog.h \ + util/virobject.c \ + util/virobject.h \ + util/virpidfile.c \ + util/virpidfile.h \ + util/virprocess.c \ + util/virprocess.h \ + util/virsocketaddr.c \ + util/virsocketaddr.h \ + util/virstring.c \ + util/virstring.h \ + util/virthread.c \ + util/virthread.h \ + util/virthreadjob.c \ + util/virthreadjob.h \ + util/virtime.c \ + util/virtime.h \ + util/virutil.c \ + util/virutil.h \ + $(NULL) + +libvirt_nss_la_CFLAGS = \ + -DLIBVIRT_NSS \ + $(AM_CFLAGS) \ + $(YAJL_CFLAGS) \ + $(NULL) +libvirt_nss_la_LDFLAGS = \ + $(AM_LDFLAGS) \ + $(NULL) + +libvirt_nss_la_LIBADD = \ + $(YAJL_LIBS) \ + $(NULL) +endif WITH_NSS +
^^ of course, here too :)
+
install-data-local: install-init install-systemd
if WITH_LIBVIRTD
$(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/lockd"
diff --git a/src/util/virfile.c b/src/util/virfile.c
index f45e18f..0fae0f5 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -554,7 +554,7 @@ int virFileUpdatePerm(const char *path,
#if defined(__linux__) && HAVE_DECL_LO_FLAGS_AUTOCLEAR && \
- !defined(LIBVIRT_SETUID_RPC_CLIENT)
+ !defined(LIBVIRT_SETUID_RPC_CLIENT) && !defined(LIBVIRT_NSS)
# if HAVE_DECL_LOOP_CTL_GET_FREE
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 90981dc..55e8432 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -65,7 +65,7 @@ GNULIB_LIBS = \
../gnulib/lib/libgnu.la
LDADDS = \
- $(WARN_CFLAGS) \
+ $(WARN_CFLAGS) \
$(NO_INDIRECT_LDFLAGS) \
$(PROBES_O) \
$(GNULIB_LIBS) \
If I were to nit-pick I would say this has nothing to do here... But I'm not like that... I won't even mention anything related to that...
diff --git a/tools/Makefile.am b/tools/Makefile.am index 9754e42..8312111 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -433,7 +433,13 @@ nss_libnss_libvirt_impl_la_CFLAGS = \ $(AM_CFLAGS) \ $(WARN_CFLAGS) \ $(PIE_CFLAGS) \ - $(COVERAGE_CFLAGS) + $(COVERAGE_CFLAGS) \ + $(LIBXML_CFLAGS)
Couldn't we use more #undef's in config-post to get rid of this? It's not used anywhere in the module implementation.
+
+nss_libnss_libvirt_impl_la_LIBADD = \
+ $(LIBXML_LIBS) \
+ ../gnulib/lib/libgnu.la \
+ ../src/libvirt-nss.la
nss_libnss_libvirt_la_SOURCES =
nss_libnss_libvirt_la_LDFLAGS = \
diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c
index 461d8ca..192d8e3 100644
--- a/tools/nss/libvirt_nss.c
+++ b/tools/nss/libvirt_nss.c
@@ -29,8 +29,343 @@
#include "libvirt_nss.h"
-int
-blah(int c)
+#include <resolv.h>
+#include <sys/types.h>
+#include <dirent.h>
+#include <arpa/inet.h>
+
+#include "virlease.h"
+#include "viralloc.h"
+#include "virfile.h"
+#include "virerror.h"
+#include "virstring.h"
+#include "virsocketaddr.h"
+#include "configmake.h"
+
+#if 0
+# define ERROR(...) \
+do { \
+ char ebuf[1024]; \
+ fprintf(stderr, "ERROR %s:%d : ", __FUNCTION__, __LINE__); \
+ fprintf(stderr, __VA_ARGS__); \
+ fprintf(stderr, " : %s\n", virStrerror(errno, ebuf, sizeof(ebuf))); \
+ fprintf(stderr, "\n"); \
+} while (0)
+
+# define DEBUG(...) \
+do { \
+ fprintf(stderr, "DEBUG %s:%d : ", __FUNCTION__, __LINE__); \
+ fprintf(stderr, __VA_ARGS__); \
+ fprintf(stderr, "\n"); \
+} while (0)
+#else
+# define ERROR(...) do { } while (0)
+# define DEBUG(...) do { } while (0)
+#endif
+
+#define LEASEDIR LOCALSTATEDIR "/lib/libvirt/dnsmasq/"
+
+#define ALIGN4(l) (((l) + 3) & ~3)
+#define ALIGN8(l) (((l) + 7) & ~7)
+
+#if __SIZEOF_POINTER__ == 8
+# define ALIGN(l) ALIGN8(l)
+#elif __SIZEOF_POINTER__ == 4
+# define ALIGN(l) ALIGN4(l)
+#else
+# error "Wut? Pointers are neither 4 nor 8 bytes long?"
+#endif
+
This is kinda gross. You can avoid needing to do alignment but proper casts (as I will point out later on so this compiles on clang as well). But even if you need to go with this, why not do: #define ALIGN(x) (((x) + __SIZEOF_POINTER__ - 1) & ~(__SIZEOF_POINTER__ - 1)) so that you don't have to create the macro for each size and then select between them?
+#define FAMILY_ADDRESS_SIZE(family) ((family) == AF_INET6 ? 16 : 4)
+
+typedef struct {
+ unsigned char addr[16];
+ int af;
+} leaseAddress;
+
+/**
+ * findLease:
+ * @name: domain name to lookup
+ * @af: address family
+ * @address: all the addresses found for selected @af
+ * @naddress: number of elements in @address array
+ * @found: whether @name has been found
+ * @errnop: errno pointer
+ *
+ * Lookup @name in libvirt's IP database, parse it and store all
+ * addresses found in @address array. Callers can choose which
+ * address family (@af) should be returned. Currently only
+ * AF_INET (IPv4) and AF_INET6 (IPv6) are supported. As a corner
+ * case, AF_UNSPEC may be passed to @af in which case no address
+ * filtering is done and addresses from both families are
+ * returned.
+ *
+ * Returns -1 on error
+ * 0 on success
+ */
+static int
+findLease(const char *name,
+ int af,
+ leaseAddress **address,
+ size_t *naddress,
+ bool *found,
+ int *errnop)
{
- return c;
+ DIR *dir = NULL;
+ int ret = -1;
+ const char *leaseDir = LEASEDIR;
+ struct dirent *entry;
+ virJSONValuePtr leases_array = NULL;
+ char *server_duid = NULL;
+ ssize_t i, nleases;
+ leaseAddress *tmpAddress = NULL;
+ size_t ntmpAddress = 0;
+
+ *address = NULL;
+ *naddress = 0;
+ *found = false;
+
+ if (af != AF_UNSPEC && af != AF_INET && af != AF_INET6) {
+ errno = EAFNOSUPPORT;
+ goto cleanup;
+ }
+
+
+ if (!(dir = opendir(leaseDir))) {
+ ERROR("Failed to open dir '%s'", leaseDir);
+ goto cleanup;
+ }
+
+ if (!(leases_array = virJSONValueNewArray())) {
+ ERROR("Failed to create json array");
+ goto cleanup;
+ }
+
+ DEBUG("Dir: %s", leaseDir);
+ while ((ret = virDirRead(dir, &entry, leaseDir)) > 0) {
+ char *path;
+
+ if (entry->d_name[0] == '.')
+ continue;
+
+ if (!virFileHasSuffix(entry->d_name, ".status"))
+ continue;
+
+ if (!(path = virFileBuildPath(leaseDir, entry->d_name, NULL)))
+ goto cleanup;
+
+ DEBUG("Processing %s", path);
+
+ if (virLeaseReadCustomLeaseFile(leases_array, path, NULL, &server_duid) < 0) {
+ ERROR("Unable to parse %s", path);
+ VIR_FREE(path);
+ goto cleanup;
+ }
+
+ VIR_FREE(path);
+ VIR_FREE(server_duid);
I would rather change virLeaseReadCustomLeaseFile() so that it treats server_duid == NULL gracefully so that we don't need to do such things here. But no harm done so far ;)
+ }
+
+ closedir(dir);
+ dir = NULL;
+
+ nleases = virJSONValueArraySize(leases_array);
+ DEBUG("Read %zd leases", nleases);
+
+ for (i = 0; i < nleases; i++) {
+ virJSONValue const *lease;
+ const char *lease_name;
+ virSocketAddr sa;
+ const char *ipAddr;
+ int family;
+
+ lease = virJSONValueArrayGet(leases_array, i);
+
+ if (!lease) {
+ /* This should never happen (TM) */
+ ERROR("Unable to get element %zd of %zd", i, nleases);
+ goto cleanup;
+ }
+
+ lease_name = virJSONValueObjectGetString(lease, "hostname");
+
+ if (STRNEQ_NULLABLE(name, lease_name))
+ continue;
+
+ DEBUG("Found record for %s", lease_name);
+ *found = true;
+
+ if (!(ipAddr = virJSONValueObjectGetString(lease, "ip-address"))) {
+ ERROR("ip-address field missing for %s", name);
+ goto cleanup;
+ }
+
+ DEBUG("IP address: %s", ipAddr);
+
+ if (virSocketAddrParse(&sa, ipAddr, AF_UNSPEC) < 0) {
+ ERROR("Unable to parse %s", ipAddr);
+ goto cleanup;
+ }
+
+ family = VIR_SOCKET_ADDR_FAMILY(&sa);
+ if (af != AF_UNSPEC && af != family) {
+ DEBUG("Skipping address which family is %d, %d requested", family, af);
+ continue;
+ }
+
+ if (VIR_REALLOC_N_QUIET(tmpAddress, ntmpAddress + 1) < 0) {
+ ERROR("Out of memory");
+ goto cleanup;
+ }
+
+ tmpAddress[ntmpAddress].af = family;
+ memcpy(tmpAddress[ntmpAddress].addr,
+ (family == AF_INET ?
+ (void *) &sa.data.inet4.sin_addr.s_addr :
+ (void *) &sa.data.inet6.sin6_addr.s6_addr),
+ FAMILY_ADDRESS_SIZE(family));
+ ntmpAddress++;
+ }
+
+ *address = tmpAddress;
+ *naddress = ntmpAddress;
+ tmpAddress = NULL;
+ ntmpAddress = 0;
+
+ ret = 0;
+
+ cleanup:
+ *errnop = errno;
+ VIR_FREE(tmpAddress);
+ virJSONValueFree(leases_array);
+ if (dir)
+ closedir(dir);
+ return ret;
+}
+
+
+enum nss_status
+_nss_libvirt_gethostbyname_r(const char *name, struct hostent *result,
+ char *buffer, size_t buflen, int *errnop,
+ int *herrnop)
+{
+ int af = ((_res.options & RES_USE_INET6) ? AF_INET6 : AF_INET);
+
+ return _nss_libvirt_gethostbyname3_r(name, af, result, buffer, buflen,
+ errnop, herrnop, NULL, NULL);
+}
+
+enum nss_status
+_nss_libvirt_gethostbyname2_r(const char *name, int af, struct hostent *result,
+ char *buffer, size_t buflen, int *errnop,
+ int *herrnop)
+{
+ return _nss_libvirt_gethostbyname3_r(name, af, result, buffer, buflen,
+ errnop, herrnop, NULL, NULL);
+}
+
+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)
+{
+ enum nss_status ret = NSS_STATUS_UNAVAIL;
+ char *r_name, *r_aliases, *r_addr, *r_addr_list;
Most of these point to memory that we don't address by bytes, so they could be changed to void pointers to indicate that. Or to whatever they point to so you can use the pointer arithmetic and not that many casts.
+ leaseAddress *addr = NULL;
+ size_t naddr, i;
+ bool found = false;
+ size_t nameLen, need, idx;
+ int alen;
+ int r;
+
+ if ((r = findLease(name, af, &addr, &naddr, &found, errnop)) < 0) {
+ /* Error occurred. Return immediately. */
+ if (*errnop == EAGAIN) {
+ *herrnop = TRY_AGAIN;
+ return NSS_STATUS_TRYAGAIN;
+ } else {
+ *herrnop = NO_RECOVERY;
+ return NSS_STATUS_UNAVAIL;
+ }
+ }
+
+ if (!found) {
+ /* NOT found */
+ *errnop = ESRCH;
+ *herrnop = HOST_NOT_FOUND;
+ return NSS_STATUS_NOTFOUND;
+ } else if (!naddr) {
+ /* Found, but no data */
+ *errnop = ENXIO;
+ *herrnop = NO_DATA;
+ return NSS_STATUS_UNAVAIL;
+ }
+
+ /* Found and have data */
+
+ af = addr[0].af;
+ alen = FAMILY_ADDRESS_SIZE(af);
+
So here you base every address' size on the family of the first returned one. But if there are IPv6 addresses, this function returns random stuff (tried). I started fixing this and then I realised there were no links in the commits on how these functions should behave, so I can only guess how they are supposed to work in case multiple addresses are returned. I'm guessing the biggest one is used and then it just works because of the ::<ipv4 address here> ipv6 feature or something like that, right? I have some changes for this stored, you might like some of them or not. They are not split by commit, but see them on my public github and feel free to incorporate some of it if you like it. At least some pointers are handled nicely but I haven't rewritten all of the uses. If you say you only support IPv4 for now, I think we can accept that for the time being and deal with IPv6 later. But if you want to write the whole thing, be my guest. Martin
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list