[PATCH 0 / 1] Move NFS mount code from util-linux to nfs-utils - take2

[PATCH 0 / 1] Move NFS mount code from util-linux to nfs-utils - take2

[Amit Gud]

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



AG
-- 
May the source be with you.
http://www.cis.ksu.edu/~gud


[-- Attachment #2: nfsmount-migration-to-nfsutils-v4.patch.desc --]
[-- Type: text/plain, Size: 1139 bytes --]

Moves the NFS mount code out of util-linux to nfs-utils. The primary reason being ease of maintainability, and keeping util-linux away from all the filesystem-specifc worries.

Adds a new directory 'mount' under utils. Creates a binary mount.nfs. Three symbolic links are created to this binary - mount.nfs4, umount.nfs and umount.nfs4. It is simpler and avoids code duplication (or overhead of fork->exec) by keeping umount code in a single binary along with the mount code. Mount code uses umount to handle certain fail cases. It also makes sense to keep single binary for all the versions of the NFS, as opposed to having seperate binaries mount.nfs and mount.nfs4, since the code falls back to lower version if mounting with v4 fails. Adds two man pages - mount.nfs(8) and umount.nfs(8).

Patches are split into two parts:
[1/2] nfsmount-migration-to-nfsutils-v2.patch
[2/2] nfsmount-migration-to-nfsutils-support-v2.patch

After applying the patches before configuring and compiling, do:
$ rpcgen -c utils/mount/nfsmount.x > utils/mount/nfsmount_xdr.c
$ rpcgen -h utils/mount/nfsmount.x > utils/mount/nfsmount.h
$ sh autogen.sh

---


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



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

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

Re: [PATCH 0 / 1] Move NFS mount code from util-linux to nfs-utils - take2

[Chakravarthi P]

in mount.c :

+void mount_usage()
+{
+	printf("usage: %s remotetarget dir [-rvVwfnh] [-t version] [-o
nfsoptions]\n", progname);
+	printf("options:\n\t-r\t\tMount file system readonly\n");
+	printf("\t-v\t\tVerbose\n");
+	printf("\t-V\t\tPrint version\n");
+	printf("\t-w\t\tMount file system read-write\n");

1. isn't it generally a good practice to have usage message over stderr
?
so the user doesn't miss it. even if he chances to redirect the output
...
just a suggestion.


+			break;
+		case 't':
+			nfs_mount_vers = (strncmp(optarg, "nfs4", 4)) ?
0 : 4;
+			break;

2.  i dont think a -t option makes sense for mount.nfs  
     mount.nfs itself says it is a nfs file system
     you can pick up the version from -o nfsvers=4  to cater to nfs
version
     -t is by legacy meant for specifying the filesystem type.
      
      i also don't see why bind and other options need to be there for
mount.nfs


+
+int add_mtab(char *fsname, char *mount_point, char *fstype, int flags,
char *opts, int freq, int passno)
+{

3. felt that there can be just be one function called mtab_update that
can be kept in 
    the common utility source files you had so that both umount and
mount code can use it.

in nfs4mount.c
+#if defined(VAR_LOCK_DIR)
+#define DEFAULT_DIR VAR_LOCK_DIR
+#else
+#define DEFAULT_DIR "/var/lock/subsys"
+#endif
+
+extern int verbose;
+
+char *IDMAPLCK = DEFAULT_DIR "/rpcidmapd";
+#define idmapd_check() do { \
+	if (access(IDMAPLCK, F_OK)) { \
+		printf(_("Warning: rpc.idmapd appears not to be
running.\n" \
+			"         All uids will be mapped to the nobody
uid.\n")); \
+	} \
+} while(0);
+
+char *GSSDLCK = DEFAULT_DIR "/rpcgssd";
+#define gssd_check() do { \
+		if (access(GSSDLCK, F_OK)) { \
+			printf(_("Warning: rpc.gssd appears not to be
running.\n")); \
+		} \
+} while(0); 


4. very nice thing to do ... warn the user if idmapd and gssd are not
running.
    But then idmapd on suse doesn't seem to be writing into
/var/lock/subsys ... 
    Is /var/lock/subsys/  the standard way for daemons to show their
existence. ?
    or is creation of  /var/run/<daemon_name>  a standard ?
    then idmapd scripts might need some change .. or else ..
    code needs some modification here to be able to work fine in all
distros ...

in nfsumount.c :

+	spec = argv[1];
+
+	argv += 1;
+	argc -= 1;
+
+	while ((c = getopt_long (argc, argv, "fvnrlh",
+				umount_longopts, NULL)) != -1) {

 it'll be nice if you include -a option and have all nfs or nfs4 mounts
umounted on calling
 umount.nfs -a 
 this is should be easy enough with an appropriate mtab utility
function.


 thanks
 chax.



    




P. S. Chakravarthi
Senior Software Engineer
pchakravarthi@novell.com
Phone: 25731856 Extn: 3198

Novell Inc.
Software for the Open Enterprise



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

Re: [PATCH 0 / 1] Move NFS mount code from util-linux to nfs-utils - take2

[Frank Filz]

On Thu, 2006-06-15 at 05:10 -0600, Chakravarthi P wrote:
> +			break;
> +		case 't':
> +			nfs_mount_vers = (strncmp(optarg, "nfs4", 4)) ?
> 0 : 4;
> +			break;
> 
> 2.  i dont think a -t option makes sense for mount.nfs  
>      mount.nfs itself says it is a nfs file system
>      you can pick up the version from -o nfsvers=4  to cater to nfs
> version
>      -t is by legacy meant for specifying the filesystem type.

-t nfs4 is required for compatibility with existing scripts
and /etc/fstab entries.

Frank




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

Re: [PATCH 0 / 1] Move NFS mount code from util-linux to nfs-utils - take2

[Chuck Lever]

On 6/15/06, Frank Filz <ffilzlnx@us.ibm.com> wrote:
> On Thu, 2006-06-15 at 05:10 -0600, Chakravarthi P wrote:
> > +                     break;
> > +             case 't':
> > +                     nfs_mount_vers = (strncmp(optarg, "nfs4", 4)) ?
> > 0 : 4;
> > +                     break;
> >
> > 2.  i dont think a -t option makes sense for mount.nfs
> >      mount.nfs itself says it is a nfs file system
> >      you can pick up the version from -o nfsvers=4  to cater to nfs
> > version
> >      -t is by legacy meant for specifying the filesystem type.
>
> -t nfs4 is required for compatibility with existing scripts
> and /etc/fstab entries.

As far as I understand this, the mount command chooses which
subcommand to execute based on the "-t" option.  So the subcommands
(such as mount.nfs) shouldn't have a "-t" option.

Instead, use two subcommands for NFS:  mount.nfs and mount.nfs4.

-- 
"We who cut mere stones must always be envisioning cathedrals"
   -- Quarry worker's creed


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

Re: [PATCH 0 / 1] Move NFS mount code from util-linux to nfs-utils - take2

[Amit Gud]

Chakravarthi P wrote:
> in mount.c :
> 
> +void mount_usage()
> +{
> +	printf("usage: %s remotetarget dir [-rvVwfnh] [-t version] [-o
> nfsoptions]\n", progname);
> +	printf("options:\n\t-r\t\tMount file system readonly\n");
> +	printf("\t-v\t\tVerbose\n");
> +	printf("\t-V\t\tPrint version\n");
> +	printf("\t-w\t\tMount file system read-write\n");
> 
> 1. isn't it generally a good practice to have usage message over stderr
> ?
> so the user doesn't miss it. even if he chances to redirect the output
> ...
> just a suggestion.
> 

yes, good idea.

> +			break;
> +		case 't':
> +			nfs_mount_vers = (strncmp(optarg, "nfs4", 4)) ?
> 0 : 4;
> +			break;
> 
> 2.  i dont think a -t option makes sense for mount.nfs  
>      mount.nfs itself says it is a nfs file system
>      you can pick up the version from -o nfsvers=4  to cater to nfs
> version
>      -t is by legacy meant for specifying the filesystem type.
>       
>       i also don't see why bind and other options need to be there for
> mount.nfs
> 

The mount.nfs, though a subcommand for mount, is meant to be used even 
as a standalone binary with the limited functionality. -t option is 
there because theres just one binary for the different NFS versions, and 
-t is just used as a way to give the filesystem type, just like for 
mount there are nfs and nfs4 options for -t.


> +
> +int add_mtab(char *fsname, char *mount_point, char *fstype, int flags,
> char *opts, int freq, int passno)
> +{
> 
> 3. felt that there can be just be one function called mtab_update that
> can be kept in 
>     the common utility source files you had so that both umount and
> mount code can use it.

Yes, I will reorganize this.

> in nfs4mount.c
> +#if defined(VAR_LOCK_DIR)
> +#define DEFAULT_DIR VAR_LOCK_DIR
> +#else
> +#define DEFAULT_DIR "/var/lock/subsys"
> +#endif
> +
> +extern int verbose;
> +
> +char *IDMAPLCK = DEFAULT_DIR "/rpcidmapd";
> +#define idmapd_check() do { \
> +	if (access(IDMAPLCK, F_OK)) { \
> +		printf(_("Warning: rpc.idmapd appears not to be
> running.\n" \
> +			"         All uids will be mapped to the nobody
> uid.\n")); \
> +	} \
> +} while(0);
> +
> +char *GSSDLCK = DEFAULT_DIR "/rpcgssd";
> +#define gssd_check() do { \
> +		if (access(GSSDLCK, F_OK)) { \
> +			printf(_("Warning: rpc.gssd appears not to be
> running.\n")); \
> +		} \
> +} while(0); 
> 
> 
> 4. very nice thing to do ... warn the user if idmapd and gssd are not
> running.
>     But then idmapd on suse doesn't seem to be writing into
> /var/lock/subsys ... 
>     Is /var/lock/subsys/  the standard way for daemons to show their
> existence. ?
>     or is creation of  /var/run/<daemon_name>  a standard ?
>     then idmapd scripts might need some change .. or else ..
>     code needs some modification here to be able to work fine in all
> distros ...

I suppose /var/lock/subsys/ is the standard directory. I inherited the 
DEFAULT_DIR, that was being used in the util-linux mount.

> in nfsumount.c :
> 
> +	spec = argv[1];
> +
> +	argv += 1;
> +	argc -= 1;
> +
> +	while ((c = getopt_long (argc, argv, "fvnrlh",
> +				umount_longopts, NULL)) != -1) {
> 
>  it'll be nice if you include -a option and have all nfs or nfs4 mounts
> umounted on calling
>  umount.nfs -a 
>  this is should be easy enough with an appropriate mtab utility
> function.
> 

Yes, nice idea.


Best,
AG
-- 
May the source be with you.
http://www.cis.ksu.edu/~gud



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

Re: [PATCH 0 / 1] Move NFS mount code from util-linux to nfs-utils - take2

[Neil Brown]

On Monday June 12, agud@redhat.com wrote:
> 
> 
> AG
> -- 
> May the source be with you.
> http://www.cis.ksu.edu/~gud
> 
> Moves the NFS mount code out of util-linux to nfs-utils. The primary reason being ease of maintainability, and keeping util-linux away from all the filesystem-specifc worries.
> 
> Adds a new directory 'mount' under utils. Creates a binary mount.nfs. Three symbolic links are created to this binary - mount.nfs4, umount.nfs and umount.nfs4. It is simpler and avoids code duplication (or overhead of fork->exec) by keeping umount code in a single binary along with the mount code. Mount code uses umount to handle certain fail cases. It also makes sense to keep single binary for all the versions of the NFS, as opposed to having seperate binaries mount.nfs and mount.nfs4, since the code falls back to lower version if mounting with v4 fails. Adds two man pages - mount.nfs(8) and umount.nfs(8).
> 
> Patches are split into two parts:
> [1/2] nfsmount-migration-to-nfsutils-v2.patch
> [2/2] nfsmount-migration-to-nfsutils-support-v2.patch
> 
> After applying the patches before configuring and compiling, do:
> $ rpcgen -c utils/mount/nfsmount.x > utils/mount/nfsmount_xdr.c
> $ rpcgen -h utils/mount/nfsmount.x > utils/mount/nfsmount.h
> $ sh autogen.sh

The 'rpcgen' should be done by the Makefile.  I have added a patch
which does this.
However there are now two slightly different nfsmount.x files in the
tree

   ./support/export/mount.x
   ./utils/mount/nfsmount.x

If they could be unified (leave the one in support/export, but merge
in any changes you want from nfsmount.x) and then use the libexport.a
library to get the nfsmount_clnt.o etc, that would be really good.

You can see the current code at
  git://linux-nfs.org/nfs-utils

Further patches against that would be great.

Also, it seems really odd that you need
   ./configure --without-mount
it you want it to compile the mount client.
Surely it should be --with-mount!!!
What is the reasoning there?

NeilBrown


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

Re: [PATCH 0 / 1] Move NFS mount code from util-linux to nfs-utils - take2

[Amit Gud]

Neil Brown wrote:
> On Monday June 12, agud@redhat.com wrote:
>>
>> AG
>> -- 
>> May the source be with you.
>> http://www.cis.ksu.edu/~gud
>>
>> Moves the NFS mount code out of util-linux to nfs-utils. The primary reason being ease of maintainability, and keeping util-linux away from all the filesystem-specifc worries.
>>
>> Adds a new directory 'mount' under utils. Creates a binary mount.nfs. Three symbolic links are created to this binary - mount.nfs4, umount.nfs and umount.nfs4. It is simpler and avoids code duplication (or overhead of fork->exec) by keeping umount code in a single binary along with the mount code. Mount code uses umount to handle certain fail cases. It also makes sense to keep single binary for all the versions of the NFS, as opposed to having seperate binaries mount.nfs and mount.nfs4, since the code falls back to lower version if mounting with v4 fails. Adds two man pages - mount.nfs(8) and umount.nfs(8).
>>
>> Patches are split into two parts:
>> [1/2] nfsmount-migration-to-nfsutils-v2.patch
>> [2/2] nfsmount-migration-to-nfsutils-support-v2.patch
>>
>> After applying the patches before configuring and compiling, do:
>> $ rpcgen -c utils/mount/nfsmount.x > utils/mount/nfsmount_xdr.c
>> $ rpcgen -h utils/mount/nfsmount.x > utils/mount/nfsmount.h
>> $ sh autogen.sh
> 
> The 'rpcgen' should be done by the Makefile.  I have added a patch
> which does this.
> However there are now two slightly different nfsmount.x files in the
> tree
> 
>    ./support/export/mount.x
>    ./utils/mount/nfsmount.x
> 
> If they could be unified (leave the one in support/export, but merge
> in any changes you want from nfsmount.x) and then use the libexport.a
> library to get the nfsmount_clnt.o etc, that would be really good.

sounds good.

> You can see the current code at
>   git://linux-nfs.org/nfs-utils
> 
> Further patches against that would be great.
> 
> Also, it seems really odd that you need
>    ./configure --without-mount
> it you want it to compile the mount client.
> Surely it should be --with-mount!!!
> What is the reasoning there?

I went with the GNU Autoconf Manual ;), 
http://www.gnu.org/software/autoconf/manual/autoconf-2.57/html_mono/autoconf.html#SEC130, 
which says --with-foo, when external program foo is to be used, and 
--without-foo otherwise. But I know, it sounds rather odd. I think, I'II 
change it to --with-mount.


AG
-- 
May the source be with you.
http://www.cis.ksu.edu/~gud



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

Re: [PATCH 0 / 1] Move NFS mount code from util-linux to nfs-utils - take2

[Neil Brown]

On Thursday June 15, agud@redhat.com wrote:
> > 
> > Also, it seems really odd that you need
> >    ./configure --without-mount
> > it you want it to compile the mount client.
> > Surely it should be --with-mount!!!
> > What is the reasoning there?
> 
> I went with the GNU Autoconf Manual ;), 
> http://www.gnu.org/software/autoconf/manual/autoconf-2.57/html_mono/autoconf.html#SEC130, 
> which says --with-foo, when external program foo is to be used, and 
> --without-foo otherwise. But I know, it sounds rather odd. I think, I'II 
> change it to --with-mount.
> 

Ahh... I see what you were thinking, and it does seem to make sense in
a way...

I think the '--with-foo' is when you want the package you are using to
make use of some other program.
In this case nfs-utils doesn't have the option of using mount.  It is
the sysadmin who may use mount, or may use nfs-utils/mount.

I think you should make it an enable/disable feature.
Default to --enable-mount, but allow --disable-mount.

NeilBrown


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