[PATCH] [nfs-utils: mountd] exports too verbose.

Steve Dickson

author

2007-03-13 16:02 UTC #0 lore raw

PATCH 1/1 exports too verbose.

This patch is relative to the git tree at:
git://git.infradead.org/~steved/nfs-utils.git
which has been updated to the nfs-utils-1.0.12
release.

Please consider...

steved.

commit daf1b781bf92177ed616e4ced1a3f90353acde25

Author: Steve Dickson <[email protected]>
Date:   Tue Mar 13 11:55:00 2007 -0400

This patch tones down the verbosity of exports that export the same directory for both read-only and read-write. Something similar to:

/var 10.12.32.32(rw,nohide,fsid=1)
/var *(ro,nohide,fsid=1)

Every time the export is access, the following syslog is logged:

/var exported to both * and 10.12.32.32 in *,10.12.32.32

Since the exports are working as expected, this syslog is really not needed. So this patch changes that syslog from a warning to a debugging message.

Signed-off-by Steve Dickson <[email protected]>

utils/mountd/cache.c +1 -1

index a14f4f2..3e30a76 100644
@@ -511,7 +511,7 @@ void nfsd_export(FILE *f)
 			if (!found)
 				found = exp;
 			else {
-				xlog(L_WARNING, "%s exported to both %s and %s in %s",
+				xlog(D_GENERAL, "%s exported to both %s and %s in %s",
 				     path, exp->m_client->m_hostname, found->m_client->m_hostname,
 				     dom);
 			}

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

Neil Brown

2007-03-16 00:36 UTC #1 lore raw

Quoted 6 lines · On Tuesday March 13, [email protected] wrote:

This patch is relative to the git tree at:
git://git.infradead.org/~steved/nfs-utils.git
which has been updated to the nfs-utils-1.0.12
release.
Please consider...

How about this instead...
nfs-utils imposes an ordering to some extent. hostnames first, then subnets, the wildcards, then netgroups.
If the two names are in different places in that ordering, then there really is no need to report anything, as in you case.

OK?

NeilBrown

utils/mountd/cache.c +4 -2

index 6cf24ce..ed7fe35 100644
@@ -490,6 +490,7 @@ void nfsd_export(FILE *f)
 	int i;
 	char *dom, *path;
 	nfs_export *exp, *found = NULL;
+	int found_type = 0;
 
 
 	if (readline(fileno(f), &lbuf, &lbuflen) != 1)

@@ -516,9 +517,10 @@ void nfsd_export(FILE *f)
 				continue;
 			if (strcmp(path, exp->m_export.e_path))
 				continue;
-			if (!found)
+			if (!found) {
 				found = exp;
-			else {
+				found_type = i;
+			} else if (found_type == i) {
 				xlog(L_WARNING, "%s exported to both %s and %s in %s",
 				     path, exp->m_client->m_hostname, found->m_client->m_hostname,
 				     dom);

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

Steve Dickson

author

2007-03-16 13:22 UTC #2 lore raw

Quoted 33 lines · Neil Brown

Quoted 6 lines · On Tuesday March 13, [email protected] wrote:
This patch is relative to the git tree at:
git://git.infradead.org/~steved/nfs-utils.git
which has been updated to the nfs-utils-1.0.12
release.
Please consider...
How about this instead...
nfs-utils imposes an ordering to some extent. hostnames first, then subnets, the wildcards, then netgroups.
If the two names are in different places in that ordering, then there really is no need to report anything, as in you case.
OK?
NeilBrown
utils/mountd/cache.c +1 -0
index 6cf24ce..ed7fe35 100644
@@ -490,6 +490,7 @@ void nfsd_export(FILE *f)
 	int i;
 	char *dom, *path;
 	nfs_export *exp, *found = NULL;
+	int found_type = 0;
 
 
 	if (readline(fileno(f), &lbuf, &lbuflen) != 1)
@@ -516,9 +517,10 @@ void nfsd_export(FILE *f)
 				continue;
 			if (strcmp(path, exp->m_export.e_path))

utils/mountd/cache.c void nfsd_export(FILE *f) · line 526

Quoted 10 lines

utils/mountd/cache.c +3 -2

 				continue;
-			if (!found)
+			if (!found) {
 				found = exp;
-			else {
+				found_type = i;
+			} else if (found_type == i) {
 				xlog(L_WARNING, "%s exported to both %s and %s in %s",
 				     path, exp->m_client->m_hostname, found->m_client->m_hostname,
 				     dom);

hmm... isn't there still a possibility that every time the mis-orders exported is accessed, the a message will be logged? So it might make sense to only log this message once, right?

Also since this is a reordering problem, shouldn't the logged message say its a re-ordering problem, so people will know how to fix it? At this point, the message is stating the obvious... which is really not that helpful as to how to avoid the message... imho..

steved.

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

Neil Brown

2007-03-18 23:23 UTC #3 lore raw

Quoted 40 lines · On Friday March 16, [email protected] wrote:

Quoted 39 lines · Neil Brown
Quoted 6 lines · On Tuesday March 13, [email protected] wrote:
This patch is relative to the git tree at:
git://git.infradead.org/~steved/nfs-utils.git
which has been updated to the nfs-utils-1.0.12
release.
Please consider...
How about this instead...
nfs-utils imposes an ordering to some extent. hostnames first, then subnets, the wildcards, then netgroups.
If the two names are in different places in that ordering, then there really is no need to report anything, as in you case.
OK?
NeilBrown
utils/mountd/cache.c +3 -2
index 6cf24ce..ed7fe35 100644
@@ -490,6 +490,7 @@ void nfsd_export(FILE *f)
 	int i;
 	char *dom, *path;
 	nfs_export *exp, *found = NULL;
+	int found_type = 0;
 
 
 	if (readline(fileno(f), &lbuf, &lbuflen) != 1)
@@ -516,9 +517,10 @@ void nfsd_export(FILE *f)
 				continue;
 			if (strcmp(path, exp->m_export.e_path))
 				continue;
-			if (!found)
+			if (!found) {
 				found = exp;
-			else {
+				found_type = i;

Quoted 8 lines

utils/mountd/cache.c void nfsd_export(FILE *f) · line 526
Quoted 4 lines
utils/mountd/cache.c +1 -0
+			} else if (found_type == i) {
 				xlog(L_WARNING, "%s exported to both %s and %s in %s",
 				     path, exp->m_client->m_hostname, found->m_client->m_hostname,
 				     dom);
hmm... isn't there still a possibility that every time the mis-orders exported is accessed, the a message will be logged? So it might make sense to only log this message once, right?

Not on every access, but only every time the info in the kernel-cache is refreshed. But yes, probably once each would be best.

Quoted 6 lines

Also since this is a reordering problem, shouldn't the logged message say its a re-ordering problem, so people will know how to fix it? At this point, the message is stating the obvious... which is really not that helpful as to how to avoid the message... imho..

The point of the message is really to make the sysadmin aware that their /etc/exports might not be doing exactly what they expect. e.g. if you have
/path @somegroup(ro) @othergroup(rw)

and some host is in both @somegroup and @othergroup, then there is not guarantees about whether that host will get 'ro' or 'rw'. How to fix it some no something that we can even begin to approach without knowing the intension and expectation of sysadmin.

Anyway, how about this on top of the previous patch... any improvement?

Thanks,
NeilBrown

Quoted 1 line

From e3f2d2262fed12f0f6b77c5ac1c4072d82f1e754 Mon Sep 17 00:00:00 2001

From: Neil Brown <[email protected]>
Date: Mon, 19 Mar 2007 10:22:22 +1100
Subject: [PATCH] Make warning about host matching multiple exports more helpful.

1/ only warn once per export, as it could get too noisy. 2/ make it a little clearer why this might be a problem.

---
 support/export/export.c    |    2 ++
 support/include/exportfs.h |    4 +++-
 utils/exportfs/exportfs.c  |    2 ++
 utils/mountd/cache.c       |    8 +++++---
 4 files changed, 12 insertions(+), 4 deletions(-)

support/export/export.c +2 -0

index a4b0788..74e1d1b 100644
@@ -90,6 +90,7 @@ export_init(nfs_export *exp, nfs_client *clp, struct exportent *nep)
 	exp->m_xtabent = 0;
 	exp->m_mayexport = 0;
 	exp->m_changed = 0;
+	exp->m_warned = 0;
 	exp->m_client = clp;
 	clp->m_count++;
 }

@@ -115,6 +116,7 @@ export_dup(nfs_export *exp, struct hostent *hp)
 	new->m_exported = 0;
 	new->m_xtabent = 0;
 	new->m_changed = 0;
+	new->m_warned = 0;
 	export_add(new);
 
 	return new;

support/include/exportfs.h +3 -1

index 458611b..431b5ce 100644
@@ -47,7 +47,9 @@ typedef struct mexport {
 	int			m_exported;	/* known to knfsd. -1 means not sure */
 	int			m_xtabent  : 1,	/* xtab entry exists */
 				m_mayexport: 1,	/* derived from xtabbed */
-				m_changed  : 1; /* options (may) have changed */
+				m_changed  : 1, /* options (may) have changed */
+				m_warned   : 1; /* warned about multiple exports
+						 * matching one client */
 } nfs_export;
 
 extern nfs_client *		clientlist[MCL_MAXTYPES];

utils/exportfs/exportfs.c +2 -0

index 1eca567..8c3f634 100644
@@ -218,6 +218,7 @@ export_all(int verbose)
 			exp->m_xtabent = 1;
 			exp->m_mayexport = 1;
 			exp->m_changed = 1;
+			exp->m_warned = 0;
 		}
 	}
 }

@@ -274,6 +275,7 @@ exportfs(char *arg, char *options, int verbose)
 	exp->m_xtabent = 1;
 	exp->m_mayexport = 1;
 	exp->m_changed = 1;
+	exp->m_warned = 0;
 	if (hp) free (hp);
 }

utils/mountd/cache.c +5 -3

index ed7fe35..5612a9e 100644
@@ -520,10 +520,12 @@ void nfsd_export(FILE *f)
 			if (!found) {
 				found = exp;
 				found_type = i;
-			} else if (found_type == i) {
-				xlog(L_WARNING, "%s exported to both %s and %s in %s",
-				     path, exp->m_client->m_hostname, found->m_client->m_hostname,
+			} else if (found_type == i && found->m_warned == 0) {
+				xlog(L_WARNING, "%s exported to both %s and %s, "
+				     "arbitrarily choosing options from first",
+				     path, found->m_client->m_hostname, exp->m_client->m_hostname,
 				     dom);
+				found->m_warned = 1;
 			}
 		}
 	}

Signature · 11 lines

1.4.4.4


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

Steve Dickson

author

2007-03-19 18:47 UTC #4 lore raw

Quoted 2 lines · Neil Brown

Anyway, how about this on top of the previous patch... any improvement?

Looks good... The main complaint was the number of times the message was being logged so logging it once is good... But there was some grumbling about a message being logged for an valid export configuration...

steved.

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs