[PATCH 0/5] nfs-utils: Improving NFS re-exports

[PATCH 0/5] nfs-utils: Improving NFS re-exports

[Richard Weinberger]

This is the first non-RFC iteration of the NFS re-export
improvement series for nfs-utils.
While the kernel side[0] didn't change at all and is still small,
the userspace side saw much more changes.

The core idea is adding new export option: reeport=
Using reexport= it is possible to mark an export entry in the exports
file explicitly as NFS re-export and select a strategy how unique
identifiers should be provided.
Currently two strategies are supported, "auto-fsidnum" and
"predefined-fsidnum", both use a SQLite database as backend to keep
track of generated ids.
For a more detailed description see patch "exports: Implement new export option reexport=".
I choose SQLite because nfs-utils already uses it and using SQL ids can nicely
generated and maintained. It will also scale for large setups where the amount
of subvolumes is high.

Beside of id generation this series also addresses the reboot problem.
If the re-exporting NFS server reboots, uncovered NFS subvolumes are not yet
mounted and file handles become stale.
Now mountd/exportd keeps track of uncovered subvolumes and makes sure they get
uncovered while nfsd starts.

The whole set of features is currently opt-in via --enable-reexport.
I'm also not sure about the rearrangement of the reexport code,
currently it is a helper library.

A typical export entry on a re-exporting server looks like:
	/nfs *(rw,no_root_squash,no_subtree_check,crossmnt,reexport=auto-fsidnum)
reexport=auto-fsidnum will automatically assign an fsid= to /nfs and all
uncovered subvolumes.

Richard Weinberger (5):
  Implement reexport helper library
  exports: Implement new export option reexport=
  export: Implement logic behind reexport=
  export: Avoid fsid= conflicts
  reexport: Make state database location configurable

[0] https://git.kernel.org/pub/scm/linux/kernel/git/rw/misc.git/log/?h=nfs_reexport_clean

 configure.ac                   |  12 ++
 nfs.conf                       |   3 +
 support/Makefile.am            |   4 +
 support/export/Makefile.am     |   2 +
 support/export/cache.c         |  71 ++++++-
 support/export/export.c        |  27 ++-
 support/include/nfslib.h       |   1 +
 support/nfs/Makefile.am        |   1 +
 support/nfs/exports.c          |  68 +++++++
 support/reexport/Makefile.am   |   6 +
 support/reexport/reexport.c    | 354 +++++++++++++++++++++++++++++++++
 support/reexport/reexport.h    |  39 ++++
 systemd/Makefile.am            |   4 +
 systemd/nfs-server-generator.c |  14 +-
 systemd/nfs.conf.man           |   6 +
 utils/exportd/Makefile.am      |   8 +-
 utils/exportd/exportd.c        |   5 +
 utils/exportfs/Makefile.am     |   6 +
 utils/exportfs/exportfs.c      |  21 +-
 utils/exportfs/exports.man     |  31 +++
 utils/mount/Makefile.am        |   7 +
 utils/mountd/Makefile.am       |   6 +
 utils/mountd/mountd.c          |   1 +
 utils/mountd/svc_run.c         |   6 +
 24 files changed, 690 insertions(+), 13 deletions(-)
 create mode 100644 support/reexport/Makefile.am
 create mode 100644 support/reexport/reexport.c
 create mode 100644 support/reexport/reexport.h

-- 
2.31.1

[PATCH 1/5] Implement reexport helper library

[Richard Weinberger]

This internal library contains code that will be used by various
tools within the nfs-utils package to deal better with NFS re-export,
especially cross mounts.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 configure.ac                 |  12 ++
 support/Makefile.am          |   4 +
 support/reexport/Makefile.am |   6 +
 support/reexport/reexport.c  | 285 +++++++++++++++++++++++++++++++++++
 support/reexport/reexport.h  |  39 +++++
 5 files changed, 346 insertions(+)
 create mode 100644 support/reexport/Makefile.am
 create mode 100644 support/reexport/reexport.c
 create mode 100644 support/reexport/reexport.h

diff --git a/configure.ac b/configure.ac
index 93626d62..86bf8ba9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -274,6 +274,17 @@ AC_ARG_ENABLE(nfsv4server,
 	fi
 	AM_CONDITIONAL(CONFIG_NFSV4SERVER, [test "$enable_nfsv4server" = "yes" ])
 
+AC_ARG_ENABLE(reexport,
+	[AC_HELP_STRING([--enable-reexport],
+			[enable support for re-exporting NFS mounts  @<:@default=no@:>@])],
+	enable_reexport=$enableval,
+	enable_reexport="no")
+	if test "$enable_reexport" = yes; then
+		AC_DEFINE(HAVE_REEXPORT_SUPPORT, 1,
+                          [Define this if you want NFS re-export support compiled in])
+	fi
+	AM_CONDITIONAL(CONFIG_REEXPORT, [test "$enable_reexport" = "yes" ])
+
 dnl Check for TI-RPC library and headers
 AC_LIBTIRPC
 
@@ -730,6 +741,7 @@ AC_CONFIG_FILES([
 	support/nsm/Makefile
 	support/nfsidmap/Makefile
 	support/nfsidmap/libnfsidmap.pc
+	support/reexport/Makefile
 	tools/Makefile
 	tools/locktest/Makefile
 	tools/nlmtest/Makefile
diff --git a/support/Makefile.am b/support/Makefile.am
index c962d4d4..986e9b5f 100644
--- a/support/Makefile.am
+++ b/support/Makefile.am
@@ -10,6 +10,10 @@ if CONFIG_JUNCTION
 OPTDIRS += junction
 endif
 
+if CONFIG_REEXPORT
+OPTDIRS += reexport
+endif
+
 SUBDIRS = export include misc nfs nsm $(OPTDIRS)
 
 MAINTAINERCLEANFILES = Makefile.in
diff --git a/support/reexport/Makefile.am b/support/reexport/Makefile.am
new file mode 100644
index 00000000..9d544a8f
--- /dev/null
+++ b/support/reexport/Makefile.am
@@ -0,0 +1,6 @@
+## Process this file with automake to produce Makefile.in
+
+noinst_LIBRARIES = libreexport.a
+libreexport_a_SOURCES = reexport.c
+
+MAINTAINERCLEANFILES = Makefile.in
diff --git a/support/reexport/reexport.c b/support/reexport/reexport.c
new file mode 100644
index 00000000..5474a21f
--- /dev/null
+++ b/support/reexport/reexport.c
@@ -0,0 +1,285 @@
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <sqlite3.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <sys/random.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <sys/vfs.h>
+#include <unistd.h>
+
+#include "nfsd_path.h"
+#include "nfslib.h"
+#include "reexport.h"
+#include "xcommon.h"
+#include "xlog.h"
+
+#define REEXPDB_DBFILE NFS_STATEDIR "/reexpdb.sqlite3"
+#define REEXPDB_DBFILE_WAIT_USEC (5000)
+
+static sqlite3 *db;
+static int init_done;
+
+static int prng_init(void)
+{
+	int seed;
+
+	if (getrandom(&seed, sizeof(seed), 0) != sizeof(seed)) {
+		xlog(L_ERROR, "Unable to obtain seed for PRNG via getrandom()");
+		return -1;
+	}
+
+	srand(seed);
+	return 0;
+}
+
+static void wait_for_dbaccess(void)
+{
+	usleep(REEXPDB_DBFILE_WAIT_USEC + (rand() % REEXPDB_DBFILE_WAIT_USEC));
+}
+
+/*
+ * reexpdb_init - Initialize reexport database
+ */
+int reexpdb_init(void)
+{
+	char *sqlerr;
+	int ret;
+
+	if (init_done)
+		return 0;
+
+	if (prng_init() != 0)
+		return -1;
+
+	ret = sqlite3_open_v2(REEXPDB_DBFILE, &db, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_FULLMUTEX, NULL);
+	if (ret != SQLITE_OK) {
+		xlog(L_ERROR, "Unable to open reexport database: %s", sqlite3_errstr(ret));
+		return -1;
+	}
+
+again:
+	ret = sqlite3_exec(db, "CREATE TABLE IF NOT EXISTS fsidnums (num INTEGER PRIMARY KEY CHECK (num > 0 AND num < 4294967296), path TEXT UNIQUE); CREATE INDEX IF NOT EXISTS idx_ids_path ON fsidnums (path);", NULL, NULL, &sqlerr);
+	switch (ret) {
+	case SQLITE_OK:
+		init_done = 1;
+		ret = 0;
+		break;
+	case SQLITE_BUSY:
+	case SQLITE_LOCKED:
+		wait_for_dbaccess();
+		goto again;
+	default:
+		xlog(L_ERROR, "Unable to init reexport database: %s", sqlite3_errstr(ret));
+		sqlite3_free(sqlerr);
+		sqlite3_close_v2(db);
+		ret = -1;
+	}
+
+	return ret;
+}
+
+/*
+ * reexpdb_destroy - Undo reexpdb_init().
+ */
+void reexpdb_destroy(void)
+{
+	if (!init_done)
+		return;
+
+	sqlite3_close_v2(db);
+}
+
+static int get_fsidnum_by_path(char *path, uint32_t *fsidnum)
+{
+	static const char fsidnum_by_path_sql[] = "SELECT num FROM fsidnums WHERE path = ?1;";
+	sqlite3_stmt *stmt = NULL;
+	int found = 0;
+	int ret;
+
+	ret = sqlite3_prepare_v2(db, fsidnum_by_path_sql, sizeof(fsidnum_by_path_sql), &stmt, NULL);
+	if (ret != SQLITE_OK) {
+		xlog(L_WARNING, "Unable to prepare SQL query '%s': %s", fsidnum_by_path_sql, sqlite3_errstr(ret));
+		goto out;
+	}
+
+	ret = sqlite3_bind_text(stmt, 1, path, -1, NULL);
+	if (ret != SQLITE_OK) {
+		xlog(L_WARNING, "Unable to bind SQL query '%s': %s", fsidnum_by_path_sql, sqlite3_errstr(ret));
+		goto out;
+	}
+
+again:
+	ret = sqlite3_step(stmt);
+	switch (ret) {
+	case SQLITE_ROW:
+		*fsidnum = sqlite3_column_int(stmt, 0);
+		found = 1;
+		break;
+	case SQLITE_DONE:
+		/* No hit */
+		found = 0;
+		break;
+	case SQLITE_BUSY:
+	case SQLITE_LOCKED:
+		wait_for_dbaccess();
+		goto again;
+	default:
+		xlog(L_WARNING, "Error while looking up '%s' in database: %s", path, sqlite3_errstr(ret));
+	}
+
+out:
+	sqlite3_finalize(stmt);
+	return found;
+}
+
+static int get_path_by_fsidnum(uint32_t fsidnum, char **path)
+{
+	static const char path_by_fsidnum_sql[] = "SELECT path FROM fsidnums WHERE num = ?1;";
+	sqlite3_stmt *stmt = NULL;
+	int found = 0;
+	int ret;
+
+	ret = sqlite3_prepare_v2(db, path_by_fsidnum_sql, sizeof(path_by_fsidnum_sql), &stmt, NULL);
+	if (ret != SQLITE_OK) {
+		xlog(L_WARNING, "Unable to prepare SQL query '%s': %s", path_by_fsidnum_sql, sqlite3_errstr(ret));
+		goto out;
+	}
+
+	ret = sqlite3_bind_int(stmt, 1, fsidnum);
+	if (ret != SQLITE_OK) {
+		xlog(L_WARNING, "Unable to bind SQL query '%s': %s", path_by_fsidnum_sql, sqlite3_errstr(ret));
+		goto out;
+	}
+
+again:
+	ret = sqlite3_step(stmt);
+	switch (ret) {
+	case SQLITE_ROW:
+		*path = xstrdup((char *)sqlite3_column_text(stmt, 0));
+		found = 1;
+		break;
+	case SQLITE_DONE:
+		/* No hit */
+		found = 0;
+		break;
+	case SQLITE_BUSY:
+	case SQLITE_LOCKED:
+		wait_for_dbaccess();
+		goto again;
+	default:
+		xlog(L_WARNING, "Error while looking up '%i' in database: %s", fsidnum, sqlite3_errstr(ret));
+	}
+
+out:
+	sqlite3_finalize(stmt);
+	return found;
+}
+
+static int new_fsidnum_by_path(char *path, uint32_t *fsidnum)
+{
+	/*
+	 * This query is a little tricky. We use SQL to find and claim the smallest free fsid number.
+	 * To find a free fsid the fsidnums is left joined to itself but with an offset of 1.
+	 * Everything after the UNION statement is to handle the corner case where fsidnums
+	 * is empty. In this case we want 1 as first fsid number.
+	 */
+	static const char new_fsidnum_by_path_sql[] = "INSERT INTO fsidnums VALUES ((SELECT ids1.num + 1 FROM fsidnums AS ids1 LEFT JOIN fsidnums AS ids2 ON ids2.num = ids1.num + 1 WHERE ids2.num IS NULL UNION SELECT 1 WHERE NOT EXISTS (SELECT NULL FROM fsidnums WHERE num = 1) LIMIT 1), ?1) RETURNING num;";
+
+	sqlite3_stmt *stmt = NULL;
+	int found = 0, check = 0;
+	int ret;
+
+	ret = sqlite3_prepare_v2(db, new_fsidnum_by_path_sql, sizeof(new_fsidnum_by_path_sql), &stmt, NULL);
+	if (ret != SQLITE_OK) {
+		xlog(L_WARNING, "Unable to prepare SQL query '%s': %s", new_fsidnum_by_path_sql, sqlite3_errstr(ret));
+		goto out;
+	}
+
+	ret = sqlite3_bind_text(stmt, 1, path, -1, NULL);
+	if (ret != SQLITE_OK) {
+		xlog(L_WARNING, "Unable to bind SQL query '%s': %s", new_fsidnum_by_path_sql, sqlite3_errstr(ret));
+		goto out;
+	}
+
+again:
+	ret = sqlite3_step(stmt);
+	switch (ret) {
+	case SQLITE_ROW:
+		*fsidnum = sqlite3_column_int(stmt, 0);
+		found = 1;
+		break;
+	case SQLITE_CONSTRAINT:
+		/* Maybe we lost the race against another writer and the path is now present. */
+		check = 1;
+		break;
+	case SQLITE_BUSY:
+	case SQLITE_LOCKED:
+		wait_for_dbaccess();
+		goto again;
+	default:
+		xlog(L_WARNING, "Error while looking up '%s' in database: %s", path, sqlite3_errstr(ret));
+	}
+
+out:
+	sqlite3_finalize(stmt);
+
+	if (check) {
+		found = get_fsidnum_by_path(path, fsidnum);
+		if (!found)
+			xlog(L_WARNING, "SQLITE_CONSTRAINT error while inserting '%s' in database", path);
+	}
+
+	return found;
+}
+
+/*
+ * reexpdb_fsidnum_by_path - Lookup a fsid by path.
+ *
+ * @path: File system path used as lookup key
+ * @fsidnum: Pointer where found fsid is written to
+ * @may_create: If non-zero, allocate new fsid if lookup failed
+ *
+ */
+int reexpdb_fsidnum_by_path(char *path, uint32_t *fsidnum, int may_create)
+{
+	int found;
+
+	found = get_fsidnum_by_path(path, fsidnum);
+
+	if (!found && may_create)
+		found = new_fsidnum_by_path(path, fsidnum);
+
+	return found;
+}
+
+/*
+ * reexpdb_uncover_subvolume - Make sure a subvolume is present.
+ *
+ * @fsidnum: Numerical fsid number to look for
+ *
+ * Subvolumes (NFS cross mounts) get automatically mounted upon first
+ * access and can vanish after fs.nfs.nfs_mountpoint_timeout seconds.
+ * Also if the NFS server reboots, clients can still have valid file
+ * handles for such a subvolume.
+ *
+ * If kNFSd asks mountd for the path of a given fsidnum it can
+ * trigger an automount by calling statfs() on the given path.
+ */
+void reexpdb_uncover_subvolume(uint32_t fsidnum)
+{
+	struct statfs64 st;
+	char *path = NULL;
+	int ret;
+
+	if (get_path_by_fsidnum(fsidnum, &path)) {
+		ret = nfsd_path_statfs64(path, &st);
+		if (ret == -1)
+			xlog(L_WARNING, "statfs() failed");
+	}
+
+	free(path);
+}
diff --git a/support/reexport/reexport.h b/support/reexport/reexport.h
new file mode 100644
index 00000000..bb6d2a1b
--- /dev/null
+++ b/support/reexport/reexport.h
@@ -0,0 +1,39 @@
+#ifndef REEXPORT_H
+#define REEXPORT_H
+
+enum {
+	REEXP_NONE = 0,
+	REEXP_AUTO_FSIDNUM,
+	REEXP_PREDEFINED_FSIDNUM,
+};
+
+#ifdef HAVE_REEXPORT_SUPPORT
+int reexpdb_init(void);
+void reexpdb_destroy(void);
+int reexpdb_fsidnum_by_path(char *path, uint32_t *fsidnum, int may_create);
+int reexpdb_apply_reexport_settings(struct exportent *ep, char *flname, int flline);
+void reexpdb_uncover_subvolume(uint32_t fsidnum);
+#else
+static inline int reexpdb_init(void) { return 0; }
+static inline void reexpdb_destroy(void) {}
+static inline int reexpdb_fsidnum_by_path(char *path, uint32_t *fsidnum, int may_create)
+{
+	(void)path;
+	(void)may_create;
+	*fsidnum = 0;
+	return 0;
+}
+static inline int reexpdb_apply_reexport_settings(struct exportent *ep, char *flname, int flline)
+{
+	(void)ep;
+	(void)flname;
+	(void)flline;
+	return 0;
+}
+static inline void reexpdb_uncover_subvolume(uint32_t fsidnum)
+{
+	(void)fsidnum;
+}
+#endif /* HAVE_REEXPORT_SUPPORT */
+
+#endif /* REEXPORT_H */
-- 
2.31.1

Re: [PATCH 1/5] Implement reexport helper library

[Steve Dickson]

Hello,

On 5/2/22 4:50 AM, Richard Weinberger wrote:
> This internal library contains code that will be used by various
> tools within the nfs-utils package to deal better with NFS re-export,
> especially cross mounts.
> 
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>   configure.ac                 |  12 ++
>   support/Makefile.am          |   4 +
>   support/reexport/Makefile.am |   6 +
>   support/reexport/reexport.c  | 285 +++++++++++++++++++++++++++++++++++
>   support/reexport/reexport.h  |  39 +++++
>   5 files changed, 346 insertions(+)
>   create mode 100644 support/reexport/Makefile.am
>   create mode 100644 support/reexport/reexport.c
>   create mode 100644 support/reexport/reexport.h
> 
> diff --git a/configure.ac b/configure.ac
> index 93626d62..86bf8ba9 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -274,6 +274,17 @@ AC_ARG_ENABLE(nfsv4server,
>   	fi
>   	AM_CONDITIONAL(CONFIG_NFSV4SERVER, [test "$enable_nfsv4server" = "yes" ])
>   
> +AC_ARG_ENABLE(reexport,
> +	[AC_HELP_STRING([--enable-reexport],
> +			[enable support for re-exporting NFS mounts  @<:@default=no@:>@])],
> +	enable_reexport=$enableval,
> +	enable_reexport="no")
> +	if test "$enable_reexport" = yes; then
> +		AC_DEFINE(HAVE_REEXPORT_SUPPORT, 1,
> +                          [Define this if you want NFS re-export support compiled in])
> +	fi
> +	AM_CONDITIONAL(CONFIG_REEXPORT, [test "$enable_reexport" = "yes" ])
> +
To get this moving I'm going to add a --disable-reexport flag

+AC_ARG_ENABLE(reexport,
+       [AC_HELP_STRING([--disable-reexport],
+                       [disable support for re-exporting NFS mounts 
@<:@default=no@:>@])],
+       enable_reexport=$enableval,
+       enable_reexport="yes")
+       if test "$enable_reexport" = yes; then
+               AC_DEFINE(HAVE_REEXPORT_SUPPORT, 1,
+                          [Define this if you want NFS re-export 
support compiled in])
+       fi
+       AM_CONDITIONAL(CONFIG_REEXPORT, [test "$enable_reexport" = "yes" ])
+

steved.

>   dnl Check for TI-RPC library and headers
>   AC_LIBTIRPC
>   
> @@ -730,6 +741,7 @@ AC_CONFIG_FILES([
>   	support/nsm/Makefile
>   	support/nfsidmap/Makefile
>   	support/nfsidmap/libnfsidmap.pc
> +	support/reexport/Makefile
>   	tools/Makefile
>   	tools/locktest/Makefile
>   	tools/nlmtest/Makefile
> diff --git a/support/Makefile.am b/support/Makefile.am
> index c962d4d4..986e9b5f 100644
> --- a/support/Makefile.am
> +++ b/support/Makefile.am
> @@ -10,6 +10,10 @@ if CONFIG_JUNCTION
>   OPTDIRS += junction
>   endif
>   
> +if CONFIG_REEXPORT
> +OPTDIRS += reexport
> +endif
> +
>   SUBDIRS = export include misc nfs nsm $(OPTDIRS)
>   
>   MAINTAINERCLEANFILES = Makefile.in
> diff --git a/support/reexport/Makefile.am b/support/reexport/Makefile.am
> new file mode 100644
> index 00000000..9d544a8f
> --- /dev/null
> +++ b/support/reexport/Makefile.am
> @@ -0,0 +1,6 @@
> +## Process this file with automake to produce Makefile.in
> +
> +noinst_LIBRARIES = libreexport.a
> +libreexport_a_SOURCES = reexport.c
> +
> +MAINTAINERCLEANFILES = Makefile.in
> diff --git a/support/reexport/reexport.c b/support/reexport/reexport.c
> new file mode 100644
> index 00000000..5474a21f
> --- /dev/null
> +++ b/support/reexport/reexport.c
> @@ -0,0 +1,285 @@
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <sqlite3.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <sys/random.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <sys/vfs.h>
> +#include <unistd.h>
> +
> +#include "nfsd_path.h"
> +#include "nfslib.h"
> +#include "reexport.h"
> +#include "xcommon.h"
> +#include "xlog.h"
> +
> +#define REEXPDB_DBFILE NFS_STATEDIR "/reexpdb.sqlite3"
> +#define REEXPDB_DBFILE_WAIT_USEC (5000)
> +
> +static sqlite3 *db;
> +static int init_done;
> +
> +static int prng_init(void)
> +{
> +	int seed;
> +
> +	if (getrandom(&seed, sizeof(seed), 0) != sizeof(seed)) {
> +		xlog(L_ERROR, "Unable to obtain seed for PRNG via getrandom()");
> +		return -1;
> +	}
> +
> +	srand(seed);
> +	return 0;
> +}
> +
> +static void wait_for_dbaccess(void)
> +{
> +	usleep(REEXPDB_DBFILE_WAIT_USEC + (rand() % REEXPDB_DBFILE_WAIT_USEC));
> +}
> +
> +/*
> + * reexpdb_init - Initialize reexport database
> + */
> +int reexpdb_init(void)
> +{
> +	char *sqlerr;
> +	int ret;
> +
> +	if (init_done)
> +		return 0;
> +
> +	if (prng_init() != 0)
> +		return -1;
> +
> +	ret = sqlite3_open_v2(REEXPDB_DBFILE, &db, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_FULLMUTEX, NULL);
> +	if (ret != SQLITE_OK) {
> +		xlog(L_ERROR, "Unable to open reexport database: %s", sqlite3_errstr(ret));
> +		return -1;
> +	}
> +
> +again:
> +	ret = sqlite3_exec(db, "CREATE TABLE IF NOT EXISTS fsidnums (num INTEGER PRIMARY KEY CHECK (num > 0 AND num < 4294967296), path TEXT UNIQUE); CREATE INDEX IF NOT EXISTS idx_ids_path ON fsidnums (path);", NULL, NULL, &sqlerr);
> +	switch (ret) {
> +	case SQLITE_OK:
> +		init_done = 1;
> +		ret = 0;
> +		break;
> +	case SQLITE_BUSY:
> +	case SQLITE_LOCKED:
> +		wait_for_dbaccess();
> +		goto again;
> +	default:
> +		xlog(L_ERROR, "Unable to init reexport database: %s", sqlite3_errstr(ret));
> +		sqlite3_free(sqlerr);
> +		sqlite3_close_v2(db);
> +		ret = -1;
> +	}
> +
> +	return ret;
> +}
> +
> +/*
> + * reexpdb_destroy - Undo reexpdb_init().
> + */
> +void reexpdb_destroy(void)
> +{
> +	if (!init_done)
> +		return;
> +
> +	sqlite3_close_v2(db);
> +}
> +
> +static int get_fsidnum_by_path(char *path, uint32_t *fsidnum)
> +{
> +	static const char fsidnum_by_path_sql[] = "SELECT num FROM fsidnums WHERE path = ?1;";
> +	sqlite3_stmt *stmt = NULL;
> +	int found = 0;
> +	int ret;
> +
> +	ret = sqlite3_prepare_v2(db, fsidnum_by_path_sql, sizeof(fsidnum_by_path_sql), &stmt, NULL);
> +	if (ret != SQLITE_OK) {
> +		xlog(L_WARNING, "Unable to prepare SQL query '%s': %s", fsidnum_by_path_sql, sqlite3_errstr(ret));
> +		goto out;
> +	}
> +
> +	ret = sqlite3_bind_text(stmt, 1, path, -1, NULL);
> +	if (ret != SQLITE_OK) {
> +		xlog(L_WARNING, "Unable to bind SQL query '%s': %s", fsidnum_by_path_sql, sqlite3_errstr(ret));
> +		goto out;
> +	}
> +
> +again:
> +	ret = sqlite3_step(stmt);
> +	switch (ret) {
> +	case SQLITE_ROW:
> +		*fsidnum = sqlite3_column_int(stmt, 0);
> +		found = 1;
> +		break;
> +	case SQLITE_DONE:
> +		/* No hit */
> +		found = 0;
> +		break;
> +	case SQLITE_BUSY:
> +	case SQLITE_LOCKED:
> +		wait_for_dbaccess();
> +		goto again;
> +	default:
> +		xlog(L_WARNING, "Error while looking up '%s' in database: %s", path, sqlite3_errstr(ret));
> +	}
> +
> +out:
> +	sqlite3_finalize(stmt);
> +	return found;
> +}
> +
> +static int get_path_by_fsidnum(uint32_t fsidnum, char **path)
> +{
> +	static const char path_by_fsidnum_sql[] = "SELECT path FROM fsidnums WHERE num = ?1;";
> +	sqlite3_stmt *stmt = NULL;
> +	int found = 0;
> +	int ret;
> +
> +	ret = sqlite3_prepare_v2(db, path_by_fsidnum_sql, sizeof(path_by_fsidnum_sql), &stmt, NULL);
> +	if (ret != SQLITE_OK) {
> +		xlog(L_WARNING, "Unable to prepare SQL query '%s': %s", path_by_fsidnum_sql, sqlite3_errstr(ret));
> +		goto out;
> +	}
> +
> +	ret = sqlite3_bind_int(stmt, 1, fsidnum);
> +	if (ret != SQLITE_OK) {
> +		xlog(L_WARNING, "Unable to bind SQL query '%s': %s", path_by_fsidnum_sql, sqlite3_errstr(ret));
> +		goto out;
> +	}
> +
> +again:
> +	ret = sqlite3_step(stmt);
> +	switch (ret) {
> +	case SQLITE_ROW:
> +		*path = xstrdup((char *)sqlite3_column_text(stmt, 0));
> +		found = 1;
> +		break;
> +	case SQLITE_DONE:
> +		/* No hit */
> +		found = 0;
> +		break;
> +	case SQLITE_BUSY:
> +	case SQLITE_LOCKED:
> +		wait_for_dbaccess();
> +		goto again;
> +	default:
> +		xlog(L_WARNING, "Error while looking up '%i' in database: %s", fsidnum, sqlite3_errstr(ret));
> +	}
> +
> +out:
> +	sqlite3_finalize(stmt);
> +	return found;
> +}
> +
> +static int new_fsidnum_by_path(char *path, uint32_t *fsidnum)
> +{
> +	/*
> +	 * This query is a little tricky. We use SQL to find and claim the smallest free fsid number.
> +	 * To find a free fsid the fsidnums is left joined to itself but with an offset of 1.
> +	 * Everything after the UNION statement is to handle the corner case where fsidnums
> +	 * is empty. In this case we want 1 as first fsid number.
> +	 */
> +	static const char new_fsidnum_by_path_sql[] = "INSERT INTO fsidnums VALUES ((SELECT ids1.num + 1 FROM fsidnums AS ids1 LEFT JOIN fsidnums AS ids2 ON ids2.num = ids1.num + 1 WHERE ids2.num IS NULL UNION SELECT 1 WHERE NOT EXISTS (SELECT NULL FROM fsidnums WHERE num = 1) LIMIT 1), ?1) RETURNING num;";
> +
> +	sqlite3_stmt *stmt = NULL;
> +	int found = 0, check = 0;
> +	int ret;
> +
> +	ret = sqlite3_prepare_v2(db, new_fsidnum_by_path_sql, sizeof(new_fsidnum_by_path_sql), &stmt, NULL);
> +	if (ret != SQLITE_OK) {
> +		xlog(L_WARNING, "Unable to prepare SQL query '%s': %s", new_fsidnum_by_path_sql, sqlite3_errstr(ret));
> +		goto out;
> +	}
> +
> +	ret = sqlite3_bind_text(stmt, 1, path, -1, NULL);
> +	if (ret != SQLITE_OK) {
> +		xlog(L_WARNING, "Unable to bind SQL query '%s': %s", new_fsidnum_by_path_sql, sqlite3_errstr(ret));
> +		goto out;
> +	}
> +
> +again:
> +	ret = sqlite3_step(stmt);
> +	switch (ret) {
> +	case SQLITE_ROW:
> +		*fsidnum = sqlite3_column_int(stmt, 0);
> +		found = 1;
> +		break;
> +	case SQLITE_CONSTRAINT:
> +		/* Maybe we lost the race against another writer and the path is now present. */
> +		check = 1;
> +		break;
> +	case SQLITE_BUSY:
> +	case SQLITE_LOCKED:
> +		wait_for_dbaccess();
> +		goto again;
> +	default:
> +		xlog(L_WARNING, "Error while looking up '%s' in database: %s", path, sqlite3_errstr(ret));
> +	}
> +
> +out:
> +	sqlite3_finalize(stmt);
> +
> +	if (check) {
> +		found = get_fsidnum_by_path(path, fsidnum);
> +		if (!found)
> +			xlog(L_WARNING, "SQLITE_CONSTRAINT error while inserting '%s' in database", path);
> +	}
> +
> +	return found;
> +}
> +
> +/*
> + * reexpdb_fsidnum_by_path - Lookup a fsid by path.
> + *
> + * @path: File system path used as lookup key
> + * @fsidnum: Pointer where found fsid is written to
> + * @may_create: If non-zero, allocate new fsid if lookup failed
> + *
> + */
> +int reexpdb_fsidnum_by_path(char *path, uint32_t *fsidnum, int may_create)
> +{
> +	int found;
> +
> +	found = get_fsidnum_by_path(path, fsidnum);
> +
> +	if (!found && may_create)
> +		found = new_fsidnum_by_path(path, fsidnum);
> +
> +	return found;
> +}
> +
> +/*
> + * reexpdb_uncover_subvolume - Make sure a subvolume is present.
> + *
> + * @fsidnum: Numerical fsid number to look for
> + *
> + * Subvolumes (NFS cross mounts) get automatically mounted upon first
> + * access and can vanish after fs.nfs.nfs_mountpoint_timeout seconds.
> + * Also if the NFS server reboots, clients can still have valid file
> + * handles for such a subvolume.
> + *
> + * If kNFSd asks mountd for the path of a given fsidnum it can
> + * trigger an automount by calling statfs() on the given path.
> + */
> +void reexpdb_uncover_subvolume(uint32_t fsidnum)
> +{
> +	struct statfs64 st;
> +	char *path = NULL;
> +	int ret;
> +
> +	if (get_path_by_fsidnum(fsidnum, &path)) {
> +		ret = nfsd_path_statfs64(path, &st);
> +		if (ret == -1)
> +			xlog(L_WARNING, "statfs() failed");
> +	}
> +
> +	free(path);
> +}
> diff --git a/support/reexport/reexport.h b/support/reexport/reexport.h
> new file mode 100644
> index 00000000..bb6d2a1b
> --- /dev/null
> +++ b/support/reexport/reexport.h
> @@ -0,0 +1,39 @@
> +#ifndef REEXPORT_H
> +#define REEXPORT_H
> +
> +enum {
> +	REEXP_NONE = 0,
> +	REEXP_AUTO_FSIDNUM,
> +	REEXP_PREDEFINED_FSIDNUM,
> +};
> +
> +#ifdef HAVE_REEXPORT_SUPPORT
> +int reexpdb_init(void);
> +void reexpdb_destroy(void);
> +int reexpdb_fsidnum_by_path(char *path, uint32_t *fsidnum, int may_create);
> +int reexpdb_apply_reexport_settings(struct exportent *ep, char *flname, int flline);
> +void reexpdb_uncover_subvolume(uint32_t fsidnum);
> +#else
> +static inline int reexpdb_init(void) { return 0; }
> +static inline void reexpdb_destroy(void) {}
> +static inline int reexpdb_fsidnum_by_path(char *path, uint32_t *fsidnum, int may_create)
> +{
> +	(void)path;
> +	(void)may_create;
> +	*fsidnum = 0;
> +	return 0;
> +}
> +static inline int reexpdb_apply_reexport_settings(struct exportent *ep, char *flname, int flline)
> +{
> +	(void)ep;
> +	(void)flname;
> +	(void)flline;
> +	return 0;
> +}
> +static inline void reexpdb_uncover_subvolume(uint32_t fsidnum)
> +{
> +	(void)fsidnum;
> +}
> +#endif /* HAVE_REEXPORT_SUPPORT */
> +
> +#endif /* REEXPORT_H */

Re: [PATCH 1/5] Implement reexport helper library

[Chuck Lever III]

> On May 10, 2022, at 9:32 AM, Steve Dickson <steved@redhat.com> wrote:
> 
> Hello,
> 
> On 5/2/22 4:50 AM, Richard Weinberger wrote:
>> This internal library contains code that will be used by various
>> tools within the nfs-utils package to deal better with NFS re-export,
>> especially cross mounts.
>> Signed-off-by: Richard Weinberger <richard@nod.at>
>> ---
>>  configure.ac                 |  12 ++
>>  support/Makefile.am          |   4 +
>>  support/reexport/Makefile.am |   6 +
>>  support/reexport/reexport.c  | 285 +++++++++++++++++++++++++++++++++++
>>  support/reexport/reexport.h  |  39 +++++
>>  5 files changed, 346 insertions(+)
>>  create mode 100644 support/reexport/Makefile.am
>>  create mode 100644 support/reexport/reexport.c
>>  create mode 100644 support/reexport/reexport.h
>> diff --git a/configure.ac b/configure.ac
>> index 93626d62..86bf8ba9 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -274,6 +274,17 @@ AC_ARG_ENABLE(nfsv4server,
>>  	fi
>>  	AM_CONDITIONAL(CONFIG_NFSV4SERVER, [test "$enable_nfsv4server" = "yes" ])
>>  +AC_ARG_ENABLE(reexport,
>> +	[AC_HELP_STRING([--enable-reexport],
>> +			[enable support for re-exporting NFS mounts  @<:@default=no@:>@])],
>> +	enable_reexport=$enableval,
>> +	enable_reexport="no")
>> +	if test "$enable_reexport" = yes; then
>> +		AC_DEFINE(HAVE_REEXPORT_SUPPORT, 1,
>> +                          [Define this if you want NFS re-export support compiled in])
>> +	fi
>> +	AM_CONDITIONAL(CONFIG_REEXPORT, [test "$enable_reexport" = "yes" ])
>> +
> To get this moving I'm going to add a --disable-reexport flag

Hi Steve, no-one has given a reason why disabling support
for re-exports would be necessary. Therefore, can't this
switch just be removed?


> +AC_ARG_ENABLE(reexport,
> +       [AC_HELP_STRING([--disable-reexport],
> +                       [disable support for re-exporting NFS mounts @<:@default=no@:>@])],
> +       enable_reexport=$enableval,
> +       enable_reexport="yes")
> +       if test "$enable_reexport" = yes; then
> +               AC_DEFINE(HAVE_REEXPORT_SUPPORT, 1,
> +                          [Define this if you want NFS re-export support compiled in])
> +       fi
> +       AM_CONDITIONAL(CONFIG_REEXPORT, [test "$enable_reexport" = "yes" ])
> +
> 
> steved.
> 
>>  dnl Check for TI-RPC library and headers
>>  AC_LIBTIRPC
>>  @@ -730,6 +741,7 @@ AC_CONFIG_FILES([
>>  	support/nsm/Makefile
>>  	support/nfsidmap/Makefile
>>  	support/nfsidmap/libnfsidmap.pc
>> +	support/reexport/Makefile
>>  	tools/Makefile
>>  	tools/locktest/Makefile
>>  	tools/nlmtest/Makefile
>> diff --git a/support/Makefile.am b/support/Makefile.am
>> index c962d4d4..986e9b5f 100644
>> --- a/support/Makefile.am
>> +++ b/support/Makefile.am
>> @@ -10,6 +10,10 @@ if CONFIG_JUNCTION
>>  OPTDIRS += junction
>>  endif
>>  +if CONFIG_REEXPORT
>> +OPTDIRS += reexport
>> +endif
>> +
>>  SUBDIRS = export include misc nfs nsm $(OPTDIRS)
>>    MAINTAINERCLEANFILES = Makefile.in
>> diff --git a/support/reexport/Makefile.am b/support/reexport/Makefile.am
>> new file mode 100644
>> index 00000000..9d544a8f
>> --- /dev/null
>> +++ b/support/reexport/Makefile.am
>> @@ -0,0 +1,6 @@
>> +## Process this file with automake to produce Makefile.in
>> +
>> +noinst_LIBRARIES = libreexport.a
>> +libreexport_a_SOURCES = reexport.c
>> +
>> +MAINTAINERCLEANFILES = Makefile.in
>> diff --git a/support/reexport/reexport.c b/support/reexport/reexport.c
>> new file mode 100644
>> index 00000000..5474a21f
>> --- /dev/null
>> +++ b/support/reexport/reexport.c
>> @@ -0,0 +1,285 @@
>> +#ifdef HAVE_CONFIG_H
>> +#include <config.h>
>> +#endif
>> +
>> +#include <sqlite3.h>
>> +#include <stdint.h>
>> +#include <stdio.h>
>> +#include <sys/random.h>
>> +#include <sys/stat.h>
>> +#include <sys/types.h>
>> +#include <sys/vfs.h>
>> +#include <unistd.h>
>> +
>> +#include "nfsd_path.h"
>> +#include "nfslib.h"
>> +#include "reexport.h"
>> +#include "xcommon.h"
>> +#include "xlog.h"
>> +
>> +#define REEXPDB_DBFILE NFS_STATEDIR "/reexpdb.sqlite3"
>> +#define REEXPDB_DBFILE_WAIT_USEC (5000)
>> +
>> +static sqlite3 *db;
>> +static int init_done;
>> +
>> +static int prng_init(void)
>> +{
>> +	int seed;
>> +
>> +	if (getrandom(&seed, sizeof(seed), 0) != sizeof(seed)) {
>> +		xlog(L_ERROR, "Unable to obtain seed for PRNG via getrandom()");
>> +		return -1;
>> +	}
>> +
>> +	srand(seed);
>> +	return 0;
>> +}
>> +
>> +static void wait_for_dbaccess(void)
>> +{
>> +	usleep(REEXPDB_DBFILE_WAIT_USEC + (rand() % REEXPDB_DBFILE_WAIT_USEC));
>> +}
>> +
>> +/*
>> + * reexpdb_init - Initialize reexport database
>> + */
>> +int reexpdb_init(void)
>> +{
>> +	char *sqlerr;
>> +	int ret;
>> +
>> +	if (init_done)
>> +		return 0;
>> +
>> +	if (prng_init() != 0)
>> +		return -1;
>> +
>> +	ret = sqlite3_open_v2(REEXPDB_DBFILE, &db, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_FULLMUTEX, NULL);
>> +	if (ret != SQLITE_OK) {
>> +		xlog(L_ERROR, "Unable to open reexport database: %s", sqlite3_errstr(ret));
>> +		return -1;
>> +	}
>> +
>> +again:
>> +	ret = sqlite3_exec(db, "CREATE TABLE IF NOT EXISTS fsidnums (num INTEGER PRIMARY KEY CHECK (num > 0 AND num < 4294967296), path TEXT UNIQUE); CREATE INDEX IF NOT EXISTS idx_ids_path ON fsidnums (path);", NULL, NULL, &sqlerr);
>> +	switch (ret) {
>> +	case SQLITE_OK:
>> +		init_done = 1;
>> +		ret = 0;
>> +		break;
>> +	case SQLITE_BUSY:
>> +	case SQLITE_LOCKED:
>> +		wait_for_dbaccess();
>> +		goto again;
>> +	default:
>> +		xlog(L_ERROR, "Unable to init reexport database: %s", sqlite3_errstr(ret));
>> +		sqlite3_free(sqlerr);
>> +		sqlite3_close_v2(db);
>> +		ret = -1;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +/*
>> + * reexpdb_destroy - Undo reexpdb_init().
>> + */
>> +void reexpdb_destroy(void)
>> +{
>> +	if (!init_done)
>> +		return;
>> +
>> +	sqlite3_close_v2(db);
>> +}
>> +
>> +static int get_fsidnum_by_path(char *path, uint32_t *fsidnum)
>> +{
>> +	static const char fsidnum_by_path_sql[] = "SELECT num FROM fsidnums WHERE path = ?1;";
>> +	sqlite3_stmt *stmt = NULL;
>> +	int found = 0;
>> +	int ret;
>> +
>> +	ret = sqlite3_prepare_v2(db, fsidnum_by_path_sql, sizeof(fsidnum_by_path_sql), &stmt, NULL);
>> +	if (ret != SQLITE_OK) {
>> +		xlog(L_WARNING, "Unable to prepare SQL query '%s': %s", fsidnum_by_path_sql, sqlite3_errstr(ret));
>> +		goto out;
>> +	}
>> +
>> +	ret = sqlite3_bind_text(stmt, 1, path, -1, NULL);
>> +	if (ret != SQLITE_OK) {
>> +		xlog(L_WARNING, "Unable to bind SQL query '%s': %s", fsidnum_by_path_sql, sqlite3_errstr(ret));
>> +		goto out;
>> +	}
>> +
>> +again:
>> +	ret = sqlite3_step(stmt);
>> +	switch (ret) {
>> +	case SQLITE_ROW:
>> +		*fsidnum = sqlite3_column_int(stmt, 0);
>> +		found = 1;
>> +		break;
>> +	case SQLITE_DONE:
>> +		/* No hit */
>> +		found = 0;
>> +		break;
>> +	case SQLITE_BUSY:
>> +	case SQLITE_LOCKED:
>> +		wait_for_dbaccess();
>> +		goto again;
>> +	default:
>> +		xlog(L_WARNING, "Error while looking up '%s' in database: %s", path, sqlite3_errstr(ret));
>> +	}
>> +
>> +out:
>> +	sqlite3_finalize(stmt);
>> +	return found;
>> +}
>> +
>> +static int get_path_by_fsidnum(uint32_t fsidnum, char **path)
>> +{
>> +	static const char path_by_fsidnum_sql[] = "SELECT path FROM fsidnums WHERE num = ?1;";
>> +	sqlite3_stmt *stmt = NULL;
>> +	int found = 0;
>> +	int ret;
>> +
>> +	ret = sqlite3_prepare_v2(db, path_by_fsidnum_sql, sizeof(path_by_fsidnum_sql), &stmt, NULL);
>> +	if (ret != SQLITE_OK) {
>> +		xlog(L_WARNING, "Unable to prepare SQL query '%s': %s", path_by_fsidnum_sql, sqlite3_errstr(ret));
>> +		goto out;
>> +	}
>> +
>> +	ret = sqlite3_bind_int(stmt, 1, fsidnum);
>> +	if (ret != SQLITE_OK) {
>> +		xlog(L_WARNING, "Unable to bind SQL query '%s': %s", path_by_fsidnum_sql, sqlite3_errstr(ret));
>> +		goto out;
>> +	}
>> +
>> +again:
>> +	ret = sqlite3_step(stmt);
>> +	switch (ret) {
>> +	case SQLITE_ROW:
>> +		*path = xstrdup((char *)sqlite3_column_text(stmt, 0));
>> +		found = 1;
>> +		break;
>> +	case SQLITE_DONE:
>> +		/* No hit */
>> +		found = 0;
>> +		break;
>> +	case SQLITE_BUSY:
>> +	case SQLITE_LOCKED:
>> +		wait_for_dbaccess();
>> +		goto again;
>> +	default:
>> +		xlog(L_WARNING, "Error while looking up '%i' in database: %s", fsidnum, sqlite3_errstr(ret));
>> +	}
>> +
>> +out:
>> +	sqlite3_finalize(stmt);
>> +	return found;
>> +}
>> +
>> +static int new_fsidnum_by_path(char *path, uint32_t *fsidnum)
>> +{
>> +	/*
>> +	 * This query is a little tricky. We use SQL to find and claim the smallest free fsid number.
>> +	 * To find a free fsid the fsidnums is left joined to itself but with an offset of 1.
>> +	 * Everything after the UNION statement is to handle the corner case where fsidnums
>> +	 * is empty. In this case we want 1 as first fsid number.
>> +	 */
>> +	static const char new_fsidnum_by_path_sql[] = "INSERT INTO fsidnums VALUES ((SELECT ids1.num + 1 FROM fsidnums AS ids1 LEFT JOIN fsidnums AS ids2 ON ids2.num = ids1.num + 1 WHERE ids2.num IS NULL UNION SELECT 1 WHERE NOT EXISTS (SELECT NULL FROM fsidnums WHERE num = 1) LIMIT 1), ?1) RETURNING num;";
>> +
>> +	sqlite3_stmt *stmt = NULL;
>> +	int found = 0, check = 0;
>> +	int ret;
>> +
>> +	ret = sqlite3_prepare_v2(db, new_fsidnum_by_path_sql, sizeof(new_fsidnum_by_path_sql), &stmt, NULL);
>> +	if (ret != SQLITE_OK) {
>> +		xlog(L_WARNING, "Unable to prepare SQL query '%s': %s", new_fsidnum_by_path_sql, sqlite3_errstr(ret));
>> +		goto out;
>> +	}
>> +
>> +	ret = sqlite3_bind_text(stmt, 1, path, -1, NULL);
>> +	if (ret != SQLITE_OK) {
>> +		xlog(L_WARNING, "Unable to bind SQL query '%s': %s", new_fsidnum_by_path_sql, sqlite3_errstr(ret));
>> +		goto out;
>> +	}
>> +
>> +again:
>> +	ret = sqlite3_step(stmt);
>> +	switch (ret) {
>> +	case SQLITE_ROW:
>> +		*fsidnum = sqlite3_column_int(stmt, 0);
>> +		found = 1;
>> +		break;
>> +	case SQLITE_CONSTRAINT:
>> +		/* Maybe we lost the race against another writer and the path is now present. */
>> +		check = 1;
>> +		break;
>> +	case SQLITE_BUSY:
>> +	case SQLITE_LOCKED:
>> +		wait_for_dbaccess();
>> +		goto again;
>> +	default:
>> +		xlog(L_WARNING, "Error while looking up '%s' in database: %s", path, sqlite3_errstr(ret));
>> +	}
>> +
>> +out:
>> +	sqlite3_finalize(stmt);
>> +
>> +	if (check) {
>> +		found = get_fsidnum_by_path(path, fsidnum);
>> +		if (!found)
>> +			xlog(L_WARNING, "SQLITE_CONSTRAINT error while inserting '%s' in database", path);
>> +	}
>> +
>> +	return found;
>> +}
>> +
>> +/*
>> + * reexpdb_fsidnum_by_path - Lookup a fsid by path.
>> + *
>> + * @path: File system path used as lookup key
>> + * @fsidnum: Pointer where found fsid is written to
>> + * @may_create: If non-zero, allocate new fsid if lookup failed
>> + *
>> + */
>> +int reexpdb_fsidnum_by_path(char *path, uint32_t *fsidnum, int may_create)
>> +{
>> +	int found;
>> +
>> +	found = get_fsidnum_by_path(path, fsidnum);
>> +
>> +	if (!found && may_create)
>> +		found = new_fsidnum_by_path(path, fsidnum);
>> +
>> +	return found;
>> +}
>> +
>> +/*
>> + * reexpdb_uncover_subvolume - Make sure a subvolume is present.
>> + *
>> + * @fsidnum: Numerical fsid number to look for
>> + *
>> + * Subvolumes (NFS cross mounts) get automatically mounted upon first
>> + * access and can vanish after fs.nfs.nfs_mountpoint_timeout seconds.
>> + * Also if the NFS server reboots, clients can still have valid file
>> + * handles for such a subvolume.
>> + *
>> + * If kNFSd asks mountd for the path of a given fsidnum it can
>> + * trigger an automount by calling statfs() on the given path.
>> + */
>> +void reexpdb_uncover_subvolume(uint32_t fsidnum)
>> +{
>> +	struct statfs64 st;
>> +	char *path = NULL;
>> +	int ret;
>> +
>> +	if (get_path_by_fsidnum(fsidnum, &path)) {
>> +		ret = nfsd_path_statfs64(path, &st);
>> +		if (ret == -1)
>> +			xlog(L_WARNING, "statfs() failed");
>> +	}
>> +
>> +	free(path);
>> +}
>> diff --git a/support/reexport/reexport.h b/support/reexport/reexport.h
>> new file mode 100644
>> index 00000000..bb6d2a1b
>> --- /dev/null
>> +++ b/support/reexport/reexport.h
>> @@ -0,0 +1,39 @@
>> +#ifndef REEXPORT_H
>> +#define REEXPORT_H
>> +
>> +enum {
>> +	REEXP_NONE = 0,
>> +	REEXP_AUTO_FSIDNUM,
>> +	REEXP_PREDEFINED_FSIDNUM,
>> +};
>> +
>> +#ifdef HAVE_REEXPORT_SUPPORT
>> +int reexpdb_init(void);
>> +void reexpdb_destroy(void);
>> +int reexpdb_fsidnum_by_path(char *path, uint32_t *fsidnum, int may_create);
>> +int reexpdb_apply_reexport_settings(struct exportent *ep, char *flname, int flline);
>> +void reexpdb_uncover_subvolume(uint32_t fsidnum);
>> +#else
>> +static inline int reexpdb_init(void) { return 0; }
>> +static inline void reexpdb_destroy(void) {}
>> +static inline int reexpdb_fsidnum_by_path(char *path, uint32_t *fsidnum, int may_create)
>> +{
>> +	(void)path;
>> +	(void)may_create;
>> +	*fsidnum = 0;
>> +	return 0;
>> +}
>> +static inline int reexpdb_apply_reexport_settings(struct exportent *ep, char *flname, int flline)
>> +{
>> +	(void)ep;
>> +	(void)flname;
>> +	(void)flline;
>> +	return 0;
>> +}
>> +static inline void reexpdb_uncover_subvolume(uint32_t fsidnum)
>> +{
>> +	(void)fsidnum;
>> +}
>> +#endif /* HAVE_REEXPORT_SUPPORT */
>> +
>> +#endif /* REEXPORT_H */
> 

--
Chuck Lever

Re: [PATCH 1/5] Implement reexport helper library

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "chuck lever" <chuck.lever@oracle.com>
> An: "Steve Dickson" <steved@redhat.com>
> CC: "richard" <richard@nod.at>, "linux-nfs" <linux-nfs@vger.kernel.org>, "david" <david@sigma-star.at>, "bfields"
> <bfields@fieldses.org>, "luis turcitu" <luis.turcitu@appsbroker.com>, "david young" <david.young@appsbroker.com>,
> "david oberhollenzer" <david.oberhollenzer@sigma-star.at>, "trond myklebust" <trond.myklebust@hammerspace.com>, "anna
> schumaker" <anna.schumaker@netapp.com>, "chris chilvers" <chris.chilvers@appsbroker.com>
> Gesendet: Dienstag, 10. Mai 2022 15:48:49
> Betreff: Re: [PATCH 1/5] Implement reexport helper library

>> On May 10, 2022, at 9:32 AM, Steve Dickson <steved@redhat.com> wrote:
>> 
>> Hello,
>> 
>> On 5/2/22 4:50 AM, Richard Weinberger wrote:
>>> This internal library contains code that will be used by various
>>> tools within the nfs-utils package to deal better with NFS re-export,
>>> especially cross mounts.
>>> Signed-off-by: Richard Weinberger <richard@nod.at>
>>> ---
>>>  configure.ac                 |  12 ++
>>>  support/Makefile.am          |   4 +
>>>  support/reexport/Makefile.am |   6 +
>>>  support/reexport/reexport.c  | 285 +++++++++++++++++++++++++++++++++++
>>>  support/reexport/reexport.h  |  39 +++++
>>>  5 files changed, 346 insertions(+)
>>>  create mode 100644 support/reexport/Makefile.am
>>>  create mode 100644 support/reexport/reexport.c
>>>  create mode 100644 support/reexport/reexport.h
>>> diff --git a/configure.ac b/configure.ac
>>> index 93626d62..86bf8ba9 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -274,6 +274,17 @@ AC_ARG_ENABLE(nfsv4server,
>>>  	fi
>>>  	AM_CONDITIONAL(CONFIG_NFSV4SERVER, [test "$enable_nfsv4server" = "yes" ])
>>>  +AC_ARG_ENABLE(reexport,
>>> +	[AC_HELP_STRING([--enable-reexport],
>>> +			[enable support for re-exporting NFS mounts  @<:@default=no@:>@])],
>>> +	enable_reexport=$enableval,
>>> +	enable_reexport="no")
>>> +	if test "$enable_reexport" = yes; then
>>> +		AC_DEFINE(HAVE_REEXPORT_SUPPORT, 1,
>>> +                          [Define this if you want NFS re-export support
>>> compiled in])
>>> +	fi
>>> +	AM_CONDITIONAL(CONFIG_REEXPORT, [test "$enable_reexport" = "yes" ])
>>> +
>> To get this moving I'm going to add a --disable-reexport flag
> 
> Hi Steve, no-one has given a reason why disabling support
> for re-exports would be necessary. Therefore, can't this
> switch just be removed?

Sure can we remove it. My idea was that new/experimental features should be opt-in.

Thanks,
//richard

Re: [PATCH 1/5] Implement reexport helper library

[Steve Dickson]

Hey!

On 5/10/22 9:48 AM, Chuck Lever III wrote:
> 
> 
>> On May 10, 2022, at 9:32 AM, Steve Dickson <steved@redhat.com> wrote:
>>
>> Hello,
>>
>> On 5/2/22 4:50 AM, Richard Weinberger wrote:
>>> This internal library contains code that will be used by various
>>> tools within the nfs-utils package to deal better with NFS re-export,
>>> especially cross mounts.
>>> Signed-off-by: Richard Weinberger <richard@nod.at>
>>> ---
>>>   configure.ac                 |  12 ++
>>>   support/Makefile.am          |   4 +
>>>   support/reexport/Makefile.am |   6 +
>>>   support/reexport/reexport.c  | 285 +++++++++++++++++++++++++++++++++++
>>>   support/reexport/reexport.h  |  39 +++++
>>>   5 files changed, 346 insertions(+)
>>>   create mode 100644 support/reexport/Makefile.am
>>>   create mode 100644 support/reexport/reexport.c
>>>   create mode 100644 support/reexport/reexport.h
>>> diff --git a/configure.ac b/configure.ac
>>> index 93626d62..86bf8ba9 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -274,6 +274,17 @@ AC_ARG_ENABLE(nfsv4server,
>>>   	fi
>>>   	AM_CONDITIONAL(CONFIG_NFSV4SERVER, [test "$enable_nfsv4server" = "yes" ])
>>>   +AC_ARG_ENABLE(reexport,
>>> +	[AC_HELP_STRING([--enable-reexport],
>>> +			[enable support for re-exporting NFS mounts  @<:@default=no@:>@])],
>>> +	enable_reexport=$enableval,
>>> +	enable_reexport="no")
>>> +	if test "$enable_reexport" = yes; then
>>> +		AC_DEFINE(HAVE_REEXPORT_SUPPORT, 1,
>>> +                          [Define this if you want NFS re-export support compiled in])
>>> +	fi
>>> +	AM_CONDITIONAL(CONFIG_REEXPORT, [test "$enable_reexport" = "yes" ])
>>> +
>> To get this moving I'm going to add a --disable-reexport flag
> 
> Hi Steve, no-one has given a reason why disabling support
> for re-exports would be necessary. Therefore, can't this
> switch just be removed?The precedence has been that we used --disable-XXX flag
a lot in configure.ac... -nfsv4, -nfsv41, etc...
I'm just following along with that.

Yes, at this point, nobody is asking to turn it off but
in future somebody may want to... due some security hole
or just make the footprint of the package smaller.

I've always though it was a good idea to be able
to turn things off.. esp if the feature will
not be used.

steved.

> 
> 
>> +AC_ARG_ENABLE(reexport,
>> +       [AC_HELP_STRING([--disable-reexport],
>> +                       [disable support for re-exporting NFS mounts @<:@default=no@:>@])],
>> +       enable_reexport=$enableval,
>> +       enable_reexport="yes")
>> +       if test "$enable_reexport" = yes; then
>> +               AC_DEFINE(HAVE_REEXPORT_SUPPORT, 1,
>> +                          [Define this if you want NFS re-export support compiled in])
>> +       fi
>> +       AM_CONDITIONAL(CONFIG_REEXPORT, [test "$enable_reexport" = "yes" ])
>> +
>>
>> steved.
>>
>>>   dnl Check for TI-RPC library and headers
>>>   AC_LIBTIRPC
>>>   @@ -730,6 +741,7 @@ AC_CONFIG_FILES([
>>>   	support/nsm/Makefile
>>>   	support/nfsidmap/Makefile
>>>   	support/nfsidmap/libnfsidmap.pc
>>> +	support/reexport/Makefile
>>>   	tools/Makefile
>>>   	tools/locktest/Makefile
>>>   	tools/nlmtest/Makefile
>>> diff --git a/support/Makefile.am b/support/Makefile.am
>>> index c962d4d4..986e9b5f 100644
>>> --- a/support/Makefile.am
>>> +++ b/support/Makefile.am
>>> @@ -10,6 +10,10 @@ if CONFIG_JUNCTION
>>>   OPTDIRS += junction
>>>   endif
>>>   +if CONFIG_REEXPORT
>>> +OPTDIRS += reexport
>>> +endif
>>> +
>>>   SUBDIRS = export include misc nfs nsm $(OPTDIRS)
>>>     MAINTAINERCLEANFILES = Makefile.in
>>> diff --git a/support/reexport/Makefile.am b/support/reexport/Makefile.am
>>> new file mode 100644
>>> index 00000000..9d544a8f
>>> --- /dev/null
>>> +++ b/support/reexport/Makefile.am
>>> @@ -0,0 +1,6 @@
>>> +## Process this file with automake to produce Makefile.in
>>> +
>>> +noinst_LIBRARIES = libreexport.a
>>> +libreexport_a_SOURCES = reexport.c
>>> +
>>> +MAINTAINERCLEANFILES = Makefile.in
>>> diff --git a/support/reexport/reexport.c b/support/reexport/reexport.c
>>> new file mode 100644
>>> index 00000000..5474a21f
>>> --- /dev/null
>>> +++ b/support/reexport/reexport.c
>>> @@ -0,0 +1,285 @@
>>> +#ifdef HAVE_CONFIG_H
>>> +#include <config.h>
>>> +#endif
>>> +
>>> +#include <sqlite3.h>
>>> +#include <stdint.h>
>>> +#include <stdio.h>
>>> +#include <sys/random.h>
>>> +#include <sys/stat.h>
>>> +#include <sys/types.h>
>>> +#include <sys/vfs.h>
>>> +#include <unistd.h>
>>> +
>>> +#include "nfsd_path.h"
>>> +#include "nfslib.h"
>>> +#include "reexport.h"
>>> +#include "xcommon.h"
>>> +#include "xlog.h"
>>> +
>>> +#define REEXPDB_DBFILE NFS_STATEDIR "/reexpdb.sqlite3"
>>> +#define REEXPDB_DBFILE_WAIT_USEC (5000)
>>> +
>>> +static sqlite3 *db;
>>> +static int init_done;
>>> +
>>> +static int prng_init(void)
>>> +{
>>> +	int seed;
>>> +
>>> +	if (getrandom(&seed, sizeof(seed), 0) != sizeof(seed)) {
>>> +		xlog(L_ERROR, "Unable to obtain seed for PRNG via getrandom()");
>>> +		return -1;
>>> +	}
>>> +
>>> +	srand(seed);
>>> +	return 0;
>>> +}
>>> +
>>> +static void wait_for_dbaccess(void)
>>> +{
>>> +	usleep(REEXPDB_DBFILE_WAIT_USEC + (rand() % REEXPDB_DBFILE_WAIT_USEC));
>>> +}
>>> +
>>> +/*
>>> + * reexpdb_init - Initialize reexport database
>>> + */
>>> +int reexpdb_init(void)
>>> +{
>>> +	char *sqlerr;
>>> +	int ret;
>>> +
>>> +	if (init_done)
>>> +		return 0;
>>> +
>>> +	if (prng_init() != 0)
>>> +		return -1;
>>> +
>>> +	ret = sqlite3_open_v2(REEXPDB_DBFILE, &db, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_FULLMUTEX, NULL);
>>> +	if (ret != SQLITE_OK) {
>>> +		xlog(L_ERROR, "Unable to open reexport database: %s", sqlite3_errstr(ret));
>>> +		return -1;
>>> +	}
>>> +
>>> +again:
>>> +	ret = sqlite3_exec(db, "CREATE TABLE IF NOT EXISTS fsidnums (num INTEGER PRIMARY KEY CHECK (num > 0 AND num < 4294967296), path TEXT UNIQUE); CREATE INDEX IF NOT EXISTS idx_ids_path ON fsidnums (path);", NULL, NULL, &sqlerr);
>>> +	switch (ret) {
>>> +	case SQLITE_OK:
>>> +		init_done = 1;
>>> +		ret = 0;
>>> +		break;
>>> +	case SQLITE_BUSY:
>>> +	case SQLITE_LOCKED:
>>> +		wait_for_dbaccess();
>>> +		goto again;
>>> +	default:
>>> +		xlog(L_ERROR, "Unable to init reexport database: %s", sqlite3_errstr(ret));
>>> +		sqlite3_free(sqlerr);
>>> +		sqlite3_close_v2(db);
>>> +		ret = -1;
>>> +	}
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +/*
>>> + * reexpdb_destroy - Undo reexpdb_init().
>>> + */
>>> +void reexpdb_destroy(void)
>>> +{
>>> +	if (!init_done)
>>> +		return;
>>> +
>>> +	sqlite3_close_v2(db);
>>> +}
>>> +
>>> +static int get_fsidnum_by_path(char *path, uint32_t *fsidnum)
>>> +{
>>> +	static const char fsidnum_by_path_sql[] = "SELECT num FROM fsidnums WHERE path = ?1;";
>>> +	sqlite3_stmt *stmt = NULL;
>>> +	int found = 0;
>>> +	int ret;
>>> +
>>> +	ret = sqlite3_prepare_v2(db, fsidnum_by_path_sql, sizeof(fsidnum_by_path_sql), &stmt, NULL);
>>> +	if (ret != SQLITE_OK) {
>>> +		xlog(L_WARNING, "Unable to prepare SQL query '%s': %s", fsidnum_by_path_sql, sqlite3_errstr(ret));
>>> +		goto out;
>>> +	}
>>> +
>>> +	ret = sqlite3_bind_text(stmt, 1, path, -1, NULL);
>>> +	if (ret != SQLITE_OK) {
>>> +		xlog(L_WARNING, "Unable to bind SQL query '%s': %s", fsidnum_by_path_sql, sqlite3_errstr(ret));
>>> +		goto out;
>>> +	}
>>> +
>>> +again:
>>> +	ret = sqlite3_step(stmt);
>>> +	switch (ret) {
>>> +	case SQLITE_ROW:
>>> +		*fsidnum = sqlite3_column_int(stmt, 0);
>>> +		found = 1;
>>> +		break;
>>> +	case SQLITE_DONE:
>>> +		/* No hit */
>>> +		found = 0;
>>> +		break;
>>> +	case SQLITE_BUSY:
>>> +	case SQLITE_LOCKED:
>>> +		wait_for_dbaccess();
>>> +		goto again;
>>> +	default:
>>> +		xlog(L_WARNING, "Error while looking up '%s' in database: %s", path, sqlite3_errstr(ret));
>>> +	}
>>> +
>>> +out:
>>> +	sqlite3_finalize(stmt);
>>> +	return found;
>>> +}
>>> +
>>> +static int get_path_by_fsidnum(uint32_t fsidnum, char **path)
>>> +{
>>> +	static const char path_by_fsidnum_sql[] = "SELECT path FROM fsidnums WHERE num = ?1;";
>>> +	sqlite3_stmt *stmt = NULL;
>>> +	int found = 0;
>>> +	int ret;
>>> +
>>> +	ret = sqlite3_prepare_v2(db, path_by_fsidnum_sql, sizeof(path_by_fsidnum_sql), &stmt, NULL);
>>> +	if (ret != SQLITE_OK) {
>>> +		xlog(L_WARNING, "Unable to prepare SQL query '%s': %s", path_by_fsidnum_sql, sqlite3_errstr(ret));
>>> +		goto out;
>>> +	}
>>> +
>>> +	ret = sqlite3_bind_int(stmt, 1, fsidnum);
>>> +	if (ret != SQLITE_OK) {
>>> +		xlog(L_WARNING, "Unable to bind SQL query '%s': %s", path_by_fsidnum_sql, sqlite3_errstr(ret));
>>> +		goto out;
>>> +	}
>>> +
>>> +again:
>>> +	ret = sqlite3_step(stmt);
>>> +	switch (ret) {
>>> +	case SQLITE_ROW:
>>> +		*path = xstrdup((char *)sqlite3_column_text(stmt, 0));
>>> +		found = 1;
>>> +		break;
>>> +	case SQLITE_DONE:
>>> +		/* No hit */
>>> +		found = 0;
>>> +		break;
>>> +	case SQLITE_BUSY:
>>> +	case SQLITE_LOCKED:
>>> +		wait_for_dbaccess();
>>> +		goto again;
>>> +	default:
>>> +		xlog(L_WARNING, "Error while looking up '%i' in database: %s", fsidnum, sqlite3_errstr(ret));
>>> +	}
>>> +
>>> +out:
>>> +	sqlite3_finalize(stmt);
>>> +	return found;
>>> +}
>>> +
>>> +static int new_fsidnum_by_path(char *path, uint32_t *fsidnum)
>>> +{
>>> +	/*
>>> +	 * This query is a little tricky. We use SQL to find and claim the smallest free fsid number.
>>> +	 * To find a free fsid the fsidnums is left joined to itself but with an offset of 1.
>>> +	 * Everything after the UNION statement is to handle the corner case where fsidnums
>>> +	 * is empty. In this case we want 1 as first fsid number.
>>> +	 */
>>> +	static const char new_fsidnum_by_path_sql[] = "INSERT INTO fsidnums VALUES ((SELECT ids1.num + 1 FROM fsidnums AS ids1 LEFT JOIN fsidnums AS ids2 ON ids2.num = ids1.num + 1 WHERE ids2.num IS NULL UNION SELECT 1 WHERE NOT EXISTS (SELECT NULL FROM fsidnums WHERE num = 1) LIMIT 1), ?1) RETURNING num;";
>>> +
>>> +	sqlite3_stmt *stmt = NULL;
>>> +	int found = 0, check = 0;
>>> +	int ret;
>>> +
>>> +	ret = sqlite3_prepare_v2(db, new_fsidnum_by_path_sql, sizeof(new_fsidnum_by_path_sql), &stmt, NULL);
>>> +	if (ret != SQLITE_OK) {
>>> +		xlog(L_WARNING, "Unable to prepare SQL query '%s': %s", new_fsidnum_by_path_sql, sqlite3_errstr(ret));
>>> +		goto out;
>>> +	}
>>> +
>>> +	ret = sqlite3_bind_text(stmt, 1, path, -1, NULL);
>>> +	if (ret != SQLITE_OK) {
>>> +		xlog(L_WARNING, "Unable to bind SQL query '%s': %s", new_fsidnum_by_path_sql, sqlite3_errstr(ret));
>>> +		goto out;
>>> +	}
>>> +
>>> +again:
>>> +	ret = sqlite3_step(stmt);
>>> +	switch (ret) {
>>> +	case SQLITE_ROW:
>>> +		*fsidnum = sqlite3_column_int(stmt, 0);
>>> +		found = 1;
>>> +		break;
>>> +	case SQLITE_CONSTRAINT:
>>> +		/* Maybe we lost the race against another writer and the path is now present. */
>>> +		check = 1;
>>> +		break;
>>> +	case SQLITE_BUSY:
>>> +	case SQLITE_LOCKED:
>>> +		wait_for_dbaccess();
>>> +		goto again;
>>> +	default:
>>> +		xlog(L_WARNING, "Error while looking up '%s' in database: %s", path, sqlite3_errstr(ret));
>>> +	}
>>> +
>>> +out:
>>> +	sqlite3_finalize(stmt);
>>> +
>>> +	if (check) {
>>> +		found = get_fsidnum_by_path(path, fsidnum);
>>> +		if (!found)
>>> +			xlog(L_WARNING, "SQLITE_CONSTRAINT error while inserting '%s' in database", path);
>>> +	}
>>> +
>>> +	return found;
>>> +}
>>> +
>>> +/*
>>> + * reexpdb_fsidnum_by_path - Lookup a fsid by path.
>>> + *
>>> + * @path: File system path used as lookup key
>>> + * @fsidnum: Pointer where found fsid is written to
>>> + * @may_create: If non-zero, allocate new fsid if lookup failed
>>> + *
>>> + */
>>> +int reexpdb_fsidnum_by_path(char *path, uint32_t *fsidnum, int may_create)
>>> +{
>>> +	int found;
>>> +
>>> +	found = get_fsidnum_by_path(path, fsidnum);
>>> +
>>> +	if (!found && may_create)
>>> +		found = new_fsidnum_by_path(path, fsidnum);
>>> +
>>> +	return found;
>>> +}
>>> +
>>> +/*
>>> + * reexpdb_uncover_subvolume - Make sure a subvolume is present.
>>> + *
>>> + * @fsidnum: Numerical fsid number to look for
>>> + *
>>> + * Subvolumes (NFS cross mounts) get automatically mounted upon first
>>> + * access and can vanish after fs.nfs.nfs_mountpoint_timeout seconds.
>>> + * Also if the NFS server reboots, clients can still have valid file
>>> + * handles for such a subvolume.
>>> + *
>>> + * If kNFSd asks mountd for the path of a given fsidnum it can
>>> + * trigger an automount by calling statfs() on the given path.
>>> + */
>>> +void reexpdb_uncover_subvolume(uint32_t fsidnum)
>>> +{
>>> +	struct statfs64 st;
>>> +	char *path = NULL;
>>> +	int ret;
>>> +
>>> +	if (get_path_by_fsidnum(fsidnum, &path)) {
>>> +		ret = nfsd_path_statfs64(path, &st);
>>> +		if (ret == -1)
>>> +			xlog(L_WARNING, "statfs() failed");
>>> +	}
>>> +
>>> +	free(path);
>>> +}
>>> diff --git a/support/reexport/reexport.h b/support/reexport/reexport.h
>>> new file mode 100644
>>> index 00000000..bb6d2a1b
>>> --- /dev/null
>>> +++ b/support/reexport/reexport.h
>>> @@ -0,0 +1,39 @@
>>> +#ifndef REEXPORT_H
>>> +#define REEXPORT_H
>>> +
>>> +enum {
>>> +	REEXP_NONE = 0,
>>> +	REEXP_AUTO_FSIDNUM,
>>> +	REEXP_PREDEFINED_FSIDNUM,
>>> +};
>>> +
>>> +#ifdef HAVE_REEXPORT_SUPPORT
>>> +int reexpdb_init(void);
>>> +void reexpdb_destroy(void);
>>> +int reexpdb_fsidnum_by_path(char *path, uint32_t *fsidnum, int may_create);
>>> +int reexpdb_apply_reexport_settings(struct exportent *ep, char *flname, int flline);
>>> +void reexpdb_uncover_subvolume(uint32_t fsidnum);
>>> +#else
>>> +static inline int reexpdb_init(void) { return 0; }
>>> +static inline void reexpdb_destroy(void) {}
>>> +static inline int reexpdb_fsidnum_by_path(char *path, uint32_t *fsidnum, int may_create)
>>> +{
>>> +	(void)path;
>>> +	(void)may_create;
>>> +	*fsidnum = 0;
>>> +	return 0;
>>> +}
>>> +static inline int reexpdb_apply_reexport_settings(struct exportent *ep, char *flname, int flline)
>>> +{
>>> +	(void)ep;
>>> +	(void)flname;
>>> +	(void)flline;
>>> +	return 0;
>>> +}
>>> +static inline void reexpdb_uncover_subvolume(uint32_t fsidnum)
>>> +{
>>> +	(void)fsidnum;
>>> +}
>>> +#endif /* HAVE_REEXPORT_SUPPORT */
>>> +
>>> +#endif /* REEXPORT_H */
>>
> 
> --
> Chuck Lever
> 
> 
> 

Re: [PATCH 1/5] Implement reexport helper library

[Chuck Lever III]

> On May 10, 2022, at 10:04 AM, Steve Dickson <steved@redhat.com> wrote:
> 
> Hey!
> 
> On 5/10/22 9:48 AM, Chuck Lever III wrote:
>>> On May 10, 2022, at 9:32 AM, Steve Dickson <steved@redhat.com> wrote:
>>> 
>>> Hello,
>>> 
>>> On 5/2/22 4:50 AM, Richard Weinberger wrote:
>>>> This internal library contains code that will be used by various
>>>> tools within the nfs-utils package to deal better with NFS re-export,
>>>> especially cross mounts.
>>>> Signed-off-by: Richard Weinberger <richard@nod.at>
>>>> ---
>>>>  configure.ac                 |  12 ++
>>>>  support/Makefile.am          |   4 +
>>>>  support/reexport/Makefile.am |   6 +
>>>>  support/reexport/reexport.c  | 285 +++++++++++++++++++++++++++++++++++
>>>>  support/reexport/reexport.h  |  39 +++++
>>>>  5 files changed, 346 insertions(+)
>>>>  create mode 100644 support/reexport/Makefile.am
>>>>  create mode 100644 support/reexport/reexport.c
>>>>  create mode 100644 support/reexport/reexport.h
>>>> diff --git a/configure.ac b/configure.ac
>>>> index 93626d62..86bf8ba9 100644
>>>> --- a/configure.ac
>>>> +++ b/configure.ac
>>>> @@ -274,6 +274,17 @@ AC_ARG_ENABLE(nfsv4server,
>>>>  	fi
>>>>  	AM_CONDITIONAL(CONFIG_NFSV4SERVER, [test "$enable_nfsv4server" = "yes" ])
>>>>  +AC_ARG_ENABLE(reexport,
>>>> +	[AC_HELP_STRING([--enable-reexport],
>>>> +			[enable support for re-exporting NFS mounts  @<:@default=no@:>@])],
>>>> +	enable_reexport=$enableval,
>>>> +	enable_reexport="no")
>>>> +	if test "$enable_reexport" = yes; then
>>>> +		AC_DEFINE(HAVE_REEXPORT_SUPPORT, 1,
>>>> +                          [Define this if you want NFS re-export support compiled in])
>>>> +	fi
>>>> +	AM_CONDITIONAL(CONFIG_REEXPORT, [test "$enable_reexport" = "yes" ])
>>>> +
>>> To get this moving I'm going to add a --disable-reexport flag
>> Hi Steve, no-one has given a reason why disabling support
>> for re-exports would be necessary. Therefore, can't this
>> switch just be removed?

> The precedence has been that we used --disable-XXX flag
> a lot in configure.ac... -nfsv4, -nfsv41, etc...
> I'm just following along with that.

I get that... but no-one has given a technical reason
why disabling this code would even be necessary.


> Yes, at this point, nobody is asking to turn it off but
> in future somebody may want to... due some security hole
> or just make the footprint of the package smaller.

I'll bite. What is the added footprint of this patch
series? I didn't think there was a new library dependency
or anything like that...


> I've always though it was a good idea to be able
> to turn things off.. esp if the feature will
> not be used.

If there is a hazard to leaving it on all the time, we
should find out now before the series is applied. So, is
there harm to leaving it on all the time? That should be
documented or fixed before merging.

I must be missing something.


> steved.
> 
>>> +AC_ARG_ENABLE(reexport,
>>> +       [AC_HELP_STRING([--disable-reexport],
>>> +                       [disable support for re-exporting NFS mounts @<:@default=no@:>@])],
>>> +       enable_reexport=$enableval,
>>> +       enable_reexport="yes")
>>> +       if test "$enable_reexport" = yes; then
>>> +               AC_DEFINE(HAVE_REEXPORT_SUPPORT, 1,
>>> +                          [Define this if you want NFS re-export support compiled in])
>>> +       fi
>>> +       AM_CONDITIONAL(CONFIG_REEXPORT, [test "$enable_reexport" = "yes" ])
>>> +
>>> 
>>> steved.
>>> 
>>>>  dnl Check for TI-RPC library and headers
>>>>  AC_LIBTIRPC
>>>>  @@ -730,6 +741,7 @@ AC_CONFIG_FILES([
>>>>  	support/nsm/Makefile
>>>>  	support/nfsidmap/Makefile
>>>>  	support/nfsidmap/libnfsidmap.pc
>>>> +	support/reexport/Makefile
>>>>  	tools/Makefile
>>>>  	tools/locktest/Makefile
>>>>  	tools/nlmtest/Makefile
>>>> diff --git a/support/Makefile.am b/support/Makefile.am
>>>> index c962d4d4..986e9b5f 100644
>>>> --- a/support/Makefile.am
>>>> +++ b/support/Makefile.am
>>>> @@ -10,6 +10,10 @@ if CONFIG_JUNCTION
>>>>  OPTDIRS += junction
>>>>  endif
>>>>  +if CONFIG_REEXPORT
>>>> +OPTDIRS += reexport
>>>> +endif
>>>> +
>>>>  SUBDIRS = export include misc nfs nsm $(OPTDIRS)
>>>>    MAINTAINERCLEANFILES = Makefile.in
>>>> diff --git a/support/reexport/Makefile.am b/support/reexport/Makefile.am
>>>> new file mode 100644
>>>> index 00000000..9d544a8f
>>>> --- /dev/null
>>>> +++ b/support/reexport/Makefile.am
>>>> @@ -0,0 +1,6 @@
>>>> +## Process this file with automake to produce Makefile.in
>>>> +
>>>> +noinst_LIBRARIES = libreexport.a
>>>> +libreexport_a_SOURCES = reexport.c
>>>> +
>>>> +MAINTAINERCLEANFILES = Makefile.in
>>>> diff --git a/support/reexport/reexport.c b/support/reexport/reexport.c
>>>> new file mode 100644
>>>> index 00000000..5474a21f
>>>> --- /dev/null
>>>> +++ b/support/reexport/reexport.c
>>>> @@ -0,0 +1,285 @@
>>>> +#ifdef HAVE_CONFIG_H
>>>> +#include <config.h>
>>>> +#endif
>>>> +
>>>> +#include <sqlite3.h>
>>>> +#include <stdint.h>
>>>> +#include <stdio.h>
>>>> +#include <sys/random.h>
>>>> +#include <sys/stat.h>
>>>> +#include <sys/types.h>
>>>> +#include <sys/vfs.h>
>>>> +#include <unistd.h>
>>>> +
>>>> +#include "nfsd_path.h"
>>>> +#include "nfslib.h"
>>>> +#include "reexport.h"
>>>> +#include "xcommon.h"
>>>> +#include "xlog.h"
>>>> +
>>>> +#define REEXPDB_DBFILE NFS_STATEDIR "/reexpdb.sqlite3"
>>>> +#define REEXPDB_DBFILE_WAIT_USEC (5000)
>>>> +
>>>> +static sqlite3 *db;
>>>> +static int init_done;
>>>> +
>>>> +static int prng_init(void)
>>>> +{
>>>> +	int seed;
>>>> +
>>>> +	if (getrandom(&seed, sizeof(seed), 0) != sizeof(seed)) {
>>>> +		xlog(L_ERROR, "Unable to obtain seed for PRNG via getrandom()");
>>>> +		return -1;
>>>> +	}
>>>> +
>>>> +	srand(seed);
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static void wait_for_dbaccess(void)
>>>> +{
>>>> +	usleep(REEXPDB_DBFILE_WAIT_USEC + (rand() % REEXPDB_DBFILE_WAIT_USEC));
>>>> +}
>>>> +
>>>> +/*
>>>> + * reexpdb_init - Initialize reexport database
>>>> + */
>>>> +int reexpdb_init(void)
>>>> +{
>>>> +	char *sqlerr;
>>>> +	int ret;
>>>> +
>>>> +	if (init_done)
>>>> +		return 0;
>>>> +
>>>> +	if (prng_init() != 0)
>>>> +		return -1;
>>>> +
>>>> +	ret = sqlite3_open_v2(REEXPDB_DBFILE, &db, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_FULLMUTEX, NULL);
>>>> +	if (ret != SQLITE_OK) {
>>>> +		xlog(L_ERROR, "Unable to open reexport database: %s", sqlite3_errstr(ret));
>>>> +		return -1;
>>>> +	}
>>>> +
>>>> +again:
>>>> +	ret = sqlite3_exec(db, "CREATE TABLE IF NOT EXISTS fsidnums (num INTEGER PRIMARY KEY CHECK (num > 0 AND num < 4294967296), path TEXT UNIQUE); CREATE INDEX IF NOT EXISTS idx_ids_path ON fsidnums (path);", NULL, NULL, &sqlerr);
>>>> +	switch (ret) {
>>>> +	case SQLITE_OK:
>>>> +		init_done = 1;
>>>> +		ret = 0;
>>>> +		break;
>>>> +	case SQLITE_BUSY:
>>>> +	case SQLITE_LOCKED:
>>>> +		wait_for_dbaccess();
>>>> +		goto again;
>>>> +	default:
>>>> +		xlog(L_ERROR, "Unable to init reexport database: %s", sqlite3_errstr(ret));
>>>> +		sqlite3_free(sqlerr);
>>>> +		sqlite3_close_v2(db);
>>>> +		ret = -1;
>>>> +	}
>>>> +
>>>> +	return ret;
>>>> +}
>>>> +
>>>> +/*
>>>> + * reexpdb_destroy - Undo reexpdb_init().
>>>> + */
>>>> +void reexpdb_destroy(void)
>>>> +{
>>>> +	if (!init_done)
>>>> +		return;
>>>> +
>>>> +	sqlite3_close_v2(db);
>>>> +}
>>>> +
>>>> +static int get_fsidnum_by_path(char *path, uint32_t *fsidnum)
>>>> +{
>>>> +	static const char fsidnum_by_path_sql[] = "SELECT num FROM fsidnums WHERE path = ?1;";
>>>> +	sqlite3_stmt *stmt = NULL;
>>>> +	int found = 0;
>>>> +	int ret;
>>>> +
>>>> +	ret = sqlite3_prepare_v2(db, fsidnum_by_path_sql, sizeof(fsidnum_by_path_sql), &stmt, NULL);
>>>> +	if (ret != SQLITE_OK) {
>>>> +		xlog(L_WARNING, "Unable to prepare SQL query '%s': %s", fsidnum_by_path_sql, sqlite3_errstr(ret));
>>>> +		goto out;
>>>> +	}
>>>> +
>>>> +	ret = sqlite3_bind_text(stmt, 1, path, -1, NULL);
>>>> +	if (ret != SQLITE_OK) {
>>>> +		xlog(L_WARNING, "Unable to bind SQL query '%s': %s", fsidnum_by_path_sql, sqlite3_errstr(ret));
>>>> +		goto out;
>>>> +	}
>>>> +
>>>> +again:
>>>> +	ret = sqlite3_step(stmt);
>>>> +	switch (ret) {
>>>> +	case SQLITE_ROW:
>>>> +		*fsidnum = sqlite3_column_int(stmt, 0);
>>>> +		found = 1;
>>>> +		break;
>>>> +	case SQLITE_DONE:
>>>> +		/* No hit */
>>>> +		found = 0;
>>>> +		break;
>>>> +	case SQLITE_BUSY:
>>>> +	case SQLITE_LOCKED:
>>>> +		wait_for_dbaccess();
>>>> +		goto again;
>>>> +	default:
>>>> +		xlog(L_WARNING, "Error while looking up '%s' in database: %s", path, sqlite3_errstr(ret));
>>>> +	}
>>>> +
>>>> +out:
>>>> +	sqlite3_finalize(stmt);
>>>> +	return found;
>>>> +}
>>>> +
>>>> +static int get_path_by_fsidnum(uint32_t fsidnum, char **path)
>>>> +{
>>>> +	static const char path_by_fsidnum_sql[] = "SELECT path FROM fsidnums WHERE num = ?1;";
>>>> +	sqlite3_stmt *stmt = NULL;
>>>> +	int found = 0;
>>>> +	int ret;
>>>> +
>>>> +	ret = sqlite3_prepare_v2(db, path_by_fsidnum_sql, sizeof(path_by_fsidnum_sql), &stmt, NULL);
>>>> +	if (ret != SQLITE_OK) {
>>>> +		xlog(L_WARNING, "Unable to prepare SQL query '%s': %s", path_by_fsidnum_sql, sqlite3_errstr(ret));
>>>> +		goto out;
>>>> +	}
>>>> +
>>>> +	ret = sqlite3_bind_int(stmt, 1, fsidnum);
>>>> +	if (ret != SQLITE_OK) {
>>>> +		xlog(L_WARNING, "Unable to bind SQL query '%s': %s", path_by_fsidnum_sql, sqlite3_errstr(ret));
>>>> +		goto out;
>>>> +	}
>>>> +
>>>> +again:
>>>> +	ret = sqlite3_step(stmt);
>>>> +	switch (ret) {
>>>> +	case SQLITE_ROW:
>>>> +		*path = xstrdup((char *)sqlite3_column_text(stmt, 0));
>>>> +		found = 1;
>>>> +		break;
>>>> +	case SQLITE_DONE:
>>>> +		/* No hit */
>>>> +		found = 0;
>>>> +		break;
>>>> +	case SQLITE_BUSY:
>>>> +	case SQLITE_LOCKED:
>>>> +		wait_for_dbaccess();
>>>> +		goto again;
>>>> +	default:
>>>> +		xlog(L_WARNING, "Error while looking up '%i' in database: %s", fsidnum, sqlite3_errstr(ret));
>>>> +	}
>>>> +
>>>> +out:
>>>> +	sqlite3_finalize(stmt);
>>>> +	return found;
>>>> +}
>>>> +
>>>> +static int new_fsidnum_by_path(char *path, uint32_t *fsidnum)
>>>> +{
>>>> +	/*
>>>> +	 * This query is a little tricky. We use SQL to find and claim the smallest free fsid number.
>>>> +	 * To find a free fsid the fsidnums is left joined to itself but with an offset of 1.
>>>> +	 * Everything after the UNION statement is to handle the corner case where fsidnums
>>>> +	 * is empty. In this case we want 1 as first fsid number.
>>>> +	 */
>>>> +	static const char new_fsidnum_by_path_sql[] = "INSERT INTO fsidnums VALUES ((SELECT ids1.num + 1 FROM fsidnums AS ids1 LEFT JOIN fsidnums AS ids2 ON ids2.num = ids1.num + 1 WHERE ids2.num IS NULL UNION SELECT 1 WHERE NOT EXISTS (SELECT NULL FROM fsidnums WHERE num = 1) LIMIT 1), ?1) RETURNING num;";
>>>> +
>>>> +	sqlite3_stmt *stmt = NULL;
>>>> +	int found = 0, check = 0;
>>>> +	int ret;
>>>> +
>>>> +	ret = sqlite3_prepare_v2(db, new_fsidnum_by_path_sql, sizeof(new_fsidnum_by_path_sql), &stmt, NULL);
>>>> +	if (ret != SQLITE_OK) {
>>>> +		xlog(L_WARNING, "Unable to prepare SQL query '%s': %s", new_fsidnum_by_path_sql, sqlite3_errstr(ret));
>>>> +		goto out;
>>>> +	}
>>>> +
>>>> +	ret = sqlite3_bind_text(stmt, 1, path, -1, NULL);
>>>> +	if (ret != SQLITE_OK) {
>>>> +		xlog(L_WARNING, "Unable to bind SQL query '%s': %s", new_fsidnum_by_path_sql, sqlite3_errstr(ret));
>>>> +		goto out;
>>>> +	}
>>>> +
>>>> +again:
>>>> +	ret = sqlite3_step(stmt);
>>>> +	switch (ret) {
>>>> +	case SQLITE_ROW:
>>>> +		*fsidnum = sqlite3_column_int(stmt, 0);
>>>> +		found = 1;
>>>> +		break;
>>>> +	case SQLITE_CONSTRAINT:
>>>> +		/* Maybe we lost the race against another writer and the path is now present. */
>>>> +		check = 1;
>>>> +		break;
>>>> +	case SQLITE_BUSY:
>>>> +	case SQLITE_LOCKED:
>>>> +		wait_for_dbaccess();
>>>> +		goto again;
>>>> +	default:
>>>> +		xlog(L_WARNING, "Error while looking up '%s' in database: %s", path, sqlite3_errstr(ret));
>>>> +	}
>>>> +
>>>> +out:
>>>> +	sqlite3_finalize(stmt);
>>>> +
>>>> +	if (check) {
>>>> +		found = get_fsidnum_by_path(path, fsidnum);
>>>> +		if (!found)
>>>> +			xlog(L_WARNING, "SQLITE_CONSTRAINT error while inserting '%s' in database", path);
>>>> +	}
>>>> +
>>>> +	return found;
>>>> +}
>>>> +
>>>> +/*
>>>> + * reexpdb_fsidnum_by_path - Lookup a fsid by path.
>>>> + *
>>>> + * @path: File system path used as lookup key
>>>> + * @fsidnum: Pointer where found fsid is written to
>>>> + * @may_create: If non-zero, allocate new fsid if lookup failed
>>>> + *
>>>> + */
>>>> +int reexpdb_fsidnum_by_path(char *path, uint32_t *fsidnum, int may_create)
>>>> +{
>>>> +	int found;
>>>> +
>>>> +	found = get_fsidnum_by_path(path, fsidnum);
>>>> +
>>>> +	if (!found && may_create)
>>>> +		found = new_fsidnum_by_path(path, fsidnum);
>>>> +
>>>> +	return found;
>>>> +}
>>>> +
>>>> +/*
>>>> + * reexpdb_uncover_subvolume - Make sure a subvolume is present.
>>>> + *
>>>> + * @fsidnum: Numerical fsid number to look for
>>>> + *
>>>> + * Subvolumes (NFS cross mounts) get automatically mounted upon first
>>>> + * access and can vanish after fs.nfs.nfs_mountpoint_timeout seconds.
>>>> + * Also if the NFS server reboots, clients can still have valid file
>>>> + * handles for such a subvolume.
>>>> + *
>>>> + * If kNFSd asks mountd for the path of a given fsidnum it can
>>>> + * trigger an automount by calling statfs() on the given path.
>>>> + */
>>>> +void reexpdb_uncover_subvolume(uint32_t fsidnum)
>>>> +{
>>>> +	struct statfs64 st;
>>>> +	char *path = NULL;
>>>> +	int ret;
>>>> +
>>>> +	if (get_path_by_fsidnum(fsidnum, &path)) {
>>>> +		ret = nfsd_path_statfs64(path, &st);
>>>> +		if (ret == -1)
>>>> +			xlog(L_WARNING, "statfs() failed");
>>>> +	}
>>>> +
>>>> +	free(path);
>>>> +}
>>>> diff --git a/support/reexport/reexport.h b/support/reexport/reexport.h
>>>> new file mode 100644
>>>> index 00000000..bb6d2a1b
>>>> --- /dev/null
>>>> +++ b/support/reexport/reexport.h
>>>> @@ -0,0 +1,39 @@
>>>> +#ifndef REEXPORT_H
>>>> +#define REEXPORT_H
>>>> +
>>>> +enum {
>>>> +	REEXP_NONE = 0,
>>>> +	REEXP_AUTO_FSIDNUM,
>>>> +	REEXP_PREDEFINED_FSIDNUM,
>>>> +};
>>>> +
>>>> +#ifdef HAVE_REEXPORT_SUPPORT
>>>> +int reexpdb_init(void);
>>>> +void reexpdb_destroy(void);
>>>> +int reexpdb_fsidnum_by_path(char *path, uint32_t *fsidnum, int may_create);
>>>> +int reexpdb_apply_reexport_settings(struct exportent *ep, char *flname, int flline);
>>>> +void reexpdb_uncover_subvolume(uint32_t fsidnum);
>>>> +#else
>>>> +static inline int reexpdb_init(void) { return 0; }
>>>> +static inline void reexpdb_destroy(void) {}
>>>> +static inline int reexpdb_fsidnum_by_path(char *path, uint32_t *fsidnum, int may_create)
>>>> +{
>>>> +	(void)path;
>>>> +	(void)may_create;
>>>> +	*fsidnum = 0;
>>>> +	return 0;
>>>> +}
>>>> +static inline int reexpdb_apply_reexport_settings(struct exportent *ep, char *flname, int flline)
>>>> +{
>>>> +	(void)ep;
>>>> +	(void)flname;
>>>> +	(void)flline;
>>>> +	return 0;
>>>> +}
>>>> +static inline void reexpdb_uncover_subvolume(uint32_t fsidnum)
>>>> +{
>>>> +	(void)fsidnum;
>>>> +}
>>>> +#endif /* HAVE_REEXPORT_SUPPORT */
>>>> +
>>>> +#endif /* REEXPORT_H */
>>> 
>> --
>> Chuck Lever

--
Chuck Lever

Re: [PATCH 1/5] Implement reexport helper library

[Steve Dickson]

On 5/10/22 10:17 AM, Chuck Lever III wrote:
> 
> 
>> On May 10, 2022, at 10:04 AM, Steve Dickson <steved@redhat.com> wrote:
>>
>> Hey!
>>
>> On 5/10/22 9:48 AM, Chuck Lever III wrote:
>>>> On May 10, 2022, at 9:32 AM, Steve Dickson <steved@redhat.com> wrote:
>>>>
>>>> Hello,
>>>>
>>>> On 5/2/22 4:50 AM, Richard Weinberger wrote:
>>>>> This internal library contains code that will be used by various
>>>>> tools within the nfs-utils package to deal better with NFS re-export,
>>>>> especially cross mounts.
>>>>> Signed-off-by: Richard Weinberger <richard@nod.at>
>>>>> ---
>>>>>   configure.ac                 |  12 ++
>>>>>   support/Makefile.am          |   4 +
>>>>>   support/reexport/Makefile.am |   6 +
>>>>>   support/reexport/reexport.c  | 285 +++++++++++++++++++++++++++++++++++
>>>>>   support/reexport/reexport.h  |  39 +++++
>>>>>   5 files changed, 346 insertions(+)
>>>>>   create mode 100644 support/reexport/Makefile.am
>>>>>   create mode 100644 support/reexport/reexport.c
>>>>>   create mode 100644 support/reexport/reexport.h
>>>>> diff --git a/configure.ac b/configure.ac
>>>>> index 93626d62..86bf8ba9 100644
>>>>> --- a/configure.ac
>>>>> +++ b/configure.ac
>>>>> @@ -274,6 +274,17 @@ AC_ARG_ENABLE(nfsv4server,
>>>>>   	fi
>>>>>   	AM_CONDITIONAL(CONFIG_NFSV4SERVER, [test "$enable_nfsv4server" = "yes" ])
>>>>>   +AC_ARG_ENABLE(reexport,
>>>>> +	[AC_HELP_STRING([--enable-reexport],
>>>>> +			[enable support for re-exporting NFS mounts  @<:@default=no@:>@])],
>>>>> +	enable_reexport=$enableval,
>>>>> +	enable_reexport="no")
>>>>> +	if test "$enable_reexport" = yes; then
>>>>> +		AC_DEFINE(HAVE_REEXPORT_SUPPORT, 1,
>>>>> +                          [Define this if you want NFS re-export support compiled in])
>>>>> +	fi
>>>>> +	AM_CONDITIONAL(CONFIG_REEXPORT, [test "$enable_reexport" = "yes" ])
>>>>> +
>>>> To get this moving I'm going to add a --disable-reexport flag
>>> Hi Steve, no-one has given a reason why disabling support
>>> for re-exports would be necessary. Therefore, can't this
>>> switch just be removed?
> 
>> The precedence has been that we used --disable-XXX flag
>> a lot in configure.ac... -nfsv4, -nfsv41, etc...
>> I'm just following along with that.
> 
> I get that... but no-one has given a technical reason
> why disabling this code would even be necessary.
Nobody has see the code yet... :-)

There is a lot of churn in these patch.

> 
> 
>> Yes, at this point, nobody is asking to turn it off but
>> in future somebody may want to... due some security hole
>> or just make the footprint of the package smaller.
> 
> I'll bite. What is the added footprint of this patch
> series? I didn't think there was a new library dependency
> or anything like that...
Well mountd is now using an database and there
is a static reexport lib that a number daemons
and command like with...

In the I would like to bring (or be able) bring the
foot print of nfs-utils down so it will be more container
friendly... IDK if is possible... but that is the idea.

> 
> 
>> I've always though it was a good idea to be able
>> to turn things off.. esp if the feature will
>> not be used.
> 
> If there is a hazard to leaving it on all the time, we
> should find out now before the series is applied. So, is
> there harm to leaving it on all the time? That should be
> documented or fixed before merging.
Putting my distro maintainer hat on... If the flux
capacitor breaks and the server is no longer able
to exports things correctly... As I said there is
a lot of churn.... I would like a way to put things
back, so while we fix the flux capacitor, I can still
maintain a stable server.

I have found.... Have a way to get back...
is very useful! ;-)

steved.

> 
> I must be missing something.
> 
> 
>> steved.
>>
>>>> +AC_ARG_ENABLE(reexport,
>>>> +       [AC_HELP_STRING([--disable-reexport],
>>>> +                       [disable support for re-exporting NFS mounts @<:@default=no@:>@])],
>>>> +       enable_reexport=$enableval,
>>>> +       enable_reexport="yes")
>>>> +       if test "$enable_reexport" = yes; then
>>>> +               AC_DEFINE(HAVE_REEXPORT_SUPPORT, 1,
>>>> +                          [Define this if you want NFS re-export support compiled in])
>>>> +       fi
>>>> +       AM_CONDITIONAL(CONFIG_REEXPORT, [test "$enable_reexport" = "yes" ])
>>>> +
>>>>
>>>> steved.
>>>>
>>>>>   dnl Check for TI-RPC library and headers
>>>>>   AC_LIBTIRPC
>>>>>   @@ -730,6 +741,7 @@ AC_CONFIG_FILES([
>>>>>   	support/nsm/Makefile
>>>>>   	support/nfsidmap/Makefile
>>>>>   	support/nfsidmap/libnfsidmap.pc
>>>>> +	support/reexport/Makefile
>>>>>   	tools/Makefile
>>>>>   	tools/locktest/Makefile
>>>>>   	tools/nlmtest/Makefile
>>>>> diff --git a/support/Makefile.am b/support/Makefile.am
>>>>> index c962d4d4..986e9b5f 100644
>>>>> --- a/support/Makefile.am
>>>>> +++ b/support/Makefile.am
>>>>> @@ -10,6 +10,10 @@ if CONFIG_JUNCTION
>>>>>   OPTDIRS += junction
>>>>>   endif
>>>>>   +if CONFIG_REEXPORT
>>>>> +OPTDIRS += reexport
>>>>> +endif
>>>>> +
>>>>>   SUBDIRS = export include misc nfs nsm $(OPTDIRS)
>>>>>     MAINTAINERCLEANFILES = Makefile.in
>>>>> diff --git a/support/reexport/Makefile.am b/support/reexport/Makefile.am
>>>>> new file mode 100644
>>>>> index 00000000..9d544a8f
>>>>> --- /dev/null
>>>>> +++ b/support/reexport/Makefile.am
>>>>> @@ -0,0 +1,6 @@
>>>>> +## Process this file with automake to produce Makefile.in
>>>>> +
>>>>> +noinst_LIBRARIES = libreexport.a
>>>>> +libreexport_a_SOURCES = reexport.c
>>>>> +
>>>>> +MAINTAINERCLEANFILES = Makefile.in
>>>>> diff --git a/support/reexport/reexport.c b/support/reexport/reexport.c
>>>>> new file mode 100644
>>>>> index 00000000..5474a21f
>>>>> --- /dev/null
>>>>> +++ b/support/reexport/reexport.c
>>>>> @@ -0,0 +1,285 @@
>>>>> +#ifdef HAVE_CONFIG_H
>>>>> +#include <config.h>
>>>>> +#endif
>>>>> +
>>>>> +#include <sqlite3.h>
>>>>> +#include <stdint.h>
>>>>> +#include <stdio.h>
>>>>> +#include <sys/random.h>
>>>>> +#include <sys/stat.h>
>>>>> +#include <sys/types.h>
>>>>> +#include <sys/vfs.h>
>>>>> +#include <unistd.h>
>>>>> +
>>>>> +#include "nfsd_path.h"
>>>>> +#include "nfslib.h"
>>>>> +#include "reexport.h"
>>>>> +#include "xcommon.h"
>>>>> +#include "xlog.h"
>>>>> +
>>>>> +#define REEXPDB_DBFILE NFS_STATEDIR "/reexpdb.sqlite3"
>>>>> +#define REEXPDB_DBFILE_WAIT_USEC (5000)
>>>>> +
>>>>> +static sqlite3 *db;
>>>>> +static int init_done;
>>>>> +
>>>>> +static int prng_init(void)
>>>>> +{
>>>>> +	int seed;
>>>>> +
>>>>> +	if (getrandom(&seed, sizeof(seed), 0) != sizeof(seed)) {
>>>>> +		xlog(L_ERROR, "Unable to obtain seed for PRNG via getrandom()");
>>>>> +		return -1;
>>>>> +	}
>>>>> +
>>>>> +	srand(seed);
>>>>> +	return 0;
>>>>> +}
>>>>> +
>>>>> +static void wait_for_dbaccess(void)
>>>>> +{
>>>>> +	usleep(REEXPDB_DBFILE_WAIT_USEC + (rand() % REEXPDB_DBFILE_WAIT_USEC));
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * reexpdb_init - Initialize reexport database
>>>>> + */
>>>>> +int reexpdb_init(void)
>>>>> +{
>>>>> +	char *sqlerr;
>>>>> +	int ret;
>>>>> +
>>>>> +	if (init_done)
>>>>> +		return 0;
>>>>> +
>>>>> +	if (prng_init() != 0)
>>>>> +		return -1;
>>>>> +
>>>>> +	ret = sqlite3_open_v2(REEXPDB_DBFILE, &db, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_FULLMUTEX, NULL);
>>>>> +	if (ret != SQLITE_OK) {
>>>>> +		xlog(L_ERROR, "Unable to open reexport database: %s", sqlite3_errstr(ret));
>>>>> +		return -1;
>>>>> +	}
>>>>> +
>>>>> +again:
>>>>> +	ret = sqlite3_exec(db, "CREATE TABLE IF NOT EXISTS fsidnums (num INTEGER PRIMARY KEY CHECK (num > 0 AND num < 4294967296), path TEXT UNIQUE); CREATE INDEX IF NOT EXISTS idx_ids_path ON fsidnums (path);", NULL, NULL, &sqlerr);
>>>>> +	switch (ret) {
>>>>> +	case SQLITE_OK:
>>>>> +		init_done = 1;
>>>>> +		ret = 0;
>>>>> +		break;
>>>>> +	case SQLITE_BUSY:
>>>>> +	case SQLITE_LOCKED:
>>>>> +		wait_for_dbaccess();
>>>>> +		goto again;
>>>>> +	default:
>>>>> +		xlog(L_ERROR, "Unable to init reexport database: %s", sqlite3_errstr(ret));
>>>>> +		sqlite3_free(sqlerr);
>>>>> +		sqlite3_close_v2(db);
>>>>> +		ret = -1;
>>>>> +	}
>>>>> +
>>>>> +	return ret;
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * reexpdb_destroy - Undo reexpdb_init().
>>>>> + */
>>>>> +void reexpdb_destroy(void)
>>>>> +{
>>>>> +	if (!init_done)
>>>>> +		return;
>>>>> +
>>>>> +	sqlite3_close_v2(db);
>>>>> +}
>>>>> +
>>>>> +static int get_fsidnum_by_path(char *path, uint32_t *fsidnum)
>>>>> +{
>>>>> +	static const char fsidnum_by_path_sql[] = "SELECT num FROM fsidnums WHERE path = ?1;";
>>>>> +	sqlite3_stmt *stmt = NULL;
>>>>> +	int found = 0;
>>>>> +	int ret;
>>>>> +
>>>>> +	ret = sqlite3_prepare_v2(db, fsidnum_by_path_sql, sizeof(fsidnum_by_path_sql), &stmt, NULL);
>>>>> +	if (ret != SQLITE_OK) {
>>>>> +		xlog(L_WARNING, "Unable to prepare SQL query '%s': %s", fsidnum_by_path_sql, sqlite3_errstr(ret));
>>>>> +		goto out;
>>>>> +	}
>>>>> +
>>>>> +	ret = sqlite3_bind_text(stmt, 1, path, -1, NULL);
>>>>> +	if (ret != SQLITE_OK) {
>>>>> +		xlog(L_WARNING, "Unable to bind SQL query '%s': %s", fsidnum_by_path_sql, sqlite3_errstr(ret));
>>>>> +		goto out;
>>>>> +	}
>>>>> +
>>>>> +again:
>>>>> +	ret = sqlite3_step(stmt);
>>>>> +	switch (ret) {
>>>>> +	case SQLITE_ROW:
>>>>> +		*fsidnum = sqlite3_column_int(stmt, 0);
>>>>> +		found = 1;
>>>>> +		break;
>>>>> +	case SQLITE_DONE:
>>>>> +		/* No hit */
>>>>> +		found = 0;
>>>>> +		break;
>>>>> +	case SQLITE_BUSY:
>>>>> +	case SQLITE_LOCKED:
>>>>> +		wait_for_dbaccess();
>>>>> +		goto again;
>>>>> +	default:
>>>>> +		xlog(L_WARNING, "Error while looking up '%s' in database: %s", path, sqlite3_errstr(ret));
>>>>> +	}
>>>>> +
>>>>> +out:
>>>>> +	sqlite3_finalize(stmt);
>>>>> +	return found;
>>>>> +}
>>>>> +
>>>>> +static int get_path_by_fsidnum(uint32_t fsidnum, char **path)
>>>>> +{
>>>>> +	static const char path_by_fsidnum_sql[] = "SELECT path FROM fsidnums WHERE num = ?1;";
>>>>> +	sqlite3_stmt *stmt = NULL;
>>>>> +	int found = 0;
>>>>> +	int ret;
>>>>> +
>>>>> +	ret = sqlite3_prepare_v2(db, path_by_fsidnum_sql, sizeof(path_by_fsidnum_sql), &stmt, NULL);
>>>>> +	if (ret != SQLITE_OK) {
>>>>> +		xlog(L_WARNING, "Unable to prepare SQL query '%s': %s", path_by_fsidnum_sql, sqlite3_errstr(ret));
>>>>> +		goto out;
>>>>> +	}
>>>>> +
>>>>> +	ret = sqlite3_bind_int(stmt, 1, fsidnum);
>>>>> +	if (ret != SQLITE_OK) {
>>>>> +		xlog(L_WARNING, "Unable to bind SQL query '%s': %s", path_by_fsidnum_sql, sqlite3_errstr(ret));
>>>>> +		goto out;
>>>>> +	}
>>>>> +
>>>>> +again:
>>>>> +	ret = sqlite3_step(stmt);
>>>>> +	switch (ret) {
>>>>> +	case SQLITE_ROW:
>>>>> +		*path = xstrdup((char *)sqlite3_column_text(stmt, 0));
>>>>> +		found = 1;
>>>>> +		break;
>>>>> +	case SQLITE_DONE:
>>>>> +		/* No hit */
>>>>> +		found = 0;
>>>>> +		break;
>>>>> +	case SQLITE_BUSY:
>>>>> +	case SQLITE_LOCKED:
>>>>> +		wait_for_dbaccess();
>>>>> +		goto again;
>>>>> +	default:
>>>>> +		xlog(L_WARNING, "Error while looking up '%i' in database: %s", fsidnum, sqlite3_errstr(ret));
>>>>> +	}
>>>>> +
>>>>> +out:
>>>>> +	sqlite3_finalize(stmt);
>>>>> +	return found;
>>>>> +}
>>>>> +
>>>>> +static int new_fsidnum_by_path(char *path, uint32_t *fsidnum)
>>>>> +{
>>>>> +	/*
>>>>> +	 * This query is a little tricky. We use SQL to find and claim the smallest free fsid number.
>>>>> +	 * To find a free fsid the fsidnums is left joined to itself but with an offset of 1.
>>>>> +	 * Everything after the UNION statement is to handle the corner case where fsidnums
>>>>> +	 * is empty. In this case we want 1 as first fsid number.
>>>>> +	 */
>>>>> +	static const char new_fsidnum_by_path_sql[] = "INSERT INTO fsidnums VALUES ((SELECT ids1.num + 1 FROM fsidnums AS ids1 LEFT JOIN fsidnums AS ids2 ON ids2.num = ids1.num + 1 WHERE ids2.num IS NULL UNION SELECT 1 WHERE NOT EXISTS (SELECT NULL FROM fsidnums WHERE num = 1) LIMIT 1), ?1) RETURNING num;";
>>>>> +
>>>>> +	sqlite3_stmt *stmt = NULL;
>>>>> +	int found = 0, check = 0;
>>>>> +	int ret;
>>>>> +
>>>>> +	ret = sqlite3_prepare_v2(db, new_fsidnum_by_path_sql, sizeof(new_fsidnum_by_path_sql), &stmt, NULL);
>>>>> +	if (ret != SQLITE_OK) {
>>>>> +		xlog(L_WARNING, "Unable to prepare SQL query '%s': %s", new_fsidnum_by_path_sql, sqlite3_errstr(ret));
>>>>> +		goto out;
>>>>> +	}
>>>>> +
>>>>> +	ret = sqlite3_bind_text(stmt, 1, path, -1, NULL);
>>>>> +	if (ret != SQLITE_OK) {
>>>>> +		xlog(L_WARNING, "Unable to bind SQL query '%s': %s", new_fsidnum_by_path_sql, sqlite3_errstr(ret));
>>>>> +		goto out;
>>>>> +	}
>>>>> +
>>>>> +again:
>>>>> +	ret = sqlite3_step(stmt);
>>>>> +	switch (ret) {
>>>>> +	case SQLITE_ROW:
>>>>> +		*fsidnum = sqlite3_column_int(stmt, 0);
>>>>> +		found = 1;
>>>>> +		break;
>>>>> +	case SQLITE_CONSTRAINT:
>>>>> +		/* Maybe we lost the race against another writer and the path is now present. */
>>>>> +		check = 1;
>>>>> +		break;
>>>>> +	case SQLITE_BUSY:
>>>>> +	case SQLITE_LOCKED:
>>>>> +		wait_for_dbaccess();
>>>>> +		goto again;
>>>>> +	default:
>>>>> +		xlog(L_WARNING, "Error while looking up '%s' in database: %s", path, sqlite3_errstr(ret));
>>>>> +	}
>>>>> +
>>>>> +out:
>>>>> +	sqlite3_finalize(stmt);
>>>>> +
>>>>> +	if (check) {
>>>>> +		found = get_fsidnum_by_path(path, fsidnum);
>>>>> +		if (!found)
>>>>> +			xlog(L_WARNING, "SQLITE_CONSTRAINT error while inserting '%s' in database", path);
>>>>> +	}
>>>>> +
>>>>> +	return found;
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * reexpdb_fsidnum_by_path - Lookup a fsid by path.
>>>>> + *
>>>>> + * @path: File system path used as lookup key
>>>>> + * @fsidnum: Pointer where found fsid is written to
>>>>> + * @may_create: If non-zero, allocate new fsid if lookup failed
>>>>> + *
>>>>> + */
>>>>> +int reexpdb_fsidnum_by_path(char *path, uint32_t *fsidnum, int may_create)
>>>>> +{
>>>>> +	int found;
>>>>> +
>>>>> +	found = get_fsidnum_by_path(path, fsidnum);
>>>>> +
>>>>> +	if (!found && may_create)
>>>>> +		found = new_fsidnum_by_path(path, fsidnum);
>>>>> +
>>>>> +	return found;
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * reexpdb_uncover_subvolume - Make sure a subvolume is present.
>>>>> + *
>>>>> + * @fsidnum: Numerical fsid number to look for
>>>>> + *
>>>>> + * Subvolumes (NFS cross mounts) get automatically mounted upon first
>>>>> + * access and can vanish after fs.nfs.nfs_mountpoint_timeout seconds.
>>>>> + * Also if the NFS server reboots, clients can still have valid file
>>>>> + * handles for such a subvolume.
>>>>> + *
>>>>> + * If kNFSd asks mountd for the path of a given fsidnum it can
>>>>> + * trigger an automount by calling statfs() on the given path.
>>>>> + */
>>>>> +void reexpdb_uncover_subvolume(uint32_t fsidnum)
>>>>> +{
>>>>> +	struct statfs64 st;
>>>>> +	char *path = NULL;
>>>>> +	int ret;
>>>>> +
>>>>> +	if (get_path_by_fsidnum(fsidnum, &path)) {
>>>>> +		ret = nfsd_path_statfs64(path, &st);
>>>>> +		if (ret == -1)
>>>>> +			xlog(L_WARNING, "statfs() failed");
>>>>> +	}
>>>>> +
>>>>> +	free(path);
>>>>> +}
>>>>> diff --git a/support/reexport/reexport.h b/support/reexport/reexport.h
>>>>> new file mode 100644
>>>>> index 00000000..bb6d2a1b
>>>>> --- /dev/null
>>>>> +++ b/support/reexport/reexport.h
>>>>> @@ -0,0 +1,39 @@
>>>>> +#ifndef REEXPORT_H
>>>>> +#define REEXPORT_H
>>>>> +
>>>>> +enum {
>>>>> +	REEXP_NONE = 0,
>>>>> +	REEXP_AUTO_FSIDNUM,
>>>>> +	REEXP_PREDEFINED_FSIDNUM,
>>>>> +};
>>>>> +
>>>>> +#ifdef HAVE_REEXPORT_SUPPORT
>>>>> +int reexpdb_init(void);
>>>>> +void reexpdb_destroy(void);
>>>>> +int reexpdb_fsidnum_by_path(char *path, uint32_t *fsidnum, int may_create);
>>>>> +int reexpdb_apply_reexport_settings(struct exportent *ep, char *flname, int flline);
>>>>> +void reexpdb_uncover_subvolume(uint32_t fsidnum);
>>>>> +#else
>>>>> +static inline int reexpdb_init(void) { return 0; }
>>>>> +static inline void reexpdb_destroy(void) {}
>>>>> +static inline int reexpdb_fsidnum_by_path(char *path, uint32_t *fsidnum, int may_create)
>>>>> +{
>>>>> +	(void)path;
>>>>> +	(void)may_create;
>>>>> +	*fsidnum = 0;
>>>>> +	return 0;
>>>>> +}
>>>>> +static inline int reexpdb_apply_reexport_settings(struct exportent *ep, char *flname, int flline)
>>>>> +{
>>>>> +	(void)ep;
>>>>> +	(void)flname;
>>>>> +	(void)flline;
>>>>> +	return 0;
>>>>> +}
>>>>> +static inline void reexpdb_uncover_subvolume(uint32_t fsidnum)
>>>>> +{
>>>>> +	(void)fsidnum;
>>>>> +}
>>>>> +#endif /* HAVE_REEXPORT_SUPPORT */
>>>>> +
>>>>> +#endif /* REEXPORT_H */
>>>>
>>> --
>>> Chuck Lever
> 
> --
> Chuck Lever
> 
> 
> 

Re: [PATCH 1/5] Implement reexport helper library

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Steve Dickson" <steved@redhat.com>
>> I'll bite. What is the added footprint of this patch
>> series? I didn't think there was a new library dependency
>> or anything like that...
> Well mountd is now using an database and there
> is a static reexport lib that a number daemons
> and command like with...
> 
> In the I would like to bring (or be able) bring the
> foot print of nfs-utils down so it will be more container
> friendly... IDK if is possible... but that is the idea.

Getting rid of the SQLite dependency shouldn't be a big deal.
I'm already working on a plugin interface for reexport such that
a sysadmin can easily use other databases than SQLite.

E.g. in nfs.conf you point to a shared library which hides all the
database stuff from you.

Thanks,
//richard

Re: [PATCH 1/5] Implement reexport helper library

[Chuck Lever III]

> On May 10, 2022, at 4:08 PM, Steve Dickson <steved@redhat.com> wrote:
> 
> On 5/10/22 10:17 AM, Chuck Lever III wrote:
>>> On May 10, 2022, at 10:04 AM, Steve Dickson <steved@redhat.com> wrote:
>>> 
>>> Hey!
>>> 
>>> On 5/10/22 9:48 AM, Chuck Lever III wrote:
>>>>> On May 10, 2022, at 9:32 AM, Steve Dickson <steved@redhat.com> wrote:
>>>>> 
>>>>> Hello,
>>>>> 
>>>>> On 5/2/22 4:50 AM, Richard Weinberger wrote:
>>>>>> This internal library contains code that will be used by various
>>>>>> tools within the nfs-utils package to deal better with NFS re-export,
>>>>>> especially cross mounts.
>>>>>> Signed-off-by: Richard Weinberger <richard@nod.at>
>>>>>> ---
>>>>>>  configure.ac                 |  12 ++
>>>>>>  support/Makefile.am          |   4 +
>>>>>>  support/reexport/Makefile.am |   6 +
>>>>>>  support/reexport/reexport.c  | 285 +++++++++++++++++++++++++++++++++++
>>>>>>  support/reexport/reexport.h  |  39 +++++
>>>>>>  5 files changed, 346 insertions(+)
>>>>>>  create mode 100644 support/reexport/Makefile.am
>>>>>>  create mode 100644 support/reexport/reexport.c
>>>>>>  create mode 100644 support/reexport/reexport.h
>>>>>> diff --git a/configure.ac b/configure.ac
>>>>>> index 93626d62..86bf8ba9 100644
>>>>>> --- a/configure.ac
>>>>>> +++ b/configure.ac
>>>>>> @@ -274,6 +274,17 @@ AC_ARG_ENABLE(nfsv4server,
>>>>>>  	fi
>>>>>>  	AM_CONDITIONAL(CONFIG_NFSV4SERVER, [test "$enable_nfsv4server" = "yes" ])
>>>>>>  +AC_ARG_ENABLE(reexport,
>>>>>> +	[AC_HELP_STRING([--enable-reexport],
>>>>>> +			[enable support for re-exporting NFS mounts  @<:@default=no@:>@])],
>>>>>> +	enable_reexport=$enableval,
>>>>>> +	enable_reexport="no")
>>>>>> +	if test "$enable_reexport" = yes; then
>>>>>> +		AC_DEFINE(HAVE_REEXPORT_SUPPORT, 1,
>>>>>> +                          [Define this if you want NFS re-export support compiled in])
>>>>>> +	fi
>>>>>> +	AM_CONDITIONAL(CONFIG_REEXPORT, [test "$enable_reexport" = "yes" ])
>>>>>> +
>>>>> To get this moving I'm going to add a --disable-reexport flag
>>>> Hi Steve, no-one has given a reason why disabling support
>>>> for re-exports would be necessary. Therefore, can't this
>>>> switch just be removed?
>>> The precedence has been that we used --disable-XXX flag
>>> a lot in configure.ac... -nfsv4, -nfsv41, etc...
>>> I'm just following along with that.
>> I get that... but no-one has given a technical reason
>> why disabling this code would even be necessary.
> Nobody has see the code yet... :-)
> 
> There is a lot of churn in these patch.
> 
>>> Yes, at this point, nobody is asking to turn it off but
>>> in future somebody may want to... due some security hole
>>> or just make the footprint of the package smaller.
>> I'll bite. What is the added footprint of this patch
>> series? I didn't think there was a new library dependency
>> or anything like that...
> Well mountd is now using an database and there
> is a static reexport lib that a number daemons
> and command like with...

nfs-utils already depends on sqlite for the nfsd
cltrack stuff, so I don't really see an additional
dependency for nfs-utils there.

However, there is a configure.ac switch that says
if there's no sqlite in the build environment,
then nfsdcltrack is entirely disabled. I guess
the build environment needs to deal correctly
with that situation and re-export support.


--
Chuck Lever

[PATCH 2/5] exports: Implement new export option reexport=

[Richard Weinberger]

When re-exporting a NFS volume it is mandatory to specify
either a UUID or numerical fsid= option because nfsd is unable
to derive a identifier on its own.

For NFS cross mounts this becomes a problem because nfsd also
needs a identifier for every crossed mount.
A common workaround is stating every single subvolume in the
exports list too.
But this defeats the purpose of the crossmnt option and is tedious.

This is where the reexport= tries to help.
It offers various strategies to automatically derive a identifier
for NFS volumes and sub volumes.
Each have their pros and cons.

Currently two modes are implemented:

1. auto-fsidnum
	In this mode mountd/exportd will create a new numerical fsid
	for a NFS volume and subvolume. The numbers are stored in a database
	such that the server will always use the same fsid.
	The entry in the exports file allowed to skip fsid= entiry but
	stating a UUID is allowed, if needed.

	This mode has the obvious downside that load balancing is not
	possible since multiple re-exporting NFS servers would generate
	different ids.

2. predefined-fsidnum
	This mode works just like auto-fsidnum but does not generate ids
	for you. It helps in the load balancing case. A system administrator
	has to manually maintain the database and install it on all re-exporting
	NFS servers. If you have a massive amount of subvolumes this mode
	will help because you don't have to bloat the exports list.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 support/export/Makefile.am  |  2 ++
 support/include/nfslib.h    |  1 +
 support/nfs/Makefile.am     |  1 +
 support/nfs/exports.c       | 68 +++++++++++++++++++++++++++++++++++++
 support/reexport/reexport.c | 65 +++++++++++++++++++++++++++++++++++
 systemd/Makefile.am         |  4 +++
 utils/exportfs/Makefile.am  |  6 ++++
 utils/exportfs/exportfs.c   | 11 ++++++
 utils/exportfs/exports.man  | 31 +++++++++++++++++
 utils/mount/Makefile.am     |  7 ++++
 10 files changed, 196 insertions(+)

diff --git a/support/export/Makefile.am b/support/export/Makefile.am
index eec737f6..7338e1c7 100644
--- a/support/export/Makefile.am
+++ b/support/export/Makefile.am
@@ -14,6 +14,8 @@ libexport_a_SOURCES = client.c export.c hostname.c \
 		      xtab.c mount_clnt.c mount_xdr.c \
 		      cache.c auth.c v4root.c fsloc.c \
 		      v4clients.c
+libexport_a_CPPFLAGS = $(AM_CPPFLAGS) $(CPPFLAGS) -I$(top_srcdir)/support/reexport
+
 BUILT_SOURCES 	= $(GENFILES)
 
 noinst_HEADERS = mount.h
diff --git a/support/include/nfslib.h b/support/include/nfslib.h
index 6faba71b..0465a1ff 100644
--- a/support/include/nfslib.h
+++ b/support/include/nfslib.h
@@ -85,6 +85,7 @@ struct exportent {
 	struct sec_entry e_secinfo[SECFLAVOR_COUNT+1];
 	unsigned int	e_ttl;
 	char *		e_realpath;
+	int		e_reexport;
 };
 
 struct rmtabent {
diff --git a/support/nfs/Makefile.am b/support/nfs/Makefile.am
index 67e3a8e1..2e1577cc 100644
--- a/support/nfs/Makefile.am
+++ b/support/nfs/Makefile.am
@@ -9,6 +9,7 @@ libnfs_la_SOURCES = exports.c rmtab.c xio.c rpcmisc.c rpcdispatch.c \
 		   svc_socket.c cacheio.c closeall.c nfs_mntent.c \
 		   svc_create.c atomicio.c strlcat.c strlcpy.c
 libnfs_la_LIBADD = libnfsconf.la
+libnfs_la_CPPFLAGS = $(AM_CPPFLAGS) $(CPPFLAGS) -I$(top_srcdir)/support/reexport
 
 libnfsconf_la_SOURCES = conffile.c xlog.c
 
diff --git a/support/nfs/exports.c b/support/nfs/exports.c
index 2c8f0752..bc2b1d93 100644
--- a/support/nfs/exports.c
+++ b/support/nfs/exports.c
@@ -31,6 +31,7 @@
 #include "xlog.h"
 #include "xio.h"
 #include "pseudoflavors.h"
+#include "reexport.h"
 
 #define EXPORT_DEFAULT_FLAGS	\
   (NFSEXP_READONLY|NFSEXP_ROOTSQUASH|NFSEXP_GATHERED_WRITES|NFSEXP_NOSUBTREECHECK)
@@ -103,6 +104,7 @@ static void init_exportent (struct exportent *ee, int fromkernel)
 	ee->e_nsqgids = 0;
 	ee->e_uuid = NULL;
 	ee->e_ttl = default_ttl;
+	ee->e_reexport = REEXP_NONE;
 }
 
 struct exportent *
@@ -302,6 +304,23 @@ putexportent(struct exportent *ep)
 	}
 	if (ep->e_uuid)
 		fprintf(fp, "fsid=%s,", ep->e_uuid);
+
+	if (ep->e_reexport) {
+		fprintf(fp, "reexport=");
+		switch (ep->e_reexport) {
+			case REEXP_AUTO_FSIDNUM:
+				fprintf(fp, "auto-fsidnum");
+				break;
+			case REEXP_PREDEFINED_FSIDNUM:
+				fprintf(fp, "predefined-fsidnum");
+				break;
+			default:
+				xlog(L_ERROR, "unknown reexport method %i", ep->e_reexport);
+				fprintf(fp, "none");
+		}
+		fprintf(fp, ",");
+	}
+
 	if (ep->e_mountpoint)
 		fprintf(fp, "mountpoint%s%s,",
 			ep->e_mountpoint[0]?"=":"", ep->e_mountpoint);
@@ -538,6 +557,7 @@ parseopts(char *cp, struct exportent *ep, int warn, int *had_subtree_opt_ptr)
 	char 	*flname = efname?efname:"command line";
 	int	flline = efp?efp->x_line:0;
 	unsigned int active = 0;
+	int saw_reexport = 0;
 
 	squids = ep->e_squids; nsquids = ep->e_nsquids;
 	sqgids = ep->e_sqgids; nsqgids = ep->e_nsqgids;
@@ -644,6 +664,13 @@ bad_option:
 			}
 		} else if (strncmp(opt, "fsid=", 5) == 0) {
 			char *oe;
+
+			if (saw_reexport) {
+				xlog(L_ERROR, "%s:%d: 'fsid=' has to be before 'reexport=' %s\n",
+				     flname, flline, opt);
+				goto bad_option;
+			}
+
 			if (strcmp(opt+5, "root") == 0) {
 				ep->e_fsid = 0;
 				setflags(NFSEXP_FSID, active, ep);
@@ -688,6 +715,47 @@ bad_option:
 			active = parse_flavors(opt+4, ep);
 			if (!active)
 				goto bad_option;
+		} else if (strncmp(opt, "reexport=", 9) == 0) {
+#ifdef HAVE_REEXPORT_SUPPORT
+			char *strategy = strchr(opt, '=');
+
+			if (!strategy) {
+				xlog(L_ERROR, "%s:%d: bad option %s\n",
+				     flname, flline, opt);
+				goto bad_option;
+			}
+			strategy++;
+
+			if (saw_reexport) {
+				xlog(L_ERROR, "%s:%d: only one 'reexport=' is allowed%s\n",
+				     flname, flline, opt);
+				goto bad_option;
+			}
+
+			if (strcmp(strategy, "auto-fsidnum") == 0) {
+				ep->e_reexport = REEXP_AUTO_FSIDNUM;
+			} else if (strcmp(strategy, "predefined-fsidnum") == 0) {
+				ep->e_reexport = REEXP_PREDEFINED_FSIDNUM;
+			} else if (strcmp(strategy, "none") == 0) {
+				ep->e_reexport = REEXP_NONE;
+			} else {
+				xlog(L_ERROR, "%s:%d: bad option %s\n",
+				     flname, flline, strategy);
+				goto bad_option;
+			}
+
+			if (reexpdb_apply_reexport_settings(ep, flname, flline) != 0)
+				goto bad_option;
+
+			if (ep->e_fsid)
+				setflags(NFSEXP_FSID, active, ep);
+
+			saw_reexport = 1;
+#else
+			xlog(L_ERROR, "%s:%d: 'reexport=' not available, rebuild with --enable-reexport\n",
+			     flname, flline);
+			goto bad_option;
+#endif
 		} else {
 			xlog(L_ERROR, "%s:%d: unknown keyword \"%s\"\n",
 					flname, flline, opt);
diff --git a/support/reexport/reexport.c b/support/reexport/reexport.c
index 5474a21f..a9529b2b 100644
--- a/support/reexport/reexport.c
+++ b/support/reexport/reexport.c
@@ -283,3 +283,68 @@ void reexpdb_uncover_subvolume(uint32_t fsidnum)
 
 	free(path);
 }
+
+/*
+ * reexpdb_apply_reexport_settings - Apply reexport specific settings to an exportent
+ *
+ * @ep: exportent to apply to
+ * @flname: Current export file, only useful for logging
+ * @flline: Current line, only useful for logging
+ *
+ * This is a helper function for applying reexport specific settings to an exportent.
+ * It searches a suitable fsid an sets @ep->e_fsid.
+ */
+int reexpdb_apply_reexport_settings(struct exportent *ep, char *flname, int flline)
+{
+	uint32_t fsidnum;
+	int found;
+	int ret = 0;
+
+	if (ep->e_reexport == REEXP_NONE)
+		goto out;
+
+	if (ep->e_uuid)
+		goto out;
+
+	/*
+	 * We do a lazy database init because we want to init the db only
+	 * when at least one reexport= option is present.
+	 */
+	if (reexpdb_init() != 0) {
+		ret = -1;
+		goto out;
+	}
+
+	found = reexpdb_fsidnum_by_path(ep->e_path, &fsidnum, 0);
+	if (!found) {
+		if (ep->e_reexport == REEXP_AUTO_FSIDNUM) {
+			found = reexpdb_fsidnum_by_path(ep->e_path, &fsidnum, 1);
+			if (!found) {
+				xlog(L_ERROR, "%s:%i: Unable to generate fsid for %s",
+				     flname, flline, ep->e_path);
+				ret = -1;
+				goto out;
+			}
+		} else {
+			if (!ep->e_fsid) {
+				xlog(L_ERROR, "%s:%i: Selected 'reexport=' mode requires either a UUID 'fsid=' or a numerical 'fsid=' or a reexport db entry %d",
+				     flname, flline, ep->e_fsid);
+				ret = -1;
+			}
+
+			goto out;
+		}
+	}
+
+	if (ep->e_fsid) {
+		if (ep->e_fsid != fsidnum) {
+			xlog(L_ERROR, "%s:%i: Selected 'reexport=' mode requires configured numerical 'fsid=' to agree with reexport db entry",
+			     flname, flline);
+			ret = -1;
+		}
+	} else {
+		ep->e_fsid = fsidnum;
+	}
+
+	return ret;
+}
diff --git a/systemd/Makefile.am b/systemd/Makefile.am
index e7f5d818..f254b218 100644
--- a/systemd/Makefile.am
+++ b/systemd/Makefile.am
@@ -69,6 +69,10 @@ nfs_server_generator_LDADD = ../support/export/libexport.a \
 			     ../support/misc/libmisc.a \
 			     $(LIBPTHREAD)
 
+if CONFIG_REEXPORT
+nfs_server_generator_LDADD += ../support/reexport/libreexport.a $(LIBSQLITE) -lrt
+endif
+
 rpc_pipefs_generator_LDADD = ../support/nfs/libnfs.la
 
 if INSTALL_SYSTEMD
diff --git a/utils/exportfs/Makefile.am b/utils/exportfs/Makefile.am
index 96524c72..451637a0 100644
--- a/utils/exportfs/Makefile.am
+++ b/utils/exportfs/Makefile.am
@@ -12,4 +12,10 @@ exportfs_LDADD = ../../support/export/libexport.a \
 		 ../../support/misc/libmisc.a \
 		 $(LIBWRAP) $(LIBNSL) $(LIBPTHREAD)
 
+if CONFIG_REEXPORT
+exportfs_LDADD += ../../support/reexport/libreexport.a $(LIBSQLITE) -lrt
+endif
+
+exportfs_CPPFLAGS = $(AM_CPPFLAGS) $(CPPFLAGS) -I$(top_srcdir)/support/reexport
+
 MAINTAINERCLEANFILES = Makefile.in
diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
index 6ba615d1..7f21edcf 100644
--- a/utils/exportfs/exportfs.c
+++ b/utils/exportfs/exportfs.c
@@ -38,6 +38,7 @@
 #include "exportfs.h"
 #include "xlog.h"
 #include "conffile.h"
+#include "reexport.h"
 
 static void	export_all(int verbose);
 static void	exportfs(char *arg, char *options, int verbose);
@@ -719,6 +720,16 @@ dump(int verbose, int export_format)
 				c = dumpopt(c, "fsid=%d", ep->e_fsid);
 			if (ep->e_uuid)
 				c = dumpopt(c, "fsid=%s", ep->e_uuid);
+			if (ep->e_reexport) {
+				switch (ep->e_reexport) {
+					case REEXP_AUTO_FSIDNUM:
+						c = dumpopt(c, "reexport=%s", "auto-fsidnum");
+						break;
+					case REEXP_PREDEFINED_FSIDNUM:
+						c = dumpopt(c, "reexport=%s", "predefined-fsidnum");
+						break;
+				}
+			}
 			if (ep->e_mountpoint)
 				c = dumpopt(c, "mountpoint%s%s",
 					    ep->e_mountpoint[0]?"=":"",
diff --git a/utils/exportfs/exports.man b/utils/exportfs/exports.man
index 54b3f877..ad2c2c59 100644
--- a/utils/exportfs/exports.man
+++ b/utils/exportfs/exports.man
@@ -420,6 +420,37 @@ will only work if all clients use a consistent security policy.  Note
 that early kernels did not support this export option, and instead
 enabled security labels by default.
 
+.TP
+.IR reexport= auto-fsidnum|predefined-fsidnum
+This option helps when a NFS share is re-exported. Since the NFS server
+needs a unique identifier for each exported filesystem and a NFS share
+cannot provide such, usually a manual fsid is needed.
+As soon
+.IR crossmnt
+is used manually assigning fsid won't work anymore. This is where this
+option becomes handy. It will automatically assign a numerical fsid
+to exported NFS shares. The fsid and path relations are stored in a SQLite
+database. If
+.IR auto-fsidnum
+is selected, the fsid is also autmatically allocated.
+.IR predefined-fsidnum
+assumes pre-allocated fsid numbers and will just look them up.
+This option depends also on the kernel, you will need at least kernel version
+5.19.
+Since
+.IR reexport=
+can automatically allocate and assign numerical fsids, it is no longer possible
+to have numerical fsids in other exports as soon this option is used in at least
+one export entry.
+
+The association between fsid numbers and paths is stored in a SQLite database.
+Don't edit or remove the database unless you know exactly what you're doing.
+.IR predefined-fsidnum
+is useful when you have used
+.IR auto-fsidnum
+before and don't want further entries stored.
+
+
 .SS User ID Mapping
 .PP
 .B nfsd
diff --git a/utils/mount/Makefile.am b/utils/mount/Makefile.am
index 3101f7ab..0268488c 100644
--- a/utils/mount/Makefile.am
+++ b/utils/mount/Makefile.am
@@ -32,6 +32,13 @@ mount_nfs_LDADD = ../../support/nfs/libnfs.la \
 		  ../../support/misc/libmisc.a \
 		  $(LIBTIRPC)
 
+if CONFIG_REEXPORT
+mount_nfs_LDADD += ../../support/reexport/libreexport.a \
+		   ../../support/misc/libmisc.a \
+		   $(LIBSQLITE) -lrt $(LIBPTHREAD)
+endif
+
+
 mount_nfs_SOURCES = $(mount_common)
 
 if CONFIG_LIBMOUNT
-- 
2.31.1

Re: [PATCH 2/5] exports: Implement new export option reexport=

[Steve Dickson]

Hey,

A compile error...

On 5/2/22 4:50 AM, Richard Weinberger wrote:
> When re-exporting a NFS volume it is mandatory to specify
> either a UUID or numerical fsid= option because nfsd is unable
> to derive a identifier on its own.
> 
> For NFS cross mounts this becomes a problem because nfsd also
> needs a identifier for every crossed mount.
> A common workaround is stating every single subvolume in the
> exports list too.
> But this defeats the purpose of the crossmnt option and is tedious.
> 
> This is where the reexport= tries to help.
> It offers various strategies to automatically derive a identifier
> for NFS volumes and sub volumes.
> Each have their pros and cons.
> 
> Currently two modes are implemented:
> 
> 1. auto-fsidnum
> 	In this mode mountd/exportd will create a new numerical fsid
> 	for a NFS volume and subvolume. The numbers are stored in a database
> 	such that the server will always use the same fsid.
> 	The entry in the exports file allowed to skip fsid= entiry but
> 	stating a UUID is allowed, if needed.
> 
> 	This mode has the obvious downside that load balancing is not
> 	possible since multiple re-exporting NFS servers would generate
> 	different ids.
> 
> 2. predefined-fsidnum
> 	This mode works just like auto-fsidnum but does not generate ids
> 	for you. It helps in the load balancing case. A system administrator
> 	has to manually maintain the database and install it on all re-exporting
> 	NFS servers. If you have a massive amount of subvolumes this mode
> 	will help because you don't have to bloat the exports list.
> 
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>   support/export/Makefile.am  |  2 ++
>   support/include/nfslib.h    |  1 +
>   support/nfs/Makefile.am     |  1 +
>   support/nfs/exports.c       | 68 +++++++++++++++++++++++++++++++++++++
>   support/reexport/reexport.c | 65 +++++++++++++++++++++++++++++++++++
>   systemd/Makefile.am         |  4 +++
>   utils/exportfs/Makefile.am  |  6 ++++
>   utils/exportfs/exportfs.c   | 11 ++++++
>   utils/exportfs/exports.man  | 31 +++++++++++++++++
>   utils/mount/Makefile.am     |  7 ++++
>   10 files changed, 196 insertions(+)
> 
> diff --git a/support/export/Makefile.am b/support/export/Makefile.am
> index eec737f6..7338e1c7 100644
> --- a/support/export/Makefile.am
> +++ b/support/export/Makefile.am
> @@ -14,6 +14,8 @@ libexport_a_SOURCES = client.c export.c hostname.c \
>   		      xtab.c mount_clnt.c mount_xdr.c \
>   		      cache.c auth.c v4root.c fsloc.c \
>   		      v4clients.c
> +libexport_a_CPPFLAGS = $(AM_CPPFLAGS) $(CPPFLAGS) -I$(top_srcdir)/support/reexport
> +
>   BUILT_SOURCES 	= $(GENFILES)
>   
>   noinst_HEADERS = mount.h
> diff --git a/support/include/nfslib.h b/support/include/nfslib.h
> index 6faba71b..0465a1ff 100644
> --- a/support/include/nfslib.h
> +++ b/support/include/nfslib.h
> @@ -85,6 +85,7 @@ struct exportent {
>   	struct sec_entry e_secinfo[SECFLAVOR_COUNT+1];
>   	unsigned int	e_ttl;
>   	char *		e_realpath;
> +	int		e_reexport;
>   };
>   
>   struct rmtabent {
> diff --git a/support/nfs/Makefile.am b/support/nfs/Makefile.am
> index 67e3a8e1..2e1577cc 100644
> --- a/support/nfs/Makefile.am
> +++ b/support/nfs/Makefile.am
> @@ -9,6 +9,7 @@ libnfs_la_SOURCES = exports.c rmtab.c xio.c rpcmisc.c rpcdispatch.c \
>   		   svc_socket.c cacheio.c closeall.c nfs_mntent.c \
>   		   svc_create.c atomicio.c strlcat.c strlcpy.c
>   libnfs_la_LIBADD = libnfsconf.la
> +libnfs_la_CPPFLAGS = $(AM_CPPFLAGS) $(CPPFLAGS) -I$(top_srcdir)/support/reexport
>   
>   libnfsconf_la_SOURCES = conffile.c xlog.c
>   
> diff --git a/support/nfs/exports.c b/support/nfs/exports.c
> index 2c8f0752..bc2b1d93 100644
> --- a/support/nfs/exports.c
> +++ b/support/nfs/exports.c
> @@ -31,6 +31,7 @@
>   #include "xlog.h"
>   #include "xio.h"
>   #include "pseudoflavors.h"
> +#include "reexport.h"
>   
>   #define EXPORT_DEFAULT_FLAGS	\
>     (NFSEXP_READONLY|NFSEXP_ROOTSQUASH|NFSEXP_GATHERED_WRITES|NFSEXP_NOSUBTREECHECK)
> @@ -103,6 +104,7 @@ static void init_exportent (struct exportent *ee, int fromkernel)
>   	ee->e_nsqgids = 0;
>   	ee->e_uuid = NULL;
>   	ee->e_ttl = default_ttl;
> +	ee->e_reexport = REEXP_NONE;
>   }
>   
>   struct exportent *
> @@ -302,6 +304,23 @@ putexportent(struct exportent *ep)
>   	}
>   	if (ep->e_uuid)
>   		fprintf(fp, "fsid=%s,", ep->e_uuid);
> +
> +	if (ep->e_reexport) {
> +		fprintf(fp, "reexport=");
> +		switch (ep->e_reexport) {
> +			case REEXP_AUTO_FSIDNUM:
> +				fprintf(fp, "auto-fsidnum");
> +				break;
> +			case REEXP_PREDEFINED_FSIDNUM:
> +				fprintf(fp, "predefined-fsidnum");
> +				break;
> +			default:
> +				xlog(L_ERROR, "unknown reexport method %i", ep->e_reexport);
> +				fprintf(fp, "none");
> +		}
> +		fprintf(fp, ",");
> +	}
> +
>   	if (ep->e_mountpoint)
>   		fprintf(fp, "mountpoint%s%s,",
>   			ep->e_mountpoint[0]?"=":"", ep->e_mountpoint);
> @@ -538,6 +557,7 @@ parseopts(char *cp, struct exportent *ep, int warn, int *had_subtree_opt_ptr)
>   	char 	*flname = efname?efname:"command line";
>   	int	flline = efp?efp->x_line:0;
>   	unsigned int active = 0;
> +	int saw_reexport = 0;
>   
>   	squids = ep->e_squids; nsquids = ep->e_nsquids;
>   	sqgids = ep->e_sqgids; nsqgids = ep->e_nsqgids;
> @@ -644,6 +664,13 @@ bad_option:
>   			}
>   		} else if (strncmp(opt, "fsid=", 5) == 0) {
>   			char *oe;
> +
> +			if (saw_reexport) {
> +				xlog(L_ERROR, "%s:%d: 'fsid=' has to be before 'reexport=' %s\n",
> +				     flname, flline, opt);
> +				goto bad_option;
> +			}
> +
>   			if (strcmp(opt+5, "root") == 0) {
>   				ep->e_fsid = 0;
>   				setflags(NFSEXP_FSID, active, ep);
> @@ -688,6 +715,47 @@ bad_option:
>   			active = parse_flavors(opt+4, ep);
>   			if (!active)
>   				goto bad_option;
> +		} else if (strncmp(opt, "reexport=", 9) == 0) {
> +#ifdef HAVE_REEXPORT_SUPPORT
> +			char *strategy = strchr(opt, '=');
> +
> +			if (!strategy) {
> +				xlog(L_ERROR, "%s:%d: bad option %s\n",
> +				     flname, flline, opt);
> +				goto bad_option;
> +			}
> +			strategy++;
> +
> +			if (saw_reexport) {
> +				xlog(L_ERROR, "%s:%d: only one 'reexport=' is allowed%s\n",
> +				     flname, flline, opt);
> +				goto bad_option;
> +			}
> +
> +			if (strcmp(strategy, "auto-fsidnum") == 0) {
> +				ep->e_reexport = REEXP_AUTO_FSIDNUM;
> +			} else if (strcmp(strategy, "predefined-fsidnum") == 0) {
> +				ep->e_reexport = REEXP_PREDEFINED_FSIDNUM;
> +			} else if (strcmp(strategy, "none") == 0) {
> +				ep->e_reexport = REEXP_NONE;
> +			} else {
> +				xlog(L_ERROR, "%s:%d: bad option %s\n",
> +				     flname, flline, strategy);
> +				goto bad_option;
> +			}
> +
> +			if (reexpdb_apply_reexport_settings(ep, flname, flline) != 0)
> +				goto bad_option;
> +
> +			if (ep->e_fsid)
> +				setflags(NFSEXP_FSID, active, ep);
> +
> +			saw_reexport = 1;
> +#else
> +			xlog(L_ERROR, "%s:%d: 'reexport=' not available, rebuild with --enable-reexport\n",
> +			     flname, flline);
> +			goto bad_option;
> +#endif
>   		} else {
>   			xlog(L_ERROR, "%s:%d: unknown keyword \"%s\"\n",
>   					flname, flline, opt);
> diff --git a/support/reexport/reexport.c b/support/reexport/reexport.c
> index 5474a21f..a9529b2b 100644
> --- a/support/reexport/reexport.c
> +++ b/support/reexport/reexport.c
> @@ -283,3 +283,68 @@ void reexpdb_uncover_subvolume(uint32_t fsidnum)
>   
>   	free(path);
>   }
> +
> +/*
> + * reexpdb_apply_reexport_settings - Apply reexport specific settings to an exportent
> + *
> + * @ep: exportent to apply to
> + * @flname: Current export file, only useful for logging
> + * @flline: Current line, only useful for logging
> + *
> + * This is a helper function for applying reexport specific settings to an exportent.
> + * It searches a suitable fsid an sets @ep->e_fsid.
> + */
> +int reexpdb_apply_reexport_settings(struct exportent *ep, char *flname, int flline)
> +{
> +	uint32_t fsidnum;
> +	int found;
> +	int ret = 0;
> +
> +	if (ep->e_reexport == REEXP_NONE)
> +		goto out;
> +
> +	if (ep->e_uuid)
> +		goto out;
> +
> +	/*
> +	 * We do a lazy database init because we want to init the db only
> +	 * when at least one reexport= option is present.
> +	 */
> +	if (reexpdb_init() != 0) {
> +		ret = -1;
> +		goto out;
> +	}
> +
> +	found = reexpdb_fsidnum_by_path(ep->e_path, &fsidnum, 0);
> +	if (!found) {
> +		if (ep->e_reexport == REEXP_AUTO_FSIDNUM) {
> +			found = reexpdb_fsidnum_by_path(ep->e_path, &fsidnum, 1);
> +			if (!found) {
> +				xlog(L_ERROR, "%s:%i: Unable to generate fsid for %s",
> +				     flname, flline, ep->e_path);
> +				ret = -1;
> +				goto out;
> +			}
> +		} else {
> +			if (!ep->e_fsid) {
> +				xlog(L_ERROR, "%s:%i: Selected 'reexport=' mode requires either a UUID 'fsid=' or a numerical 'fsid=' or a reexport db entry %d",
> +				     flname, flline, ep->e_fsid);
> +				ret = -1;
> +			}
> +
> +			goto out;
reexport.c: In function ‘reexpdb_apply_reexport_settings’:
reexport.c:335:25: error: label ‘out’ used but not defined
   335 |                         goto out;
       |                         ^~~~


> +		}
> +	}
> +
> +	if (ep->e_fsid) {
> +		if (ep->e_fsid != fsidnum) {
> +			xlog(L_ERROR, "%s:%i: Selected 'reexport=' mode requires configured numerical 'fsid=' to agree with reexport db entry",
> +			     flname, flline);
> +			ret = -1;
> +		}
> +	} else {
> +		ep->e_fsid = fsidnum;
> +	}
I'm assuming this is where the out needs to be

out:
> +	return ret;
> +}
> diff --git a/systemd/Makefile.am b/systemd/Makefile.am
> index e7f5d818..f254b218 100644
> --- a/systemd/Makefile.am
> +++ b/systemd/Makefile.am
> @@ -69,6 +69,10 @@ nfs_server_generator_LDADD = ../support/export/libexport.a \
>   			     ../support/misc/libmisc.a \
>   			     $(LIBPTHREAD)
>   
> +if CONFIG_REEXPORT
> +nfs_server_generator_LDADD += ../support/reexport/libreexport.a $(LIBSQLITE) -lrt
> +endif
> +
>   rpc_pipefs_generator_LDADD = ../support/nfs/libnfs.la
>   
>   if INSTALL_SYSTEMD
> diff --git a/utils/exportfs/Makefile.am b/utils/exportfs/Makefile.am
> index 96524c72..451637a0 100644
> --- a/utils/exportfs/Makefile.am
> +++ b/utils/exportfs/Makefile.am
> @@ -12,4 +12,10 @@ exportfs_LDADD = ../../support/export/libexport.a \
>   		 ../../support/misc/libmisc.a \
>   		 $(LIBWRAP) $(LIBNSL) $(LIBPTHREAD)
>   
> +if CONFIG_REEXPORT
> +exportfs_LDADD += ../../support/reexport/libreexport.a $(LIBSQLITE) -lrt
> +endif
> +
> +exportfs_CPPFLAGS = $(AM_CPPFLAGS) $(CPPFLAGS) -I$(top_srcdir)/support/reexport
> +
>   MAINTAINERCLEANFILES = Makefile.in
> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
> index 6ba615d1..7f21edcf 100644
> --- a/utils/exportfs/exportfs.c
> +++ b/utils/exportfs/exportfs.c
> @@ -38,6 +38,7 @@
>   #include "exportfs.h"
>   #include "xlog.h"
>   #include "conffile.h"
> +#include "reexport.h"
>   
>   static void	export_all(int verbose);
>   static void	exportfs(char *arg, char *options, int verbose);
> @@ -719,6 +720,16 @@ dump(int verbose, int export_format)
>   				c = dumpopt(c, "fsid=%d", ep->e_fsid);
>   			if (ep->e_uuid)
>   				c = dumpopt(c, "fsid=%s", ep->e_uuid);
> +			if (ep->e_reexport) {
> +				switch (ep->e_reexport) {
> +					case REEXP_AUTO_FSIDNUM:
> +						c = dumpopt(c, "reexport=%s", "auto-fsidnum");
> +						break;
> +					case REEXP_PREDEFINED_FSIDNUM:
> +						c = dumpopt(c, "reexport=%s", "predefined-fsidnum");
> +						break;
> +				}
> +			}
>   			if (ep->e_mountpoint)
>   				c = dumpopt(c, "mountpoint%s%s",
>   					    ep->e_mountpoint[0]?"=":"",
> diff --git a/utils/exportfs/exports.man b/utils/exportfs/exports.man
> index 54b3f877..ad2c2c59 100644
> --- a/utils/exportfs/exports.man
> +++ b/utils/exportfs/exports.man
> @@ -420,6 +420,37 @@ will only work if all clients use a consistent security policy.  Note
>   that early kernels did not support this export option, and instead
>   enabled security labels by default.
>   
> +.TP
> +.IR reexport= auto-fsidnum|predefined-fsidnum
> +This option helps when a NFS share is re-exported. Since the NFS server
> +needs a unique identifier for each exported filesystem and a NFS share
> +cannot provide such, usually a manual fsid is needed.
> +As soon
> +.IR crossmnt
> +is used manually assigning fsid won't work anymore. This is where this
> +option becomes handy. It will automatically assign a numerical fsid
> +to exported NFS shares. The fsid and path relations are stored in a SQLite
> +database. If
> +.IR auto-fsidnum
> +is selected, the fsid is also autmatically allocated.
> +.IR predefined-fsidnum
> +assumes pre-allocated fsid numbers and will just look them up.
> +This option depends also on the kernel, you will need at least kernel version
> +5.19.
> +Since
> +.IR reexport=
> +can automatically allocate and assign numerical fsids, it is no longer possible
> +to have numerical fsids in other exports as soon this option is used in at least
> +one export entry.
> +
> +The association between fsid numbers and paths is stored in a SQLite database.
> +Don't edit or remove the database unless you know exactly what you're doing.
> +.IR predefined-fsidnum
> +is useful when you have used
> +.IR auto-fsidnum
> +before and don't want further entries stored.
> +
> +
>   .SS User ID Mapping
>   .PP
>   .B nfsd
> diff --git a/utils/mount/Makefile.am b/utils/mount/Makefile.am
> index 3101f7ab..0268488c 100644
> --- a/utils/mount/Makefile.am
> +++ b/utils/mount/Makefile.am
> @@ -32,6 +32,13 @@ mount_nfs_LDADD = ../../support/nfs/libnfs.la \
>   		  ../../support/misc/libmisc.a \
>   		  $(LIBTIRPC)
>   
> +if CONFIG_REEXPORT
> +mount_nfs_LDADD += ../../support/reexport/libreexport.a \
> +		   ../../support/misc/libmisc.a \
> +		   $(LIBSQLITE) -lrt $(LIBPTHREAD)
> +endif
> +
> +
>   mount_nfs_SOURCES = $(mount_common)
>   
>   if CONFIG_LIBMOUNT

Re: [PATCH 2/5] exports: Implement new export option reexport=

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Steve Dickson" <steved@redhat.com>
> A compile error...
> reexport.c: In function ‘reexpdb_apply_reexport_settings’:
> reexport.c:335:25: error: label ‘out’ used but not defined
>   335 |                         goto out;
>       |                         ^~~~
> 
> 
>> +		}
>> +	}
>> +
>> +	if (ep->e_fsid) {
>> +		if (ep->e_fsid != fsidnum) {
>> +			xlog(L_ERROR, "%s:%i: Selected 'reexport=' mode requires configured
>> numerical 'fsid=' to agree with reexport db entry",
>> +			     flname, flline);
>> +			ret = -1;
>> +		}
>> +	} else {
>> +		ep->e_fsid = fsidnum;
>> +	}
> I'm assuming this is where the out needs to be
> 
> out:

Patch 3/5 adds the label.
Looks like I messed up something while reordering patches. ;-\

Do you want me to resend the patch series immediately or shall I want for further comments?

Thanks,
//richard

Re: [PATCH 2/5] exports: Implement new export option reexport=

[Steve Dickson]

On 5/10/22 12:06 PM, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
>> Von: "Steve Dickson" <steved@redhat.com>
>> A compile error...
>> reexport.c: In function ‘reexpdb_apply_reexport_settings’:
>> reexport.c:335:25: error: label ‘out’ used but not defined
>>    335 |                         goto out;
>>        |                         ^~~~
>>
>>
>>> +		}
>>> +	}
>>> +
>>> +	if (ep->e_fsid) {
>>> +		if (ep->e_fsid != fsidnum) {
>>> +			xlog(L_ERROR, "%s:%i: Selected 'reexport=' mode requires configured
>>> numerical 'fsid=' to agree with reexport db entry",
>>> +			     flname, flline);
>>> +			ret = -1;
>>> +		}
>>> +	} else {
>>> +		ep->e_fsid = fsidnum;
>>> +	}
>> I'm assuming this is where the out needs to be
>>
>> out:
> 
> Patch 3/5 adds the label.
> Looks like I messed up something while reordering patches. ;-\
NP... there were a couple linking errors that also fixed in patch
3... I figured it out.

> 
> Do you want me to resend the patch series immediately or shall I want for further comments?
No... I have the on a branch and at least the compile... cleanly...
Now on to testing! :-)

steved.

> 
> Thanks,
> //richard
> 

[PATCH 3/5] export: Implement logic behind reexport=

[Richard Weinberger]

This covers the cross mount case. When mountd/exportd detect
a cross mount on a re-exported NFS volume a identifier has to
be found to make nfsd happy.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 support/export/cache.c      | 71 +++++++++++++++++++++++++++++++++----
 support/reexport/reexport.c |  1 +
 utils/exportd/Makefile.am   |  8 ++++-
 utils/exportd/exportd.c     |  5 +++
 utils/mountd/Makefile.am    |  6 ++++
 utils/mountd/mountd.c       |  1 +
 utils/mountd/svc_run.c      |  6 ++++
 7 files changed, 91 insertions(+), 7 deletions(-)

diff --git a/support/export/cache.c b/support/export/cache.c
index a5823e92..307d183b 100644
--- a/support/export/cache.c
+++ b/support/export/cache.c
@@ -33,6 +33,7 @@
 #include "export.h"
 #include "pseudoflavors.h"
 #include "xcommon.h"
+#include "reexport.h"
 
 #ifdef HAVE_JUNCTION_SUPPORT
 #include "fsloc.h"
@@ -235,6 +236,16 @@ static void auth_unix_gid(int f)
 		xlog(L_ERROR, "auth_unix_gid: error writing reply");
 }
 
+static int match_crossmnt_fsidnum(uint32_t parsed_fsidnum, char *path)
+{
+	uint32_t fsidnum;
+
+	if (reexpdb_fsidnum_by_path(path, &fsidnum, 0) == 0)
+		return 0;
+
+	return fsidnum == parsed_fsidnum;
+}
+
 #ifdef USE_BLKID
 static const char *get_uuid_blkdev(char *path)
 {
@@ -684,8 +695,13 @@ static int match_fsid(struct parsed_fsid *parsed, nfs_export *exp, char *path)
 		goto match;
 	case FSID_NUM:
 		if (((exp->m_export.e_flags & NFSEXP_FSID) == 0 ||
-		     exp->m_export.e_fsid != parsed->fsidnum))
+		     exp->m_export.e_fsid != parsed->fsidnum)) {
+			if (exp->m_export.e_flags & NFSEXP_CROSSMOUNT &&
+			    match_crossmnt_fsidnum(parsed->fsidnum, path))
+				goto match;
+
 			goto nomatch;
+		}
 		goto match;
 	case FSID_UUID4_INUM:
 	case FSID_UUID16_INUM:
@@ -789,6 +805,9 @@ static void nfsd_fh(int f)
 			goto out;
 	}
 
+	if (parsed.fsidtype == FSID_NUM)
+		reexpdb_uncover_subvolume(parsed.fsidnum);
+
 	/* Now determine export point for this fsid/domain */
 	for (i=0 ; i < MCL_MAXTYPES; i++) {
 		nfs_export *next_exp;
@@ -932,7 +951,7 @@ static void write_fsloc(char **bp, int *blen, struct exportent *ep)
 	release_replicas(servers);
 }
 #endif
-static void write_secinfo(char **bp, int *blen, struct exportent *ep, int flag_mask)
+static void write_secinfo(char **bp, int *blen, struct exportent *ep, int flag_mask, int extra_flag)
 {
 	struct sec_entry *p;
 
@@ -947,11 +966,20 @@ static void write_secinfo(char **bp, int *blen, struct exportent *ep, int flag_m
 	qword_addint(bp, blen, p - ep->e_secinfo);
 	for (p = ep->e_secinfo; p->flav; p++) {
 		qword_addint(bp, blen, p->flav->fnum);
-		qword_addint(bp, blen, p->flags & flag_mask);
+		qword_addint(bp, blen, (p->flags | extra_flag) & flag_mask);
 	}
 
 }
 
+static int can_reexport_via_fsidnum(struct exportent *exp, struct statfs64 *st)
+{
+	if (st->f_type != 0x6969 /* NFS_SUPER_MAGIC */)
+		return 0;
+
+	return exp->e_reexport == REEXP_PREDEFINED_FSIDNUM ||
+	       exp->e_reexport == REEXP_AUTO_FSIDNUM;
+}
+
 static int dump_to_cache(int f, char *buf, int blen, char *domain,
 			 char *path, struct exportent *exp, int ttl)
 {
@@ -968,17 +996,48 @@ static int dump_to_cache(int f, char *buf, int blen, char *domain,
 	if (exp) {
 		int different_fs = strcmp(path, exp->e_path) != 0;
 		int flag_mask = different_fs ? ~NFSEXP_FSID : ~0;
+		int rc, do_fsidnum = 0;
+		uint32_t fsidnum = exp->e_fsid;
+
+		if (different_fs) {
+			struct statfs64 st;
+
+			rc = nfsd_path_statfs64(path, &st);
+			if (rc) {
+				xlog(L_WARNING, "unable to statfs %s", path);
+				errno = EINVAL;
+				return -1;
+			}
+
+			if (can_reexport_via_fsidnum(exp, &st)) {
+				do_fsidnum = 1;
+				flag_mask = ~0;
+			}
+		}
 
 		qword_adduint(&bp, &blen, now + exp->e_ttl);
-		qword_addint(&bp, &blen, exp->e_flags & flag_mask);
+
+		if (do_fsidnum) {
+			uint32_t search_fsidnum = 0;
+			if (reexpdb_fsidnum_by_path(path, &search_fsidnum,
+			    exp->e_reexport == REEXP_AUTO_FSIDNUM) == 0) {
+				errno = EINVAL;
+				return -1;
+			}
+			fsidnum = search_fsidnum;
+			qword_addint(&bp, &blen, exp->e_flags | NFSEXP_FSID);
+		} else {
+			qword_addint(&bp, &blen, exp->e_flags & flag_mask);
+		}
+
 		qword_addint(&bp, &blen, exp->e_anonuid);
 		qword_addint(&bp, &blen, exp->e_anongid);
-		qword_addint(&bp, &blen, exp->e_fsid);
+		qword_addint(&bp, &blen, fsidnum);
 
 #ifdef HAVE_JUNCTION_SUPPORT
 		write_fsloc(&bp, &blen, exp);
 #endif
-		write_secinfo(&bp, &blen, exp, flag_mask);
+		write_secinfo(&bp, &blen, exp, flag_mask, do_fsidnum ? NFSEXP_FSID : 0);
 		if (exp->e_uuid == NULL || different_fs) {
 			char u[16];
 			if ((exp->e_flags & flag_mask & NFSEXP_FSID) == 0 &&
diff --git a/support/reexport/reexport.c b/support/reexport/reexport.c
index a9529b2b..51e49834 100644
--- a/support/reexport/reexport.c
+++ b/support/reexport/reexport.c
@@ -346,5 +346,6 @@ int reexpdb_apply_reexport_settings(struct exportent *ep, char *flname, int flli
 		ep->e_fsid = fsidnum;
 	}
 
+out:
 	return ret;
 }
diff --git a/utils/exportd/Makefile.am b/utils/exportd/Makefile.am
index c95bdee7..b0ec9034 100644
--- a/utils/exportd/Makefile.am
+++ b/utils/exportd/Makefile.am
@@ -16,11 +16,17 @@ exportd_SOURCES = exportd.c
 exportd_LDADD = ../../support/export/libexport.a \
 			../../support/nfs/libnfs.la \
 			../../support/misc/libmisc.a \
-			$(OPTLIBS) $(LIBBLKID) $(LIBPTHREAD) -luuid
+			$(OPTLIBS) $(LIBBLKID) $(LIBPTHREAD) \
+			-luuid
+if CONFIG_REEXPORT
+exportd_LDADD += ../../support/reexport/libreexport.a $(LIBSQLITE) -lrt
+endif
 
 exportd_CPPFLAGS = $(AM_CPPFLAGS) $(CPPFLAGS) \
 		-I$(top_srcdir)/support/export
 
+exportd_CPPFLAGS += -I$(top_srcdir)/support/reexport
+
 MAINTAINERCLEANFILES = Makefile.in
 
 #######################################################################
diff --git a/utils/exportd/exportd.c b/utils/exportd/exportd.c
index 2dd12cb6..e076a295 100644
--- a/utils/exportd/exportd.c
+++ b/utils/exportd/exportd.c
@@ -22,6 +22,7 @@
 #include "conffile.h"
 #include "exportfs.h"
 #include "export.h"
+#include "reexport.h"
 
 extern void my_svc_run(void);
 
@@ -296,6 +297,10 @@ main(int argc, char **argv)
 	/* Open files now to avoid sharing descriptors among forked processes */
 	cache_open();
 	v4clients_init();
+	if (reexpdb_init() != 0) {
+		xlog(L_ERROR, "%s: Failed to init reexport database", __func__);
+		exit(1);
+	}
 
 	/* Process incoming upcalls */
 	cache_process_loop();
diff --git a/utils/mountd/Makefile.am b/utils/mountd/Makefile.am
index 13b25c90..569d335a 100644
--- a/utils/mountd/Makefile.am
+++ b/utils/mountd/Makefile.am
@@ -20,10 +20,16 @@ mountd_LDADD = ../../support/export/libexport.a \
 	       $(OPTLIBS) \
 	       $(LIBBSD) $(LIBWRAP) $(LIBNSL) $(LIBBLKID) -luuid $(LIBTIRPC) \
 	       $(LIBPTHREAD)
+if CONFIG_REEXPORT
+mountd_LDADD += ../../support/reexport/libreexport.a $(LIBSQLITE) -lrt
+endif
+
 mountd_CPPFLAGS = $(AM_CPPFLAGS) $(CPPFLAGS) \
 		  -I$(top_builddir)/support/include \
 		  -I$(top_srcdir)/support/export
 
+mountd_CPPFLAGS += -I$(top_srcdir)/support/reexport
+
 MAINTAINERCLEANFILES = Makefile.in
 
 #######################################################################
diff --git a/utils/mountd/mountd.c b/utils/mountd/mountd.c
index bcf749fa..8555d746 100644
--- a/utils/mountd/mountd.c
+++ b/utils/mountd/mountd.c
@@ -32,6 +32,7 @@
 #include "nfsd_path.h"
 #include "nfslib.h"
 #include "export.h"
+#include "reexport.h"
 
 extern void my_svc_run(void);
 
diff --git a/utils/mountd/svc_run.c b/utils/mountd/svc_run.c
index 167b9757..6b5f47f1 100644
--- a/utils/mountd/svc_run.c
+++ b/utils/mountd/svc_run.c
@@ -57,6 +57,7 @@
 #include <rpc/rpc_com.h>
 #endif
 #include "export.h"
+#include "reexport.h"
 
 void my_svc_run(void);
 
@@ -96,6 +97,11 @@ my_svc_run(void)
 	fd_set	readfds;
 	int	selret;
 
+	if (reexpdb_init() != 0) {
+		xlog(L_ERROR, "%s: Failed to init reexport database", __func__);
+		return;
+	}
+
 	for (;;) {
 
 		readfds = svc_fdset;
-- 
2.31.1

[PATCH 4/5] export: Avoid fsid= conflicts

[Richard Weinberger]

As soon reexport= is used, numerical fsids are automatically
assigned, therefore other fsid= options are no longer possible
without the risk of a conflict.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 support/export/export.c        | 27 +++++++++++++++++++++++++--
 systemd/nfs-server-generator.c | 14 ++++++++++++--
 utils/exportfs/exportfs.c      | 10 ++++++++--
 3 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/support/export/export.c b/support/export/export.c
index 03390dfc..c5e56b1a 100644
--- a/support/export/export.c
+++ b/support/export/export.c
@@ -25,6 +25,7 @@
 #include "exportfs.h"
 #include "nfsd_path.h"
 #include "xlog.h"
+#include "reexport.h"
 
 exp_hash_table exportlist[MCL_MAXTYPES] = {{NULL, {{NULL,NULL}, }}, }; 
 static int export_hash(char *);
@@ -115,6 +116,7 @@ export_read(char *fname, int ignore_hosts)
 	nfs_export		*exp;
 
 	int volumes = 0;
+	int reexport_found = 0;
 
 	setexportent(fname, "r");
 	while ((eep = getexportent(0,1)) != NULL) {
@@ -126,7 +128,25 @@ export_read(char *fname, int ignore_hosts)
 		}
 		else
 			warn_duplicated_exports(exp, eep);
+
+		if (eep->e_reexport)
+			reexport_found = 1;
+	}
+
+	if (reexport_found) {
+		for (int i = 0; i < MCL_MAXTYPES; i++) {
+			for (exp = exportlist[i].p_head; exp; exp = exp->m_next) {
+				if (exp->m_export.e_reexport)
+					continue;
+
+				if (exp->m_export.e_flags & NFSEXP_FSID) {
+					xlog(L_ERROR, "When a reexport= option is present no manully assigned numerical fsid= options are allowed");
+					return -1;
+				}
+			}
+		}
 	}
+
 	endexportent();
 
 	return volumes;
@@ -147,7 +167,7 @@ export_d_read(const char *dname, int ignore_hosts)
 	int n = 0, i;
 	struct dirent **namelist = NULL;
 	int volumes = 0;
-
+	int num_exports;
 
 	n = scandir(dname, &namelist, NULL, versionsort);
 	if (n < 0) {
@@ -186,7 +206,10 @@ export_d_read(const char *dname, int ignore_hosts)
 			continue;
 		}
 
-		volumes += export_read(fname, ignore_hosts);
+		num_exports = export_read(fname, ignore_hosts);
+		if (num_exports < 0)
+			return -1;
+		volumes += num_exports;
 	}
 
 	for (i = 0; i < n; i++)
diff --git a/systemd/nfs-server-generator.c b/systemd/nfs-server-generator.c
index eec98fd2..e4202954 100644
--- a/systemd/nfs-server-generator.c
+++ b/systemd/nfs-server-generator.c
@@ -89,6 +89,8 @@ int main(int argc, char *argv[])
 	struct list	*list = NULL;
 	FILE		*f, *fstab;
 	struct mntent	*mnt;
+	int		num_exports;
+	int		num_exports_d;
 
 	/* Avoid using any external services */
 	xlog_syslog(0);
@@ -102,8 +104,16 @@ int main(int argc, char *argv[])
 	path = alloca(strlen(argv[1]) + sizeof(dirbase) + sizeof(filebase));
 	if (!path)
 		exit(2);
-	if (export_read(_PATH_EXPORTS, 1) +
-	    export_d_read(_PATH_EXPORTS_D, 1) == 0)
+
+	num_exports = export_read(_PATH_EXPORTS, 1);
+	if (num_exports < 0)
+		exit(1);
+
+	num_exports_d = export_d_read(_PATH_EXPORTS_D, 1);
+	if (num_exports_d < 0)
+		exit(1);
+
+	if (num_exports + num_exports_d == 0)
 		/* Nothing is exported, so nothing to do */
 		exit(0);
 
diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
index 7f21edcf..7a67c4d3 100644
--- a/utils/exportfs/exportfs.c
+++ b/utils/exportfs/exportfs.c
@@ -206,8 +206,14 @@ main(int argc, char **argv)
 	atexit(release_lockfile);
 
 	if (f_export && ! f_ignore) {
-		if (! (export_read(_PATH_EXPORTS, 0) +
-		       export_d_read(_PATH_EXPORTS_D, 0))) {
+		int num_exports, num_exports_d;
+
+		num_exports = export_read(_PATH_EXPORTS, 0);
+		num_exports_d = export_d_read(_PATH_EXPORTS_D, 0);
+		if (num_exports < 0 || num_exports_d < 0)
+			return 1;
+
+		if (!(num_exports + num_exports_d)) {
 			if (f_verbose)
 				xlog(L_WARNING, "No file systems exported!");
 		}
-- 
2.31.1

[PATCH 5/5] reexport: Make state database location configurable

[Richard Weinberger]

With the database location configurable it is possible to
place the sqlite database on a shared filesystem.
That way the reexport state can be shared among multiple
re-exporting NFS servers.

Be careful with shared filesystems, SQLite assumes that file locking
works on such filesystems. Not all filesystems implement this
correctly.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 nfs.conf                    | 3 +++
 support/reexport/reexport.c | 5 ++++-
 systemd/nfs.conf.man        | 6 ++++++
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/nfs.conf b/nfs.conf
index 8c714ff7..f19c346a 100644
--- a/nfs.conf
+++ b/nfs.conf
@@ -96,3 +96,6 @@ rdma-port=20049
 #
 [svcgssd]
 # principal=
+
+[reexport]
+# sqlitedb=
diff --git a/support/reexport/reexport.c b/support/reexport/reexport.c
index 51e49834..61574fc5 100644
--- a/support/reexport/reexport.c
+++ b/support/reexport/reexport.c
@@ -12,6 +12,7 @@
 #include <unistd.h>
 
 #include "nfsd_path.h"
+#include "conffile.h"
 #include "nfslib.h"
 #include "reexport.h"
 #include "xcommon.h"
@@ -55,7 +56,9 @@ int reexpdb_init(void)
 	if (prng_init() != 0)
 		return -1;
 
-	ret = sqlite3_open_v2(REEXPDB_DBFILE, &db, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_FULLMUTEX, NULL);
+	ret = sqlite3_open_v2(conf_get_str_with_def("reexport", "sqlitedb", REEXPDB_DBFILE),
+			      &db, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_FULLMUTEX,
+			      NULL);
 	if (ret != SQLITE_OK) {
 		xlog(L_ERROR, "Unable to open reexport database: %s", sqlite3_errstr(ret));
 		return -1;
diff --git a/systemd/nfs.conf.man b/systemd/nfs.conf.man
index 4436a38a..afd2b3f8 100644
--- a/systemd/nfs.conf.man
+++ b/systemd/nfs.conf.man
@@ -295,6 +295,12 @@ Only
 .B debug=
 is recognized.
 
+.TP
+.B reexport
+Only
+.B sqlitedb=
+is recognized, path to the state database.
+
 .SH FILES
 .TP 10n
 .I /etc/nfs.conf
-- 
2.31.1

Re: [PATCH 0/5] nfs-utils: Improving NFS re-exports

[J. Bruce Fields]

On Mon, May 02, 2022 at 10:50:40AM +0200, Richard Weinberger wrote:
> This is the first non-RFC iteration of the NFS re-export
> improvement series for nfs-utils.
> While the kernel side[0] didn't change at all and is still small,
> the userspace side saw much more changes.
> 
> The core idea is adding new export option: reeport=
> Using reexport= it is possible to mark an export entry in the exports
> file explicitly as NFS re-export and select a strategy how unique
> identifiers should be provided.
> Currently two strategies are supported, "auto-fsidnum" and
> "predefined-fsidnum", both use a SQLite database as backend to keep
> track of generated ids.
> For a more detailed description see patch "exports: Implement new export option reexport=".
> I choose SQLite because nfs-utils already uses it and using SQL ids can nicely
> generated and maintained. It will also scale for large setups where the amount
> of subvolumes is high.
> 
> Beside of id generation this series also addresses the reboot problem.
> If the re-exporting NFS server reboots, uncovered NFS subvolumes are not yet
> mounted and file handles become stale.
> Now mountd/exportd keeps track of uncovered subvolumes and makes sure they get
> uncovered while nfsd starts.
> 
> The whole set of features is currently opt-in via --enable-reexport.

Can we remove that option before upstreaming?

For testing purposes it may makes sense to be able to turn the new code
on and off quickly.  But for something we're really going to support,
it's just another hurdle for users to jump through, and another case we
probably won't remember to test.  The export options themselves should
be enough configuration.

Anyway, basically sounds reasonable to me.  I'll try to give it a proper
review sometime, feel free to bug me if I don't get to it in a week or
so.

--b.


> I'm also not sure about the rearrangement of the reexport code,
> currently it is a helper library.
> 
> A typical export entry on a re-exporting server looks like:
> 	/nfs *(rw,no_root_squash,no_subtree_check,crossmnt,reexport=auto-fsidnum)
> reexport=auto-fsidnum will automatically assign an fsid= to /nfs and all
> uncovered subvolumes.
> 
> Richard Weinberger (5):
>   Implement reexport helper library
>   exports: Implement new export option reexport=
>   export: Implement logic behind reexport=
>   export: Avoid fsid= conflicts
>   reexport: Make state database location configurable
> 
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/rw/misc.git/log/?h=nfs_reexport_clean
> 
>  configure.ac                   |  12 ++
>  nfs.conf                       |   3 +
>  support/Makefile.am            |   4 +
>  support/export/Makefile.am     |   2 +
>  support/export/cache.c         |  71 ++++++-
>  support/export/export.c        |  27 ++-
>  support/include/nfslib.h       |   1 +
>  support/nfs/Makefile.am        |   1 +
>  support/nfs/exports.c          |  68 +++++++
>  support/reexport/Makefile.am   |   6 +
>  support/reexport/reexport.c    | 354 +++++++++++++++++++++++++++++++++
>  support/reexport/reexport.h    |  39 ++++
>  systemd/Makefile.am            |   4 +
>  systemd/nfs-server-generator.c |  14 +-
>  systemd/nfs.conf.man           |   6 +
>  utils/exportd/Makefile.am      |   8 +-
>  utils/exportd/exportd.c        |   5 +
>  utils/exportfs/Makefile.am     |   6 +
>  utils/exportfs/exportfs.c      |  21 +-
>  utils/exportfs/exports.man     |  31 +++
>  utils/mount/Makefile.am        |   7 +
>  utils/mountd/Makefile.am       |   6 +
>  utils/mountd/mountd.c          |   1 +
>  utils/mountd/svc_run.c         |   6 +
>  24 files changed, 690 insertions(+), 13 deletions(-)
>  create mode 100644 support/reexport/Makefile.am
>  create mode 100644 support/reexport/reexport.c
>  create mode 100644 support/reexport/reexport.h
> 
> -- 
> 2.31.1

Re: [PATCH 0/5] nfs-utils: Improving NFS re-exports

[Steve Dickson]

On 5/2/22 12:17 PM, J. Bruce Fields wrote:
> On Mon, May 02, 2022 at 10:50:40AM +0200, Richard Weinberger wrote:
>> This is the first non-RFC iteration of the NFS re-export
>> improvement series for nfs-utils.
>> While the kernel side[0] didn't change at all and is still small,
>> the userspace side saw much more changes.
>>
>> The core idea is adding new export option: reeport=
>> Using reexport= it is possible to mark an export entry in the exports
>> file explicitly as NFS re-export and select a strategy how unique
>> identifiers should be provided.
>> Currently two strategies are supported, "auto-fsidnum" and
>> "predefined-fsidnum", both use a SQLite database as backend to keep
>> track of generated ids.
>> For a more detailed description see patch "exports: Implement new export option reexport=".
>> I choose SQLite because nfs-utils already uses it and using SQL ids can nicely
>> generated and maintained. It will also scale for large setups where the amount
>> of subvolumes is high.
>>
>> Beside of id generation this series also addresses the reboot problem.
>> If the re-exporting NFS server reboots, uncovered NFS subvolumes are not yet
>> mounted and file handles become stale.
>> Now mountd/exportd keeps track of uncovered subvolumes and makes sure they get
>> uncovered while nfsd starts.
>>
>> The whole set of features is currently opt-in via --enable-reexport.
> 
> Can we remove that option before upstreaming?
> 
> For testing purposes it may makes sense to be able to turn the new code
> on and off quickly.  But for something we're really going to support,
> it's just another hurdle for users to jump through, and another case we
> probably won't remember to test.  The export options themselves should
> be enough configuration.
> 
> Anyway, basically sounds reasonable to me.  I'll try to give it a proper
> review sometime, feel free to bug me if I don't get to it in a week or
> so.
How about --disable-reexport? So it is on by default to help testing
but if there are issues we can turn it off...

steved.

> 
> --b.
> 
> 
>> I'm also not sure about the rearrangement of the reexport code,
>> currently it is a helper library.
>>
>> A typical export entry on a re-exporting server looks like:
>> 	/nfs *(rw,no_root_squash,no_subtree_check,crossmnt,reexport=auto-fsidnum)
>> reexport=auto-fsidnum will automatically assign an fsid= to /nfs and all
>> uncovered subvolumes.
>>
>> Richard Weinberger (5):
>>    Implement reexport helper library
>>    exports: Implement new export option reexport=
>>    export: Implement logic behind reexport=
>>    export: Avoid fsid= conflicts
>>    reexport: Make state database location configurable
>>
>> [0] https://git.kernel.org/pub/scm/linux/kernel/git/rw/misc.git/log/?h=nfs_reexport_clean
>>
>>   configure.ac                   |  12 ++
>>   nfs.conf                       |   3 +
>>   support/Makefile.am            |   4 +
>>   support/export/Makefile.am     |   2 +
>>   support/export/cache.c         |  71 ++++++-
>>   support/export/export.c        |  27 ++-
>>   support/include/nfslib.h       |   1 +
>>   support/nfs/Makefile.am        |   1 +
>>   support/nfs/exports.c          |  68 +++++++
>>   support/reexport/Makefile.am   |   6 +
>>   support/reexport/reexport.c    | 354 +++++++++++++++++++++++++++++++++
>>   support/reexport/reexport.h    |  39 ++++
>>   systemd/Makefile.am            |   4 +
>>   systemd/nfs-server-generator.c |  14 +-
>>   systemd/nfs.conf.man           |   6 +
>>   utils/exportd/Makefile.am      |   8 +-
>>   utils/exportd/exportd.c        |   5 +
>>   utils/exportfs/Makefile.am     |   6 +
>>   utils/exportfs/exportfs.c      |  21 +-
>>   utils/exportfs/exports.man     |  31 +++
>>   utils/mount/Makefile.am        |   7 +
>>   utils/mountd/Makefile.am       |   6 +
>>   utils/mountd/mountd.c          |   1 +
>>   utils/mountd/svc_run.c         |   6 +
>>   24 files changed, 690 insertions(+), 13 deletions(-)
>>   create mode 100644 support/reexport/Makefile.am
>>   create mode 100644 support/reexport/reexport.c
>>   create mode 100644 support/reexport/reexport.h
>>
>> -- 
>> 2.31.1
> 

Re: [PATCH 0/5] nfs-utils: Improving NFS re-exports

[Chuck Lever III]

> On May 2, 2022, at 6:46 PM, Steve Dickson <steved@redhat.com> wrote:
> 
> 
> 
> On 5/2/22 12:17 PM, J. Bruce Fields wrote:
>> On Mon, May 02, 2022 at 10:50:40AM +0200, Richard Weinberger wrote:
>>> This is the first non-RFC iteration of the NFS re-export
>>> improvement series for nfs-utils.
>>> While the kernel side[0] didn't change at all and is still small,
>>> the userspace side saw much more changes.
>>> 
>>> The core idea is adding new export option: reeport=
>>> Using reexport= it is possible to mark an export entry in the exports
>>> file explicitly as NFS re-export and select a strategy how unique
>>> identifiers should be provided.
>>> Currently two strategies are supported, "auto-fsidnum" and
>>> "predefined-fsidnum", both use a SQLite database as backend to keep
>>> track of generated ids.
>>> For a more detailed description see patch "exports: Implement new export option reexport=".
>>> I choose SQLite because nfs-utils already uses it and using SQL ids can nicely
>>> generated and maintained. It will also scale for large setups where the amount
>>> of subvolumes is high.
>>> 
>>> Beside of id generation this series also addresses the reboot problem.
>>> If the re-exporting NFS server reboots, uncovered NFS subvolumes are not yet
>>> mounted and file handles become stale.
>>> Now mountd/exportd keeps track of uncovered subvolumes and makes sure they get
>>> uncovered while nfsd starts.
>>> 
>>> The whole set of features is currently opt-in via --enable-reexport.
>> Can we remove that option before upstreaming?

Somehow I missed Bruce's comment, but I had the same thought.
The cover letter does not explain why someone would want to
build nfs-utils without re-export support. If there isn't a
good reason for needing this switch, IMO it can be removed.


>> For testing purposes it may makes sense to be able to turn the new code
>> on and off quickly.  But for something we're really going to support,
>> it's just another hurdle for users to jump through, and another case we
>> probably won't remember to test.  The export options themselves should
>> be enough configuration.
>> Anyway, basically sounds reasonable to me.  I'll try to give it a proper
>> review sometime, feel free to bug me if I don't get to it in a week or
>> so.
> How about --disable-reexport? So it is on by default to help testing
> but if there are issues we can turn it off...
> 
> steved.
> 
>> --b.
>>> I'm also not sure about the rearrangement of the reexport code,
>>> currently it is a helper library.
>>> 
>>> A typical export entry on a re-exporting server looks like:
>>> 	/nfs *(rw,no_root_squash,no_subtree_check,crossmnt,reexport=auto-fsidnum)
>>> reexport=auto-fsidnum will automatically assign an fsid= to /nfs and all
>>> uncovered subvolumes.
>>> 
>>> Richard Weinberger (5):
>>>   Implement reexport helper library
>>>   exports: Implement new export option reexport=
>>>   export: Implement logic behind reexport=
>>>   export: Avoid fsid= conflicts
>>>   reexport: Make state database location configurable
>>> 
>>> [0] https://git.kernel.org/pub/scm/linux/kernel/git/rw/misc.git/log/?h=nfs_reexport_clean
>>> 
>>>  configure.ac                   |  12 ++
>>>  nfs.conf                       |   3 +
>>>  support/Makefile.am            |   4 +
>>>  support/export/Makefile.am     |   2 +
>>>  support/export/cache.c         |  71 ++++++-
>>>  support/export/export.c        |  27 ++-
>>>  support/include/nfslib.h       |   1 +
>>>  support/nfs/Makefile.am        |   1 +
>>>  support/nfs/exports.c          |  68 +++++++
>>>  support/reexport/Makefile.am   |   6 +
>>>  support/reexport/reexport.c    | 354 +++++++++++++++++++++++++++++++++
>>>  support/reexport/reexport.h    |  39 ++++
>>>  systemd/Makefile.am            |   4 +
>>>  systemd/nfs-server-generator.c |  14 +-
>>>  systemd/nfs.conf.man           |   6 +
>>>  utils/exportd/Makefile.am      |   8 +-
>>>  utils/exportd/exportd.c        |   5 +
>>>  utils/exportfs/Makefile.am     |   6 +
>>>  utils/exportfs/exportfs.c      |  21 +-
>>>  utils/exportfs/exports.man     |  31 +++
>>>  utils/mount/Makefile.am        |   7 +
>>>  utils/mountd/Makefile.am       |   6 +
>>>  utils/mountd/mountd.c          |   1 +
>>>  utils/mountd/svc_run.c         |   6 +
>>>  24 files changed, 690 insertions(+), 13 deletions(-)
>>>  create mode 100644 support/reexport/Makefile.am
>>>  create mode 100644 support/reexport/reexport.c
>>>  create mode 100644 support/reexport/reexport.h
>>> 
>>> -- 
>>> 2.31.1
> 

--
Chuck Lever

Re: [PATCH 0/5] nfs-utils: Improving NFS re-exports

[Richard Weinberger]

Bruce,

----- Ursprüngliche Mail -----
> Von: "bfields" <bfields@fieldses.org>
>> The whole set of features is currently opt-in via --enable-reexport.
> 
> Can we remove that option before upstreaming?

What is the final resolution regarding this option?
I can think of embedded/memory constraint systems where the dependency on SQLite
is not wanted.

On the other hand, with my latest patch, the plugin interface, the could be solved
too.

> For testing purposes it may makes sense to be able to turn the new code
> on and off quickly.  But for something we're really going to support,
> it's just another hurdle for users to jump through, and another case we
> probably won't remember to test.  The export options themselves should
> be enough configuration.
> 
> Anyway, basically sounds reasonable to me.  I'll try to give it a proper
> review sometime, feel free to bug me if I don't get to it in a week or
> so.

*kind ping* :-)

Please also don't forget the kernel side of this work. It is small but still needed.
See: https://lore.kernel.org/linux-nfs/20220110184419.27665-1-richard@nod.at/
or
https://git.kernel.org/pub/scm/linux/kernel/git/rw/misc.git/log/?h=nfs_reexport_clean

Since the kernel changes don't change the ABI, it should not really matter which part
is merged first. Kernel or userspace. The only important part is stating the right
kernel version in the nfs-utils manpages.

Thanks,
//richard

Re: [PATCH 0/5] nfs-utils: Improving NFS re-exports

[Chuck Lever III]

> On May 23, 2022, at 3:53 AM, Richard Weinberger <richard@nod.at> wrote:
> 
> Bruce,
> 
> ----- Ursprüngliche Mail -----
>> Von: "bfields" <bfields@fieldses.org>
>>> The whole set of features is currently opt-in via --enable-reexport.
>> 
>> Can we remove that option before upstreaming?
> 
> What is the final resolution regarding this option?
> I can think of embedded/memory constraint systems where the dependency on SQLite
> is not wanted.
> 
> On the other hand, with my latest patch, the plugin interface, the could be solved
> too.
> 
>> For testing purposes it may makes sense to be able to turn the new code
>> on and off quickly.  But for something we're really going to support,
>> it's just another hurdle for users to jump through, and another case we
>> probably won't remember to test.  The export options themselves should
>> be enough configuration.
>> 
>> Anyway, basically sounds reasonable to me.  I'll try to give it a proper
>> review sometime, feel free to bug me if I don't get to it in a week or
>> so.
> 
> *kind ping* :-)
> 
> Please also don't forget the kernel side of this work. It is small but still needed.
> See: https://lore.kernel.org/linux-nfs/20220110184419.27665-1-richard@nod.at/
> or
> https://git.kernel.org/pub/scm/linux/kernel/git/rw/misc.git/log/?h=nfs_reexport_clean

In his review comments, Bruce asked if testing with NFSv4 has been done.

Also, can an idle client allow the re-export server to unmount an
automounted export?

Once you have NFSv4 results, the kernel patches need to be posted again
with Cc: linux-fsdevel@vger.kernel.org and autofs@vger.kernel.org


> Since the kernel changes don't change the ABI, it should not really matter which part
> is merged first. Kernel or userspace. The only important part is stating the right
> kernel version in the nfs-utils manpages.
> 
> Thanks,
> //richard

--
Chuck Lever

Re: [PATCH 0/5] nfs-utils: Improving NFS re-exports

[bfields]

On Mon, May 23, 2022 at 09:53:45AM +0200, Richard Weinberger wrote:
> Please also don't forget the kernel side of this work. It is small but
> still needed.  See:
> https://lore.kernel.org/linux-nfs/20220110184419.27665-1-richard@nod.at/
> or
> https://git.kernel.org/pub/scm/linux/kernel/git/rw/misc.git/log/?h=nfs_reexport_clean

The follow_down() code, at least, needs vfs review, maybe resend with a
cc to Al Viro and linux-fsdevel.

I'm not really familiar with how the vfs automounting code works.

> Since the kernel changes don't change the ABI, it should not really
> matter which part is merged first. Kernel or userspace. The only
> important part is stating the right kernel version in the nfs-utils
> manpages.

OK, as long as we make sure nothing annoying happens in the
old-kernel/new-nfs-utils case.  (Looks to me like it should be OK.)

--b.

Re: [PATCH 0/5] nfs-utils: Improving NFS re-exports

[bfields]

On Mon, May 23, 2022 at 09:53:45AM +0200, Richard Weinberger wrote:
> > Von: "bfields" <bfields@fieldses.org>
> > Anyway, basically sounds reasonable to me.  I'll try to give it a proper
> > review sometime, feel free to bug me if I don't get to it in a week or
> > so.
> 
> *kind ping* :-)

Anyway, yeah, still planning to, but I have a few other things I wanted
to get out of the way first.

--b.