[PATCH] nfs-utils 1 of 10 - Removed sync warning on readonly exports

[PATCH] nfs-utils 1 of 10 - Removed sync warning on readonly exports

[Steve Dickson]

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

The following 10 patches are a result of bugs that were reported
to us and have been fixed in various Red Hat releases.
I'm hopeful that at least some of them will incorporated into
the SourceForge CVS tree....

steved.



[-- Attachment #2: nfs-utils-1.0.6-expwarn.patch --]
[-- Type: text/x-patch, Size: 610 bytes --]

Removed sync warning on readonly exports.

Signed-off-by: Steve Dickson <steved@redhat.com>
---------
--- src/support/nfs/exports.c.org	2003-08-06 01:19:42.000000000 -0400
+++ src/support/nfs/exports.c	2004-06-14 06:48:29.000000000 -0400
@@ -442,7 +442,7 @@ bad_option:
 	ep->e_nsqgids = nsqgids;
 
 out:
-	if (warn && !had_sync_opt)
+	if (warn && !had_sync_opt && !(ep->e_flags & NFSEXP_READONLY))
 		xlog(L_WARNING, "%s [%d]: No 'sync' or 'async' option specified for export \"%s:%s\".\n"
 				"  Assuming default behaviour ('sync').\n"
 		     		"  NOTE: this default has changed from previous versions\n",

Re: [PATCH] nfs-utils 1 of 10 - Removed sync warning on readonly exports

[Neil Brown]

On Friday September 23, SteveD@redhat.com wrote:
> The following 10 patches are a result of bugs that were reported
> to us and have been fixed in various Red Hat releases.
> I'm hopeful that at least some of them will incorporated into
> the SourceForge CVS tree....
> 
> steved.
> 

Thanks Steve (and sorry about the delay).
I've incorporated some of them into SourceForge CVS (oh how I wish it
was something other than CVS !!).

Tree is tagged nfs-utils-1-0-7-post6 after the commit, and -post5
before.

 "[NFS] [PATCH] nfs-utils 1 of 10  - Removed sync warning on readonly exports"
   Included unchanged.

 "[NFS] [PATCH] nfs-utils 2 of 10 - Incorporate some clean up code from Ulrich Drepper"
   I re-wrote this with a separate function 'closeall' which is called
   from various places.  
   The 'signal' changes I left out because there was no explanatory
   text :-)

 "[NFS] [PATCH] nfs-utils 3 of 10 - Make sure check_new_cache() looks in the right place"
   I think the change to check_new_cache is wrong.  It really needs to
   check if /proc/fs/nfs{,d} is mounted, and it doesn't do that any
   more.
   The other change in this patch -removing the warning - is included
   unchanged. 

 "[NFS] [PATCH] nfs-utils 4 of 10 - Replaced a memcopy with explicit assignment"
   Already in the CVS.

 "[NFS] [PATCH] nfs-utils 5 of 10 - make sure the correct hostname is used in the SM_NOTIFY"
   Rather ugly, but then statd is fairly ugly already.  I looks
   believable so I included it unchanged.

 "[NFS] [PATCH] nfs-utils 6 of 10 - Fixed subscripting problem in idmapd"
   Yep, good.

 "[NFS] [PATCH] nfs-utils 7 of 10 - Changed mountd to use stat64()"
   Not included. 
   Can you say more about:

           I wonder if it would be better just to make mountd a 64bit
           application by compiling with -D_FILE_OFFSET_BITS=64?

   I thought that when you compiled with a newer glibc, you
   automagically got 64bit stuff...


 "[NFS] [PATCH] nfs-utils 8 of 10 - added the command line arguments to rpc.nfsd"
   Not included.  Uses functionality specific to redhat kernel.

 "[NFS] [PATCH] nfs-utils 9 of 10 - Fixed some security issues found by SGI"
   The bugzilla entry isn't world-readable, but the patch looks fine
   and is included as-is.

 "[NFS] [PATCH] nfs-utils 9 of 10 - Changed xlog to use LOG_INFO instead of LOG_DEBUG"
   Yep, that's ok too.

If you would like to 
 - comment more on the 'signal' changes in patch 3
 - talk about the underlying problem wrt check_new_cache()
 - help me understand the 64-bit options wrt patch 7
 - re-open the debate on specifying version/protocol option to the
   kernel,

I am all ears.

NeilBrown


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] nfs-utils 1 of 10 - Removed sync warning on readonly exports

[Steve Dickson]

Neil Brown wrote:
> Thanks Steve (and sorry about the delay).
np...

> I've incorporated some of them into SourceForge CVS 
Excellent! The more I can get in your world the less I need
to keep in mine... ;-)

> 
>  "[NFS] [PATCH] nfs-utils 2 of 10 - Incorporate some clean up code from Ulrich Drepper"
>    I re-wrote this with a separate function 'closeall' which is called
>    from various places.  
Cool...

>    The 'signal' changes I left out because there was no explanatory
>    text :-)
yeah, our glibc guys notice the fact we weren't using the newer
signal management API (i.e. setset()) and for some reason
(which I can't remember at the moment) they thought that was bad.
I'll ping them and ask what their reason was...

> 
>  "[NFS] [PATCH] nfs-utils 3 of 10 - Make sure check_new_cache() looks in the right place"
>    I think the change to check_new_cache is wrong.  It really needs to
>    check if /proc/fs/nfs{,d} is mounted, and it doesn't do that any
>    more.
Well why should mountd and exportfs care if /proc/fs/nfs exists?
Maybe I'm missing something, but it appears that all the cache_XXX
routines only need the auth.unix.ip, nfsd.export, and nfsd.fh files
that are in /proc/fs/nfsd. So I thought it made sense that
check_new_cache() should run through a file list (similar to what the
other cache_XXX routines do) making sure those files exist. With the
obvious point being, if those files do exist, the filesystem is mounted.


> 
>  "[NFS] [PATCH] nfs-utils 7 of 10 - Changed mountd to use stat64()"
>    Not included. 
>    Can you say more about:
> 
>            I wonder if it would be better just to make mountd a 64bit
>            application by compiling with -D_FILE_OFFSET_BITS=64?
> 
>    I thought that when you compiled with a newer glibc, you
>    automagically got 64bit stuff...
Even on 32bit machine? I thought you had to explicitly ask the
complier for 64bit offsets...  Anyways, this patch stop
rpc.mountd from failing when a file > 2g was mounted.
(i.e. 'mount server:/export/3gigfile /mnt/3gigfile' would cause
rpc.mountd to fail with with "Value too large for defined data type").

My fix was was to change all the stat() calls to stat64() calls
which fix the problem. But I was wondering if it made more sense
to back out my patch and simply compile with the
-D_FILE_OFFSET_BITS=64 compile flag....


> 
>  "[NFS] [PATCH] nfs-utils 8 of 10 - added the command line arguments to rpc.nfsd"
>    Not included.  Uses functionality specific to redhat kernel.
> 
Right... but the patch is not depended on any particular kernel.
Meaning rpc.nfsd, with this patch, will function correctly
with or without the kernel patch...


>  - re-open the debate on specifying version/protocol option to the
>    kernel,
> 
> I am all ears.
Good... I'll post the latest version asap...

steved.


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs