From: Trond Myklebust <[email protected]>
It was failing because fcntl.h is not one of the standard
includes.
Signed-off-by: Trond Myklebust <[email protected]>
---
configure.ac | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/configure.ac b/configure.ac
index 00b32800c526..df88e58fd0d4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -531,7 +531,12 @@ AC_TYPE_PID_T
AC_TYPE_SIZE_T
AC_HEADER_TIME
AC_STRUCT_TM
-AC_CHECK_TYPES([struct file_handle])
+AC_CHECK_TYPES([struct file_handle], [], [], [[
+ #define _GNU_SOURCE
+ #include <sys/types.h>
+ #include <sys/stat.h>
+ #include <fcntl.h>
+ ]])
dnl *************************************************************
dnl Check for functions
--
2.25.2
From: Trond Myklebust <[email protected]>
cache_write() will set errno if it returns -1, so that callers can
handle errors appropriately, however dump_to_cache() needs to do
so too.
Signed-off-by: Trond Myklebust <[email protected]>
---
utils/mountd/cache.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index 94e9e44b46b9..0f323226b12a 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -936,12 +936,13 @@ static void write_secinfo(char **bp, int *blen, struct exportent *ep, int flag_m
}
-static int dump_to_cache(int f, char *buf, int buflen, char *domain,
+static int dump_to_cache(int f, char *buf, int blen, char *domain,
char *path, struct exportent *exp, int ttl)
{
char *bp = buf;
- int blen = buflen;
time_t now = time(0);
+ size_t buflen;
+ ssize_t err;
if (ttl <= 1)
ttl = DEFAULT_TTL;
@@ -974,8 +975,18 @@ static int dump_to_cache(int f, char *buf, int buflen, char *domain,
} else
qword_adduint(&bp, &blen, now + ttl);
qword_addeol(&bp, &blen);
- if (blen <= 0) return -1;
- if (cache_write(f, buf, bp - buf) != bp - buf) return -1;
+ if (blen <= 0) {
+ errno = ENOBUFS;
+ return -1;
+ }
+ buflen = bp - buf;
+ err = cache_write(f, buf, buflen);
+ if (err < 0)
+ return err;
+ if ((size_t)err != buflen) {
+ errno = ENOSPC;
+ return -1;
+ }
return 0;
}
--
2.25.2
From: Trond Myklebust <[email protected]>
In nfsd_fh(), if the error returned by the downcall is transient,
then we should ignore it. Only reject the export if the filesystem
path is truly not exportable.
This fixes a case where we can see spurious NFSERR_STALE errors
being returned by knfsd.
Signed-off-by: Trond Myklebust <[email protected]>
---
utils/mountd/cache.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index 0f323226b12a..79d3ee085a90 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -843,8 +843,14 @@ static void nfsd_fh(int f)
}
}
}
- if (found &&
- found->e_mountpoint &&
+
+ if (!found) {
+ /* The missing dev could be what we want, so just be
+ * quiet rather than returning stale yet
+ */
+ if (dev_missing)
+ goto out;
+ } else if (found->e_mountpoint &&
!is_mountpoint(found->e_mountpoint[0]?
found->e_mountpoint:
found->e_path)) {
@@ -855,17 +861,12 @@ static void nfsd_fh(int f)
*/
/* FIXME we need to make sure we re-visit this later */
goto out;
+ } else if (cache_export_ent(buf, sizeof(buf), dom, found, found_path) < 0) {
+ if (!path_lookup_error(errno))
+ goto out;
+ /* The kernel is saying the path is unexportable */
+ found = NULL;
}
- if (!found && dev_missing) {
- /* The missing dev could be what we want, so just be
- * quite rather than returning stale yet
- */
- goto out;
- }
-
- if (found)
- if (cache_export_ent(buf, sizeof(buf), dom, found, found_path) < 0)
- found = 0;
bp = buf; blen = sizeof(buf);
qword_add(&bp, &blen, dom);
--
2.25.2
From: Trond Myklebust <[email protected]>
Propagate errors from the stat() calls in match_fsid() so that the
caller can be more careful about how they are handled.
Signed-off-by: Trond Myklebust <[email protected]>
---
utils/mountd/cache.c | 45 +++++++++++++++++++++++++++++---------------
1 file changed, 30 insertions(+), 15 deletions(-)
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index 79d3ee085a90..6cba2883026f 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -659,55 +659,66 @@ static int parse_fsid(int fsidtype, int fsidlen, char *fsid,
return 0;
}
-static bool match_fsid(struct parsed_fsid *parsed, nfs_export *exp, char *path)
+static int match_fsid(struct parsed_fsid *parsed, nfs_export *exp, char *path)
{
struct stat stb;
int type;
char u[16];
if (nfsd_path_stat(path, &stb) != 0)
- return false;
+ goto path_error;
if (!S_ISDIR(stb.st_mode) && !S_ISREG(stb.st_mode))
- return false;
+ goto nomatch;
switch (parsed->fsidtype) {
case FSID_DEV:
case FSID_MAJOR_MINOR:
case FSID_ENCODE_DEV:
if (stb.st_ino != parsed->inode)
- return false;
+ goto nomatch;
if (parsed->major != major(stb.st_dev) ||
parsed->minor != minor(stb.st_dev))
- return false;
- return true;
+ goto nomatch;
+ goto match;
case FSID_NUM:
if (((exp->m_export.e_flags & NFSEXP_FSID) == 0 ||
exp->m_export.e_fsid != parsed->fsidnum))
- return false;
- return true;
+ goto nomatch;
+ goto match;
case FSID_UUID4_INUM:
case FSID_UUID16_INUM:
if (stb.st_ino != parsed->inode)
- return false;
+ goto nomatch;
goto check_uuid;
case FSID_UUID8:
case FSID_UUID16:
- if (!is_mountpoint(path))
- return false;
+ errno = 0;
+ if (!is_mountpoint(path)) {
+ if (!errno)
+ goto nomatch;
+ goto path_error;
+ }
check_uuid:
if (exp->m_export.e_uuid) {
get_uuid(exp->m_export.e_uuid, parsed->uuidlen, u);
if (memcmp(u, parsed->fhuuid, parsed->uuidlen) == 0)
- return true;
+ goto match;
}
else
for (type = 0;
uuid_by_path(path, type, parsed->uuidlen, u);
type++)
if (memcmp(u, parsed->fhuuid, parsed->uuidlen) == 0)
- return true;
+ goto match;
}
- return false;
+nomatch:
+ return 0;
+match:
+ return 1;
+path_error:
+ if (path_lookup_error(errno))
+ goto nomatch;
+ return -1;
}
static struct addrinfo *lookup_client_addr(char *dom)
@@ -815,8 +826,12 @@ static void nfsd_fh(int f)
exp->m_export.e_path))
dev_missing ++;
- if (!match_fsid(&parsed, exp, path))
+ switch(match_fsid(&parsed, exp, path)) {
+ case 0:
continue;
+ case -1:
+ goto out;
+ }
if (is_ipaddr_client(dom)
&& !ipaddr_client_matches(exp, ai))
continue;
--
2.25.2
On Thu, Apr 16, 2020 at 06:12:51PM -0400, [email protected] wrote:
> From: Trond Myklebust <[email protected]>
>
> In nfsd_fh(), if the error returned by the downcall is transient,
> then we should ignore it. Only reject the export if the filesystem
> path is truly not exportable.
> This fixes a case where we can see spurious NFSERR_STALE errors
> being returned by knfsd.
>
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> utils/mountd/cache.c | 25 +++++++++++++------------
> 1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
> index 0f323226b12a..79d3ee085a90 100644
> --- a/utils/mountd/cache.c
> +++ b/utils/mountd/cache.c
> @@ -843,8 +843,14 @@ static void nfsd_fh(int f)
> }
> }
> }
> - if (found &&
> - found->e_mountpoint &&
> +
> + if (!found) {
> + /* The missing dev could be what we want, so just be
> + * quiet rather than returning stale yet
> + */
> + if (dev_missing)
> + goto out;
> + } else if (found->e_mountpoint &&
> !is_mountpoint(found->e_mountpoint[0]?
> found->e_mountpoint:
> found->e_path)) {
> @@ -855,17 +861,12 @@ static void nfsd_fh(int f)
> */
> /* FIXME we need to make sure we re-visit this later */
> goto out;
> + } else if (cache_export_ent(buf, sizeof(buf), dom, found, found_path) < 0) {
> + if (!path_lookup_error(errno))
> + goto out;
> + /* The kernel is saying the path is unexportable */
> + found = NULL;
> }
> - if (!found && dev_missing) {
> - /* The missing dev could be what we want, so just be
> - * quite rather than returning stale yet
> - */
> - goto out;
> - }
> -
> - if (found)
> - if (cache_export_ent(buf, sizeof(buf), dom, found, found_path) < 0)
> - found = 0;
>
> bp = buf; blen = sizeof(buf);
> qword_add(&bp, &blen, dom);
Is everybody else better at reading this kind of patch? Between the
code reshuffling and the conditionals, it's almost totally opaque to me.
I'd have split it up something like the below.--b.
commit b0b623281017
Author: Trond Myklebust <[email protected]>
Date: Thu Apr 16 18:12:51 2020 -0400
mountd: Ignore transient and non-fatal filesystem errors in nfsd_fh()
In nfsd_fh(), if the error returned by the downcall is transient,
then we should ignore it. Only reject the export if the filesystem
path is truly not exportable.
This fixes a case where we can see spurious NFSERR_STALE errors
being returned by knfsd.
Signed-off-by: Trond Myklebust <[email protected]>
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index bcd4b3ce5bb2..0426b171f040 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -826,8 +826,12 @@ static void nfsd_fh(int f)
*/
/* FIXME we need to make sure we re-visit this later */
goto out;
- } else if (cache_export_ent(buf, sizeof(buf), dom, found, found_path) < 0)
- found = 0;
+ } else if (cache_export_ent(buf, sizeof(buf), dom, found, found_path) < 0) {
+ if (!path_lookup_error(errno))
+ goto out;
+ /* The kernel is saying the path is unexportable */
+ found = NULL;
+ }
bp = buf; blen = sizeof(buf);
qword_add(&bp, &blen, dom);
commit 931a2e77f954
Author: J. Bruce Fields <[email protected]>
Date: Mon Apr 27 13:22:30 2020 -0400
rearrange logic; no change in behavior
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index 8f54e37b7936..bcd4b3ce5bb2 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -808,8 +808,14 @@ static void nfsd_fh(int f)
}
}
}
- if (found &&
- found->e_mountpoint &&
+
+ if (!found) {
+ /* The missing dev could be what we want, so just be
+ * quiet rather than returning stale yet
+ */
+ if (dev_missing)
+ goto out;
+ } else if (found->e_mountpoint &&
!is_mountpoint(found->e_mountpoint[0]?
found->e_mountpoint:
found->e_path)) {
@@ -820,16 +826,7 @@ static void nfsd_fh(int f)
*/
/* FIXME we need to make sure we re-visit this later */
goto out;
- }
- if (!found && dev_missing) {
- /* The missing dev could be what we want, so just be
- * quite rather than returning stale yet
- */
- goto out;
- }
-
- if (found)
- if (cache_export_ent(buf, sizeof(buf), dom, found, found_path) < 0)
+ } else if (cache_export_ent(buf, sizeof(buf), dom, found, found_path) < 0)
found = 0;
bp = buf; blen = sizeof(buf);
Ack to the series, otherwise, looks good.--b.