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

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

[Steve Dickson]

[-- Attachment #1: Type: text/plain, Size: 176 bytes --]

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.



[-- Attachment #2: exports-nonverbose.patch --]
[-- Type: text/x-patch, Size: 1172 bytes --]

commit daf1b781bf92177ed616e4ced1a3f90353acde25
Author: Steve Dickson <steved@redhat.com>
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 <steved@redhat.com>

diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index a14f4f2..3e30a76 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -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);
 			}

[-- Attachment #3: Type: text/plain, Size: 345 bytes --]

-------------------------------------------------------------------------
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

[-- Attachment #4: Type: text/plain, Size: 140 bytes --]

_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

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

[Neil Brown]

On Tuesday March 13, SteveD@redhat.com 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

diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index 6cf24ce..ed7fe35 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -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  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

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

[Steve Dickson]

Neil Brown wrote:
> On Tuesday March 13, SteveD@redhat.com 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
> 
> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
> index 6cf24ce..ed7fe35 100644
> --- a/utils/mountd/cache.c
> +++ b/utils/mountd/cache.c
> @@ -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);
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  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

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

[Neil Brown]

On Friday March 16, SteveD@redhat.com wrote:
> Neil Brown wrote:
> > On Tuesday March 13, SteveD@redhat.com 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
> > 
> > diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
> > index 6cf24ce..ed7fe35 100644
> > --- a/utils/mountd/cache.c
> > +++ b/utils/mountd/cache.c
> > @@ -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);
> 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.

> 
> 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

>From e3f2d2262fed12f0f6b77c5ac1c4072d82f1e754 Mon Sep 17 00:00:00 2001
From: Neil Brown <neilb@suse.de>
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(-)

diff --git a/support/export/export.c b/support/export/export.c
index a4b0788..74e1d1b 100644
--- a/support/export/export.c
+++ b/support/export/export.c
@@ -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;
diff --git a/support/include/exportfs.h b/support/include/exportfs.h
index 458611b..431b5ce 100644
--- a/support/include/exportfs.h
+++ b/support/include/exportfs.h
@@ -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];
diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
index 1eca567..8c3f634 100644
--- a/utils/exportfs/exportfs.c
+++ b/utils/exportfs/exportfs.c
@@ -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);
 }
 
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index ed7fe35..5612a9e 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -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;
 			}
 		}
 	}
-- 
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  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

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

[Steve Dickson]

Neil Brown wrote:
> 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  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs