[PATCH 0/8] Junctions should inherit parent's export options

[PATCH 0/8] Junctions should inherit parent's export options

[Chuck Lever]

Following suggestions by Steve and Bruce, I've at long last
implemented a mechanism by which junctions inherit the export
options of their parent exports.

While I was there, I took the opportunity to clean up the junction
resolution logic.

---

Chuck Lever (8):
      mountd: Simplify "no junction support" case
      mountd: Dynamically allocate exportent that represents junctions
      mountd: Add exportent_release()
      mountd: Junctions inherit parent export's options
      mountd: Add lookup_export_parent()
      mountd: Set e_fslocdata field directly
      mountd: Use static buffer when constructing junction export options
      mountd: Warn when a broken junction is encountered


 support/export/export.c    |   19 ++--
 support/include/exportfs.h |    1 
 utils/mountd/cache.c       |  217 ++++++++++++++++++++++++++++++--------------
 3 files changed, 162 insertions(+), 75 deletions(-)

-- 
Chuck Lever

[PATCH 1/8] mountd: Warn when a broken junction is encountered

[Chuck Lever]

A broken junction is a problem that administrators will want to
know about and correct.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 utils/mountd/cache.c |   19 ++++++++++++++-----
 1 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index 7d80432..942fdbd 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -975,8 +975,8 @@ static struct exportent *locations_to_export(struct jp_ops *ops,
 static struct exportent *invoke_junction_ops(void *handle,
 		const char *junction)
 {
+	struct exportent *exp = NULL;
 	nfs_fsloc_set_t locations;
-	struct exportent *exp;
 	enum jp_status status;
 	struct jp_ops *ops;
 	char *error;
@@ -1002,15 +1002,24 @@ static struct exportent *invoke_junction_ops(void *handle,
 	}
 
 	status = ops->jp_get_locations(junction, &locations);
-	if (status != JP_OK) {
-		xlog(D_GENERAL, "%s: failed to resolve %s: %s",
-			__func__, junction, ops->jp_error(status));
-		return NULL;
+	switch (status) {
+	case JP_OK:
+		break;
+	case JP_NOTJUNCTION:
+		xlog(D_GENERAL, "%s: %s is not a junction",
+			__func__, junction);
+		goto out;
+	default:
+		xlog(L_WARNING, "Dangling junction %s: %s",
+			junction, ops->jp_error(status));
+		goto out;
 	}
 
 	exp = locations_to_export(ops, locations, junction);
 
 	ops->jp_put_locations(locations);
+
+out:
 	ops->jp_done();
 	return exp;
 }

[PATCH 2/8] mountd: Use static buffer when constructing junction export options

[Chuck Lever]

Clean up: Simplify locations_to_export() by constructing a junction's
export options in a static buffer.

We can do this because all of this code is called serially, in one
thread, and the result is thrown away immediately after the caller
is finished.  The returned exportent itself is static.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 utils/mountd/cache.c |   36 +++++-------------------------------
 1 files changed, 5 insertions(+), 31 deletions(-)

diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index 942fdbd..8de2eac 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -917,14 +917,15 @@ out_false:
  *
  * Returned exportent points to static memory.
  */
-static struct exportent *do_locations_to_export(struct jp_ops *ops,
-		nfs_fsloc_set_t locations, const char *junction,
-		char *options, size_t options_len)
+static struct exportent *locations_to_export(struct jp_ops *ops,
+		nfs_fsloc_set_t locations, const char *junction)
 {
+	static char options[BUFSIZ];
 	struct exportent *exp;
 	int ttl;
 
-	if (!locations_to_options(ops, locations, options, options_len, &ttl))
+	options[0] = '\0';
+	if (!locations_to_options(ops, locations, options, sizeof(options), &ttl))
 		return NULL;
 
 	exp = mkexportent("*", (char *)junction, options);
@@ -939,33 +940,6 @@ static struct exportent *do_locations_to_export(struct jp_ops *ops,
 }
 
 /*
- * Convert set of FS locations to an exportent.  Returns pointer to
- * an exportent if "junction" refers to a junction.
- *
- * Returned exportent points to static memory.
- */
-static struct exportent *locations_to_export(struct jp_ops *ops,
-		nfs_fsloc_set_t locations, const char *junction)
-{
-	struct exportent *exp;
-	char *options;
-
-	options = malloc(BUFSIZ);
-	if (options == NULL) {
-		xlog(D_GENERAL, "%s: failed to allocate options buffer",
-			__func__);
-		return NULL;
-	}
-	options[0] = '\0';
-
-	exp = do_locations_to_export(ops, locations, junction,
-						options, BUFSIZ);
-
-	free(options);
-	return exp;
-}
-
-/*
  * Retrieve locations information in "junction" and dump it to the
  * kernel.  Returns pointer to an exportent if "junction" refers
  * to a junction.

[PATCH 3/8] mountd: Set e_fslocdata field directly

[Chuck Lever]

To create an export entry for a junction, an options string is
constructed from the set of locations in the junction.  This options
string is then passed to mkexportent() where it is parsed and
converted into an exportent.

There is only one export option that is used to create a junction's
exportent: "refer=".  When that option is parsed, it's value is
simply copied to a fresh string and planted in the new export's
e_fslocdata field.

Let's avoid the option parsing and extra string copy.  Construct
a string for the new e_fslocdata field and plant it in the exportent
directly.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 utils/mountd/cache.c |   37 +++++++++++++++++++++++--------------
 1 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index 8de2eac..f63803b 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -831,12 +831,12 @@ lookup_export(char *dom, char *path, struct addrinfo *ai)
 #include <nfs-plugin.h>
 
 /*
- * Walk through a set of FS locations and build a set of export options.
+ * Walk through a set of FS locations and build an e_fslocdata string.
  * Returns true if all went to plan; otherwise, false.
  */
-static _Bool
-locations_to_options(struct jp_ops *ops, nfs_fsloc_set_t locations,
-		char *options, size_t remaining, int *ttl)
+static bool locations_to_fslocdata(struct jp_ops *ops,
+		nfs_fsloc_set_t locations, char *fslocdata,
+		size_t remaining, int *ttl)
 {
 	char *server, *last_path, *rootpath, *ptr;
 	_Bool seen = false;
@@ -844,7 +844,7 @@ locations_to_options(struct jp_ops *ops, nfs_fsloc_set_t locations,
 	last_path = NULL;
 	rootpath = NULL;
 	server = NULL;
-	ptr = options;
+	ptr = fslocdata;
 	*ttl = 0;
 
 	for (;;) {
@@ -870,14 +870,14 @@ locations_to_options(struct jp_ops *ops, nfs_fsloc_set_t locations,
 				goto out_false;
 			}
 			if ((size_t)len >= remaining) {
-				xlog(D_GENERAL, "%s: options buffer overflow", __func__);
+				xlog(D_GENERAL, "%s: fslocdata buffer overflow", __func__);
 				goto out_false;
 			}
 			remaining -= (size_t)len;
 			ptr += len;
 		} else {
 			if (last_path == NULL)
-				len = snprintf(ptr, remaining, "refer=%s@%s",
+				len = snprintf(ptr, remaining, "%s@%s",
 							rootpath, server);
 			else
 				len = snprintf(ptr, remaining, ":%s@%s",
@@ -887,7 +887,7 @@ locations_to_options(struct jp_ops *ops, nfs_fsloc_set_t locations,
 				goto out_false;
 			}
 			if ((size_t)len >= remaining) {
-				xlog(D_GENERAL, "%s: options buffer overflow",
+				xlog(D_GENERAL, "%s: fslocdata buffer overflow",
 					__func__);
 				goto out_false;
 			}
@@ -901,8 +901,8 @@ locations_to_options(struct jp_ops *ops, nfs_fsloc_set_t locations,
 		free(server);
 	}
 
-	xlog(D_CALL, "%s: options='%s', ttl=%d",
-		__func__, options, *ttl);
+	xlog(D_CALL, "%s: fslocdata='%s', ttl=%d",
+		__func__, fslocdata, *ttl);
 	return seen;
 
 out_false:
@@ -920,15 +920,16 @@ out_false:
 static struct exportent *locations_to_export(struct jp_ops *ops,
 		nfs_fsloc_set_t locations, const char *junction)
 {
-	static char options[BUFSIZ];
+	static char fslocdata[BUFSIZ];
 	struct exportent *exp;
 	int ttl;
 
-	options[0] = '\0';
-	if (!locations_to_options(ops, locations, options, sizeof(options), &ttl))
+	fslocdata[0] = '\0';
+	if (!locations_to_fslocdata(ops, locations,
+					fslocdata, sizeof(fslocdata), &ttl))
 		return NULL;
 
-	exp = mkexportent("*", (char *)junction, options);
+	exp = mkexportent("*", (char *)junction, "");
 	if (exp == NULL) {
 		xlog(L_ERROR, "%s: Failed to construct exportent", __func__);
 		return NULL;
@@ -936,6 +937,14 @@ static struct exportent *locations_to_export(struct jp_ops *ops,
 
 	exp->e_uuid = NULL;
 	exp->e_ttl = ttl;
+
+	free(exp->e_fslocdata);
+	exp->e_fslocmethod = FSLOC_REFER;
+	exp->e_fslocdata = strdup(fslocdata);
+	if (exp->e_fslocdata == NULL) {
+		xlog(L_ERROR, "%s: No memory", __func__);
+		return NULL;
+	}
 	return exp;
 }
 

[PATCH 4/8] mountd: Add lookup_export_parent()

[Chuck Lever]

In a moment I will be adding some logic that needs to know an
junction's parent export.

Here's a function that can discover an export's parent.  It takes
the target export's pathname, chops off the rightmost component, and
tries a lookup_export().  If that succeeds, we have our answer.
If not, it chops off the next rightmost component and tries again,
until the root is reached.

At the same time, infrastructure is added to pass the parent export
down into the functions that convert locations into a new junction
export entry.  For now the parent export remains unused.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 utils/mountd/cache.c |   79 ++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 70 insertions(+), 9 deletions(-)

diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index f63803b..b858e60 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -831,6 +831,60 @@ lookup_export(char *dom, char *path, struct addrinfo *ai)
 #include <nfs-plugin.h>
 
 /*
+ * Find the export entry for the parent of "pathname".
+ * Caller must not free returned exportent.
+ */
+static struct exportent *lookup_parent_export(char *dom,
+		const char *pathname, struct addrinfo *ai)
+{
+	char *parent, *slash;
+	nfs_export *result;
+
+	parent = strdup(pathname);
+	if (parent == NULL) {
+		xlog(D_GENERAL, "%s: failed to allocate parent path buffer",
+			__func__);
+		goto out_default;
+	}
+	xlog(D_CALL, "%s: pathname = '%s'", __func__, pathname);
+
+again:
+	/* shorten pathname by one component */
+	slash = strrchr(parent, '/');
+	if (slash == NULL) {
+		xlog(D_GENERAL, "%s: no slash found in pathname",
+			__func__);
+		goto out_default;
+	}
+	*slash = '\0';
+
+	if (strlen(parent) == 0) {
+		result = lookup_export(dom, "/", ai);
+		if (result == NULL) {
+			xlog(L_ERROR, "%s: no root export found.", __func__);
+			goto out_default;
+		}
+		goto out;
+	}
+
+	result = lookup_export(dom, parent, ai);
+	if (result == NULL) {
+		xlog(D_GENERAL, "%s: lookup_export(%s) found nothing",
+			__func__, parent);
+		goto again;
+	}
+
+out:
+	xlog(D_CALL, "%s: found export for %s", __func__, parent);
+	free(parent);
+	return &result->m_export;
+
+out_default:
+	free(parent);
+	return mkexportent("*", "/", "insecure");
+}
+
+/*
  * Walk through a set of FS locations and build an e_fslocdata string.
  * Returns true if all went to plan; otherwise, false.
  */
@@ -918,7 +972,8 @@ out_false:
  * Returned exportent points to static memory.
  */
 static struct exportent *locations_to_export(struct jp_ops *ops,
-		nfs_fsloc_set_t locations, const char *junction)
+		nfs_fsloc_set_t locations, const char *junction,
+		struct exportent *UNUSED(parent))
 {
 	static char fslocdata[BUFSIZ];
 	struct exportent *exp;
@@ -955,10 +1010,10 @@ static struct exportent *locations_to_export(struct jp_ops *ops,
  *
  * Returned exportent points to static memory.
  */
-static struct exportent *invoke_junction_ops(void *handle,
-		const char *junction)
+static struct exportent *invoke_junction_ops(void *handle, char *dom,
+		const char *junction, struct addrinfo *ai)
 {
-	struct exportent *exp = NULL;
+	struct exportent *parent, *exp = NULL;
 	nfs_fsloc_set_t locations;
 	enum jp_status status;
 	struct jp_ops *ops;
@@ -998,7 +1053,11 @@ static struct exportent *invoke_junction_ops(void *handle,
 		goto out;
 	}
 
-	exp = locations_to_export(ops, locations, junction);
+	parent = lookup_parent_export(dom, junction, ai);
+	if (parent == NULL)
+		goto out;
+
+	exp = locations_to_export(ops, locations, junction, parent);
 
 	ops->jp_put_locations(locations);
 
@@ -1014,7 +1073,8 @@ out:
  *
  * Returned exportent points to static memory.
  */
-static struct exportent *lookup_junction(const char *pathname)
+static struct exportent *lookup_junction(char *dom, const char *pathname,
+		struct addrinfo *ai)
 {
 	struct exportent *exp;
 	void *handle;
@@ -1026,7 +1086,7 @@ static struct exportent *lookup_junction(const char *pathname)
 	}
 	(void)dlerror();	/* Clear any error */
 
-	exp = invoke_junction_ops(handle, pathname);
+	exp = invoke_junction_ops(handle, dom, pathname, ai);
 
 	/* We could leave it loaded to make junction resolution
 	 * faster next time.  However, if we want to replace the
@@ -1035,7 +1095,8 @@ static struct exportent *lookup_junction(const char *pathname)
 	return exp;
 }
 #else	/* !HAVE_NFS_PLUGIN_H */
-static inline struct exportent *lookup_junction(const char *UNUSED(pathname))
+static inline struct exportent *lookup_junction(char *UNUSED(dom),
+		const char *UNUSED(pathname), struct addrinfo *UNUSED(ai))
 {
 	return NULL;
 }
@@ -1089,7 +1150,7 @@ static void nfsd_export(FILE *f)
 			dump_to_cache(f, dom, path, NULL);
 		}
 	} else {
-		dump_to_cache(f, dom, path, lookup_junction(path));
+		dump_to_cache(f, dom, path, lookup_junction(dom, path, ai));
 	}
  out:
 	xlog(D_CALL, "nfsd_export: found %p path %s", found, path ? path : NULL);

[PATCH 5/8] mountd: Junctions inherit parent export's options

[Chuck Lever]

Attempting to access junctions on a Linux NFS server from an NFS
client connected via an ephemeral source port fails with a "client
insecure" error on the server.  This happens even when the
"insecure" export option is specified on the junction's parent
export.

As a test, via a mountd code change, I added "insecure" to the fixed
export options that mountd sets up for each junction, and the error
disappeared.

It's simple enough for old-school referrals configured directly in
/etc/exports ("refer=") to have the needed options specified there.
Cache entries for junctions, however, are created on the fly by
mountd, and don't ever appear in /etc/exports.  So there's nowhere
obvious that export options for junctions can be specified.

Bruce suggested that in order to specify unique export options for
junctions, they should inherit the export options of their parent
export.  The junction's parent's exportent is duplicated in order
to create an exportent for the junction itself.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 utils/mountd/cache.c |   55 ++++++++++++++++++++++++++++++++------------------
 1 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index b858e60..9c2b972 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -966,6 +966,39 @@ out_false:
 }
 
 /*
+ * Duplicate the junction's parent's export options and graft in
+ * the fslocdata we constructed from the locations list.
+ *
+ * Returned exportent points to static memory.
+ */
+static struct exportent *create_junction_exportent(struct exportent *parent,
+		const char *junction, const char *fslocdata, int ttl)
+{
+	static struct exportent ee;
+
+	dupexportent(&ee, parent);
+	strcpy(ee.e_path, junction);
+	ee.e_hostname = strdup(parent->e_hostname);
+	if (ee.e_hostname == NULL)
+		goto out_nomem;
+	free(ee.e_uuid);
+	ee.e_uuid = NULL;
+	ee.e_ttl = (unsigned int)ttl;
+
+	free(ee.e_fslocdata);
+	ee.e_fslocmethod = FSLOC_REFER;
+	ee.e_fslocdata = strdup(fslocdata);
+	if (ee.e_fslocdata == NULL)
+		goto out_nomem;
+
+	return &ee;
+
+out_nomem:
+	xlog(L_ERROR, "%s: No memory", __func__);
+	return NULL;
+}
+
+/*
  * Walk through the set of FS locations and build an exportent.
  * Returns pointer to an exportent if "junction" refers to a junction.
  *
@@ -973,34 +1006,16 @@ out_false:
  */
 static struct exportent *locations_to_export(struct jp_ops *ops,
 		nfs_fsloc_set_t locations, const char *junction,
-		struct exportent *UNUSED(parent))
+		struct exportent *parent)
 {
 	static char fslocdata[BUFSIZ];
-	struct exportent *exp;
 	int ttl;
 
 	fslocdata[0] = '\0';
 	if (!locations_to_fslocdata(ops, locations,
 					fslocdata, sizeof(fslocdata), &ttl))
 		return NULL;
-
-	exp = mkexportent("*", (char *)junction, "");
-	if (exp == NULL) {
-		xlog(L_ERROR, "%s: Failed to construct exportent", __func__);
-		return NULL;
-	}
-
-	exp->e_uuid = NULL;
-	exp->e_ttl = ttl;
-
-	free(exp->e_fslocdata);
-	exp->e_fslocmethod = FSLOC_REFER;
-	exp->e_fslocdata = strdup(fslocdata);
-	if (exp->e_fslocdata == NULL) {
-		xlog(L_ERROR, "%s: No memory", __func__);
-		return NULL;
-	}
-	return exp;
+	return create_junction_exportent(parent, junction, fslocdata, ttl);
 }
 
 /*

[PATCH 6/8] mountd: Add exportent_release()

[Chuck Lever]

Split out the logic that releases dynamically allocated data in an
exportent.  The junction resolution code will invoke this to clean
up the junction exportent once it has been dumped to the kernel.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 support/export/export.c    |   19 ++++++++++++-------
 support/include/exportfs.h |    1 +
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/support/export/export.c b/support/export/export.c
index 4fda30a..17ceca9 100644
--- a/support/export/export.c
+++ b/support/export/export.c
@@ -31,16 +31,21 @@ static nfs_export *
 		export_allowed_internal(const struct addrinfo *ai,
 				const char *path);
 
+void
+exportent_release(struct exportent *eep)
+{
+	xfree(eep->e_squids);
+	xfree(eep->e_sqgids);
+	free(eep->e_mountpoint);
+	free(eep->e_fslocdata);
+	free(eep->e_uuid);
+	xfree(eep->e_hostname);
+}
+
 static void
 export_free(nfs_export *exp)
 {
-	xfree(exp->m_export.e_squids);
-	xfree(exp->m_export.e_sqgids);
-	free(exp->m_export.e_mountpoint);
-	free(exp->m_export.e_fslocdata);
-	free(exp->m_export.e_uuid);
-
-	xfree(exp->m_export.e_hostname);
+	exportent_release(&exp->m_export);
 	xfree(exp);
 }
 
diff --git a/support/include/exportfs.h b/support/include/exportfs.h
index 99916e5..5960feb 100644
--- a/support/include/exportfs.h
+++ b/support/include/exportfs.h
@@ -141,6 +141,7 @@ nfs_export *			export_find(const struct addrinfo *ai,
 nfs_export *			export_allowed(const struct addrinfo *ai,
 						const char *path);
 nfs_export *			export_create(struct exportent *, int canonical);
+void				exportent_release(struct exportent *);
 void				export_freeall(void);
 int				export_export(nfs_export *);
 int				export_unexport(nfs_export *);

[PATCH 7/8] mountd: Dynamically allocate exportent that represents junctions

[Chuck Lever]

We're now duplicating a real exportent with arbitrary export options
to create a junction exportent.  After a dupexportent() call,
several of the structure's fields can point to dynamically allocated
memory.  We have to be careful about not orphaning that memory.

What's more, returning a pointer to a static structure is as 90's as
a bad mullet.  It's more straightforward to allocate the exportent
dynamically and release it when we are through with it.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 utils/mountd/cache.c |   55 ++++++++++++++++++++++++++++----------------------
 1 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index 9c2b972..457fc9c 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -968,30 +968,36 @@ out_false:
 /*
  * Duplicate the junction's parent's export options and graft in
  * the fslocdata we constructed from the locations list.
- *
- * Returned exportent points to static memory.
  */
 static struct exportent *create_junction_exportent(struct exportent *parent,
 		const char *junction, const char *fslocdata, int ttl)
 {
-	static struct exportent ee;
+	static struct exportent *eep;
 
-	dupexportent(&ee, parent);
-	strcpy(ee.e_path, junction);
-	ee.e_hostname = strdup(parent->e_hostname);
-	if (ee.e_hostname == NULL)
-		goto out_nomem;
-	free(ee.e_uuid);
-	ee.e_uuid = NULL;
-	ee.e_ttl = (unsigned int)ttl;
-
-	free(ee.e_fslocdata);
-	ee.e_fslocmethod = FSLOC_REFER;
-	ee.e_fslocdata = strdup(fslocdata);
-	if (ee.e_fslocdata == NULL)
+	eep = (struct exportent *)malloc(sizeof(*eep));
+	if (eep == NULL)
 		goto out_nomem;
 
-	return &ee;
+	dupexportent(eep, parent);
+	strcpy(eep->e_path, junction);
+	eep->e_hostname = strdup(parent->e_hostname);
+	if (eep->e_hostname == NULL) {
+		free(eep);
+		goto out_nomem;
+	}
+	free(eep->e_uuid);
+	eep->e_uuid = NULL;
+	eep->e_ttl = (unsigned int)ttl;
+
+	free(eep->e_fslocdata);
+	eep->e_fslocdata = strdup(fslocdata);
+	if (eep->e_fslocdata == NULL) {
+		free(eep->e_hostname);
+		free(eep);
+		goto out_nomem;
+	}
+	eep->e_fslocmethod = FSLOC_REFER;
+	return eep;
 
 out_nomem:
 	xlog(L_ERROR, "%s: No memory", __func__);
@@ -1001,8 +1007,6 @@ out_nomem:
 /*
  * Walk through the set of FS locations and build an exportent.
  * Returns pointer to an exportent if "junction" refers to a junction.
- *
- * Returned exportent points to static memory.
  */
 static struct exportent *locations_to_export(struct jp_ops *ops,
 		nfs_fsloc_set_t locations, const char *junction,
@@ -1022,8 +1026,6 @@ static struct exportent *locations_to_export(struct jp_ops *ops,
  * Retrieve locations information in "junction" and dump it to the
  * kernel.  Returns pointer to an exportent if "junction" refers
  * to a junction.
- *
- * Returned exportent points to static memory.
  */
 static struct exportent *invoke_junction_ops(void *handle, char *dom,
 		const char *junction, struct addrinfo *ai)
@@ -1085,8 +1087,6 @@ out:
  * Load the junction plug-in, then try to resolve "pathname".
  * Returns pointer to an initialized exportent if "junction"
  * refers to a junction, or NULL if not.
- *
- * Returned exportent points to static memory.
  */
 static struct exportent *lookup_junction(char *dom, const char *pathname,
 		struct addrinfo *ai)
@@ -1165,7 +1165,14 @@ static void nfsd_export(FILE *f)
 			dump_to_cache(f, dom, path, NULL);
 		}
 	} else {
-		dump_to_cache(f, dom, path, lookup_junction(dom, path, ai));
+		struct exportent *eep;
+
+		eep = lookup_junction(dom, path, ai);
+		dump_to_cache(f, dom, path, eep);
+		if (eep != NULL) {
+			exportent_release(eep);
+			free(eep);
+		}
 	}
  out:
 	xlog(D_CALL, "nfsd_export: found %p path %s", found, path ? path : NULL);

[PATCH 8/8] mountd: Simplify "no junction support" case

[Chuck Lever]

We've added logic in the "not an export" case in nfsd_fh(), so it's
no longer a simple function call.  Clean up this code by splitting
it into a new function, and make plain what happens when junction
support is compiled out.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 utils/mountd/cache.c |   32 +++++++++++++++++++-------------
 1 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index 457fc9c..8280234 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -1109,11 +1109,24 @@ static struct exportent *lookup_junction(char *dom, const char *pathname,
 	(void)dlclose(handle);
 	return exp;
 }
+
+static void lookup_nonexport(FILE *f, char *dom, char *path,
+		struct addrinfo *ai)
+{
+	struct exportent *eep;
+
+	eep = lookup_junction(dom, path, ai);
+	dump_to_cache(f, dom, path, eep);
+	if (eep == NULL)
+		return;
+	exportent_release(eep);
+	free(eep);
+}
 #else	/* !HAVE_NFS_PLUGIN_H */
-static inline struct exportent *lookup_junction(char *UNUSED(dom),
-		const char *UNUSED(pathname), struct addrinfo *UNUSED(ai))
+static void lookup_nonexport(FILE *f, char *dom, char *path,
+		struct addrinfo *UNUSED(ai))
 {
-	return NULL;
+	dump_to_cache(f, dom, path, NULL);
 }
 #endif	/* !HAVE_NFS_PLUGIN_H */
 
@@ -1164,16 +1177,9 @@ static void nfsd_export(FILE *f)
 			     " or fsid= required", path);
 			dump_to_cache(f, dom, path, NULL);
 		}
-	} else {
-		struct exportent *eep;
-
-		eep = lookup_junction(dom, path, ai);
-		dump_to_cache(f, dom, path, eep);
-		if (eep != NULL) {
-			exportent_release(eep);
-			free(eep);
-		}
-	}
+	} else
+		lookup_nonexport(f, dom, path, ai);
+
  out:
 	xlog(D_CALL, "nfsd_export: found %p path %s", found, path ? path : NULL);
 	if (dom) free(dom);

Re: [PATCH 8/8] mountd: Simplify "no junction support" case

[Chuck Lever]

On Oct 11, 2012, at 11:11 AM, Chuck Lever wrote:

> We've added logic in the "not an export" case in nfsd_fh(), so it's

Oops, I meant nfsd_export() .

> no longer a simple function call.  Clean up this code by splitting
> it into a new function, and make plain what happens when junction
> support is compiled out.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> 
> utils/mountd/cache.c |   32 +++++++++++++++++++-------------
> 1 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
> index 457fc9c..8280234 100644
> --- a/utils/mountd/cache.c
> +++ b/utils/mountd/cache.c
> @@ -1109,11 +1109,24 @@ static struct exportent *lookup_junction(char *dom, const char *pathname,
> 	(void)dlclose(handle);
> 	return exp;
> }
> +
> +static void lookup_nonexport(FILE *f, char *dom, char *path,
> +		struct addrinfo *ai)
> +{
> +	struct exportent *eep;
> +
> +	eep = lookup_junction(dom, path, ai);
> +	dump_to_cache(f, dom, path, eep);
> +	if (eep == NULL)
> +		return;
> +	exportent_release(eep);
> +	free(eep);
> +}
> #else	/* !HAVE_NFS_PLUGIN_H */
> -static inline struct exportent *lookup_junction(char *UNUSED(dom),
> -		const char *UNUSED(pathname), struct addrinfo *UNUSED(ai))
> +static void lookup_nonexport(FILE *f, char *dom, char *path,
> +		struct addrinfo *UNUSED(ai))
> {
> -	return NULL;
> +	dump_to_cache(f, dom, path, NULL);
> }
> #endif	/* !HAVE_NFS_PLUGIN_H */
> 
> @@ -1164,16 +1177,9 @@ static void nfsd_export(FILE *f)
> 			     " or fsid= required", path);
> 			dump_to_cache(f, dom, path, NULL);
> 		}
> -	} else {
> -		struct exportent *eep;
> -
> -		eep = lookup_junction(dom, path, ai);
> -		dump_to_cache(f, dom, path, eep);
> -		if (eep != NULL) {
> -			exportent_release(eep);
> -			free(eep);
> -		}
> -	}
> +	} else
> +		lookup_nonexport(f, dom, path, ai);
> +
>  out:
> 	xlog(D_CALL, "nfsd_export: found %p path %s", found, path ? path : NULL);
> 	if (dom) free(dom);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH 0/8] Junctions should inherit parent's export options

[J. Bruce Fields]

On Thu, Oct 11, 2012 at 11:10:39AM -0400, Chuck Lever wrote:
> Following suggestions by Steve and Bruce, I've at long last
> implemented a mechanism by which junctions inherit the export
> options of their parent exports.

Thanks!

> While I was there, I took the opportunity to clean up the junction
> resolution logic.

Are bad mullets really specifically a 90's thing?  I thought they were
sort of timeless.

Anyway, that aside, looks reasonable to me.  ACK.

(I still can't get used to ACK.  Whenever I see it I wonder what the
writer's so startled about.  ACK!)

--b.

Re: [PATCH 0/8] Junctions should inherit parent's export options

[Chuck Lever]

Sent from my iPad

On Oct 15, 2012, at 6:24 PM, "J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Thu, Oct 11, 2012 at 11:10:39AM -0400, Chuck Lever wrote:
>> Following suggestions by Steve and Bruce, I've at long last
>> implemented a mechanism by which junctions inherit the export
>> options of their parent exports.
> 
> Thanks!
> 
>> While I was there, I took the opportunity to clean up the junction
>> resolution logic.
> 
> Are bad mullets really specifically a 90's thing?  I thought they were
> sort of timeless.

I could have referenced MacGyver, but I never watched the show.

> Anyway, that aside, looks reasonable to me.  ACK.

Thanks for the review.

Steve, I think you can take these now, if you are happy with them.

> (I still can't get used to ACK.  Whenever I see it I wonder what the
> writer's so startled about.  ACK!)
> 
> --b.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8] Junctions should inherit parent's export options

[Steve Dickson]

On 11/10/12 11:10, Chuck Lever wrote:
> Following suggestions by Steve and Bruce, I've at long last
> implemented a mechanism by which junctions inherit the export
> options of their parent exports.
> 
> While I was there, I took the opportunity to clean up the junction
> resolution logic.
> 
> ---
> 
> Chuck Lever (8):
>       mountd: Simplify "no junction support" case
>       mountd: Dynamically allocate exportent that represents junctions
>       mountd: Add exportent_release()
>       mountd: Junctions inherit parent export's options
>       mountd: Add lookup_export_parent()
>       mountd: Set e_fslocdata field directly
>       mountd: Use static buffer when constructing junction export options
>       mountd: Warn when a broken junction is encountered
> 
> 
>  support/export/export.c    |   19 ++--
>  support/include/exportfs.h |    1 
>  utils/mountd/cache.c       |  217 ++++++++++++++++++++++++++++++--------------
>  3 files changed, 162 insertions(+), 75 deletions(-)
> 
Committed....

steved.