2014-08-08 06:11:55

by Timo Teras

[permalink] [raw]
Subject: [PATCH nfs-utils] rework access to /proc/net/rpc

The kernel support only access by read() and write() with exactly
one line in the buffer. FILE can be implemented with readv() and
writev(), and the mapping to syscalls is not guaranteed. The code
already does lot of extra work setvbuf() and fflush() calls to try
to ensure this, but it relays on implementation details and is
non-portable. And e.g. with musl library the current hacks do not
work at all.

Remove the FILE based qword_* API as it's fundamentally broken,
and replace it with explicitly caching one line at a time. The
qword API should probable improved to do the line caching
internally, and this is the first step towards that.

Signed-off-by: Timo Teräs <[email protected]>
---
This along with Natanael Copa's patchset makes nfs-utils somewhat
usable with musl c-library.

support/include/exportfs.h | 1 +
support/include/nfslib.h | 7 -
support/include/nfsrpc.h | 1 +
support/nfs/cacheio.c | 111 +-------------
support/nfs/nfsexport.c | 77 ++++++----
utils/gssd/gssd_proc.c | 9 +-
utils/gssd/svcgssd.h | 2 +-
utils/gssd/svcgssd_main_loop.c | 9 +-
utils/gssd/svcgssd_proc.c | 51 ++++---
utils/gssd/write_bytes.h | 1 +
utils/mountd/cache.c | 336 ++++++++++++++++++++++-------------------
utils/nfsd/nfssvc.c | 1 +
12 files changed, 265 insertions(+), 341 deletions(-)

diff --git a/support/include/exportfs.h b/support/include/exportfs.h
index 9021fae..32711e4 100644
--- a/support/include/exportfs.h
+++ b/support/include/exportfs.h
@@ -10,6 +10,7 @@
#define EXPORTFS_H

#include <netdb.h>
+#include <string.h>

#include "sockaddr.h"
#include "nfslib.h"
diff --git a/support/include/nfslib.h b/support/include/nfslib.h
index ce4b14b..9fd22ac 100644
--- a/support/include/nfslib.h
+++ b/support/include/nfslib.h
@@ -152,11 +152,6 @@ struct nfs_fh_len * getfh(const struct sockaddr_in *sin, const char *path);
struct nfs_fh_len * getfh_size(const struct sockaddr_in *sin,
const char *path, int const size);

-void qword_print(FILE *f, char *str);
-void qword_printhex(FILE *f, char *str, int slen);
-void qword_printint(FILE *f, int num);
-int qword_eol(FILE *f);
-int readline(int fd, char **buf, int *lenp);
int qword_get(char **bpp, char *dest, int bufsize);
int qword_get_int(char **bpp, int *anint);
void cache_flush(int force);
@@ -167,8 +162,6 @@ void qword_addint(char **bpp, int *lp, int n);
void qword_adduint(char **bpp, int *lp, unsigned int n);
void qword_addeol(char **bpp, int *lp);
int qword_get_uint(char **bpp, unsigned int *anint);
-void qword_printuint(FILE *f, unsigned int num);
-void qword_printtimefrom(FILE *f, unsigned int num);

void closeall(int min);

diff --git a/support/include/nfsrpc.h b/support/include/nfsrpc.h
index 1bfae7a..fbbdb6a 100644
--- a/support/include/nfsrpc.h
+++ b/support/include/nfsrpc.h
@@ -23,6 +23,7 @@
#ifndef __NFS_UTILS_NFSRPC_H
#define __NFS_UTILS_NFSRPC_H

+#include <string.h>
#include <rpc/types.h>
#include <rpc/clnt.h>

diff --git a/support/nfs/cacheio.c b/support/nfs/cacheio.c
index 61e07a8..42e2502 100644
--- a/support/nfs/cacheio.c
+++ b/support/nfs/cacheio.c
@@ -18,6 +18,7 @@
#include <nfslib.h>
#include <stdio.h>
#include <stdio_ext.h>
+#include <string.h>
#include <ctype.h>
#include <unistd.h>
#include <sys/types.h>
@@ -120,74 +121,6 @@ void qword_addeol(char **bpp, int *lp)
(*lp)--;
}

-static char qword_buf[8192];
-void qword_print(FILE *f, char *str)
-{
- char *bp = qword_buf;
- int len = sizeof(qword_buf);
- qword_add(&bp, &len, str);
- if (fwrite(qword_buf, bp-qword_buf, 1, f) != 1) {
- xlog_warn("qword_print: fwrite failed: errno %d (%s)",
- errno, strerror(errno));
- }
-}
-
-void qword_printhex(FILE *f, char *str, int slen)
-{
- char *bp = qword_buf;
- int len = sizeof(qword_buf);
- qword_addhex(&bp, &len, str, slen);
- if (fwrite(qword_buf, bp-qword_buf, 1, f) != 1) {
- xlog_warn("qword_printhex: fwrite failed: errno %d (%s)",
- errno, strerror(errno));
- }
-}
-
-void qword_printint(FILE *f, int num)
-{
- fprintf(f, "%d ", num);
-}
-
-void qword_printuint(FILE *f, unsigned int num)
-{
- fprintf(f, "%u ", num);
-}
-
-void qword_printtimefrom(FILE *f, unsigned int num)
-{
- fprintf(f, "%lu ", time(0) + num);
-}
-
-int qword_eol(FILE *f)
-{
- int err;
-
- err = fprintf(f,"\n");
- if (err < 0) {
- xlog_warn("qword_eol: fprintf failed: errno %d (%s)",
- errno, strerror(errno));
- } else {
- err = fflush(f);
- if (err) {
- xlog_warn("qword_eol: fflush failed: errno %d (%s)",
- errno, strerror(errno));
- }
- }
- /*
- * We must send one line (and one line only) in a single write
- * call. In case of a write error, libc may accumulate the
- * unwritten data and try to write it again later, resulting in a
- * multi-line write. So we must explicitly ask it to throw away
- * any such cached data. But we return any original error
- * indication to the caller.
- */
- __fpurge(f);
- fflush(f);
- return err;
-}
-
-
-
#define isodigit(c) (isdigit(c) && c <= '7')
int qword_get(char **bpp, char *dest, int bufsize)
{
@@ -265,48 +198,6 @@ int qword_get_uint(char **bpp, unsigned int *anint)
return 0;
}

-#define READLINE_BUFFER_INCREMENT 2048
-
-int readline(int fd, char **buf, int *lenp)
-{
- /* read a line into *buf, which is malloced *len long
- * realloc if needed until we find a \n
- * nul out the \n and return
- * 0 on eof, 1 on success
- */
- int len;
-
- if (*lenp == 0) {
- char *b = malloc(READLINE_BUFFER_INCREMENT);
- if (b == NULL)
- return 0;
- *buf = b;
- *lenp = READLINE_BUFFER_INCREMENT;
- }
- len = read(fd, *buf, *lenp);
- if (len <= 0)
- return 0;
- while ((*buf)[len-1] != '\n') {
- /* now the less common case. There was no newline,
- * so we have to keep reading after re-alloc
- */
- char *new;
- int nl;
- *lenp += READLINE_BUFFER_INCREMENT;
- new = realloc(*buf, *lenp);
- if (new == NULL)
- return 0;
- *buf = new;
- nl = read(fd, *buf + len, *lenp - len);
- if (nl <= 0)
- return 0;
- len += nl;
- }
- (*buf)[len-1] = '\0';
- return 1;
-}
-
-
/* Check if we should use the new caching interface
* This succeeds iff the "nfsd" filesystem is mounted on
* /proc/fs/nfs
diff --git a/support/nfs/nfsexport.c b/support/nfs/nfsexport.c
index f129fd2..afd7c90 100644
--- a/support/nfs/nfsexport.c
+++ b/support/nfs/nfsexport.c
@@ -18,6 +18,7 @@
#include <fcntl.h>

#include "nfslib.h"
+#include "misc.h"

/* if /proc/net/rpc/... exists, then
* write to it, as that interface is more stable.
@@ -32,62 +33,72 @@
static int
exp_unexp(struct nfsctl_export *exp, int export)
{
- FILE *f;
+ char buf[RPC_CHAN_BUF_SIZE], *bp;
struct stat stb;
__u32 fsid;
char fsidstr[8];
__u16 dev;
__u32 inode;
- int err;
+ int err = 0, f, blen;

+ f = open("/proc/net/rpc/nfsd.export/channel", O_WRONLY);
+ if (f < 0) return -1;

- f = fopen("/proc/net/rpc/nfsd.export/channel", "w");
- if (f == NULL) return -1;
- qword_print(f, exp->ex_client);
- qword_print(f, exp->ex_path);
+ bp = buf; blen = sizeof(buf);
+ qword_add(&bp, &blen, exp->ex_client);
+ qword_add(&bp, &blen, exp->ex_path);
if (export) {
- qword_printint(f, 0x7fffffff);
- qword_printint(f, exp->ex_flags);
- qword_printint(f, exp->ex_anon_uid);
- qword_printint(f, exp->ex_anon_gid);
- qword_printint(f, exp->ex_dev);
+ qword_addint(&bp, &blen, 0x7fffffff);
+ qword_addint(&bp, &blen, exp->ex_flags);
+ qword_addint(&bp, &blen, exp->ex_anon_uid);
+ qword_addint(&bp, &blen, exp->ex_anon_gid);
+ qword_addint(&bp, &blen, exp->ex_dev);
} else
- qword_printint(f, 1);
-
- err = qword_eol(f);
- fclose(f);
+ qword_addint(&bp, &blen, 1);
+ qword_addeol(&bp, &blen);
+ if (blen <= 0 || write(f, buf, bp - buf) != bp - buf)
+ err = -1;
+ close(f);

if (stat(exp->ex_path, &stb) != 0)
return -1;
- f = fopen("/proc/net/rpc/nfsd.fh/channel", "w");
- if (f==NULL) return -1;
+
+ f = open("/proc/net/rpc/nfsd.fh/channel", O_WRONLY);
+ if (f < 0) return -1;
if (exp->ex_flags & NFSEXP_FSID) {
- qword_print(f,exp->ex_client);
- qword_printint(f,1);
+ bp = buf; blen = sizeof(buf);
+ qword_add(&bp, &blen, exp->ex_client);
+ qword_addint(&bp, &blen, 1);
fsid = exp->ex_dev;
- qword_printhex(f, (char*)&fsid, 4);
+ qword_addhex(&bp, &blen, (char*)&fsid, 4);
if (export) {
- qword_printint(f, 0x7fffffff);
- qword_print(f, exp->ex_path);
+ qword_addint(&bp, &blen, 0x7fffffff);
+ qword_add(&bp, &blen, exp->ex_path);
} else
- qword_printint(f, 1);
-
- err = qword_eol(f) || err;
+ qword_addint(&bp, &blen, 1);
+ qword_addeol(&bp, &blen);
+ if (blen <= 0 || write(f, buf, bp - buf) != bp - buf)
+ err = -1;
}
- qword_print(f,exp->ex_client);
- qword_printint(f,0);
+
+ bp = buf; blen = sizeof(buf);
+ qword_add(&bp, &blen, exp->ex_client);
+ qword_addint(&bp, &blen, 0);
dev = htons(major(stb.st_dev)); memcpy(fsidstr, &dev, 2);
dev = htons(minor(stb.st_dev)); memcpy(fsidstr+2, &dev, 2);
inode = stb.st_ino; memcpy(fsidstr+4, &inode, 4);

- qword_printhex(f, fsidstr, 8);
+ qword_addhex(&bp, &blen, fsidstr, 8);
if (export) {
- qword_printint(f, 0x7fffffff);
- qword_print(f, exp->ex_path);
+ qword_addint(&bp, &blen, 0x7fffffff);
+ qword_add(&bp, &blen, exp->ex_path);
} else
- qword_printint(f, 1);
- err = qword_eol(f) || err;
- fclose(f);
+ qword_addint(&bp, &blen, 1);
+ qword_addeol(&bp, &blen);
+ if (blen <= 0 || write(f, buf, bp - buf) != bp - buf)
+ err = -1;
+ close(f);
+
return err;
}

diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index 121feb1..1d8e6a7 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -78,6 +78,7 @@
#include "nfsrpc.h"
#include "nfslib.h"
#include "gss_names.h"
+#include "misc.h"

/*
* pollarray:
@@ -1250,7 +1251,7 @@ void
handle_gssd_upcall(struct clnt_info *clp)
{
uid_t uid;
- char *lbuf = NULL;
+ char lbuf[RPC_CHAN_BUF_SIZE];
int lbuflen = 0;
char *p;
char *mech = NULL;
@@ -1260,11 +1261,14 @@ handle_gssd_upcall(struct clnt_info *clp)

printerr(1, "handling gssd upcall (%s)\n", clp->dirname);

- if (readline(clp->gssd_fd, &lbuf, &lbuflen) != 1) {
+ lbuflen = read(clp->gssd_fd, lbuf, sizeof(lbuf));
+ if (lbuflen <= 0 || lbuf[lbuflen-1] != '\n') {
printerr(0, "WARNING: handle_gssd_upcall: "
"failed reading request\n");
return;
}
+ lbuf[lbuflen-1] = 0;
+
printerr(2, "%s: '%s'\n", __func__, lbuf);

/* find the mechanism name */
@@ -1362,7 +1366,6 @@ handle_gssd_upcall(struct clnt_info *clp)
}

out:
- free(lbuf);
free(mech);
free(enctypes);
free(target);
diff --git a/utils/gssd/svcgssd.h b/utils/gssd/svcgssd.h
index 9a2e2e8..02b5c7a 100644
--- a/utils/gssd/svcgssd.h
+++ b/utils/gssd/svcgssd.h
@@ -35,7 +35,7 @@
#include <sys/queue.h>
#include <gssapi/gssapi.h>

-void handle_nullreq(FILE *f);
+void handle_nullreq(int f);
void gssd_run(void);

#define GSSD_SERVICE_NAME "nfs"
diff --git a/utils/gssd/svcgssd_main_loop.c b/utils/gssd/svcgssd_main_loop.c
index 2b4111c..b5681ce 100644
--- a/utils/gssd/svcgssd_main_loop.c
+++ b/utils/gssd/svcgssd_main_loop.c
@@ -54,19 +54,18 @@ void
gssd_run()
{
int ret;
- FILE *f;
+ int f;
struct pollfd pollfd;

#define NULLRPC_FILE "/proc/net/rpc/auth.rpcsec.init/channel"

- f = fopen(NULLRPC_FILE, "rw");
-
- if (!f) {
+ f = open(NULLRPC_FILE, O_RDWR);
+ if (f < 0) {
printerr(0, "failed to open %s: %s\n",
NULLRPC_FILE, strerror(errno));
exit(1);
}
- pollfd.fd = fileno(f);
+ pollfd.fd = f;
pollfd.events = POLLIN;
while (1) {
int save_err;
diff --git a/utils/gssd/svcgssd_proc.c b/utils/gssd/svcgssd_proc.c
index 5bdb438..72ec254 100644
--- a/utils/gssd/svcgssd_proc.c
+++ b/utils/gssd/svcgssd_proc.c
@@ -73,36 +73,35 @@ struct svc_cred {
int cr_ngroups;
gid_t cr_groups[NGROUPS];
};
-static char vbuf[RPC_CHAN_BUF_SIZE];

static int
do_svc_downcall(gss_buffer_desc *out_handle, struct svc_cred *cred,
gss_OID mech, gss_buffer_desc *context_token,
int32_t endtime, char *client_name)
{
- FILE *f;
- int i;
+ char buf[RPC_CHAN_BUF_SIZE], *bp;
+ int i, f, err, blen;
char *fname = NULL;
- int err;

printerr(1, "doing downcall\n");
if ((fname = mech2file(mech)) == NULL)
goto out_err;
- f = fopen(SVCGSSD_CONTEXT_CHANNEL, "w");
- if (f == NULL) {
+
+ f = open(SVCGSSD_CONTEXT_CHANNEL, O_WRONLY);
+ if (f < 0) {
printerr(0, "WARNING: unable to open downcall channel "
"%s: %s\n",
SVCGSSD_CONTEXT_CHANNEL, strerror(errno));
goto out_err;
}
- setvbuf(f, vbuf, _IOLBF, RPC_CHAN_BUF_SIZE);
- qword_printhex(f, out_handle->value, out_handle->length);
+ bp = buf, blen = sizeof(buf);
+ qword_addhex(&bp, &blen, out_handle->value, out_handle->length);
/* XXX are types OK for the rest of this? */
/* For context cache, use the actual context endtime */
- qword_printint(f, endtime);
- qword_printint(f, cred->cr_uid);
- qword_printint(f, cred->cr_gid);
- qword_printint(f, cred->cr_ngroups);
+ qword_addint(&bp, &blen, endtime);
+ qword_addint(&bp, &blen, cred->cr_uid);
+ qword_addint(&bp, &blen, cred->cr_gid);
+ qword_addint(&bp, &blen, cred->cr_ngroups);
printerr(2, "mech: %s, hndl len: %d, ctx len %d, timeout: %d (%d from now), "
"clnt: %s, uid: %d, gid: %d, num aux grps: %d:\n",
fname, out_handle->length, context_token->length,
@@ -110,19 +109,21 @@ do_svc_downcall(gss_buffer_desc *out_handle, struct svc_cred *cred,
client_name ? client_name : "<null>",
cred->cr_uid, cred->cr_gid, cred->cr_ngroups);
for (i=0; i < cred->cr_ngroups; i++) {
- qword_printint(f, cred->cr_groups[i]);
+ qword_addint(&bp, &blen, cred->cr_groups[i]);
printerr(2, " (%4d) %d\n", i+1, cred->cr_groups[i]);
}
- qword_print(f, fname);
- qword_printhex(f, context_token->value, context_token->length);
+ qword_add(&bp, &blen, fname);
+ qword_addhex(&bp, &blen, context_token->value, context_token->length);
if (client_name)
- qword_print(f, client_name);
- err = qword_eol(f);
- if (err) {
+ qword_add(&bp, &blen, client_name);
+ qword_addeol(&bp, &blen);
+ err = 0;
+ if (blen <= 0 || write(f, buf, bp - buf) != bp - buf) {
printerr(1, "WARNING: error writing to downcall channel "
"%s: %s\n", SVCGSSD_CONTEXT_CHANNEL, strerror(errno));
+ err = -1;
}
- fclose(f);
+ close(f);
return err;
out_err:
printerr(1, "WARNING: downcall failed\n");
@@ -317,7 +318,7 @@ print_hexl(const char *description, unsigned char *cp, int length)
#endif

void
-handle_nullreq(FILE *f) {
+handle_nullreq(int f) {
/* XXX initialize to a random integer to reduce chances of unnecessary
* invalidation of existing ctx's on restarting svcgssd. */
static u_int32_t handle_seq = 0;
@@ -339,19 +340,21 @@ handle_nullreq(FILE *f) {
u_int32_t maj_stat = GSS_S_FAILURE, min_stat = 0;
u_int32_t ignore_min_stat;
struct svc_cred cred;
- static char *lbuf = NULL;
- static int lbuflen = 0;
- static char *cp;
+ char lbuf[RPC_CHAN_BUF_SIZE];
+ int lbuflen = 0;
+ char *cp;
int32_t ctx_endtime;
char *hostbased_name = NULL;

printerr(1, "handling null request\n");

- if (readline(fileno(f), &lbuf, &lbuflen) != 1) {
+ lbuflen = read(f, lbuf, sizeof(lbuf));
+ if (lbuflen <= 0 || lbuf[lbuflen-1] != '\n') {
printerr(0, "WARNING: handle_nullreq: "
"failed reading request\n");
return;
}
+ lbuf[lbuflen-1] = 0;

cp = lbuf;

diff --git a/utils/gssd/write_bytes.h b/utils/gssd/write_bytes.h
index 4fc72cc..b3f342b 100644
--- a/utils/gssd/write_bytes.h
+++ b/utils/gssd/write_bytes.h
@@ -32,6 +32,7 @@
#define _WRITE_BYTES_H_

#include <stdlib.h>
+#include <string.h>
#include <sys/types.h>
#include <netinet/in.h> /* for ntohl */

diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index 663a52a..465ce65 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -61,15 +61,13 @@ enum nfsd_fsid {
* Record is terminated with newline.
*
*/
-static int cache_export_ent(char *domain, struct exportent *exp, char *p);
+static int cache_export_ent(char *buf, int buflen, char *domain, struct exportent *exp, char *path);

#define INITIAL_MANAGED_GROUPS 100

-char *lbuf = NULL;
-int lbuflen = 0;
extern int use_ipaddr;

-static void auth_unix_ip(FILE *f)
+static void auth_unix_ip(int f)
{
/* requests are
* class IP-ADDR
@@ -78,23 +76,26 @@ static void auth_unix_ip(FILE *f)
*
* "nfsd" IP-ADDR expiry domainname
*/
- char *cp;
char class[20];
char ipaddr[INET6_ADDRSTRLEN + 1];
char *client = NULL;
struct addrinfo *tmp = NULL;
- if (readline(fileno(f), &lbuf, &lbuflen) != 1)
- return;
+ char buf[RPC_CHAN_BUF_SIZE], *bp;
+ int blen;
+
+ blen = read(f, buf, sizeof(buf));
+ if (blen <= 0 || buf[blen-1] != '\n') return;
+ buf[blen-1] = 0;

- xlog(D_CALL, "auth_unix_ip: inbuf '%s'", lbuf);
+ xlog(D_CALL, "auth_unix_ip: inbuf '%s'", buf);

- cp = lbuf;
+ bp = buf;

- if (qword_get(&cp, class, 20) <= 0 ||
+ if (qword_get(&bp, class, 20) <= 0 ||
strcmp(class, "nfsd") != 0)
return;

- if (qword_get(&cp, ipaddr, sizeof(ipaddr) - 1) <= 0)
+ if (qword_get(&bp, ipaddr, sizeof(ipaddr) - 1) <= 0)
return;

tmp = host_pton(ipaddr);
@@ -113,16 +114,19 @@ static void auth_unix_ip(FILE *f)
freeaddrinfo(ai);
}
}
- qword_print(f, "nfsd");
- qword_print(f, ipaddr);
- qword_printtimefrom(f, DEFAULT_TTL);
+ bp = buf; blen = sizeof(buf);
+ qword_add(&bp, &blen, "nfsd");
+ qword_add(&bp, &blen, ipaddr);
+ qword_adduint(&bp, &blen, time(0) + DEFAULT_TTL);
if (use_ipaddr) {
memmove(ipaddr + 1, ipaddr, strlen(ipaddr) + 1);
ipaddr[0] = '$';
- qword_print(f, ipaddr);
+ qword_add(&bp, &blen, ipaddr);
} else if (client)
- qword_print(f, *client?client:"DEFAULT");
- qword_eol(f);
+ qword_add(&bp, &blen, *client?client:"DEFAULT");
+ qword_addeol(&bp, &blen);
+ if (blen > 0) write(f, buf, bp - buf);
+
xlog(D_CALL, "auth_unix_ip: client %p '%s'", client, client?client: "DEFAULT");

free(client);
@@ -130,7 +134,7 @@ static void auth_unix_ip(FILE *f)

}

-static void auth_unix_gid(FILE *f)
+static void auth_unix_gid(int f)
{
/* Request are
* uid
@@ -144,7 +148,8 @@ static void auth_unix_gid(FILE *f)
gid_t *more_groups;
int ngroups;
int rv, i;
- char *cp;
+ char buf[RPC_CHAN_BUF_SIZE], *bp;
+ int blen;

if (groups_len == 0) {
groups = malloc(sizeof(gid_t) * INITIAL_MANAGED_GROUPS);
@@ -156,11 +161,12 @@ static void auth_unix_gid(FILE *f)

ngroups = groups_len;

- if (readline(fileno(f), &lbuf, &lbuflen) != 1)
- return;
+ blen = read(f, buf, sizeof(buf));
+ if (blen <= 0 || buf[blen-1] != '\n') return;
+ buf[blen-1] = 0;

- cp = lbuf;
- if (qword_get_uint(&cp, &uid) != 0)
+ bp = buf;
+ if (qword_get_uint(&bp, &uid) != 0)
return;

pw = getpwuid(uid);
@@ -180,15 +186,18 @@ static void auth_unix_gid(FILE *f)
}
}
}
- qword_printuint(f, uid);
- qword_printtimefrom(f, DEFAULT_TTL);
+
+ bp = buf; blen = sizeof(buf);
+ qword_adduint(&bp, &blen, uid);
+ qword_adduint(&bp, &blen, time(0) + DEFAULT_TTL);
if (rv >= 0) {
- qword_printuint(f, ngroups);
+ qword_adduint(&bp, &blen, ngroups);
for (i=0; i<ngroups; i++)
- qword_printuint(f, groups[i]);
+ qword_adduint(&bp, &blen, groups[i]);
} else
- qword_printuint(f, 0);
- qword_eol(f);
+ qword_adduint(&bp, &blen, 0);
+ qword_addeol(&bp, &blen);
+ if (blen > 0) write(f, buf, bp - buf);
}

#if USE_BLKID
@@ -659,14 +668,13 @@ static struct addrinfo *lookup_client_addr(char *dom)
return ret;
}

-static void nfsd_fh(FILE *f)
+static void nfsd_fh(int f)
{
/* request are:
* domain fsidtype fsid
* interpret fsid, find export point and options, and write:
* domain fsidtype fsid expiry path
*/
- char *cp;
char *dom;
int fsidtype;
int fsidlen;
@@ -678,24 +686,27 @@ static void nfsd_fh(FILE *f)
nfs_export *exp;
int i;
int dev_missing = 0;
+ char buf[RPC_CHAN_BUF_SIZE], *bp;
+ int blen;

- if (readline(fileno(f), &lbuf, &lbuflen) != 1)
- return;
+ blen = read(f, buf, sizeof(buf));
+ if (blen <= 0 || buf[blen-1] != '\n') return;
+ buf[blen-1] = 0;

- xlog(D_CALL, "nfsd_fh: inbuf '%s'", lbuf);
+ xlog(D_CALL, "nfsd_fh: inbuf '%s'", buf);

- cp = lbuf;
+ bp = buf;

- dom = malloc(strlen(cp));
+ dom = malloc(blen);
if (dom == NULL)
return;
- if (qword_get(&cp, dom, strlen(cp)) <= 0)
+ if (qword_get(&bp, dom, blen) <= 0)
goto out;
- if (qword_get_int(&cp, &fsidtype) != 0)
+ if (qword_get_int(&bp, &fsidtype) != 0)
goto out;
if (fsidtype < 0 || fsidtype > 7)
goto out; /* unknown type */
- if ((fsidlen = qword_get(&cp, fsid, 32)) <= 0)
+ if ((fsidlen = qword_get(&bp, fsid, 32)) <= 0)
goto out;
if (parse_fsid(fsidtype, fsidlen, fsid, &parsed))
goto out;
@@ -796,12 +807,13 @@ static void nfsd_fh(FILE *f)
}

if (found)
- if (cache_export_ent(dom, found, found_path) < 0)
+ if (cache_export_ent(buf, sizeof(buf), dom, found, found_path) < 0)
found = 0;

- qword_print(f, dom);
- qword_printint(f, fsidtype);
- qword_printhex(f, fsid, fsidlen);
+ bp = buf; blen = sizeof(buf);
+ qword_add(&bp, &blen, dom);
+ qword_addint(&bp, &blen, fsidtype);
+ qword_addhex(&bp, &blen, fsid, fsidlen);
/* The fsid -> path lookup can be quite expensive as it
* potentially stats and reads lots of devices, and some of those
* might have spun-down. The Answer is not likely to
@@ -810,10 +822,11 @@ static void nfsd_fh(FILE *f)
* timeout. Maybe this should be configurable on the command
* line.
*/
- qword_printint(f, 0x7fffffff);
+ qword_addint(&bp, &blen, 0x7fffffff);
if (found)
- qword_print(f, found_path);
- qword_eol(f);
+ qword_add(&bp, &blen, found_path);
+ qword_addeol(&bp, &blen);
+ if (blen > 0) write(f, buf, bp - buf);
out:
if (found_path)
free(found_path);
@@ -823,7 +836,7 @@ static void nfsd_fh(FILE *f)
return;
}

-static void write_fsloc(FILE *f, struct exportent *ep)
+static void write_fsloc(char **bp, int *blen, struct exportent *ep)
{
struct servers *servers;

@@ -833,20 +846,20 @@ static void write_fsloc(FILE *f, struct exportent *ep)
servers = replicas_lookup(ep->e_fslocmethod, ep->e_fslocdata);
if (!servers)
return;
- qword_print(f, "fsloc");
- qword_printint(f, servers->h_num);
+ qword_add(bp, blen, "fsloc");
+ qword_addint(bp, blen, servers->h_num);
if (servers->h_num >= 0) {
int i;
for (i=0; i<servers->h_num; i++) {
- qword_print(f, servers->h_mp[i]->h_host);
- qword_print(f, servers->h_mp[i]->h_path);
+ qword_add(bp, blen, servers->h_mp[i]->h_host);
+ qword_add(bp, blen, servers->h_mp[i]->h_path);
}
}
- qword_printint(f, servers->h_referral);
+ qword_addint(bp, blen, servers->h_referral);
release_replicas(servers);
}

-static void write_secinfo(FILE *f, struct exportent *ep, int flag_mask)
+static void write_secinfo(char **bp, int *blen, struct exportent *ep, int flag_mask)
{
struct sec_entry *p;

@@ -857,45 +870,51 @@ static void write_secinfo(FILE *f, struct exportent *ep, int flag_mask)
return;
}
fix_pseudoflavor_flags(ep);
- qword_print(f, "secinfo");
- qword_printint(f, p - ep->e_secinfo);
+ qword_add(bp, blen, "secinfo");
+ qword_addint(bp, blen, p - ep->e_secinfo);
for (p = ep->e_secinfo; p->flav; p++) {
- qword_printint(f, p->flav->fnum);
- qword_printint(f, p->flags & flag_mask);
+ qword_addint(bp, blen, p->flav->fnum);
+ qword_addint(bp, blen, p->flags & flag_mask);
}

}

-static int dump_to_cache(FILE *f, char *domain, char *path, struct exportent *exp)
+static int dump_to_cache(int f, char *buf, int buflen, char *domain, char *path, struct exportent *exp)
{
- qword_print(f, domain);
- qword_print(f, path);
+ char *bp = buf;
+ int blen = buflen;
+
+ qword_add(&bp, &blen, domain);
+ qword_add(&bp, &blen, path);
if (exp) {
int different_fs = strcmp(path, exp->e_path) != 0;
int flag_mask = different_fs ? ~NFSEXP_FSID : ~0;

- qword_printtimefrom(f, exp->e_ttl);
- qword_printint(f, exp->e_flags & flag_mask);
- qword_printint(f, exp->e_anonuid);
- qword_printint(f, exp->e_anongid);
- qword_printint(f, exp->e_fsid);
- write_fsloc(f, exp);
- write_secinfo(f, exp, flag_mask);
- if (exp->e_uuid == NULL || different_fs) {
- char u[16];
- if (uuid_by_path(path, 0, 16, u)) {
- qword_print(f, "uuid");
- qword_printhex(f, u, 16);
- }
- } else {
- char u[16];
- get_uuid(exp->e_uuid, 16, u);
- qword_print(f, "uuid");
- qword_printhex(f, u, 16);
- }
+ qword_adduint(&bp, &blen, time(0) + exp->e_ttl);
+ 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);
+ write_fsloc(&bp, &blen, exp);
+ write_secinfo(&bp, &blen, exp, flag_mask);
+ if (exp->e_uuid == NULL || different_fs) {
+ char u[16];
+ if (uuid_by_path(path, 0, 16, u)) {
+ qword_add(&bp, &blen, "uuid");
+ qword_addhex(&bp, &blen, u, 16);
+ }
+ } else {
+ char u[16];
+ get_uuid(exp->e_uuid, 16, u);
+ qword_add(&bp, &blen, "uuid");
+ qword_addhex(&bp, &blen, u, 16);
+ }
} else
- qword_printtimefrom(f, DEFAULT_TTL);
- return qword_eol(f);
+ qword_adduint(&bp, &blen, time(0) + DEFAULT_TTL);
+ qword_addeol(&bp, &blen);
+ if (blen <= 0) return -1;
+ if (write(f, buf, bp - buf) != bp - buf) return -1;
+ return 0;
}

static nfs_export *
@@ -1245,27 +1264,27 @@ static struct exportent *lookup_junction(char *dom, const char *pathname,
return exp;
}

-static void lookup_nonexport(FILE *f, char *dom, char *path,
+static void lookup_nonexport(int f, char *buf, int buflen, char *dom, char *path,
struct addrinfo *ai)
{
struct exportent *eep;

eep = lookup_junction(dom, path, ai);
- dump_to_cache(f, dom, path, eep);
+ dump_to_cache(f, buf, buflen, dom, path, eep);
if (eep == NULL)
return;
exportent_release(eep);
free(eep);
}
#else /* !HAVE_NFS_PLUGIN_H */
-static void lookup_nonexport(FILE *f, char *dom, char *path,
+static void lookup_nonexport(int f, char *buf, int buflen, char *dom, char *path,
struct addrinfo *UNUSED(ai))
{
- dump_to_cache(f, dom, path, NULL);
+ dump_to_cache(f, buf, buflen, dom, path, NULL);
}
#endif /* !HAVE_NFS_PLUGIN_H */

-static void nfsd_export(FILE *f)
+static void nfsd_export(int f)
{
/* requests are:
* domain path
@@ -1273,26 +1292,28 @@ static void nfsd_export(FILE *f)
* domain path expiry flags anonuid anongid fsid
*/

- char *cp;
char *dom, *path;
nfs_export *found = NULL;
struct addrinfo *ai = NULL;
+ char buf[RPC_CHAN_BUF_SIZE], *bp;
+ int blen;

- if (readline(fileno(f), &lbuf, &lbuflen) != 1)
- return;
+ blen = read(f, buf, sizeof(buf));
+ if (blen <= 0 || buf[blen-1] != '\n') return;
+ buf[blen-1] = 0;

- xlog(D_CALL, "nfsd_export: inbuf '%s'", lbuf);
+ xlog(D_CALL, "nfsd_export: inbuf '%s'", buf);

- cp = lbuf;
- dom = malloc(strlen(cp));
- path = malloc(strlen(cp));
+ bp = buf;
+ dom = malloc(blen);
+ path = malloc(blen);

if (!dom || !path)
goto out;

- if (qword_get(&cp, dom, strlen(lbuf)) <= 0)
+ if (qword_get(&bp, dom, blen) <= 0)
goto out;
- if (qword_get(&cp, path, strlen(lbuf)) <= 0)
+ if (qword_get(&bp, path, blen) <= 0)
goto out;

auth_reload();
@@ -1306,14 +1327,14 @@ static void nfsd_export(FILE *f)
found = lookup_export(dom, path, ai);

if (found) {
- if (dump_to_cache(f, dom, path, &found->m_export) < 0) {
+ if (dump_to_cache(f, buf, sizeof(buf), dom, path, &found->m_export) < 0) {
xlog(L_WARNING,
"Cannot export %s, possibly unsupported filesystem"
" or fsid= required", path);
- dump_to_cache(f, dom, path, NULL);
+ dump_to_cache(f, buf, sizeof(buf), dom, path, NULL);
}
} else
- lookup_nonexport(f, dom, path, ai);
+ lookup_nonexport(f, buf, sizeof(buf), dom, path, ai);

out:
xlog(D_CALL, "nfsd_export: found %p path %s", found, path ? path : NULL);
@@ -1325,15 +1346,14 @@ static void nfsd_export(FILE *f)

struct {
char *cache_name;
- void (*cache_handle)(FILE *f);
- FILE *f;
- char vbuf[RPC_CHAN_BUF_SIZE];
+ void (*cache_handle)(int f);
+ int f;
} cachelist[] = {
- { "auth.unix.ip", auth_unix_ip, NULL, ""},
- { "auth.unix.gid", auth_unix_gid, NULL, ""},
- { "nfsd.export", nfsd_export, NULL, ""},
- { "nfsd.fh", nfsd_fh, NULL, ""},
- { NULL, NULL, NULL, ""}
+ { "auth.unix.ip", auth_unix_ip, -1 },
+ { "auth.unix.gid", auth_unix_gid, -1 },
+ { "nfsd.export", nfsd_export, -1 },
+ { "nfsd.fh", nfsd_fh, -1 },
+ { NULL, NULL, -1 }
};

extern int manage_gids;
@@ -1350,11 +1370,7 @@ void cache_open(void)
if (!manage_gids && cachelist[i].cache_handle == auth_unix_gid)
continue;
sprintf(path, "/proc/net/rpc/%s/channel", cachelist[i].cache_name);
- cachelist[i].f = fopen(path, "r+");
- if (cachelist[i].f != NULL) {
- setvbuf(cachelist[i].f, cachelist[i].vbuf, _IOLBF,
- RPC_CHAN_BUF_SIZE);
- }
+ cachelist[i].f = open(path, O_RDWR);
}
}

@@ -1366,8 +1382,8 @@ void cache_set_fds(fd_set *fdset)
{
int i;
for (i=0; cachelist[i].cache_name; i++) {
- if (cachelist[i].f)
- FD_SET(fileno(cachelist[i].f), fdset);
+ if (cachelist[i].f >= 0)
+ FD_SET(cachelist[i].f, fdset);
}
}

@@ -1380,11 +1396,11 @@ int cache_process_req(fd_set *readfds)
int i;
int cnt = 0;
for (i=0; cachelist[i].cache_name; i++) {
- if (cachelist[i].f != NULL &&
- FD_ISSET(fileno(cachelist[i].f), readfds)) {
+ if (cachelist[i].f >= 0 &&
+ FD_ISSET(cachelist[i].f, readfds)) {
cnt++;
cachelist[i].cache_handle(cachelist[i].f);
- FD_CLR(fileno(cachelist[i].f), readfds);
+ FD_CLR(cachelist[i].f, readfds);
}
}
return cnt;
@@ -1397,14 +1413,14 @@ int cache_process_req(fd_set *readfds)
* % echo $domain $path $[now+DEFAULT_TTL] $options $anonuid $anongid $fsid > /proc/net/rpc/nfsd.export/channel
*/

-static int cache_export_ent(char *domain, struct exportent *exp, char *path)
+static int cache_export_ent(char *buf, int buflen, char *domain, struct exportent *exp, char *path)
{
- int err;
- FILE *f = fopen("/proc/net/rpc/nfsd.export/channel", "w");
- if (!f)
- return -1;
+ int f, err;
+
+ f = open("/proc/net/rpc/nfsd.export/channel", O_WRONLY);
+ if (f < 0) return -1;

- err = dump_to_cache(f, domain, exp->e_path, exp);
+ err = dump_to_cache(f, buf, buflen, domain, exp->e_path, exp);
if (err) {
xlog(L_WARNING,
"Cannot export %s, possibly unsupported filesystem or"
@@ -1445,13 +1461,13 @@ static int cache_export_ent(char *domain, struct exportent *exp, char *path)
continue;
dev = stb.st_dev;
path[l] = 0;
- dump_to_cache(f, domain, path, exp);
+ dump_to_cache(f, buf, buflen, domain, path, exp);
path[l] = c;
}
break;
}

- fclose(f);
+ close(f);
return err;
}

@@ -1462,27 +1478,25 @@ static int cache_export_ent(char *domain, struct exportent *exp, char *path)
*/
int cache_export(nfs_export *exp, char *path)
{
- char buf[INET6_ADDRSTRLEN];
- int err;
- FILE *f;
+ char ip[INET6_ADDRSTRLEN];
+ char buf[RPC_CHAN_BUF_SIZE], *bp;
+ int blen, f;

- f = fopen("/proc/net/rpc/auth.unix.ip/channel", "w");
- if (!f)
+ f = open("/proc/net/rpc/auth.unix.ip/channel", O_WRONLY);
+ if (f < 0)
return -1;

-
- qword_print(f, "nfsd");
- qword_print(f,
- host_ntop(get_addrlist(exp->m_client, 0), buf, sizeof(buf)));
- qword_printtimefrom(f, exp->m_export.e_ttl);
- qword_print(f, exp->m_client->m_hostname);
- err = qword_eol(f);
-
- fclose(f);
-
- err = cache_export_ent(exp->m_client->m_hostname, &exp->m_export, path)
- || err;
- return err;
+ bp = buf, blen = sizeof(buf);
+ qword_add(&bp, &blen, "nfsd");
+ qword_add(&bp, &blen, host_ntop(get_addrlist(exp->m_client, 0), ip, sizeof(ip)));
+ qword_adduint(&bp, &blen, time(0) + exp->m_export.e_ttl);
+ qword_add(&bp, &blen, exp->m_client->m_hostname);
+ qword_addeol(&bp, &blen);
+ if (blen > 0) write(f, buf, bp - buf);
+ close(f);
+ if (blen <= 0) return -1;
+
+ return cache_export_ent(buf, sizeof(buf), exp->m_client->m_hostname, &exp->m_export, path);
}

/**
@@ -1501,28 +1515,34 @@ int cache_export(nfs_export *exp, char *path)
struct nfs_fh_len *
cache_get_filehandle(nfs_export *exp, int len, char *p)
{
- FILE *f = fopen("/proc/fs/nfsd/filehandle", "r+");
- char buf[200];
- char *bp = buf;
- int failed;
static struct nfs_fh_len fh;
+ char buf[200], *bp;
+ int blen, f;
+
+ f = open("/proc/fs/nfsd/filehandle", O_RDWR);
+ if (f < 0) {
+ f = open("/proc/fs/nfs/filehandle", O_RDWR);
+ if (f < 0) return NULL;
+ }

- if (!f)
- f = fopen("/proc/fs/nfs/filehandle", "r+");
- if (!f)
+ bp = buf, blen = sizeof(buf);
+ qword_add(&bp, &blen, exp->m_client->m_hostname);
+ qword_add(&bp, &blen, p);
+ qword_addint(&bp, &blen, len);
+ qword_addeol(&bp, &blen);
+ if (blen <= 0 || write(f, buf, bp - buf) != bp - buf) {
+ close(f);
return NULL;
+ }
+ blen = read(f, buf, sizeof(buf));
+ close(f);

- qword_print(f, exp->m_client->m_hostname);
- qword_print(f, p);
- qword_printint(f, len);
- failed = qword_eol(f);
-
- if (!failed)
- failed = (fgets(buf, sizeof(buf), f) == NULL);
- fclose(f);
- if (failed)
+ if (blen <= 0 || buf[blen-1] != '\n')
return NULL;
+ buf[blen-1] = 0;
+
memset(fh.fh_handle, 0, sizeof(fh.fh_handle));
fh.fh_size = qword_get(&bp, (char *)fh.fh_handle, NFS3_FHSIZE);
+
return &fh;
}
diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
index 0675b6a..027e5ac 100644
--- a/utils/nfsd/nfssvc.c
+++ b/utils/nfsd/nfssvc.c
@@ -20,6 +20,7 @@
#include <fcntl.h>
#include <errno.h>
#include <stdlib.h>
+#include <string.h>

#include "nfslib.h"
#include "xlog.h"
--
2.0.4



2014-09-13 07:05:34

by Timo Teras

[permalink] [raw]
Subject: Re: [PATCH nfs-utils] rework access to /proc/net/rpc

On Fri, 12 Sep 2014 13:00:00 -0400
Steve Dickson <[email protected]> wrote:

> Hello,
>
> On 08/08/2014 02:11 AM, Timo Teräs wrote:
> > The kernel support only access by read() and write() with exactly
> > one line in the buffer. FILE can be implemented with readv() and
> > writev(), and the mapping to syscalls is not guaranteed. The code
> > already does lot of extra work setvbuf() and fflush() calls to try
> > to ensure this, but it relays on implementation details and is
> > non-portable. And e.g. with musl library the current hacks do not
> > work at all.
> >
> > Remove the FILE based qword_* API as it's fundamentally broken,
> > and replace it with explicitly caching one line at a time. The
> > qword API should probable improved to do the line caching
> > internally, and this is the first step towards that.
> >
> > Signed-off-by: Timo Teräs <[email protected]>
> > ---
> > This along with Natanael Copa's patchset makes nfs-utils somewhat
> > usable with musl c-library.
> First of all, I apologize for taking so long to get this patch.
>
> Second of all, I'm going to NAK this patch because it breaks v3
> mounts. (Note: this patch was applied after Natanael patchset).
>
> Using a Fedora 20 server, Fedora 20 clients fail with
> mount.nfs: Unknown error 521
> and Fedora 21 clients fail with
> mount.nfs: an incorrect mount option was specified

Thanks for the testing and feedback. We had issues with nfs3 on musl
too and were uncertain if our broke something, or if it's just
something additional to be fixed.

Did you test Natanaels patch set individually first? I'm wonderinf
which patch set breaks nfs3.

> Also the following warnings occur after the patch is applied:
>
> cache.c:128:21: warning: ignoring return value of 'write', declared
> with attribute warn_unused_result [-Wunused-result] cache.c:1495:21:
> warning: ignoring return value of 'write', declared with attribute
> warn_unused_result [-Wunused-result] nfssvc.c:71:8: warning: ignoring
> return value of 'system', declared with attribute warn_unused_result
> [-Wunused-result] nfssvc.c:325:9: warning: ignoring return value of
> 'write', declared with attribute warn_unused_result [-Wunused-result]

My patch seems introduce few of these, but I think some of this come
from Natanaels patch set.

> My suggestion is to break the patch up into several small patches
> which would help with debugging the problem.

Yes. Might be a good idea.

> I'll be more than willing to reconsider this change when it
> stop braking v3 mounts and there are no warnings.

Thanks for review, testing and feedback.


2014-09-12 17:00:20

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH nfs-utils] rework access to /proc/net/rpc

Hello,

On 08/08/2014 02:11 AM, Timo Teräs wrote:
> The kernel support only access by read() and write() with exactly
> one line in the buffer. FILE can be implemented with readv() and
> writev(), and the mapping to syscalls is not guaranteed. The code
> already does lot of extra work setvbuf() and fflush() calls to try
> to ensure this, but it relays on implementation details and is
> non-portable. And e.g. with musl library the current hacks do not
> work at all.
>
> Remove the FILE based qword_* API as it's fundamentally broken,
> and replace it with explicitly caching one line at a time. The
> qword API should probable improved to do the line caching
> internally, and this is the first step towards that.
>
> Signed-off-by: Timo Teräs <[email protected]>
> ---
> This along with Natanael Copa's patchset makes nfs-utils somewhat
> usable with musl c-library.
First of all, I apologize for taking so long to get this patch.

Second of all, I'm going to NAK this patch because it breaks v3 mounts.
(Note: this patch was applied after Natanael patchset).

Using a Fedora 20 server, Fedora 20 clients fail with
mount.nfs: Unknown error 521
and Fedora 21 clients fail with
mount.nfs: an incorrect mount option was specified

Also the following warnings occur after the patch is applied:

cache.c:128:21: warning: ignoring return value of 'write', declared with attribute warn_unused_result [-Wunused-result]
cache.c:1495:21: warning: ignoring return value of 'write', declared with attribute warn_unused_result [-Wunused-result]
nfssvc.c:71:8: warning: ignoring return value of 'system', declared with attribute warn_unused_result [-Wunused-result]
nfssvc.c:325:9: warning: ignoring return value of 'write', declared with attribute warn_unused_result [-Wunused-result]

My suggestion is to break the patch up into several small patches which
would help with debugging the problem.

I'll be more than willing to reconsider this change when it
stop braking v3 mounts and there are no warnings.

steved.


2014-09-13 10:46:24

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH nfs-utils] rework access to /proc/net/rpc



On 09/13/2014 03:05 AM, Timo Teras wrote:
> On Fri, 12 Sep 2014 13:00:00 -0400
> Steve Dickson <[email protected]> wrote:
>
>> Hello,
>>
>> On 08/08/2014 02:11 AM, Timo Teräs wrote:
>>> The kernel support only access by read() and write() with exactly
>>> one line in the buffer. FILE can be implemented with readv() and
>>> writev(), and the mapping to syscalls is not guaranteed. The code
>>> already does lot of extra work setvbuf() and fflush() calls to try
>>> to ensure this, but it relays on implementation details and is
>>> non-portable. And e.g. with musl library the current hacks do not
>>> work at all.
>>>
>>> Remove the FILE based qword_* API as it's fundamentally broken,
>>> and replace it with explicitly caching one line at a time. The
>>> qword API should probable improved to do the line caching
>>> internally, and this is the first step towards that.
>>>
>>> Signed-off-by: Timo Teräs <[email protected]>
>>> ---
>>> This along with Natanael Copa's patchset makes nfs-utils somewhat
>>> usable with musl c-library.
>> First of all, I apologize for taking so long to get this patch.
>>
>> Second of all, I'm going to NAK this patch because it breaks v3
>> mounts. (Note: this patch was applied after Natanael patchset).
>>
>> Using a Fedora 20 server, Fedora 20 clients fail with
>> mount.nfs: Unknown error 521
>> and Fedora 21 clients fail with
>> mount.nfs: an incorrect mount option was specified
>
> Thanks for the testing and feedback. We had issues with nfs3 on musl
> too and were uncertain if our broke something, or if it's just
> something additional to be fixed.
>
> Did you test Natanaels patch set individually first? I'm wonderinf
> which patch set breaks nfs3.
Yes, I simply remove this patch and v3 started working again.

>
>> Also the following warnings occur after the patch is applied:
>>
>> cache.c:128:21: warning: ignoring return value of 'write', declared
>> with attribute warn_unused_result [-Wunused-result] cache.c:1495:21:
>> warning: ignoring return value of 'write', declared with attribute
>> warn_unused_result [-Wunused-result] nfssvc.c:71:8: warning: ignoring
>> return value of 'system', declared with attribute warn_unused_result
>> [-Wunused-result] nfssvc.c:325:9: warning: ignoring return value of
>> 'write', declared with attribute warn_unused_result [-Wunused-result]
>
> My patch seems introduce few of these, but I think some of this come
> from Natanaels patch set.
I didn't see them after I remove your patch....

>
>> My suggestion is to break the patch up into several small patches
>> which would help with debugging the problem.
>
> Yes. Might be a good idea.
>
>> I'll be more than willing to reconsider this change when it
>> stop braking v3 mounts and there are no warnings.
>
> Thanks for review, testing and feedback.
>
np...

steved.