* [PATCH 03/17] mount.nfs: Remove support for "-t" option
@ 2007-07-16 3:15 Chuck Lever
2007-07-16 3:37 ` Neil Brown
0 siblings, 1 reply; 3+ messages in thread
From: Chuck Lever @ 2007-07-16 3:15 UTC (permalink / raw)
To: neilb; +Cc: nfs
/bin/mount will never pass "-t" to a mount helper, since it passes the
fs-type in the name of the program it is executing.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
utils/mount/mount.c | 64 ++++++++++++++++++++++-----------------------------
1 files changed, 27 insertions(+), 37 deletions(-)
diff --git a/utils/mount/mount.c b/utils/mount/mount.c
index 5a7bd1c..337d7cc 100644
--- a/utils/mount/mount.c
+++ b/utils/mount/mount.c
@@ -57,7 +57,6 @@ static struct option longopts[] = {
{ "read-write", 0, 0, 'w' },
{ "rw", 0, 0, 'w' },
{ "options", 1, 0, 'o' },
- { "nfsvers", 1, 0, 't' },
{ "bind", 0, 0, 128 },
{ "replace", 0, 0, 129 },
{ "after", 0, 0, 130 },
@@ -217,17 +216,18 @@ int do_mount_syscall(char *spec, char *node, char *type, int flags, void *data)
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("usage: %s remotetarget dir [-rvVwfnh] [-o nfsoptions]\n",
+ progname);
+ printf("options:\n");
+ printf("\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");
- printf("\t-f\t\tFake mount, don't actually mount\n");
+ printf("\t-f\t\tFake mount, do not actually mount\n");
printf("\t-n\t\tDo not update /etc/mtab\n");
printf("\t-s\t\tTolerate sloppy mount options rather than failing.\n");
printf("\t-h\t\tPrint this help\n");
- printf("\tversion\t\tnfs4 - NFS version 4, nfs - older NFS version supported\n");
- printf("\tnfsoptions\tRefer mount.nfs(8) or nfs(5)\n\n");
+ printf("\tnfsoptions\tRefer to mount.nfs(8) or nfs(5)\n\n");
}
static inline void
@@ -381,9 +381,9 @@ static int start_statd()
int main(int argc, char *argv[])
{
- int c, flags = 0, nfs_mount_vers = 0, mnt_err = 1, fake = 0;
- char *spec, *mount_point, *extra_opts = NULL;
- char *mount_opts = NULL;
+ int c, flags = 0, mnt_err = 1, fake = 0;
+ char *spec, *mount_point, *fs_type = "nfs";
+ char *extra_opts = NULL, *mount_opts = NULL;
uid_t uid = getuid();
progname = basename(argv[0]);
@@ -413,23 +413,12 @@ int main(int argc, char *argv[])
mount_point = argv[2];
argv[2] = argv[0]; /* so that getopt error messages are correct */
- while ((c = getopt_long (argc - 2, argv + 2, "rt:vVwfno:hs",
+ while ((c = getopt_long (argc - 2, argv + 2, "r:vVwfno:hs",
longopts, NULL)) != -1) {
switch (c) {
case 'r':
flags |= MS_RDONLY;
break;
- case 't':
- if (strcmp(optarg, "nfs4") == 0)
- nfs_mount_vers = 4;
- else if (strcmp(optarg, "nfs") == 0)
- nfs_mount_vers = 0;
- else {
- fprintf(stderr, "%s: unknown filesystem type: %s\n",
- progname, optarg);
- exit(1);
- }
- break;
case 'v':
++verbose;
break;
@@ -488,26 +477,29 @@ int main(int argc, char *argv[])
}
if (strcmp(progname, "mount.nfs4") == 0)
- nfs_mount_vers = 4;
+ fs_type = "nfs4";
+ /*
+ * If a non-root user is attempting to mount, make sure the
+ * user's requested options match the options specified in
+ * /etc/fstab; otherwise, don't allow the mount.
+ */
if (uid != 0) {
- /* don't even think about it unless options exactly
- * match fstab
- */
struct mntentchn *mc;
if ((mc = getfsfile(mount_point)) == NULL ||
- strcmp(mc->m.mnt_fsname, spec) != 0 ||
- strcmp(mc->m.mnt_type, (nfs_mount_vers == 4 ? "nfs4":"nfs")) != 0
- ) {
- fprintf(stderr, "%s: permission died - no match for fstab\n",
- progname);
+ !strcmp(mc->m.mnt_fsname, spec) ||
+ !strcmp(mc->m.mnt_type, fs_type)) {
+ fprintf(stderr, "%s: permission denied: no match for %s "
+ "found in /etc/fstab\n", progname, mount_point);
exit(1);
}
- /* 'mount' munges the options from fstab before passing them
+
+ /*
+ * 'mount' munges the options from fstab before passing them
* to us, so it is non-trivial to test that we have the correct
* set of options and we don't want to trust what the user
- * gave us, so just take whatever is in fstab
+ * gave us, so just take whatever is in /etc/fstab.
*/
mount_opts = strdup(mc->m.mnt_opts);
mounttype = 0;
@@ -533,7 +525,7 @@ int main(int argc, char *argv[])
if (chk_mountpoint(mount_point))
exit(EX_FAIL);
- if (nfs_mount_vers == 4)
+ if (!strcmp(fs_type, "nfs4"))
mnt_err = nfs4mount(spec, mount_point, &flags, &extra_opts, &mount_opts, 0);
else {
int need_statd = 0;
@@ -557,8 +549,7 @@ int main(int argc, char *argv[])
exit(EX_FAIL);
if (!fake) {
- mnt_err = do_mount_syscall(spec, mount_point,
- nfs_mount_vers == 4 ? "nfs4" : "nfs",
+ mnt_err = do_mount_syscall(spec, mount_point, fs_type,
flags & ~(MS_USER|MS_USERS) ,
mount_opts);
@@ -569,8 +560,7 @@ int main(int argc, char *argv[])
}
if (!nomtab)
- add_mtab(spec, mount_point,
- nfs_mount_vers == 4 ? "nfs4" : "nfs",
+ add_mtab(spec, mount_point, fs_type,
flags, extra_opts, 0, 0);
return 0;
-------------------------------------------------------------------------
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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 03/17] mount.nfs: Remove support for "-t" option
2007-07-16 3:15 [PATCH 03/17] mount.nfs: Remove support for "-t" option Chuck Lever
@ 2007-07-16 3:37 ` Neil Brown
2007-07-16 3:44 ` Chuck Lever
0 siblings, 1 reply; 3+ messages in thread
From: Neil Brown @ 2007-07-16 3:37 UTC (permalink / raw)
To: Chuck Lever; +Cc: nfs
On Sunday July 15, chuck.lever@oracle.com wrote:
> /bin/mount will never pass "-t" to a mount helper, since it passes the
> fs-type in the name of the program it is executing.
Does it hurt to leave it there though? Maybe it makes unit-testing
easier?
Or maybe not....
> @@ -413,23 +413,12 @@ int main(int argc, char *argv[])
> mount_point = argv[2];
>
> argv[2] = argv[0]; /* so that getopt error messages are correct */
> - while ((c = getopt_long (argc - 2, argv + 2, "rt:vVwfno:hs",
> + while ((c = getopt_long (argc - 2, argv + 2, "r:vVwfno:hs",
^^
Ooops. -r takes an argument now?
> if ((mc = getfsfile(mount_point)) == NULL ||
> - strcmp(mc->m.mnt_fsname, spec) != 0 ||
> - strcmp(mc->m.mnt_type, (nfs_mount_vers == 4 ? "nfs4":"nfs")) != 0
> - ) {
> - fprintf(stderr, "%s: permission died - no match for fstab\n",
> - progname);
> + !strcmp(mc->m.mnt_fsname, spec) ||
^^^^
I absolutely LOATHE that construct. The first thing you read is "!"
which means "not" but it is actually a test for "are these things
equal" which makes it confusing (to me). So I always like to see the
result of strcmp tested with "== 0" "< 0" "> 0" as the relation
given applies exactly to the tested relation between the strings.
Further:
> - strcmp(mc->m.mnt_fsname, spec) != 0 ||
and
> + !strcmp(mc->m.mnt_fsname, spec) ||
are opposite tests, and it is not clear to me why you inverted the
test.
> @@ -533,7 +525,7 @@ int main(int argc, char *argv[])
> if (chk_mountpoint(mount_point))
> exit(EX_FAIL);
>
> - if (nfs_mount_vers == 4)
> + if (!strcmp(fs_type, "nfs4"))
^
There it is again... HATE HATE HATE HATE .....
It makes me think 'If the fs_type is not "nfs4"' which is not what is
going on at all.
But otherwise there are some fairly useful clean-ups in there.
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 03/17] mount.nfs: Remove support for "-t" option
2007-07-16 3:37 ` Neil Brown
@ 2007-07-16 3:44 ` Chuck Lever
0 siblings, 0 replies; 3+ messages in thread
From: Chuck Lever @ 2007-07-16 3:44 UTC (permalink / raw)
To: Neil Brown; +Cc: nfs
[-- Attachment #1: Type: text/plain, Size: 2223 bytes --]
Neil Brown wrote:
> On Sunday July 15, chuck.lever@oracle.com wrote:
>> /bin/mount will never pass "-t" to a mount helper, since it passes the
>> fs-type in the name of the program it is executing.
>
> Does it hurt to leave it there though? Maybe it makes unit-testing
> easier?
> Or maybe not....
Well, part of these changes remove stuff that just seems confusing to
anyone trying to understand the code, and yes, this one is quite
optional. Honestly, I couldn't think of a case where this could
possibly be useful, but I'm open to education.
The fact that we can do a "mount.nfs -t nfs4" and that's the same as
"mount.nfs4" seems a little weird. The mount command will parse the
"-t" option and invoke either "mount.nfs" or "mount.nfs4"... extra logic
here is just going to cloud things.
>> @@ -413,23 +413,12 @@ int main(int argc, char *argv[])
>> mount_point = argv[2];
>>
>> argv[2] = argv[0]; /* so that getopt error messages are correct */
>> - while ((c = getopt_long (argc - 2, argv + 2, "rt:vVwfno:hs",
>> + while ((c = getopt_long (argc - 2, argv + 2, "r:vVwfno:hs",
> ^^
>
> Ooops. -r takes an argument now?
Hmmm. I'll fix that too.
>> if ((mc = getfsfile(mount_point)) == NULL ||
>> - strcmp(mc->m.mnt_fsname, spec) != 0 ||
>> - strcmp(mc->m.mnt_type, (nfs_mount_vers == 4 ? "nfs4":"nfs")) != 0
>> - ) {
>> - fprintf(stderr, "%s: permission died - no match for fstab\n",
>> - progname);
>> + !strcmp(mc->m.mnt_fsname, spec) ||
> ^^^^
>
> I absolutely LOATHE that construct. The first thing you read is "!"
> which means "not" but it is actually a test for "are these things
> equal" which makes it confusing (to me). So I always like to see the
> result of strcmp tested with "== 0" "< 0" "> 0" as the relation
> given applies exactly to the tested relation between the strings.
OK, now that I know that's the preference, I will use "== 0".
> Further:
>> - strcmp(mc->m.mnt_fsname, spec) != 0 ||
> and
>> + !strcmp(mc->m.mnt_fsname, spec) ||
>
> are opposite tests, and it is not clear to me why you inverted the
> test.
Simple explanation: because I'm a bone-head.
[-- 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2007-07-16 3:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-07-16 3:15 [PATCH 03/17] mount.nfs: Remove support for "-t" option Chuck Lever
2007-07-16 3:37 ` Neil Brown
2007-07-16 3:44 ` Chuck Lever
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.