Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 21 Jul 2023 10:22:37 +0200
From:      Peter Eriksson <pen@lysator.liu.se>
To:        FreeBSD FS <freebsd-fs@freebsd.org>
Cc:        kevans@freebsd.org, Rick Macklem <rick.macklem@gmail.com>
Subject:   Re: RFC: Patch for mountd to handle a database for exports
Message-ID:  <2D8B626E-82BB-410C-B7C7-35B4D3834A44@lysator.liu.se>
In-Reply-To: <CAM5tNy7GkMLgy-wjonVu%2BgOMnexoA2W8vSZvMo4NDikrg3-A1A@mail.gmail.com>
References:  <CAM5tNy7GkMLgy-wjonVu%2BgOMnexoA2W8vSZvMo4NDikrg3-A1A@mail.gmail.com>

index | next in thread | previous in thread | raw e-mail

[-- Attachment #1 --]
Hi!

Here’s a couple of updated patches with some bug fixes. I’ve also moved the database location for ZFS to /etc/zfs/exports.db to mirror the current location.

I also changed the patch for mountd so that for each “exports” source specified it first tries to open <path>.db which I think makes it easier to use (no need for the “-D” option and it supports multiple sources). Cleaner I think.


Btw, you can create the database manually from /etc/zfs/exports using “makemap” like this:
 
  makemap -f btree /etc/zfs/exports.db </etc/zfs/exports


Known “bugs”:
“V4:” 
Even though you could create an /etc/exports.db with the contents of /etc/exports it will fail with the “V4:” lines since the BTree database will sort the exported entries so that the V4: key will appear last and then mountd will complain when scanning the DB. 

Also when using the file /etc/exports you can either write “V4:/export -sec=sys” or “V4: /export -sec=sys” (with a space between V4: and the path) so this probably needs some special handling (can’t simply use “makemap -f btree /etc/exports.db </etc/exports” to create it. 

I did try to fix that but the code quickly got hairy so I didn’t like it. If we really want/need that I’m thinking of creating a special case for the V4: handling, sort of like prefixing the database key with a NUL byte or something (so that it will be sorted first). 


Multiline options - well, the current ZFS code doesn’t support it either so no change from the current setup but it would be nice to have.
  Ie, support for things like:
     /export -sec=krb5
     /export -sec=sys ro




[-- Attachment #2 --]
--- usr.sbin/mountd/mountd.c.ORIG	2023-07-19 00:12:56.318540000 +0200
+++ usr.sbin/mountd/mountd.c	2023-07-21 00:28:12.786275000 +0200
@@ -85,6 +85,15 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+
+#define USE_SHAREDB 1
+
+#ifdef USE_SHAREDB
+#include <db.h>
+#include <fcntl.h>
+#include <limits.h>
+#endif
+
 #include "pathnames.h"
 #include "mntopts.h"
 
@@ -233,7 +242,7 @@
 static void	free_grp(struct grouplist *);
 static void	free_host(struct hostlist *);
 static void	free_v4rootexp(void);
-static void	get_exportlist_one(int);
+static void	get_exportlist_one(int, void *);
 static void	get_exportlist(int);
 static void	insert_exports(struct exportlist *, struct exportlisthead *);
 static void	free_exports(struct exportlisthead *);
@@ -1545,11 +1554,85 @@
 static size_t linesize;
 static FILE *exp_file;
 
+#ifdef USE_SHAREDB
+static char *dat_path = NULL;
+static char *dat_rest = NULL;
+static DBT dat_k, dat_d;
+static unsigned int dat_p = 0;
+
+static int
+get_data(void *dbp)
+{
+	DB *sdb = (DB *) dbp;
+	int rc;
+
+	
+	if (dat_path && dat_rest && dat_p < dat_d.size) {
+		/* Handle multiple share options per path */
+		char *buf = (char *) dat_d.data;
+		int len;
+
+		while (dat_p < dat_d.size && buf[dat_p] == '\0')
+			++dat_p;
+		
+		len = strnlen(buf+dat_p, dat_d.size-dat_p);
+		if (len > 0) {
+			free(dat_rest);
+			dat_rest = strndup(buf+dat_p, len);
+			dat_p += len;
+			return 1;
+		}
+	}
+	
+	if (dat_path) {
+		free(dat_path);
+		dat_path = NULL;
+	}
+	if (dat_rest) {
+		free(dat_rest);
+		dat_rest = NULL;
+	}
+	
+ Again:
+	memset(&dat_k, 0, sizeof(dat_k));
+	memset(&dat_d, 0, sizeof(dat_d));
+
+	rc = sdb->seq(sdb, &dat_k, &dat_d, R_NEXT);
+	switch (rc) {
+	case 0:
+		if (dat_k.size > 0 && ((char *) dat_k.data)[0] != '/') {
+			syslog(LOG_ERR, "%.*s: Invalid export path (ignored)",
+			       (int) dat_k.size, (char *) dat_k.data);
+			goto Again;
+		}
+		
+		dat_path = strndup(dat_k.data, dat_k.size);
+		dat_rest = strndup(dat_d.data, dat_d.size);
+		dat_p = strlen(dat_rest);
+		return 1;
+		
+	case 1:
+		return 0;
+		
+	default:
+		syslog(LOG_ERR, "Error reading from sharedb: %s", strerror(errno));
+		return -1;
+	}
+}
+#else
+static int
+get_data(void *dbp) {
+	return -1;
+}
+
+#endif
+
 /*
  * Get the export list from one, currently open file
  */
 static void
-get_exportlist_one(int passno)
+get_exportlist_one(int passno,
+		   void *db)
 {
 	struct exportlist *ep;
 	struct grouplist *grp, *tgrp, *savgrp;
@@ -1563,14 +1646,16 @@
 	v4root_phase = 0;
 	anon.cr_groups = NULL;
 	dirhead = (struct dirlist *)NULL;
-	while (get_line()) {
-		if (debug)
-			warnx("got line %s", line);
-		cp = line;
-		nextfield(&cp, &endcp);
-		if (*cp == '#')
-			goto nextline;
-
+	while ((db == NULL ? get_line() : get_data(db)) > 0) {
+		if (db == NULL) {
+			if (debug)
+				warnx("got line %s", line);
+			cp = line;
+			nextfield(&cp, &endcp);
+			if (*cp == '#')
+				goto nextline;
+		}
+		
 		/*
 		 * Set defaults.
 		 */
@@ -1585,6 +1670,13 @@
 		ep = (struct exportlist *)NULL;
 		dirp = NULL;
 
+#if USE_SHAREDB
+		if (db) {
+			cp = dat_path;
+			len = strlen(dat_path);
+			endcp = dat_rest;
+		} else {
+#endif
 		/*
 		 * Handle the V4 root dir.
 		 */
@@ -1602,10 +1694,13 @@
 			nextfield(&cp, &endcp);
 		}
 
+		len = endcp-cp;
+#if USE_SHAREDB
+		}
+#endif
 		/*
 		 * Create new exports list entry
 		 */
-		len = endcp-cp;
 		tgrp = grp = get_grp();
 		while (len > 0) {
 			if (len > MNTNAMLEN) {
@@ -2103,13 +2198,31 @@
 	 */
 	done = 0;
 	for (i = 0; exnames[i] != NULL; i++) {
+#if USE_SHAREDB
+		char dbpath[MAXPATHLEN];
+		DB *sdb;
+		
+		snprintf(dbpath, sizeof(dbpath), "%s.db", exnames[i]);
+		sdb = dbopen(dbpath, O_RDONLY|O_SHLOCK, 0, DB_BTREE, NULL);
+		if (sdb) {
+			if (debug)
+				warnx("reading exports from DB %s", dbpath);
+			get_exportlist_one(passno, sdb);
+			sdb->close(sdb);
+			done++;
+			continue;
+		}
+		if (errno != ENOENT)
+			syslog(LOG_ERR, "%s: opening DB for reading: %s",
+			       dbpath, strerror(errno));
+#endif
 		if (debug)
 			warnx("reading exports from %s", exnames[i]);
 		if ((exp_file = fopen(exnames[i], "r")) == NULL) {
 			syslog(LOG_WARNING, "can't open %s", exnames[i]);
 			continue;
 		}
-		get_exportlist_one(passno);
+		get_exportlist_one(passno, NULL);
 		fclose(exp_file);
 		done++;
 	}

[-- Attachment #3 --]
--- sys/contrib/openzfs/lib/libshare/os/freebsd/nfs.c.ORIG	2023-07-19 23:46:53.058484000 +0200
+++ sys/contrib/openzfs/lib/libshare/os/freebsd/nfs.c	2023-07-20 17:01:15.349859000 +0200
@@ -53,6 +53,19 @@
 #define	ZFS_EXPORTS_FILE	"/etc/zfs/exports"
 #define	ZFS_EXPORTS_LOCK	ZFS_EXPORTS_FILE".lock"
 
+#define USE_SHAREDB 1
+#if USE_SHAREDB
+#include <sys/types.h>
+#include <db.h>
+#include <fcntl.h>
+#include <limits.h>
+
+#define ZFS_SHAREDB_FILE "/etc/zfs/exports.db"
+
+static DB *sharedb = NULL;
+static int sharedb_wrmode = 0;
+#endif
+
 static sa_fstype_t *nfs_fstype;
 
 static int nfs_lock_fd = -1;
@@ -275,9 +288,45 @@
 static int
 nfs_enable_share(sa_share_impl_t impl_share)
 {
-	char *filename = NULL;
 	int error;
+	
+#if USE_SHAREDB
+	DBT k, d;
+	int res;
 
+	char *shareopts = FSINFO(impl_share, nfs_fstype)->shareopts;
+	if (strcmp(shareopts, "on") == 0)
+		shareopts = "";
+
+	if (!sharedb || !sharedb_wrmode) {
+		if (sharedb) {
+			res = sharedb->close(sharedb);
+			if (res < 0)
+				fprintf(stderr, "nfs_enable_share: sharedb(\"%s\")->close: %s\n", ZFS_SHAREDB_FILE, strerror(errno));
+		}
+		sharedb = dbopen(ZFS_SHAREDB_FILE, O_RDWR|O_CREAT|O_EXLOCK, 0600, DB_BTREE, NULL);
+		if (!sharedb)
+			fprintf(stderr, "nfs_enable_share: dbopen(\"%s\", O_RDWR|O_CREAT|O_EXLOCK): %s\n", ZFS_SHAREDB_FILE, strerror(errno));
+	}
+	if (!sharedb)
+		return SA_SYSTEM_ERR;
+	
+	sharedb_wrmode = 1;
+	k.data = impl_share->sa_mountpoint;
+	k.size = strlen(k.data);
+
+	d.data = translate_opts(shareopts);
+	d.size = strlen(d.data);
+	
+	res = sharedb->put(sharedb, &k, &d, 0);
+	if (res < 0)
+		fprintf(stderr, "nfs_enable_share: sharedb(\"%s\")->put(\"%s\"): %s\n",
+			ZFS_SHAREDB_FILE, (char *) k.data, strerror(errno));
+
+	return (res == 0 ? SA_OK : SA_SYSTEM_ERR);
+#else
+	char *filename = NULL;
+
 	if ((filename = nfs_init_tmpfile()) == NULL)
 		return (SA_SYSTEM_ERR);
 
@@ -329,12 +378,41 @@
 	}
 	error = nfs_fini_tmpfile(filename);
 	nfs_exports_unlock();
+#endif
 	return (error);
 }
 
 static int
 nfs_disable_share(sa_share_impl_t impl_share)
 {
+#if USE_SHAREDB
+	DBT k;
+	int res;
+
+	if (!sharedb || !sharedb_wrmode) {
+		if (sharedb) {
+			res = sharedb->close(sharedb);
+			if (res < 0)
+				fprintf(stderr, "nfs_disable_share: sharedb(\"%s\")->close: %s\n", ZFS_SHAREDB_FILE, strerror(errno));
+		}
+		sharedb = dbopen(ZFS_SHAREDB_FILE, O_RDWR|O_CREAT|O_EXLOCK, 0600, DB_BTREE, NULL);
+		if (!sharedb)
+			fprintf(stderr, "nfs_disable_share: dbopen(\"%s\", O_RDWR|O_CREAT|O_EXLOCK): %s\n", ZFS_SHAREDB_FILE, strerror(errno));
+	}
+	if (!sharedb)
+		return SA_SYSTEM_ERR;
+	
+	sharedb_wrmode = 1;	
+	k.data = impl_share->sa_mountpoint;
+	k.size = strlen(k.data);
+
+	res = sharedb->del(sharedb, &k, 0);
+	if (res < 0)
+		fprintf(stderr, "nfs_disable_share: sharedb(\"%s\")->del(\"%s\"): %s\n",
+			ZFS_SHAREDB_FILE, (char *) k.data, strerror(errno));
+	
+	return (res == 0 ? SA_OK : SA_SYSTEM_ERR);
+#else
 	int error;
 	char *filename = NULL;
 
@@ -359,14 +437,39 @@
 	error = nfs_fini_tmpfile(filename);
 	nfs_exports_unlock();
 	return (error);
+#endif
 }
 
 static boolean_t
 nfs_is_shared(sa_share_impl_t impl_share)
 {
+#if USE_SHAREDB
+	DBT k, d;
+	int res;
+
+	if (!sharedb) {
+		sharedb = dbopen(ZFS_SHAREDB_FILE, O_RDONLY|O_SHLOCK, 0, DB_BTREE, NULL);
+		if (!sharedb && errno != ENOENT) {
+			fprintf(stderr, "dbopen(\"%s\", O_RDONLY|O_SHLOCK): %s\n", ZFS_SHAREDB_FILE, strerror(errno));
+		}
+	}
+	if (!sharedb)
+		return (B_FALSE);
+	
+	k.data = impl_share->sa_mountpoint;
+	k.size = strlen(k.data);
+
+	res = sharedb->get(sharedb, &k, &d, 0);
+	if (res < 0)
+		fprintf(stderr, "sharedb(\"%s\")->get(\"%s\"): %s\n",
+			ZFS_SHAREDB_FILE, (char *) k.data, strerror(errno));
+	if (res == 0)
+		return (B_TRUE);
+	return (B_FALSE);
+#else
+	char *mntpoint = impl_share->sa_mountpoint;
 	char *s, last, line[MAXLINESIZE];
 	size_t len;
-	char *mntpoint = impl_share->sa_mountpoint;
 	size_t mntlen = strlen(mntpoint);
 
 	FILE *fp = fopen(ZFS_EXPORTS_FILE, "re");
@@ -393,6 +496,7 @@
 	}
 	fclose(fp);
 	return (B_FALSE);
+#endif
 }
 
 static int
@@ -422,13 +526,30 @@
 {
 	struct pidfh *pfh;
 	pid_t mountdpid;
+	int res = SA_OK;
 
-start:
+#if USE_SHAREDB
+	if (sharedb) {
+		/* If ShareDB in use, close the database handle so any cached changes are flushed */
+		
+		res = sharedb->close(sharedb);
+		if (res < 0) {
+			fprintf(stderr, "nfs_commit_shares: sharedb(\"%s\")->close: %s\n",
+				ZFS_SHAREDB_FILE, strerror(errno));
+			res = SA_SYSTEM_ERR;
+		}
+		
+		sharedb = NULL;
+		sharedb_wrmode = 0;
+	}
+#endif
+
+ start:
 	pfh = pidfile_open(_PATH_MOUNTDPID, 0600, &mountdpid);
 	if (pfh != NULL) {
 		/* mountd(8) is not running. */
 		pidfile_remove(pfh);
-		return (SA_OK);
+		return res;
 	}
 	if (errno != EEXIST) {
 		/* Cannot open pidfile for some reason. */
@@ -441,7 +562,7 @@
 	}
 	/* We have mountd(8) PID in mountdpid variable. */
 	kill(mountdpid, SIGHUP);
-	return (SA_OK);
+	return res;
 }
 
 static const sa_share_ops_t nfs_shareops = {

[-- Attachment #4 --]
/*
 * share.c
 *
 * CLI utility to manipulate share DBs
 *
 * Version: 1.0.2 (2023-07-21)
 *
 * Copyright (c) 2023, Peter Eriksson <pen@lysator.liu.se>
 *
 * All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions are met:
 *
 * 1. Redistributions of source code must retain the above copyright notice, this
 *    list of conditions and the following disclaimer.
 *
 * 2. Redistributions in binary form must reproduce the above copyright notice,
 *    this list of conditions and the following disclaimer in the documentation
 *    and/or other materials provided with the distribution.
 *
 * 3. Neither the name of the copyright holder nor the names of its
 *    contributors may be used to endorse or promote products derived from
 *    this software without specific prior written permission.
 *
 * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
 * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
 * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
 * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
 * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
 * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
 * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
 * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
 * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 */

/*
 * Compile with: cc -O -o share share.c
 * Usage: ./share -h
 *
 * ShareDB data format (B-Tree):
 *   key  = mountpoint (without trailing NUL characters)
 *   data = mount options (per /etc/exports), multi-mountpoint options separated by NUL
 */

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <unistd.h>
#include <ctype.h>
#include <errno.h>
#include <sys/types.h>
#include <signal.h>
#include <db.h>
#include <fcntl.h>
#include <limits.h>


char *path_sharedb = "/etc/zfs/exports.db";
char *path_pidfile = "/var/run/mountd.pid";

int f_debug = 0;
int f_print = 0;
int f_signal = 0;
int f_add = 0;
int f_remove = 0;
int f_locked = 1;


void
signal_mountd(void) {
	FILE *fp;
	int pid;

	if (!path_pidfile)
		return;

	fp = fopen(path_pidfile, "r");
	if (!fp)
		return;

	if (fscanf(fp, "%u", &pid) == 1)
		kill(pid, SIGHUP);

	fclose(fp);
}


ssize_t
scan_dbt(DBT *d,
	 size_t size,
	 char *opts) {
	char *buf = d->data;
	size_t p = 0;

	while (p < d->size) {
		int len = strnlen(buf+p, d->size-p);

		if (len == size && memcmp(opts, buf+p, len) == 0)
			return p;

		p += len;

		if (p < d->size && buf[p] == '\0')
			++p;
	}

	return -1;
}

void
print_dbt(DBT *d) {
	char *buf = d->data;
	size_t p = 0;
	int i = 0;

	while (p < d->size) {
		int len = strnlen(buf+p, d->size-p);

		printf("%2d : %.*s\n", i++, len, buf+p);
		p += len;

		if (p < d->size && buf[p] == '\0')
			++p;
	}
}


int
argv2dbt(int argc,
	 char **argv,
	 DBT *d) {
	char *buf;
	size_t len, newsize;
	int i, n;


	n = 0;
	len = 0;
	for (i = 0; i < argc; i++) {
		char *ap = argv[i];
		int alen;

		/* Drop leading space */
		while (isspace(*ap))
			++ap;

		/* Drop trailing space */
		alen = strlen(ap);
		while (len > 0 && isspace(ap[alen-1]))
			--alen;

		if (!alen)
			continue;

		if (scan_dbt(d, alen, ap) >= 0) {
			printf("%s: Already in DBT\n", ap);
			continue;
		}

		if (d->data == NULL) {
			newsize = alen;
			d->data = malloc(newsize);
			buf = (char *) d->data;
		} else {
			newsize = d->size+1+alen;
			d->data = realloc(d->data, newsize);
			buf = (char *) d->data + d->size;
			*buf++ = '\0';
		}

		d->size = newsize;
		strncpy(buf, ap, alen);
		buf += alen;
	}

	print_dbt(d);
	return n;
}

void
print_data(size_t size,
	   void *data) {
	unsigned char *buf = (unsigned char *) data;
	size_t pos = 0;


	for (pos = 0; pos < size; pos++)
		printf("%c ", isprint(buf[pos]) ? buf[pos] : '?');
	putchar('\t');
	for (pos = 0; pos < size; pos++)
		printf("%02x ", buf[pos]);
	putchar('\n');
}

int
main(int argc,
     char *argv[]) {
	DB *db;
	DBT k, d;
	int i, j, rc;
	char *buf;
	int f_mode = O_RDONLY;


	for (i = 1; i < argc && argv[i][0] == '-'; i++) {
		for (j = 1; argv[i][j]; j++) {
			switch (argv[i][j]) {
			case 'h':
				printf("Usage:\n\t%s [<options>] [<dir> <opts> [... <opts>]]\n\n",
				       argv[0]);
				puts("Options:");
				puts("\t-h             Display this usage information");
				puts("\t-d             Increase debugging level");
				puts("\t-p             Increase print level");
				puts("\t-a             Append options");
				puts("\t-r             Remove share / share options");
				puts("\t-u             Unlocked database access");
				puts("\t-s             Send signal to mountd");
				puts("\t-D <path>      ShareDB path");
				puts("\t-P <path>      Mountd pidfile path");
				exit(0);

			case 'd':
				f_debug++;
				break;

			case 'u':
				f_locked = 0;
				break;

			case 'p':
				f_print++;
				break;

			case 'a':
				f_add++;
				break;

			case 'r':
				f_remove++;
				break;

			case 's':
				f_signal++;
				break;

			case 'D':
				if (argv[i][j+1])
					path_sharedb = strdup(argv[i]+j+1);
				else if (i+1 < argc && argv[i+1][0])
					path_sharedb = strdup(argv[++i]);
				else {
					fprintf(stderr, "%s: Error: Missing argument to -D\n",
						argv[0]);
					exit(1);
				}
				goto NextArg;

			case 'P':
				if (argv[i][j+1])
					path_pidfile = strdup(argv[i]+j+1);
				else if (i+1 < argc && argv[i+1][0])
					path_pidfile = strdup(argv[++i]);
				else {
					fprintf(stderr, "%s: Error: Missing argument to -P\n",
						argv[0]);
					exit(1);
				}
				goto NextArg;

			default:
				fprintf(stderr, "%s: Error: -%c: Invalid switch\n",
					argv[0], argv[i][j]);
				exit(1);
			}
		}
	NextArg:;
	}

	if (f_add || f_remove || i+1 < argc) {
		f_mode = O_RDWR;
		if (f_add || i+1 < argc)
			f_mode |= O_CREAT;
		if (f_locked)
			f_mode |= O_EXLOCK;
	} else if (f_locked)
		f_mode |= O_SHLOCK;

	while ((db = dbopen(path_sharedb, f_mode|O_NONBLOCK, 0600, DB_BTREE, NULL)) == NULL &&
	       errno == EWOULDBLOCK) {
		fprintf(stderr, "%s: Notice: %s: Database locked, retrying in 1s\n",
			argv[0], path_sharedb);
		sleep(1);
	}
	if (!db) {
		fprintf(stderr, "%s: Error: %s: Unable to open: %s\n",
			argv[0], path_sharedb, strerror(errno));
		exit(1);
	}

	if (i < argc) {
		k.size = strlen(argv[i]);
		k.data = argv[i];

		d.size = 0;
		d.data = NULL;

		if (f_remove) {
			if (i+1 == argc) {
				/* Delete the path */
				rc = db->del(db, &k, 0);
				if (rc != 0) {
					fprintf(stderr, "%s: Error: %s: %s: DB delete failed: %s\n",
						argv[0], path_sharedb, argv[i], strerror(errno));
					exit(1);
				}
			} else {
				/* Delete matching options */
				fprintf(stderr, "%s: Error: Deleting matching options not yet implemented\n",
					argv[0]);
				exit(1);
			}
		} else {
			if (f_add) {
				/* Try to get existing entry, ignore if it doesn't exist */
				if (db->get(db, &k, &d, 0) < 0) {
					fprintf(stderr, "%s: Error: %s: DB lookup failed: %s\n",
						argv[0], path_sharedb, strerror(errno));
					exit(1);
				}
				if (d.size > 0) {
					buf = malloc(d.size);
					if (!buf) {
						fprintf(stderr, "%s: Error: %lu: malloc: %s\n",
							argv[0], d.size, strerror(errno));
						exit(1);
					}
					memcpy(buf, d.data, d.size);
					d.data = buf;
				}

				if (argv2dbt(argc-i-1, argv+i+1, &d) < 0) {
					fprintf(stderr, "%s: Error: %s [%s]: Unable to extract options: %s\n",
						argv[0], argv[i], argv[i+1], strerror(errno));
					exit(1);
				}

				rc = db->put(db, &k, &d, 0);
				if (rc != 0) {
					fprintf(stderr, "%s: Error: %s: %s: DB update failed: %s\n",
						argv[0], path_sharedb, argv[i], strerror(errno));
					exit(1);
				}
			} else {
				/* Replace */
				if (argv2dbt(argc-i-1, argv+i+1, &d) < 0) {
					fprintf(stderr, "%s: Error: %s [%s]: Unable to extract options: %s\n",
						argv[0], argv[i], argv[i+1], strerror(errno));
					exit(1);
				}

				rc = db->put(db, &k, &d, 0);
				if (rc != 0) {
					fprintf(stderr, "%s: Error: %s: %s: DB update failed: %s\n",
						argv[0], path_sharedb, argv[i], strerror(errno));
					exit(1);
				}
			}
		}

		if (d.data)
			free(d.data);
	}

	if (f_signal)
		signal_mountd();

	if (f_print) {
		rc = db->seq(db, &k, &d, R_FIRST);
		while (rc == 0) {
			unsigned int p = 0;

			if (f_debug) {
				print_data(k.size, k.data);
				putchar('\n');
				print_data(d.size, d.data);
				putchar('\n');
			}

			for (p = 0; p < d.size; p++) {
				char *buf = (char *) d.data;
				int len = strnlen(buf+p, d.size-p);

				printf("%.*s\t%.*s\n", (int) k.size, (char *) k.data, len, buf+p);
				p += len;
			}

			rc = db->seq(db, &k, &d, R_NEXT);
		}
	}

	db->close(db);
	exit(0);
}

[-- Attachment #5 --]



(I also agree that the USE_SHAREDB probably should be removed, I just have that here for now so that I can quickly disable the code)

Regarding the patch to the zfs part - I’m not sure which way to go there - the current patch switches to always use the DB. But one could argue that the code could check for an existing /etc/zfs/exports.db and only use the DB-writing code if that already exists. That way it will support both the old way and the new way, but it requires an empty /etc/zfs/exports.db to be pre-created at initial boot time for it to start using it. 

- Peter


> On 21 Jul 2023, at 00:50, Rick Macklem <rick.macklem@gmail.com> wrote:
> 
> Hi,
> 
> Peter Eriksson has submitted an interesting patch that adds support
> for a database file for exports.  My understanding is that this improves
> performance when used with the ZFS share property for exporting a
> large number of file systems.
> 
> There are a couple of user visible issues that I'd like feedback from
> others. (Hopefully Peter can correct me if I get any of these wrong.)
> 
> - The patch uses a single database file and a new "-D" option to
>  specify it on the command line.
>  --> I think it might be less confusing to just put the database file(s)
>        in the exports list and identify them via a ".db" filename suffix.
>  What do others think?
> 
> The changes are #ifdef'd on USE_SHAREDB. I'm thinking that this
> change will be always built, so maybe USE_SHAREDB is not needed?
> (Obviously mountd(8)'s semantics will only changed if/when database
> file(s) are provided.)
> 
> Once I have feedback on the above, I will put a patch up on
> phabricator.
> 
> rick
> ps: I believe kevans@ has volunteered to shepperd the ZFS share
>      property changes.
> 

help

Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2D8B626E-82BB-410C-B7C7-35B4D3834A44>