[PATCH v2 0/7] nfsdcld: add a daemon to track NFSv4 client names on stable storage

[PATCH v2 0/7] nfsdcld: add a daemon to track NFSv4 client names on stable storage

[Jeff Layton]

This patchset is the userspace portion of the knfsd client name tracking
overhaul. See this patch series for an explanation:

    nfsd: overhaul the client name tracking code (RFC)

The main changes from the last set are:

- the daemon has been renamed from clstated to nfsdcld, per Steve D's
  suggestion

- the client names are no longer tracked on a per-server-address basis.
  Chuck Lever pointed out that doing so would break the UCS migration
  model. This version just keeps a single clients table in the main
  database (which also significantly simplifies the code).

- other bugfixes and cleanups

The daemon listens for upcalls on the rpc_pipefs pipe using libevent,
and handles the requests.

The data is stored using a sqlite database. The main reason for this is
that it takes care of most of the fussy details and atomicity concerns
of tracking the information on stable storage.

Surprisingly, there is some controversy over this, but I believe it's
the right design decision. I'm willing to change that however if there
are legitimate technical concerns. In that event, I'd appreciate
constructive suggestions about what should replace it.

For now, the daemon is only suitable for single-host configurations.
My plan is to later extend this to be suitable for clustered
configurations as well.

The code is still a little rough, so be gentle. It also lacks things
like a manpage. I plan to add all that before doing a "formal" patch
submission, but I wanted to get some early review of the overall design
before to spend a lot of time knocking off the rough edges.

Jeff Layton (7):
  autoconf: fix up libevent autoconf test
  nfsdcld: add client tracking daemon stub
  nfsdcld: add autoconf goop for sqlite
  nfsdcld: add routines for a sqlite backend database
  nfsdcld: add remove functionality
  nfsdcld: add check/update functionality
  nfsdcld: add function to remove unreclaimed client records

 aclocal/libevent.m4       |    3 +-
 aclocal/libsqlite3.m4     |   33 ++++
 configure.ac              |   23 +++
 utils/Makefile.am         |    4 +
 utils/idmapd/Makefile.am  |    2 +-
 utils/nfsdcld/Makefile.am |   14 ++
 utils/nfsdcld/nfsdcld.c   |  332 ++++++++++++++++++++++++++++++++++++++
 utils/nfsdcld/sqlite.c    |  388 +++++++++++++++++++++++++++++++++++++++++++++
 utils/nfsdcld/sqlite.h    |   30 ++++
 9 files changed, 827 insertions(+), 2 deletions(-)
 create mode 100644 aclocal/libsqlite3.m4
 create mode 100644 utils/nfsdcld/Makefile.am
 create mode 100644 utils/nfsdcld/nfsdcld.c
 create mode 100644 utils/nfsdcld/sqlite.c
 create mode 100644 utils/nfsdcld/sqlite.h

[PATCH v2 1/7] autoconf: fix up libevent autoconf test

[Jeff Layton]

Have it set LIBEVENT to -levent and use that in the Makefiles instead
of hardcoding it.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 aclocal/libevent.m4      |    3 ++-
 utils/idmapd/Makefile.am |    2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/aclocal/libevent.m4 b/aclocal/libevent.m4
index 3c962b3..b5ac00f 100644
--- a/aclocal/libevent.m4
+++ b/aclocal/libevent.m4
@@ -2,8 +2,9 @@ dnl Checks for libevent
 AC_DEFUN([AC_LIBEVENT], [
 
   dnl Check for libevent, but do not add -levent to LIBS
-  AC_CHECK_LIB([event], [event_dispatch], [libevent=1],
+  AC_CHECK_LIB([event], [event_dispatch], [LIBEVENT=-levent],
                [AC_MSG_ERROR([libevent not found.])])
+  AC_SUBST(LIBEVENT)
 
   AC_CHECK_HEADERS([event.h], ,
                    [AC_MSG_ERROR([libevent headers not found.])])
diff --git a/utils/idmapd/Makefile.am b/utils/idmapd/Makefile.am
index 4328e41..f8578b0 100644
--- a/utils/idmapd/Makefile.am
+++ b/utils/idmapd/Makefile.am
@@ -16,7 +16,7 @@ idmapd_SOURCES = \
 	nfs_idmap.h \
 	queue.h
 
-idmapd_LDADD = -levent -lnfsidmap ../../support/nfs/libnfs.a
+idmapd_LDADD = $(LIBEVENT) -lnfsidmap ../../support/nfs/libnfs.a
 
 MAINTAINERCLEANFILES = Makefile.in
 
-- 
1.7.1

[PATCH v2 2/7] nfsdcld: add client tracking daemon stub

[Jeff Layton]

This program opens and "listens" on the new nfsd/cld rpc_pipefs pipe.
The code here doesn't actually do anything on stable storage yet. That
will be added in a later patch.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 configure.ac              |    1 +
 utils/Makefile.am         |    1 +
 utils/nfsdcld/Makefile.am |   14 +++
 utils/nfsdcld/nfsdcld.c   |  240 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 256 insertions(+), 0 deletions(-)
 create mode 100644 utils/nfsdcld/Makefile.am
 create mode 100644 utils/nfsdcld/nfsdcld.c

diff --git a/configure.ac b/configure.ac
index f101b86..c5b6a0f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -458,6 +458,7 @@ AC_CONFIG_FILES([
 	tools/nfs-iostat/Makefile
 	utils/Makefile
 	utils/blkmapd/Makefile
+	utils/nfsdcld/Makefile
 	utils/exportfs/Makefile
 	utils/gssd/Makefile
 	utils/idmapd/Makefile
diff --git a/utils/Makefile.am b/utils/Makefile.am
index d074b85..f42c7ee 100644
--- a/utils/Makefile.am
+++ b/utils/Makefile.am
@@ -22,6 +22,7 @@ OPTDIRS += mount
 endif
 
 SUBDIRS = \
+	nfsdcld \
 	exportfs \
 	mountd \
 	nfsd \
diff --git a/utils/nfsdcld/Makefile.am b/utils/nfsdcld/Makefile.am
new file mode 100644
index 0000000..ed7ed42
--- /dev/null
+++ b/utils/nfsdcld/Makefile.am
@@ -0,0 +1,14 @@
+## Process this file with automake to produce Makefile.in
+
+#man8_MANS	= nfsdcld.man
+#EXTRA_DIST = $(man8_MANS)
+
+AM_CFLAGS	+= -D_LARGEFILE64_SOURCE
+sbin_PROGRAMS	= nfsdcld
+
+nfsdcld_SOURCES = nfsdcld.c
+
+nfsdcld_LDADD = ../../support/nfs/libnfs.a $(LIBEVENT)
+
+MAINTAINERCLEANFILES = Makefile.in
+
diff --git a/utils/nfsdcld/nfsdcld.c b/utils/nfsdcld/nfsdcld.c
new file mode 100644
index 0000000..d82908f
--- /dev/null
+++ b/utils/nfsdcld/nfsdcld.c
@@ -0,0 +1,240 @@
+/*
+ * nfsdcld.c -- NFSv4 client name tracking daemon
+ *
+ * Copyright (C) 2011  Red Hat, Jeff Layton <jlayton@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301, USA.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif /* HAVE_CONFIG_H */
+
+#include <errno.h>
+#include <event.h>
+#include <stdbool.h>
+#include <getopt.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <linux/nfsd/cld.h>
+
+#include "xlog.h"
+#include "nfslib.h"
+
+#ifndef PIPEFS_DIR
+#define PIPEFS_DIR NFS_STATEDIR "/rpc_pipefs"
+#endif
+
+#define DEFAULT_CLD_PATH	PIPEFS_DIR "/nfsd/cld"
+
+/* private data structures */
+struct cld_client {
+	int			cl_fd;
+	struct event		cl_event;
+	struct cld_msg	cl_msg;
+};
+
+/* global variables */
+static char *pipepath = DEFAULT_CLD_PATH;
+
+static struct option longopts[] =
+{
+	{ "help", 0, NULL, 'h' },
+	{ "foreground", 0, NULL, 'F' },
+	{ "debug", 0, NULL, 'd' },
+	{ "path", 1, NULL, 'p' },
+	{ NULL, 0, 0, 0 },
+};
+
+/* forward declarations */
+static void cldcb(int UNUSED(fd), short which, void *data);
+
+static void
+usage(char *progname)
+{
+	printf("Usage:\n");
+	printf("%s [ -hFdp ]\n", progname);
+}
+
+static int
+cld_pipe_open(struct cld_client *clnt)
+{
+	clnt->cl_fd = open(pipepath, O_RDWR, 0);
+	if (clnt->cl_fd < 0) {
+		xlog(L_ERROR, "%s: unable to open %s: %m", __func__, pipepath);
+		return clnt->cl_fd;
+	}
+
+	event_set(&clnt->cl_event, clnt->cl_fd, EV_READ, cldcb, clnt);
+	event_add(&clnt->cl_event, NULL);
+
+	return clnt->cl_fd;
+}
+
+static void
+cld_pipe_reopen(struct cld_client *clnt)
+{
+	int fd;
+
+	fd = open(pipepath, O_RDWR, 0);
+	if (fd < 0) {
+		xlog_warn("%s: Re-opening of %s failed: %m");
+		return;
+	}
+
+	if ((clnt->cl_event.ev_flags & EVLIST_INIT))
+		event_del(&clnt->cl_event);
+	if (clnt->cl_fd > 0)
+		close(clnt->cl_fd);
+
+	clnt->cl_fd = fd;
+	event_set(&clnt->cl_event, clnt->cl_fd, EV_READ, cldcb, clnt);
+	/* event_add is done by the caller */
+}
+
+static void
+cld_create(struct cld_client *clnt)
+{
+	ssize_t bsize, wsize;
+	struct cld_msg *cmsg = &clnt->cl_msg;
+
+	xlog(D_GENERAL, "%s: create client record", __func__);
+
+	/* FIXME: create client record on storage here */
+
+	/* set up reply */
+	cmsg->cm_status = 0;
+
+	bsize = sizeof(*cmsg);
+
+	wsize = atomicio((void *)write, clnt->cl_fd, cmsg, bsize);
+	if (wsize != bsize) {
+		xlog(L_ERROR, "%s: problem writing to cld pipe (%ld): %m",
+			 __func__, wsize);
+		cld_pipe_reopen(clnt);
+	}
+}
+
+static void
+cldcb(int UNUSED(fd), short which, void *data)
+{
+	ssize_t len;
+	struct cld_client *clnt = data;
+	struct cld_msg *cmsg = &clnt->cl_msg;
+
+	if (which != EV_READ)
+		goto out;
+
+	len = atomicio(read, clnt->cl_fd, cmsg, sizeof(*cmsg));
+	if (len <= 0) {
+		xlog_warn("%s: pipe read failed: %m", __func__);
+		cld_pipe_reopen(clnt);
+		goto out;
+	}
+
+	switch(cmsg->cm_cmd) {
+	case Cld_Create:
+		cld_create(clnt);
+		break;
+	case Cld_Expire:
+	case Cld_NrToReclaim:
+	case Cld_Allow:
+	case Cld_GraceDone:
+		xlog_warn("%s: command %u is not yet implemented", __func__,
+			  cmsg->cm_cmd);
+		break;
+	default:
+		xlog_warn("%s: unrecognized command %u", __func__,
+			  cmsg->cm_cmd);
+		cld_pipe_reopen(clnt);
+	}
+out:
+	event_add(&clnt->cl_event, NULL);
+}
+
+int
+main(int argc, char **argv)
+{
+	char arg;
+	int rc = 0, fd;
+	bool foreground = false;
+	char *progname;
+	struct cld_client clnt;
+
+	memset(&clnt, 0, sizeof(clnt));
+
+	progname = strdup(basename(argv[0]));
+	if (!progname) {
+		fprintf(stderr, "%s: unable to allocate memory.\n", argv[0]);
+		return 1;
+	}
+
+	event_init();
+	xlog_syslog(0);
+	xlog_stderr(1);
+
+	/* process command-line options */
+	while ((arg = getopt_long(argc, argv, "hdFp:", longopts,
+				  NULL)) != EOF) {
+		switch (arg) {
+		case 'd':
+			xlog_config(D_ALL, 1);
+			break;
+		case 'F':
+			foreground = true;
+			break;
+		case 'p':
+			pipepath = optarg;
+			break;
+		default:
+			usage(progname);
+			return 0;
+		}
+	}
+
+
+	xlog_open(progname);
+	if (!foreground) {
+		xlog_syslog(1);
+		xlog_stderr(0);
+		rc = daemon(0, 0);
+		if (rc) {
+			xlog(L_ERROR, "Unable to daemonize: %m");
+			goto out;
+		}
+	}
+
+	/* set up storage db */
+
+	/* set up event handler */
+	fd = cld_pipe_open(&clnt);
+	if (fd < 0) {
+		rc = fd;
+		goto out;
+	}
+
+	rc = event_dispatch();
+	if (rc < 0)
+		xlog(L_ERROR, "%s: event_dispatch failed: %m", __func__);
+
+	close(clnt.cl_fd);
+out:
+	free(progname);
+	return rc;
+}
-- 
1.7.1

[PATCH v2 3/7] nfsdcld: add autoconf goop for sqlite

[Jeff Layton]

Mostly cribbed from Chuck Lever's new-statd rewrite a few years ago...

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 aclocal/libsqlite3.m4 |   33 +++++++++++++++++++++++++++++++++
 configure.ac          |   22 ++++++++++++++++++++++
 utils/Makefile.am     |    5 ++++-
 3 files changed, 59 insertions(+), 1 deletions(-)
 create mode 100644 aclocal/libsqlite3.m4

diff --git a/aclocal/libsqlite3.m4 b/aclocal/libsqlite3.m4
new file mode 100644
index 0000000..73d1e46
--- /dev/null
+++ b/aclocal/libsqlite3.m4
@@ -0,0 +1,33 @@
+dnl Checks for matching sqlite3 header and library, and
+dnl sufficient sqlite3 version.
+dnl
+AC_DEFUN([AC_SQLITE3_VERS], [
+  AC_CHECK_HEADERS([sqlite3.h], ,)
+
+  dnl look for the library; do not add to LIBS if found
+  AC_CHECK_LIB([sqlite3], [sqlite3_libversion_number], [LIBSQLITE=-lsqlite3], ,)
+  AC_SUBST(LIBSQLITE)
+
+  AC_MSG_CHECKING(for suitable sqlite3 version)
+
+  AC_CACHE_VAL([libsqlite3_cv_is_recent],
+   [
+    saved_LIBS="$LIBS"
+    LIBS=-lsqlite3
+    AC_TRY_RUN([
+	#include <stdio.h>
+	#include <sqlite3.h>
+	int main()
+	{
+		int vers = sqlite3_libversion_number();
+
+		return vers != SQLITE_VERSION_NUMBER ||
+			vers < 3003000;
+	}
+       ], [libsqlite3_cv_is_recent=yes], [libsqlite3_cv_is_recent=no],
+       [libsqlite3_cv_is_recent=unknown])
+    LIBS="$saved_LIBS"])
+
+  AC_MSG_RESULT($libsqlite3_cv_is_recent)
+  AM_CONDITIONAL(CONFIG_SQLITE3, [test "$libsqlite3_cv_is_recent" = "yes"])
+])dnl
diff --git a/configure.ac b/configure.ac
index c5b6a0f..bf018a2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -186,6 +186,12 @@ else
 	AM_CONDITIONAL(MOUNT_CONFIG, [test "$enable_mount" = "yes"])
 fi
 
+AC_ARG_ENABLE(nfsdcld,
+	[AC_HELP_STRING([--enable-nfsdcld],
+			[Create nfsdcld NFSv4 clientid tracking daemon. <:@default=yes@:>@])],
+	enable_nfsdcld=$enableval,
+	enable_nfsdcld="maybe")
+
 dnl Check for TI-RPC library and headers
 AC_LIBTIRPC
 
@@ -259,6 +265,9 @@ if test "$enable_nfsv4" = yes; then
   dnl check for the keyutils libraries and headers
   AC_KEYUTILS
 
+  dnl Check for sqlite3
+  AC_SQLITE3_VERS
+
   dnl librpcsecgss already has a dependency on libgssapi,
   dnl but we need to make sure we get the right version
   if test "$enable_gss" = yes; then
@@ -328,6 +337,19 @@ fi
 dnl Check for IPv6 support
 AC_IPV6
 
+dnl Decide what to do with nfsdcld based on sqlite3 test result
+if test "$enable_nfsv4" = "yes" -a "$libsqlite3_cv_is_recent" != "yes" ; then
+	if test "$enable_nfsdcld" = "yes"; then
+		AC_MSG_ERROR([nfsdcld requires sqlite3])
+	elif test "$enable_nfsdcld" != "no"; then
+		AC_MSG_WARN([nfsdcld requires sqlite3, autodisabling it])
+		enable_nfsdcld="no"
+	fi
+fi
+
+dnl Now set CONFIG_NFSDCLD properly
+AM_CONDITIONAL(CONFIG_NFSDCLD, [test "$enable_nfsdcld" != "no" ])
+
 dnl *************************************************************
 dnl Check for headers
 dnl *************************************************************
diff --git a/utils/Makefile.am b/utils/Makefile.am
index f42c7ee..5df7ca7 100644
--- a/utils/Makefile.am
+++ b/utils/Makefile.am
@@ -21,8 +21,11 @@ if CONFIG_MOUNT
 OPTDIRS += mount
 endif
 
+if CONFIG_NFSDCLD
+OPTDIRS += nfsdcld
+endif
+
 SUBDIRS = \
-	nfsdcld \
 	exportfs \
 	mountd \
 	nfsd \
-- 
1.7.1

[PATCH v2 4/7] nfsdcld: add routines for a sqlite backend database

[Jeff Layton]

Rather than roll our own "storage engine", use sqlite instead. It fits
the bill nicely as it does:

- durable on-disk storage
- the ability to constrain record uniqueness
- a facility for collating and searching the host records

...it does add a build dependency to nfs-utils, but almost all modern
distros provide those packages.

The current incarnation of this code dynamically links against a
provided sqlite library, but we could also consider including their
single-file "amalgamation" to reduce dependencies (though with all
the caveats that that entails).

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 utils/nfsdcld/Makefile.am |    4 +-
 utils/nfsdcld/nfsdcld.c   |   16 ++-
 utils/nfsdcld/sqlite.c    |  256 +++++++++++++++++++++++++++++++++++++++++++++
 utils/nfsdcld/sqlite.h    |   27 +++++
 4 files changed, 297 insertions(+), 6 deletions(-)
 create mode 100644 utils/nfsdcld/sqlite.c
 create mode 100644 utils/nfsdcld/sqlite.h

diff --git a/utils/nfsdcld/Makefile.am b/utils/nfsdcld/Makefile.am
index ed7ed42..8e4f2ab 100644
--- a/utils/nfsdcld/Makefile.am
+++ b/utils/nfsdcld/Makefile.am
@@ -6,9 +6,9 @@
 AM_CFLAGS	+= -D_LARGEFILE64_SOURCE
 sbin_PROGRAMS	= nfsdcld
 
-nfsdcld_SOURCES = nfsdcld.c
+nfsdcld_SOURCES = nfsdcld.c sqlite.c
 
-nfsdcld_LDADD = ../../support/nfs/libnfs.a $(LIBEVENT)
+nfsdcld_LDADD = ../../support/nfs/libnfs.a $(LIBEVENT) $(LIBSQLITE)
 
 MAINTAINERCLEANFILES = Makefile.in
 
diff --git a/utils/nfsdcld/nfsdcld.c b/utils/nfsdcld/nfsdcld.c
index d82908f..de2e932 100644
--- a/utils/nfsdcld/nfsdcld.c
+++ b/utils/nfsdcld/nfsdcld.c
@@ -36,6 +36,7 @@
 
 #include "xlog.h"
 #include "nfslib.h"
+#include "sqlite.h"
 
 #ifndef PIPEFS_DIR
 #define PIPEFS_DIR NFS_STATEDIR "/rpc_pipefs"
@@ -111,18 +112,20 @@ cld_pipe_reopen(struct cld_client *clnt)
 static void
 cld_create(struct cld_client *clnt)
 {
+	int ret;
 	ssize_t bsize, wsize;
 	struct cld_msg *cmsg = &clnt->cl_msg;
 
-	xlog(D_GENERAL, "%s: create client record", __func__);
+	xlog(D_GENERAL, "%s: create client record.", __func__);
 
-	/* FIXME: create client record on storage here */
+	ret = sqlite_insert_client(cmsg->cm_u.cm_name.cn_id,
+				   cmsg->cm_u.cm_name.cn_len);
 
-	/* set up reply */
-	cmsg->cm_status = 0;
+	cmsg->cm_status = ret ? -EREMOTEIO : ret;
 
 	bsize = sizeof(*cmsg);
 
+	xlog(D_GENERAL, "Doing downcall with status %d", cmsg->cm_status);
 	wsize = atomicio((void *)write, clnt->cl_fd, cmsg, bsize);
 	if (wsize != bsize) {
 		xlog(L_ERROR, "%s: problem writing to cld pipe (%ld): %m",
@@ -221,6 +224,11 @@ main(int argc, char **argv)
 	}
 
 	/* set up storage db */
+	rc = sqlite_maindb_init();
+	if (rc) {
+		xlog(L_ERROR, "Failed to open main database: %d", rc);
+		goto out;
+	}
 
 	/* set up event handler */
 	fd = cld_pipe_open(&clnt);
diff --git a/utils/nfsdcld/sqlite.c b/utils/nfsdcld/sqlite.c
new file mode 100644
index 0000000..2534ba5
--- /dev/null
+++ b/utils/nfsdcld/sqlite.c
@@ -0,0 +1,256 @@
+/*
+ * Copyright (C) 2011  Red Hat, Jeff Layton <jlayton@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301, USA.
+ */
+
+/*
+ * Explanation:
+ *
+ * This file contains the code to manage the sqlite backend database for the
+ * clstated upcall daemon.
+ *
+ * The main database is called main.sqlite and contains the following tables:
+ *
+ * parameters: simple key/value pairs for storing database info
+ *
+ * clients: one column containing a BLOB with the as sent by the client
+ * 	    and a timestamp (in epoch seconds) of when the record was
+ * 	    established
+ *
+ * FIXME: should we also record the fsid being accessed?
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif /* HAVE_CONFIG_H */
+
+#include <errno.h>
+#include <event.h>
+#include <stdbool.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <sqlite3.h>
+#include <linux/limits.h>
+
+#include "xlog.h"
+
+#define CLD_SQLITE_SCHEMA_VERSION 1
+
+#ifndef CLD_SQLITE_TOPDIR
+#define CLD_SQLITE_TOPDIR NFS_STATEDIR "/nfsdcld"
+#endif
+
+/* in milliseconds */
+#define CLD_SQLITE_BUSY_TIMEOUT 10000
+
+/* private data structures */
+
+/* global variables */
+
+/* top level DB directory */
+static char *sqlite_topdir = CLD_SQLITE_TOPDIR;
+
+/* reusable pathname and sql command buffer */
+static char buf[PATH_MAX];
+
+/* global database handle */
+static sqlite3 *dbh;
+
+/* forward declarations */
+
+void
+sqlite_set_topdir(char *topdir)
+{
+	sqlite_topdir = topdir;
+}
+
+/* make a directory, ignoring EEXIST errors unless it's not a directory */
+static int
+mkdir_if_not_exist(char *dirname)
+{
+	int ret;
+	struct stat statbuf;
+
+	ret = mkdir(dirname, S_IRWXU);
+	if (ret && errno != EEXIST)
+		return -errno;
+
+	ret = stat(dirname, &statbuf);
+	if (ret)
+		return -errno;
+
+	if (!S_ISDIR(statbuf.st_mode))
+		ret = -ENOTDIR;
+
+	return ret;
+}
+
+/*
+ * Open the "main" database, and attempt to initialize it by creating the
+ * parameters table and inserting the schema version into it. Ignore any errors
+ * from that, and then attempt to select the version out of it again. If the
+ * version appears wrong, then assume that the DB is corrupt or has been
+ * upgraded, and return an error. If all of that works, then attempt to create
+ * the "clients" table.
+ */
+int
+sqlite_maindb_init(void)
+{
+	int ret;
+	char *err = NULL;
+	sqlite3_stmt *stmt = NULL;
+
+	ret = mkdir_if_not_exist(sqlite_topdir);
+	if (ret)
+		return ret;
+
+	ret = snprintf(buf, PATH_MAX - 1, "%s/main.sqlite", sqlite_topdir);
+	if (ret < 0)
+		return ret;
+
+	buf[PATH_MAX - 1] = '\0';
+
+	ret = sqlite3_open(buf, &dbh);
+	if (ret != SQLITE_OK) {
+		xlog(D_GENERAL, "Unable to open main database: %d", ret);
+		return ret;
+	}
+
+	ret = sqlite3_busy_timeout(dbh, CLD_SQLITE_BUSY_TIMEOUT);
+	if (ret != SQLITE_OK) {
+		xlog(D_GENERAL, "Unable to set sqlite busy timeout: %d", ret);
+		goto out_err;
+	}
+
+	/* Try to create table */
+	ret = sqlite3_exec(dbh, "CREATE TABLE IF NOT EXISTS parameters "
+				"(key TEXT PRIMARY KEY, value TEXT);",
+				NULL, NULL, &err);
+	if (ret != SQLITE_OK) {
+		xlog(D_GENERAL, "Unable to create parameter table: %d", ret);
+		goto out_err;
+	}
+
+	/* insert version into table -- ignore error if it fails */
+	ret = snprintf(buf, sizeof(buf),
+		       "INSERT OR IGNORE INTO parameters values (\"version\", "
+		       "\"%d\");", CLD_SQLITE_SCHEMA_VERSION);
+	if (ret < 0) {
+		goto out_err;
+	} else if ((size_t)ret >= sizeof(buf)) {
+		ret = -EINVAL;
+		goto out_err;
+	}
+
+	ret = sqlite3_exec(dbh, (const char *)buf, NULL, NULL, &err);
+	if (ret != SQLITE_OK) {
+		xlog(D_GENERAL, "Unable to insert into parameter table: %d",
+				ret);
+		goto out_err;
+	}
+
+	ret = sqlite3_prepare_v2(dbh,
+		"SELECT value FROM parameters WHERE key == \"version\";",
+		 -1, &stmt, NULL);
+	if (ret != SQLITE_OK) {
+		xlog(D_GENERAL, "Unable to prepare select statement: %d", ret);
+		goto out_err;
+	}
+
+	/* check schema version */
+	ret = sqlite3_step(stmt);
+	if (ret != SQLITE_ROW) {
+		xlog(D_GENERAL, "Select statement execution failed: %s",
+				sqlite3_errmsg(dbh));
+		goto out_err;
+	}
+
+	/* process SELECT result */
+	ret = sqlite3_column_int(stmt, 0);
+	if (ret != CLD_SQLITE_SCHEMA_VERSION) {
+		xlog(L_ERROR, "Unsupported database schema version! "
+			"Expected %d, got %d.",
+			CLD_SQLITE_SCHEMA_VERSION, ret);
+		ret = -EINVAL;
+		goto out_err;
+	}
+
+	/* now create the "clients" table */
+	ret = sqlite3_exec(dbh, "CREATE TABLE IF NOT EXISTS clients "
+				"(id BLOB PRIMARY KEY, time INTEGER);",
+				NULL, NULL, &err);
+	if (ret != SQLITE_OK) {
+		xlog(L_ERROR, "Unable to create clients table: %s", err);
+		goto out_err;
+	}
+
+	sqlite3_free(err);
+	sqlite3_finalize(stmt);
+	return 0;
+
+out_err:
+	if (err) {
+		xlog(L_ERROR, "sqlite error: %s", err);
+		sqlite3_free(err);
+	}
+	sqlite3_finalize(stmt);
+	sqlite3_close(dbh);
+	return ret;
+}
+
+/*
+ * Create a client record
+ *
+ * Returns a non-zero sqlite error code, or SQLITE_OK (aka 0)
+ */
+int
+sqlite_insert_client(const unsigned char *clname, const size_t namelen)
+{
+	int ret;
+	sqlite3_stmt *stmt = NULL;
+
+	ret = sqlite3_prepare_v2(dbh, "INSERT OR REPLACE INTO clients VALUES "
+				      "(?, strftime('%s', 'now'));", -1,
+					&stmt, NULL);
+	if (ret != SQLITE_OK) {
+		xlog(D_GENERAL, "Insert statement prepare failed: %s",
+			sqlite3_errmsg(dbh));
+		return ret;
+	}
+
+	ret = sqlite3_bind_blob(stmt, 1, (const void *)clname, namelen,
+				SQLITE_STATIC);
+	if (ret != SQLITE_OK) {
+		xlog(D_GENERAL, "Bind blob failed: %d", sqlite3_errmsg(dbh));
+		goto out_err;
+	}
+
+	ret = sqlite3_step(stmt);
+	if (ret == SQLITE_DONE)
+		ret = SQLITE_OK;
+	else
+		xlog(D_GENERAL, "Unexpected return code from insert: %s",
+				sqlite3_errmsg(dbh));
+
+out_err:
+	xlog(D_GENERAL, "%s: returning %d", __func__, ret);
+	sqlite3_finalize(stmt);
+	return ret;
+}
diff --git a/utils/nfsdcld/sqlite.h b/utils/nfsdcld/sqlite.h
new file mode 100644
index 0000000..4b99729
--- /dev/null
+++ b/utils/nfsdcld/sqlite.h
@@ -0,0 +1,27 @@
+/*
+ * Copyright (C) 2011  Red Hat, Jeff Layton <jlayton@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301, USA.
+ */
+
+#ifndef _SQLITE_H_
+#define _SQLITE_H_
+
+void sqlite_set_topdir(char *topdir);
+int sqlite_maindb_init(void);
+int sqlite_insert_client(const unsigned char *clname, const size_t namelen);
+
+#endif /* _SQLITE_H */
-- 
1.7.1

[PATCH v2 5/7] nfsdcld: add remove functionality

[Jeff Layton]

Allow the kernel to ask for removal of a client record.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 utils/nfsdcld/nfsdcld.c |   28 ++++++++++++++++++++++++++++
 utils/nfsdcld/sqlite.c  |   35 +++++++++++++++++++++++++++++++++++
 utils/nfsdcld/sqlite.h  |    1 +
 3 files changed, 64 insertions(+), 0 deletions(-)

diff --git a/utils/nfsdcld/nfsdcld.c b/utils/nfsdcld/nfsdcld.c
index de2e932..0252e78 100644
--- a/utils/nfsdcld/nfsdcld.c
+++ b/utils/nfsdcld/nfsdcld.c
@@ -135,6 +135,32 @@ cld_create(struct cld_client *clnt)
 }
 
 static void
+cld_remove(struct cld_client *clnt)
+{
+	int ret;
+	ssize_t bsize, wsize;
+	struct cld_msg *cmsg = &clnt->cl_msg;
+
+	xlog(D_GENERAL, "%s: remove client record.", __func__);
+
+	ret = sqlite_remove_client(cmsg->cm_u.cm_name.cn_id,
+				   cmsg->cm_u.cm_name.cn_len);
+
+	/* set up reply */
+	cmsg->cm_status = ret ? -EREMOTEIO : ret;
+
+	bsize = sizeof(*cmsg);
+
+	xlog(D_GENERAL, "Doing downcall with status %d", cmsg->cm_status);
+	wsize = atomicio((void *)write, clnt->cl_fd, cmsg, bsize);
+	if (wsize != bsize) {
+		xlog(L_ERROR, "%s: problem writing to cld pipe (%ld): %m",
+			 __func__, wsize);
+		cld_pipe_reopen(clnt);
+	}
+}
+
+static void
 cldcb(int UNUSED(fd), short which, void *data)
 {
 	ssize_t len;
@@ -156,6 +182,8 @@ cldcb(int UNUSED(fd), short which, void *data)
 		cld_create(clnt);
 		break;
 	case Cld_Expire:
+		cld_remove(clnt);
+		break;
 	case Cld_NrToReclaim:
 	case Cld_Allow:
 	case Cld_GraceDone:
diff --git a/utils/nfsdcld/sqlite.c b/utils/nfsdcld/sqlite.c
index 2534ba5..5986dca 100644
--- a/utils/nfsdcld/sqlite.c
+++ b/utils/nfsdcld/sqlite.c
@@ -254,3 +254,38 @@ out_err:
 	sqlite3_finalize(stmt);
 	return ret;
 }
+
+/* Remove a client record */
+int
+sqlite_remove_client(const unsigned char *clname, const size_t namelen)
+{
+	int ret;
+	sqlite3_stmt *stmt = NULL;
+
+	ret = sqlite3_prepare_v2(dbh, "DELETE FROM clients WHERE id==?", -1,
+				 &stmt, NULL);
+	if (ret != SQLITE_OK) {
+		xlog(D_GENERAL, "Statement prepare failed: %s",
+				sqlite3_errmsg(dbh));
+		goto out_err;
+	}
+
+	ret = sqlite3_bind_blob(stmt, 1, (const void *)clname, namelen,
+				SQLITE_STATIC);
+	if (ret != SQLITE_OK) {
+		xlog(D_GENERAL, "Bind blob failed: %s", sqlite3_errmsg(dbh));
+		goto out_err;
+	}
+
+	ret = sqlite3_step(stmt);
+	if (ret == SQLITE_DONE)
+		ret = SQLITE_OK;
+	else
+		xlog(D_GENERAL, "Unexpected return code from delete: %s",
+				sqlite3_errmsg(dbh));
+
+out_err:
+	xlog(D_GENERAL, "%s: returning %d", __func__, ret);
+	sqlite3_finalize(stmt);
+	return ret;
+}
diff --git a/utils/nfsdcld/sqlite.h b/utils/nfsdcld/sqlite.h
index 4b99729..157c127 100644
--- a/utils/nfsdcld/sqlite.h
+++ b/utils/nfsdcld/sqlite.h
@@ -23,5 +23,6 @@
 void sqlite_set_topdir(char *topdir);
 int sqlite_maindb_init(void);
 int sqlite_insert_client(const unsigned char *clname, const size_t namelen);
+int sqlite_remove_client(const unsigned char *clname, const size_t namelen);
 
 #endif /* _SQLITE_H */
-- 
1.7.1

[PATCH v2 6/7] nfsdcld: add check/update functionality

[Jeff Layton]

Add functions to check whether a client is allowed to reclaim, and
update its timestamp in the DB if so. We do this with an UPDATE OR
FAIL SQL statement. If the update fails for any reason (like, say
the record doesn't exist), then the kernel can disallow the reclaim.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 utils/nfsdcld/nfsdcld.c |   30 +++++++++++++++++++-
 utils/nfsdcld/sqlite.c  |   69 +++++++++++++++++++++++++++++++++++++++++++++++
 utils/nfsdcld/sqlite.h  |    1 +
 3 files changed, 99 insertions(+), 1 deletions(-)

diff --git a/utils/nfsdcld/nfsdcld.c b/utils/nfsdcld/nfsdcld.c
index 0252e78..2322ab7 100644
--- a/utils/nfsdcld/nfsdcld.c
+++ b/utils/nfsdcld/nfsdcld.c
@@ -161,6 +161,32 @@ cld_remove(struct cld_client *clnt)
 }
 
 static void
+cld_check(struct cld_client *clnt)
+{
+	int ret;
+	ssize_t bsize, wsize;
+	struct cld_msg *cmsg = &clnt->cl_msg;
+
+	xlog(D_GENERAL, "%s: check client record", __func__);
+
+	ret = sqlite_check_client(cmsg->cm_u.cm_name.cn_id,
+				  cmsg->cm_u.cm_name.cn_len);
+
+	/* set up reply */
+	cmsg->cm_status = ret ? -EACCES : ret;
+
+	bsize = sizeof(*cmsg);
+
+	xlog(D_GENERAL, "Doing downcall with status %d", cmsg->cm_status);
+	wsize = atomicio((void *)write, clnt->cl_fd, cmsg, bsize);
+	if (wsize != bsize) {
+		xlog(L_ERROR, "%s: problem writing to cld pipe (%ld): %m",
+			 __func__, wsize);
+		cld_pipe_reopen(clnt);
+	}
+}
+
+static void
 cldcb(int UNUSED(fd), short which, void *data)
 {
 	ssize_t len;
@@ -184,8 +210,10 @@ cldcb(int UNUSED(fd), short which, void *data)
 	case Cld_Expire:
 		cld_remove(clnt);
 		break;
-	case Cld_NrToReclaim:
 	case Cld_Allow:
+		cld_check(clnt);
+		break;
+	case Cld_NrToReclaim:
 	case Cld_GraceDone:
 		xlog_warn("%s: command %u is not yet implemented", __func__,
 			  cmsg->cm_cmd);
diff --git a/utils/nfsdcld/sqlite.c b/utils/nfsdcld/sqlite.c
index 5986dca..63d5363 100644
--- a/utils/nfsdcld/sqlite.c
+++ b/utils/nfsdcld/sqlite.c
@@ -289,3 +289,72 @@ out_err:
 	sqlite3_finalize(stmt);
 	return ret;
 }
+
+/*
+ * Update a client's timestamp. Return an error if the update fails. On error,
+ * the kernel can infer that reclaim should be denied.
+ */
+int
+sqlite_check_client(const unsigned char *clname, const size_t namelen)
+{
+	int ret;
+	sqlite3_stmt *stmt = NULL;
+
+	ret = sqlite3_prepare_v2(dbh, "SELECT count(*) FROM clients WHERE "
+				      "id==?", -1, &stmt, NULL);
+	if (ret != SQLITE_OK) {
+		xlog(D_GENERAL, "Unable to prepare update statement: %s",
+				sqlite3_errmsg(dbh));
+		goto out_err;
+	}
+
+	ret = sqlite3_bind_blob(stmt, 1, (const void *)clname, namelen,
+				SQLITE_STATIC);
+	if (ret != SQLITE_OK) {
+		xlog(D_GENERAL, "Bind blob failed: %s", sqlite3_errmsg(dbh));
+		goto out_err;
+	}
+
+	ret = sqlite3_step(stmt);
+	if (ret != SQLITE_ROW) {
+		xlog(D_GENERAL, "Unexpected return code from select: %d", ret);
+		goto out_err;
+	}
+
+	ret = sqlite3_column_int(stmt, 0);
+	xlog(D_GENERAL, "Select returned %d rows", ret);
+	if (ret != 1) {
+		ret = -EACCES;
+		goto out_err;
+	}
+
+	sqlite3_finalize(stmt);
+	stmt = NULL;
+	ret = sqlite3_prepare_v2(dbh, "UPDATE OR FAIL clients SET "
+				      "time=strftime('%s', 'now') WHERE id==?",
+				 -1, &stmt, NULL);
+	if (ret != SQLITE_OK) {
+		xlog(D_GENERAL, "Unable to prepare update statement: %s",
+				sqlite3_errmsg(dbh));
+		goto out_err;
+	}
+
+	ret = sqlite3_bind_blob(stmt, 1, (const void *)clname, namelen,
+				SQLITE_STATIC);
+	if (ret != SQLITE_OK) {
+		xlog(D_GENERAL, "Bind blob failed: %s", sqlite3_errmsg(dbh));
+		goto out_err;
+	}
+
+	ret = sqlite3_step(stmt);
+	if (ret == SQLITE_DONE)
+		ret = SQLITE_OK;
+	else
+		xlog(D_GENERAL, "Unexpected return code from update: %s",
+				sqlite3_errmsg(dbh));
+
+out_err:
+	xlog(D_GENERAL, "%s: returning %d", __func__, ret);
+	sqlite3_finalize(stmt);
+	return ret;
+}
diff --git a/utils/nfsdcld/sqlite.h b/utils/nfsdcld/sqlite.h
index 157c127..613ae97 100644
--- a/utils/nfsdcld/sqlite.h
+++ b/utils/nfsdcld/sqlite.h
@@ -24,5 +24,6 @@ void sqlite_set_topdir(char *topdir);
 int sqlite_maindb_init(void);
 int sqlite_insert_client(const unsigned char *clname, const size_t namelen);
 int sqlite_remove_client(const unsigned char *clname, const size_t namelen);
+int sqlite_check_client(const unsigned char *clname, const size_t namelen);
 
 #endif /* _SQLITE_H */
-- 
1.7.1

[PATCH v2 7/7] nfsdcld: add function to remove unreclaimed client records

[Jeff Layton]

This should remove any client record that has a timestamp prior to
the given time.

Eventually, this call will need to be made cluster aware when this is
run in a clustered configuration. For now, this is only suitable for
single-host configurations.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 utils/nfsdcld/nfsdcld.c |   30 +++++++++++++++++++++++++++++-
 utils/nfsdcld/sqlite.c  |   28 ++++++++++++++++++++++++++++
 utils/nfsdcld/sqlite.h  |    1 +
 3 files changed, 58 insertions(+), 1 deletions(-)

diff --git a/utils/nfsdcld/nfsdcld.c b/utils/nfsdcld/nfsdcld.c
index 2322ab7..6fabbe7 100644
--- a/utils/nfsdcld/nfsdcld.c
+++ b/utils/nfsdcld/nfsdcld.c
@@ -187,6 +187,32 @@ cld_check(struct cld_client *clnt)
 }
 
 static void
+cld_gracedone(struct cld_client *clnt)
+{
+	int ret;
+	ssize_t bsize, wsize;
+	struct cld_msg *cmsg = &clnt->cl_msg;
+
+	xlog(D_GENERAL, "%s: grace done. cm_gracetime=%ld", __func__,
+			cmsg->cm_u.cm_gracetime);
+
+	ret = sqlite_remove_unreclaimed(cmsg->cm_u.cm_gracetime);
+
+	/* set up reply: downcall with 0 status */
+	cmsg->cm_status = ret ? -EREMOTEIO : ret;
+
+	bsize = sizeof(*cmsg);
+
+	xlog(D_GENERAL, "Doing downcall with status %d", cmsg->cm_status);
+	wsize = atomicio((void *)write, clnt->cl_fd, cmsg, bsize);
+	if (wsize != bsize) {
+		xlog(L_ERROR, "%s: problem writing to cld pipe (%ld): %m",
+			 __func__, wsize);
+		cld_pipe_reopen(clnt);
+	}
+}
+
+static void
 cldcb(int UNUSED(fd), short which, void *data)
 {
 	ssize_t len;
@@ -213,8 +239,10 @@ cldcb(int UNUSED(fd), short which, void *data)
 	case Cld_Allow:
 		cld_check(clnt);
 		break;
-	case Cld_NrToReclaim:
 	case Cld_GraceDone:
+		cld_gracedone(clnt);
+		break;
+	case Cld_NrToReclaim:
 		xlog_warn("%s: command %u is not yet implemented", __func__,
 			  cmsg->cm_cmd);
 		break;
diff --git a/utils/nfsdcld/sqlite.c b/utils/nfsdcld/sqlite.c
index 63d5363..be1bb71 100644
--- a/utils/nfsdcld/sqlite.c
+++ b/utils/nfsdcld/sqlite.c
@@ -38,6 +38,7 @@
 #include "config.h"
 #endif /* HAVE_CONFIG_H */
 
+#include <dirent.h>
 #include <errno.h>
 #include <event.h>
 #include <stdbool.h>
@@ -358,3 +359,30 @@ out_err:
 	sqlite3_finalize(stmt);
 	return ret;
 }
+
+/*
+ * remove any client records that were not reclaimed since grace_start.
+ */
+int
+sqlite_remove_unreclaimed(time_t grace_start)
+{
+	int ret;
+	char *err = NULL;
+
+	ret = snprintf(buf, sizeof(buf), "DELETE FROM clients WHERE time < %ld",
+			grace_start);
+	if (ret < 0) {
+		return ret;
+	} else if ((size_t)ret >= sizeof(buf)) {
+		ret = -EINVAL;
+		return ret;
+	}
+
+	ret = sqlite3_exec(dbh, buf, NULL, NULL, &err);
+	if (ret != SQLITE_OK)
+		xlog(D_GENERAL, "Delete failed: %s", err);
+
+	xlog(D_GENERAL, "%s: returning %d", __func__, ret);
+	sqlite3_free(err);
+	return ret;
+}
diff --git a/utils/nfsdcld/sqlite.h b/utils/nfsdcld/sqlite.h
index 613ae97..b0f3f85 100644
--- a/utils/nfsdcld/sqlite.h
+++ b/utils/nfsdcld/sqlite.h
@@ -25,5 +25,6 @@ int sqlite_maindb_init(void);
 int sqlite_insert_client(const unsigned char *clname, const size_t namelen);
 int sqlite_remove_client(const unsigned char *clname, const size_t namelen);
 int sqlite_check_client(const unsigned char *clname, const size_t namelen);
+int sqlite_remove_unreclaimed(const time_t grace_start);
 
 #endif /* _SQLITE_H */
-- 
1.7.1

Re: [PATCH v2 3/7] nfsdcld: add autoconf goop for sqlite

[Jeff Layton]

On Thu, 15 Dec 2011 13:43:57 -0500
Jeff Layton <jlayton@redhat.com> wrote:

> Mostly cribbed from Chuck Lever's new-statd rewrite a few years ago...
> 

Sending this to Bruce and Steve, but cc'ing the list in case other
people have thoughts...

I have a couple of questions about configure/build behavior when the
dependencies aren't present on the build system. The patch below
autodisables the building of nfsdcld when sqlite isn't there or isn't
functional, etc.

This is probably fine initially, but eventually I'd like to see the
old state tracking code go away. At that point, this daemon will be
required for NFSv4 serving.

Some questions:

1) does anyone have a problem with deprecating the old state tracking
code? I'd prefer we not maintain it in perpetuity, but maybe there's an
argument for keeping it around?

2) assuming we do want to eventually deprecate the old code. When
should we plan to do this. Assuming that this goes into 3.3 and we add
sufficient printk warnings when the old tracker is used, the earliest
we could remove it is 3.5. Is that too aggressive?

3) again, assuming that we want to deprecate the old code eventually,
does it make sense to autodisable the daemon when the build deps aren't
sufficient for it? With it be better to make the build error out when
--enable-nfsv4 is "yes" and not bother with a new --enable option for
this daemon?

I probably will also need to add a autoconf test for inotify too.
Having an answer to these questions may save me from a lot of complex
autoconf hacking....

Thanks,

> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  aclocal/libsqlite3.m4 |   33 +++++++++++++++++++++++++++++++++
>  configure.ac          |   22 ++++++++++++++++++++++
>  utils/Makefile.am     |    5 ++++-
>  3 files changed, 59 insertions(+), 1 deletions(-)
>  create mode 100644 aclocal/libsqlite3.m4
> 
> diff --git a/aclocal/libsqlite3.m4 b/aclocal/libsqlite3.m4
> new file mode 100644
> index 0000000..73d1e46
> --- /dev/null
> +++ b/aclocal/libsqlite3.m4
> @@ -0,0 +1,33 @@
> +dnl Checks for matching sqlite3 header and library, and
> +dnl sufficient sqlite3 version.
> +dnl
> +AC_DEFUN([AC_SQLITE3_VERS], [
> +  AC_CHECK_HEADERS([sqlite3.h], ,)
> +
> +  dnl look for the library; do not add to LIBS if found
> +  AC_CHECK_LIB([sqlite3], [sqlite3_libversion_number], [LIBSQLITE=-lsqlite3], ,)
> +  AC_SUBST(LIBSQLITE)
> +
> +  AC_MSG_CHECKING(for suitable sqlite3 version)
> +
> +  AC_CACHE_VAL([libsqlite3_cv_is_recent],
> +   [
> +    saved_LIBS="$LIBS"
> +    LIBS=-lsqlite3
> +    AC_TRY_RUN([
> +	#include <stdio.h>
> +	#include <sqlite3.h>
> +	int main()
> +	{
> +		int vers = sqlite3_libversion_number();
> +
> +		return vers != SQLITE_VERSION_NUMBER ||
> +			vers < 3003000;
> +	}
> +       ], [libsqlite3_cv_is_recent=yes], [libsqlite3_cv_is_recent=no],
> +       [libsqlite3_cv_is_recent=unknown])
> +    LIBS="$saved_LIBS"])
> +
> +  AC_MSG_RESULT($libsqlite3_cv_is_recent)
> +  AM_CONDITIONAL(CONFIG_SQLITE3, [test "$libsqlite3_cv_is_recent" = "yes"])
> +])dnl
> diff --git a/configure.ac b/configure.ac
> index c5b6a0f..bf018a2 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -186,6 +186,12 @@ else
>  	AM_CONDITIONAL(MOUNT_CONFIG, [test "$enable_mount" = "yes"])
>  fi
>  
> +AC_ARG_ENABLE(nfsdcld,
> +	[AC_HELP_STRING([--enable-nfsdcld],
> +			[Create nfsdcld NFSv4 clientid tracking daemon. <:@default=yes@:>@])],
> +	enable_nfsdcld=$enableval,
> +	enable_nfsdcld="maybe")
> +
>  dnl Check for TI-RPC library and headers
>  AC_LIBTIRPC
>  
> @@ -259,6 +265,9 @@ if test "$enable_nfsv4" = yes; then
>    dnl check for the keyutils libraries and headers
>    AC_KEYUTILS
>  
> +  dnl Check for sqlite3
> +  AC_SQLITE3_VERS
> +
>    dnl librpcsecgss already has a dependency on libgssapi,
>    dnl but we need to make sure we get the right version
>    if test "$enable_gss" = yes; then
> @@ -328,6 +337,19 @@ fi
>  dnl Check for IPv6 support
>  AC_IPV6
>  
> +dnl Decide what to do with nfsdcld based on sqlite3 test result
> +if test "$enable_nfsv4" = "yes" -a "$libsqlite3_cv_is_recent" != "yes" ; then
> +	if test "$enable_nfsdcld" = "yes"; then
> +		AC_MSG_ERROR([nfsdcld requires sqlite3])
> +	elif test "$enable_nfsdcld" != "no"; then
> +		AC_MSG_WARN([nfsdcld requires sqlite3, autodisabling it])
> +		enable_nfsdcld="no"
> +	fi
> +fi
> +
> +dnl Now set CONFIG_NFSDCLD properly
> +AM_CONDITIONAL(CONFIG_NFSDCLD, [test "$enable_nfsdcld" != "no" ])
> +
>  dnl *************************************************************
>  dnl Check for headers
>  dnl *************************************************************
> diff --git a/utils/Makefile.am b/utils/Makefile.am
> index f42c7ee..5df7ca7 100644
> --- a/utils/Makefile.am
> +++ b/utils/Makefile.am
> @@ -21,8 +21,11 @@ if CONFIG_MOUNT
>  OPTDIRS += mount
>  endif
>  
> +if CONFIG_NFSDCLD
> +OPTDIRS += nfsdcld
> +endif
> +
>  SUBDIRS = \
> -	nfsdcld \
>  	exportfs \
>  	mountd \
>  	nfsd \


-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH v2 3/7] nfsdcld: add autoconf goop for sqlite

[J. Bruce Fields]

On Tue, Dec 20, 2011 at 02:17:49PM -0500, Jeff Layton wrote:
> On Thu, 15 Dec 2011 13:43:57 -0500
> Jeff Layton <jlayton@redhat.com> wrote:
> 
> > Mostly cribbed from Chuck Lever's new-statd rewrite a few years ago...
> > 
> 
> Sending this to Bruce and Steve, but cc'ing the list in case other
> people have thoughts...
> 
> I have a couple of questions about configure/build behavior when the
> dependencies aren't present on the build system. The patch below
> autodisables the building of nfsdcld when sqlite isn't there or isn't
> functional, etc.
> 
> This is probably fine initially, but eventually I'd like to see the
> old state tracking code go away. At that point, this daemon will be
> required for NFSv4 serving.
> 
> Some questions:
> 
> 1) does anyone have a problem with deprecating the old state tracking
> code? I'd prefer we not maintain it in perpetuity, but maybe there's an
> argument for keeping it around?

Just the usual: the kernel community depends for some of its testing on
people being willing to upgrade to the latest kernel.  So we try to
ensure they can just drop in a new kernel without breaking a random
subsystem that (as non-NFS specialists) they don't ordinarily have to
think about....

> 2) assuming we do want to eventually deprecate the old code. When
> should we plan to do this. Assuming that this goes into 3.3 and we add
> sufficient printk warnings when the old tracker is used, the earliest
> we could remove it is 3.5. Is that too aggressive?

That sounds a little aggressive.  It may depend on how much trouble we
find it is to keep it up to date.

Then again I guess it depends how obnoxiously we fail in the case the
new daemon isn't running.

> 3) again, assuming that we want to deprecate the old code eventually,
> does it make sense to autodisable the daemon when the build deps aren't
> sufficient for it? With it be better to make the build error out when
> --enable-nfsv4 is "yes" and not bother with a new --enable option for
> this daemon?

Could be.  Steved gets to maintain this, so I guess it's his call....

--b.

> 
> I probably will also need to add a autoconf test for inotify too.
> Having an answer to these questions may save me from a lot of complex
> autoconf hacking....
> 
> Thanks,
> 
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> >  aclocal/libsqlite3.m4 |   33 +++++++++++++++++++++++++++++++++
> >  configure.ac          |   22 ++++++++++++++++++++++
> >  utils/Makefile.am     |    5 ++++-
> >  3 files changed, 59 insertions(+), 1 deletions(-)
> >  create mode 100644 aclocal/libsqlite3.m4
> > 
> > diff --git a/aclocal/libsqlite3.m4 b/aclocal/libsqlite3.m4
> > new file mode 100644
> > index 0000000..73d1e46
> > --- /dev/null
> > +++ b/aclocal/libsqlite3.m4
> > @@ -0,0 +1,33 @@
> > +dnl Checks for matching sqlite3 header and library, and
> > +dnl sufficient sqlite3 version.
> > +dnl
> > +AC_DEFUN([AC_SQLITE3_VERS], [
> > +  AC_CHECK_HEADERS([sqlite3.h], ,)
> > +
> > +  dnl look for the library; do not add to LIBS if found
> > +  AC_CHECK_LIB([sqlite3], [sqlite3_libversion_number], [LIBSQLITE=-lsqlite3], ,)
> > +  AC_SUBST(LIBSQLITE)
> > +
> > +  AC_MSG_CHECKING(for suitable sqlite3 version)
> > +
> > +  AC_CACHE_VAL([libsqlite3_cv_is_recent],
> > +   [
> > +    saved_LIBS="$LIBS"
> > +    LIBS=-lsqlite3
> > +    AC_TRY_RUN([
> > +	#include <stdio.h>
> > +	#include <sqlite3.h>
> > +	int main()
> > +	{
> > +		int vers = sqlite3_libversion_number();
> > +
> > +		return vers != SQLITE_VERSION_NUMBER ||
> > +			vers < 3003000;
> > +	}
> > +       ], [libsqlite3_cv_is_recent=yes], [libsqlite3_cv_is_recent=no],
> > +       [libsqlite3_cv_is_recent=unknown])
> > +    LIBS="$saved_LIBS"])
> > +
> > +  AC_MSG_RESULT($libsqlite3_cv_is_recent)
> > +  AM_CONDITIONAL(CONFIG_SQLITE3, [test "$libsqlite3_cv_is_recent" = "yes"])
> > +])dnl
> > diff --git a/configure.ac b/configure.ac
> > index c5b6a0f..bf018a2 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -186,6 +186,12 @@ else
> >  	AM_CONDITIONAL(MOUNT_CONFIG, [test "$enable_mount" = "yes"])
> >  fi
> >  
> > +AC_ARG_ENABLE(nfsdcld,
> > +	[AC_HELP_STRING([--enable-nfsdcld],
> > +			[Create nfsdcld NFSv4 clientid tracking daemon. <:@default=yes@:>@])],
> > +	enable_nfsdcld=$enableval,
> > +	enable_nfsdcld="maybe")
> > +
> >  dnl Check for TI-RPC library and headers
> >  AC_LIBTIRPC
> >  
> > @@ -259,6 +265,9 @@ if test "$enable_nfsv4" = yes; then
> >    dnl check for the keyutils libraries and headers
> >    AC_KEYUTILS
> >  
> > +  dnl Check for sqlite3
> > +  AC_SQLITE3_VERS
> > +
> >    dnl librpcsecgss already has a dependency on libgssapi,
> >    dnl but we need to make sure we get the right version
> >    if test "$enable_gss" = yes; then
> > @@ -328,6 +337,19 @@ fi
> >  dnl Check for IPv6 support
> >  AC_IPV6
> >  
> > +dnl Decide what to do with nfsdcld based on sqlite3 test result
> > +if test "$enable_nfsv4" = "yes" -a "$libsqlite3_cv_is_recent" != "yes" ; then
> > +	if test "$enable_nfsdcld" = "yes"; then
> > +		AC_MSG_ERROR([nfsdcld requires sqlite3])
> > +	elif test "$enable_nfsdcld" != "no"; then
> > +		AC_MSG_WARN([nfsdcld requires sqlite3, autodisabling it])
> > +		enable_nfsdcld="no"
> > +	fi
> > +fi
> > +
> > +dnl Now set CONFIG_NFSDCLD properly
> > +AM_CONDITIONAL(CONFIG_NFSDCLD, [test "$enable_nfsdcld" != "no" ])
> > +
> >  dnl *************************************************************
> >  dnl Check for headers
> >  dnl *************************************************************
> > diff --git a/utils/Makefile.am b/utils/Makefile.am
> > index f42c7ee..5df7ca7 100644
> > --- a/utils/Makefile.am
> > +++ b/utils/Makefile.am
> > @@ -21,8 +21,11 @@ if CONFIG_MOUNT
> >  OPTDIRS += mount
> >  endif
> >  
> > +if CONFIG_NFSDCLD
> > +OPTDIRS += nfsdcld
> > +endif
> > +
> >  SUBDIRS = \
> > -	nfsdcld \
> >  	exportfs \
> >  	mountd \
> >  	nfsd \
> 
> 
> -- 
> Jeff Layton <jlayton@redhat.com>

Re: [PATCH v2 3/7] nfsdcld: add autoconf goop for sqlite

[Jeff Layton]

On Tue, 20 Dec 2011 16:18:38 -0500
"J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Tue, Dec 20, 2011 at 02:17:49PM -0500, Jeff Layton wrote:
> > On Thu, 15 Dec 2011 13:43:57 -0500
> > Jeff Layton <jlayton@redhat.com> wrote:
> > 
> > > Mostly cribbed from Chuck Lever's new-statd rewrite a few years ago...
> > > 
> > 
> > Sending this to Bruce and Steve, but cc'ing the list in case other
> > people have thoughts...
> > 
> > I have a couple of questions about configure/build behavior when the
> > dependencies aren't present on the build system. The patch below
> > autodisables the building of nfsdcld when sqlite isn't there or isn't
> > functional, etc.
> > 
> > This is probably fine initially, but eventually I'd like to see the
> > old state tracking code go away. At that point, this daemon will be
> > required for NFSv4 serving.
> > 
> > Some questions:
> > 
> > 1) does anyone have a problem with deprecating the old state tracking
> > code? I'd prefer we not maintain it in perpetuity, but maybe there's an
> > argument for keeping it around?
> 
> Just the usual: the kernel community depends for some of its testing on
> people being willing to upgrade to the latest kernel.  So we try to
> ensure they can just drop in a new kernel without breaking a random
> subsystem that (as non-NFS specialists) they don't ordinarily have to
> think about....
> 

There's the rub. The legacy scheme is completely transparent for most
users. There's no daemon involved, so they don't need to set anything
up. That's not the case with the new scheme.

If we do want to eventually deprecate the old code, what I think we'd
do is add printk's that say we're deprecating the old code in release
3.x, and to ensure that nfsdcld is running. We'd want to do that for at
least 2 releases before removing anything, but could do so for longer
too.

> > 2) assuming we do want to eventually deprecate the old code. When
> > should we plan to do this. Assuming that this goes into 3.3 and we add
> > sufficient printk warnings when the old tracker is used, the earliest
> > we could remove it is 3.5. Is that too aggressive?
> 
> That sounds a little aggressive.  It may depend on how much trouble we
> find it is to keep it up to date.
> 
> Then again I guess it depends how obnoxiously we fail in the case the
> new daemon isn't running.
> 

Ok, it sounds like you're leaning toward keeping the old code around
for now and not forcing anyone onto the new. The only downside I can
see is the maintenance hassle (which is fairly low for now), and the
fact that it adds a new dimension to the testing matrix. If we can live
with that, then I won't worry about deprecating anything at this point.

-- 
Jeff Layton <jlayton@redhat.com>