[PATCH 04/17] mount.nfs: /bin/mount already handles --bind & friends

[PATCH 04/17] mount.nfs: /bin/mount already handles --bind & friends

[Chuck Lever]

Clean-up: remove logic to handle --bind and other such options from
mount.nfs[4].

These options are already handled in /bin/mount, and the logic for handling
them in the NFS helper is currently disabled.  Other helpers such as
mount.ocfs2 appear not to support --bind (ie. they rely on /bin/mount to
do it).

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 utils/mount/mount.c           |   45 +++++------------------------------------
 utils/mount/mount_constants.h |    6 +++++
 2 files changed, 11 insertions(+), 40 deletions(-)

diff --git a/utils/mount/mount.c b/utils/mount/mount.c
index 337d7cc..c05f907 100644
--- a/utils/mount/mount.c
+++ b/utils/mount/mount.c
@@ -43,7 +43,6 @@
 char *progname;
 int nomtab;
 int verbose;
-int mounttype;
 int sloppy;
 
 static struct option longopts[] = {
@@ -57,13 +56,6 @@ static struct option longopts[] = {
   { "read-write", 0, 0, 'w' },
   { "rw", 0, 0, 'w' },
   { "options", 1, 0, 'o' },
-  { "bind", 0, 0, 128 },
-  { "replace", 0, 0, 129 },
-  { "after", 0, 0, 130 },
-  { "before", 0, 0, 131 },
-  { "over", 0, 0, 132 },
-  { "move", 0, 0, 133 },
-  { "rbind", 0, 0, 135 },
   { NULL, 0, 0, 0 }
 };
 
@@ -75,11 +67,6 @@ struct opt_map {
   int  mask;                    /* flag mask value */
 };
 
-/* Custom mount options for our own purposes.  */
-#define MS_DUMMY	0x00000000
-#define MS_USERS	0x40000000
-#define MS_USER		0x80000000
-
 static const struct opt_map opt_map[] = {
   { "defaults", 0, 0, 0         },      /* default options */
   { "ro",       1, 0, MS_RDONLY },      /* read-only */
@@ -443,31 +430,10 @@ int main(int argc, char *argv[])
 		case 's':
 			++sloppy;
 			break;
-		case 128: /* bind */
-			mounttype = MS_BIND;
-			break;
-		case 129: /* replace */
-			mounttype = MS_REPLACE;
-			break;
-		case 130: /* after */
-			mounttype = MS_AFTER;
-			break;
-		case 131: /* before */
-			mounttype = MS_BEFORE;
-			break;
-		case 132: /* over */
-			mounttype = MS_OVER;
-			break;
-		case 133: /* move */
-			mounttype = MS_MOVE;
-			break;
-		case 135: /* rbind */
-			mounttype = MS_BIND | MS_REC;
-			break;
 		case 'h':
 		default:
 			mount_usage();
-			exit(1);
+			exit(EX_USAGE);
 		}
 	}
 	if (optind != argc-2) {
@@ -502,7 +468,6 @@ int main(int argc, char *argv[])
 		 * gave us, so just take whatever is in /etc/fstab.
 		 */
 		mount_opts = strdup(mc->m.mnt_opts);
-		mounttype = 0;
 	}
 
 	mount_point = canonicalize(mount_point);
@@ -516,10 +481,10 @@ int main(int argc, char *argv[])
 	parse_opts(mount_opts, &flags, &extra_opts);
 
 	if (uid != 0) {
-	    if (! (flags & (MS_USERS | MS_USER))) {
-		    fprintf(stderr, "%s: permission denied\n", progname);
-		    exit(1);
-	    }
+		if (!(flags & (MS_USERS|MS_USER))) {
+			fprintf(stderr, "%s: permission denied\n", progname);
+			exit(EX_USAGE);
+		}
 	}
 
 	if (chk_mountpoint(mount_point))
diff --git a/utils/mount/mount_constants.h b/utils/mount/mount_constants.h
index 19a7bf5..3e40841 100644
--- a/utils/mount/mount_constants.h
+++ b/utils/mount/mount_constants.h
@@ -39,6 +39,12 @@ if we have a stack or plain mount - mount atop of it, forming a stack. */
 #ifndef MS_VERBOSE
 #define MS_VERBOSE	0x8000	/* 32768 */
 #endif
+
+/* Custom mount options for our own purposes.  */
+#define MS_DUMMY	0x00000000
+#define MS_USERS	0x40000000
+#define MS_USER		0x80000000
+
 /*
  * Magic mount flag number. Had to be or-ed to the flag values.
  */


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 04/17] mount.nfs: /bin/mount already handles --bind & friends

[Neil Brown]

On Sunday July 15, chuck.lever@oracle.com wrote:
> Clean-up: remove logic to handle --bind and other such options from
> mount.nfs[4].
> 
> These options are already handled in /bin/mount, and the logic for handling
> them in the NFS helper is currently disabled.  Other helpers such as
> mount.ocfs2 appear not to support --bind (ie. they rely on /bin/mount to
> do it).

Fair enough - I don't suppose the protocol for how /bin/mount talks to
/sbin/mount.$fstype is documented anywhere???  I guess just in the
code :-(

> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> 
>  utils/mount/mount.c           |   45 +++++------------------------------------
>  utils/mount/mount_constants.h |    6 +++++
>  2 files changed, 11 insertions(+), 40 deletions(-)
> 
> diff --git a/utils/mount/mount.c b/utils/mount/mount.c
> index 337d7cc..c05f907 100644
> --- a/utils/mount/mount.c
> +++ b/utils/mount/mount.c
> @@ -75,11 +67,6 @@ struct opt_map {
>    int  mask;                    /* flag mask value */
>  };
>  
> -/* Custom mount options for our own purposes.  */
> -#define MS_DUMMY	0x00000000
> -#define MS_USERS	0x40000000
> -#define MS_USER		0x80000000
> -
>  static const struct opt_map opt_map[] = {
>    { "defaults", 0, 0, 0         },      /* default options */
>    { "ro",       1, 0, MS_RDONLY },      /* read-only */
> diff --git a/utils/mount/mount_constants.h b/utils/mount/mount_constants.h
> index 19a7bf5..3e40841 100644
> --- a/utils/mount/mount_constants.h
> +++ b/utils/mount/mount_constants.h
> @@ -39,6 +39,12 @@ if we have a stack or plain mount - mount atop of it, forming a stack. */
>  #ifndef MS_VERBOSE
>  #define MS_VERBOSE	0x8000	/* 32768 */
>  #endif
> +
> +/* Custom mount options for our own purposes.  */
> +#define MS_DUMMY	0x00000000
> +#define MS_USERS	0x40000000
> +#define MS_USER		0x80000000
> +
>  /*
>   * Magic mount flag number. Had to be or-ed to the flag values.
>   */

Hmmm.  That wasn't mentioned in the change-log, and I don't find it
convincing.
These flags are for internal use in mount.c so it makes lots of sense
to define them in mount.c so that the definitions never leak.
Did you have a concrete reason for moving them?

NeilBrown

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 04/17] mount.nfs: /bin/mount already handles --bind & friends

[Chuck Lever]

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

Neil Brown wrote:
> On Sunday July 15, chuck.lever@oracle.com wrote:
>> Clean-up: remove logic to handle --bind and other such options from
>> mount.nfs[4].
>>
>> These options are already handled in /bin/mount, and the logic for handling
>> them in the NFS helper is currently disabled.  Other helpers such as
>> mount.ocfs2 appear not to support --bind (ie. they rely on /bin/mount to
>> do it).
> 
> Fair enough - I don't suppose the protocol for how /bin/mount talks to
> /sbin/mount.$fstype is documented anywhere???  I guess just in the
> code :-(

There are rumors that somehow the helpers will turn into plug-ins one 
day.  But yes, this is ambiguous and somewhat crappy.  I did ask the 
developers who constructed mount.ocfs2 about this.

>> diff --git a/utils/mount/mount_constants.h b/utils/mount/mount_constants.h
>> index 19a7bf5..3e40841 100644
>> --- a/utils/mount/mount_constants.h
>> +++ b/utils/mount/mount_constants.h
>> @@ -39,6 +39,12 @@ if we have a stack or plain mount - mount atop of it, forming a stack. */
>>  #ifndef MS_VERBOSE
>>  #define MS_VERBOSE	0x8000	/* 32768 */
>>  #endif
>> +
>> +/* Custom mount options for our own purposes.  */
>> +#define MS_DUMMY	0x00000000
>> +#define MS_USERS	0x40000000
>> +#define MS_USER		0x80000000
>> +
>>  /*
>>   * Magic mount flag number. Had to be or-ed to the flag values.
>>   */
> 
> Hmmm.  That wasn't mentioned in the change-log, and I don't find it
> convincing.
> These flags are for internal use in mount.c so it makes lots of sense
> to define them in mount.c so that the definitions never leak.

Well, if we really wanted to be proper about this, we would include the 
mount_constants.c file from utils-linux instead of maintaining a local 
copy.  But since this is a local copy of the header, I figure that 
adding similar local definitions in here is OK.

I'll make a deeper dive on this to see if there's a better way to 
document this.

> Did you have a concrete reason for moving them?

Yes.  See the later patch that moves the mount system call to nfsmount.c 
and nfs4mount.c.  For some reason I don't recall, I thought it would be 
better to move these here in this patch, but that may have been ill-advised.

[-- Attachment #2: chuck.lever.vcf --]
[-- Type: text/x-vcard, Size: 290 bytes --]

begin:vcard
fn:Chuck Lever
n:Lever;Chuck
org:Oracle Corporation;Corporate Architecture: Linux Projects Group
adr:;;1015 Granger Avenue;Ann Arbor;MI;48104;USA
title:Principal Member of Staff
tel;work:+1 248 614 5091
x-mozilla-html:FALSE
url:http://oss.oracle.com/~cel
version:2.1
end:vcard


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

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

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

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

Re: [PATCH 04/17] mount.nfs: /bin/mount already handles --bind & friends

[Neil Brown]

On Sunday July 15, chuck.lever@oracle.com wrote:
> > 
> > Fair enough - I don't suppose the protocol for how /bin/mount talks to
> > /sbin/mount.$fstype is documented anywhere???  I guess just in the
> > code :-(
> 
> There are rumors that somehow the helpers will turn into plug-ins one 
> day.  But yes, this is ambiguous and somewhat crappy.  I did ask the 
> developers who constructed mount.ocfs2 about this.

Plug-ins sound like a potentially good idea.  The one thing that I
really hate about the current setup is that the helper has to be
setuid root.  Which means that it has no way to know if 'mount' has
sanity checked the options, so it has to do all the checking it self,
and read fstab, which is really dumb.

So, something to look forward to :-)

> 
> Yes.  See the later patch that moves the mount system call to nfsmount.c 
> and nfs4mount.c.

No, I didn't see that...
Oh, there it is.  Patch 11.
"Refactor mount processing slightly...."

I obviously didn't read that patch closely enough.  Makes sense
though.  Thanks.

NeilBrown

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs