| Message ID | 20110628104138.GB6600@d6fc318.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Trond Myklebust |
| Headers | show |
On Jun 28, 2011, at 6:41 AM, Prem Karat wrote: > > Currently souce and destination parameters should be passed as first and > second paramter while using mount.nfs. This patch allows them to be passed > anywhere while mounting. > > Current functionality is > mount.nfs source destn -o <options> > This patch will allow to do this > mount.nfs -o <options> source destn > or > mount.nfs -o <options> source -o <options> destn Yep, that's clear, but why is this desirable? mount.nfs should be invoked only by the mount command. It's not meant to be run by humans. > Signed-off-by: Prem Karat <prem.karat@linux.vnet.ibm.com> > --- > utils/mount/mount.c | 13 +++++++++---- > 1 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/utils/mount/mount.c b/utils/mount/mount.c > index f3f0a83..62115bb 100644 > --- a/utils/mount/mount.c > +++ b/utils/mount/mount.c > @@ -374,7 +374,7 @@ static int try_mount(char *spec, char *mount_point, int flags, > int main(int argc, char *argv[]) > { > int c, flags = 0, mnt_err = 1, fake = 0; > - char *spec, *mount_point, *fs_type = "nfs"; > + char *spec = NULL, *mount_point = NULL, *fs_type = "nfs"; > char *extra_opts = NULL, *mount_opts = NULL; > uid_t uid = getuid(); > > @@ -398,9 +398,6 @@ int main(int argc, char *argv[]) > exit(EX_USAGE); > } > > - spec = argv[1]; > - mount_point = argv[2]; > - > mount_config_init(progname); > > argv[2] = argv[0]; /* so that getopt error messages are correct */ > @@ -447,6 +444,14 @@ int main(int argc, char *argv[]) > if (optind != argc - 2) { > mount_usage(); > goto out_usage; > + } else { > + while (optind < argc) { > + if (!spec) > + spec = argv[optind]; > + else > + mount_point = argv[optind]; > + optind++; > + } > } > > if (strcmp(progname, "mount.nfs4") == 0) > -- > 1.7.4 > > -- > Cheers, > Prem > Linux Technology Center, > IBM Systems & Technology Labs > DID: 41776362 > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/28/11 12:02pm, Chuck Lever wrote: > > On Jun 28, 2011, at 6:41 AM, Prem Karat wrote: > > > > > Currently souce and destination parameters should be passed as first and > > second paramter while using mount.nfs. This patch allows them to be passed > > anywhere while mounting. > > > > Current functionality is > > mount.nfs source destn -o <options> > > This patch will allow to do this > > mount.nfs -o <options> source destn > > or > > mount.nfs -o <options> source -o <options> destn > > Yep, that's clear, but why is this desirable? mount.nfs should be invoked only by the mount command. It's not meant to be run by humans. Bare with me if my understanding is incorrect, however the man page does say that it can be used as a standalone command with limited functionality. Also it makes sense to use it if a newbie wants to know the nfs specific options. The mount.nfs command shows a pointer to the correct man page for nfs specific options. Cheers, Prem -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/28/2011 12:59 PM, Prem Karat wrote: > On 06/28/11 12:02pm, Chuck Lever wrote: >> >> On Jun 28, 2011, at 6:41 AM, Prem Karat wrote: >> >>> >>> Currently souce and destination parameters should be passed as first and >>> second paramter while using mount.nfs. This patch allows them to be passed >>> anywhere while mounting. >>> >>> Current functionality is >>> mount.nfs source destn -o <options> >>> This patch will allow to do this >>> mount.nfs -o <options> source destn >>> or >>> mount.nfs -o <options> source -o <options> destn >> >> Yep, that's clear, but why is this desirable? mount.nfs should be invoked only by the mount command. It's not meant to be run by humans. > Bare with me if my understanding is incorrect, however the man page does say that > it can be used as a standalone command with limited functionality. > > Also it makes sense to use it if a newbie wants to know the nfs specific > options. The mount.nfs command shows a pointer to the correct man page for > nfs specific options. I agree.... I always thought it as a bit annoying that the command would only take arguments in an unnatural way... steved. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Jun 28, 2011, at 12:59 PM, Prem Karat wrote: > On 06/28/11 12:02pm, Chuck Lever wrote: >> >> On Jun 28, 2011, at 6:41 AM, Prem Karat wrote: >> >>> >>> Currently souce and destination parameters should be passed as first and >>> second paramter while using mount.nfs. This patch allows them to be passed >>> anywhere while mounting. >>> >>> Current functionality is >>> mount.nfs source destn -o <options> >>> This patch will allow to do this >>> mount.nfs -o <options> source destn >>> or >>> mount.nfs -o <options> source -o <options> destn >> >> Yep, that's clear, but why is this desirable? mount.nfs should be invoked only by the mount command. It's not meant to be run by humans. > Bare with me if my understanding is incorrect, however the man page does say that > it can be used as a standalone command with limited functionality. Yes, it can be used that way, but it rarely is, and that's by design. > Also it makes sense to use it if a newbie wants to know the nfs specific > options. The mount.nfs command shows a pointer to the correct man page for > nfs specific options. I don't understand. Why would a "newbie" invoke mount.nfs? Is there a new use case that requires the additional flexibility? I don't have a specific objection here, but this seems like an arbitrary change.
On 06/28/2011 07:49 PM, Chuck Lever wrote: > > On Jun 28, 2011, at 12:59 PM, Prem Karat wrote: > >> On 06/28/11 12:02pm, Chuck Lever wrote: >>> >>> On Jun 28, 2011, at 6:41 AM, Prem Karat wrote: >>> >>>> >>>> Currently souce and destination parameters should be passed as first and >>>> second paramter while using mount.nfs. This patch allows them to be passed >>>> anywhere while mounting. >>>> >>>> Current functionality is >>>> mount.nfs source destn -o <options> >>>> This patch will allow to do this >>>> mount.nfs -o <options> source destn >>>> or >>>> mount.nfs -o <options> source -o <options> destn >>> >>> Yep, that's clear, but why is this desirable? mount.nfs should be invoked only by the mount command. It's not meant to be run by humans. >> Bare with me if my understanding is incorrect, however the man page does say that >> it can be used as a standalone command with limited functionality. > > Yes, it can be used that way, but it rarely is, and that's by design. > >> Also it makes sense to use it if a newbie wants to know the nfs specific >> options. The mount.nfs command shows a pointer to the correct man page for >> nfs specific options. > > I don't understand. Why would a "newbie" invoke mount.nfs? Is there a new use case that requires the additional flexibility? > > I don't have a specific objection here, but this seems like an arbitrary change. When debugging, it's quite handy that one can switch from mount to mount.nfs without having to change the order of the arguments. Cheers Luk -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Jun 28, 2011, at 1:59 PM, Luk Claes wrote: > On 06/28/2011 07:49 PM, Chuck Lever wrote: >> >> On Jun 28, 2011, at 12:59 PM, Prem Karat wrote: >> >>> On 06/28/11 12:02pm, Chuck Lever wrote: >>>> >>>> On Jun 28, 2011, at 6:41 AM, Prem Karat wrote: >>>> >>>>> >>>>> Currently souce and destination parameters should be passed as first and >>>>> second paramter while using mount.nfs. This patch allows them to be passed >>>>> anywhere while mounting. >>>>> >>>>> Current functionality is >>>>> mount.nfs source destn -o <options> >>>>> This patch will allow to do this >>>>> mount.nfs -o <options> source destn >>>>> or >>>>> mount.nfs -o <options> source -o <options> destn >>>> >>>> Yep, that's clear, but why is this desirable? mount.nfs should be invoked only by the mount command. It's not meant to be run by humans. >>> Bare with me if my understanding is incorrect, however the man page does say that >>> it can be used as a standalone command with limited functionality. >> >> Yes, it can be used that way, but it rarely is, and that's by design. >> >>> Also it makes sense to use it if a newbie wants to know the nfs specific >>> options. The mount.nfs command shows a pointer to the correct man page for >>> nfs specific options. The mount(8) man page already has a pointer to nfs(5), it looks like. IMO mount(8) probably shouldn't even mention mount.nfs. >> I don't understand. Why would a "newbie" invoke mount.nfs? Is there a new use case that requires the additional flexibility? >> >> I don't have a specific objection here, but this seems like an arbitrary change. > > When debugging, it's quite handy that one can switch from mount to > mount.nfs without having to change the order of the arguments. Do any of the other mount subcommands (eg. mount.fuse, mount.cifs, mount.tmpfs) need this change too?
On 06/28/2011 06:41 AM, Prem Karat wrote: > > Currently souce and destination parameters should be passed as first and > second paramter while using mount.nfs. This patch allows them to be passed > anywhere while mounting. > > Current functionality is > mount.nfs source destn -o <options> > This patch will allow to do this > mount.nfs -o <options> source destn > or > mount.nfs -o <options> source -o <options> destn > > Signed-off-by: Prem Karat <prem.karat@linux.vnet.ibm.com> Committed... steved. > --- > utils/mount/mount.c | 13 +++++++++---- > 1 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/utils/mount/mount.c b/utils/mount/mount.c > index f3f0a83..62115bb 100644 > --- a/utils/mount/mount.c > +++ b/utils/mount/mount.c > @@ -374,7 +374,7 @@ static int try_mount(char *spec, char *mount_point, int flags, > int main(int argc, char *argv[]) > { > int c, flags = 0, mnt_err = 1, fake = 0; > - char *spec, *mount_point, *fs_type = "nfs"; > + char *spec = NULL, *mount_point = NULL, *fs_type = "nfs"; > char *extra_opts = NULL, *mount_opts = NULL; > uid_t uid = getuid(); > > @@ -398,9 +398,6 @@ int main(int argc, char *argv[]) > exit(EX_USAGE); > } > > - spec = argv[1]; > - mount_point = argv[2]; > - > mount_config_init(progname); > > argv[2] = argv[0]; /* so that getopt error messages are correct */ > @@ -447,6 +444,14 @@ int main(int argc, char *argv[]) > if (optind != argc - 2) { > mount_usage(); > goto out_usage; > + } else { > + while (optind < argc) { > + if (!spec) > + spec = argv[optind]; > + else > + mount_point = argv[optind]; > + optind++; > + } > } > > if (strcmp(progname, "mount.nfs4") == 0) -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/29/2011 07:09 PM, NeilBrown wrote: > On Wed, 29 Jun 2011 10:28:23 -0400 Steve Dickson <SteveD@redhat.com> wrote: > >> >> >> On 06/28/2011 06:41 AM, Prem Karat wrote: >>> >>> Currently souce and destination parameters should be passed as first and >>> second paramter while using mount.nfs. This patch allows them to be passed >>> anywhere while mounting. >>> >>> Current functionality is >>> mount.nfs source destn -o <options> >>> This patch will allow to do this >>> mount.nfs -o <options> source destn >>> or >>> mount.nfs -o <options> source -o <options> destn >>> >>> Signed-off-by: Prem Karat <prem.karat@linux.vnet.ibm.com> >> Committed... > > Uhmm... that's unfortunate because the patch is badly broken. > > With the patch in place, argv[2] gets destroyed. > i.e. mount cannot possibly work correctly now. > > And that is just the start of the problems. > > This patch has clearly never been tested. Hmm... well this is not true... I have 1.2.4 running on a number of machines and without a problem... But I will looking what you are saying... steved. > > I guess we need a 1.2.5 pretty quickly. :-( > > NeilBrown > > >> >> steved. >> >>> --- >>> utils/mount/mount.c | 13 +++++++++---- >>> 1 files changed, 9 insertions(+), 4 deletions(-) >>> >>> diff --git a/utils/mount/mount.c b/utils/mount/mount.c >>> index f3f0a83..62115bb 100644 >>> --- a/utils/mount/mount.c >>> +++ b/utils/mount/mount.c >>> @@ -374,7 +374,7 @@ static int try_mount(char *spec, char *mount_point, int flags, >>> int main(int argc, char *argv[]) >>> { >>> int c, flags = 0, mnt_err = 1, fake = 0; >>> - char *spec, *mount_point, *fs_type = "nfs"; >>> + char *spec = NULL, *mount_point = NULL, *fs_type = "nfs"; >>> char *extra_opts = NULL, *mount_opts = NULL; >>> uid_t uid = getuid(); >>> >>> @@ -398,9 +398,6 @@ int main(int argc, char *argv[]) >>> exit(EX_USAGE); >>> } >>> >>> - spec = argv[1]; >>> - mount_point = argv[2]; >>> - >>> mount_config_init(progname); >>> >>> argv[2] = argv[0]; /* so that getopt error messages are correct */ >>> @@ -447,6 +444,14 @@ int main(int argc, char *argv[]) >>> if (optind != argc - 2) { >>> mount_usage(); >>> goto out_usage; >>> + } else { >>> + while (optind < argc) { >>> + if (!spec) >>> + spec = argv[optind]; >>> + else >>> + mount_point = argv[optind]; >>> + optind++; >>> + } >>> } >>> >>> if (strcmp(progname, "mount.nfs4") == 0) >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jun 30, 2011 at 07:19:12AM -0400, Steve Dickson wrote: > > > On 06/30/2011 07:10 AM, NeilBrown wrote: > > On Thu, 30 Jun 2011 06:58:04 -0400 Steve Dickson <SteveD@redhat.com> wrote: > > > >> > >> > >> On 06/29/2011 07:09 PM, NeilBrown wrote: > >>> On Wed, 29 Jun 2011 10:28:23 -0400 Steve Dickson <SteveD@redhat.com> wrote: > >>> > >>>> > >>>> > >>>> On 06/28/2011 06:41 AM, Prem Karat wrote: > >>>>> > >>>>> Currently souce and destination parameters should be passed as first and > >>>>> second paramter while using mount.nfs. This patch allows them to be passed > >>>>> anywhere while mounting. > >>>>> > >>>>> Current functionality is > >>>>> mount.nfs source destn -o <options> > >>>>> This patch will allow to do this > >>>>> mount.nfs -o <options> source destn > >>>>> or > >>>>> mount.nfs -o <options> source -o <options> destn > >>>>> > >>>>> Signed-off-by: Prem Karat <prem.karat@linux.vnet.ibm.com> > >>>> Committed... > >>> > >>> Uhmm... that's unfortunate because the patch is badly broken. > >>> > >>> With the patch in place, argv[2] gets destroyed. > >>> i.e. mount cannot possibly work correctly now. > >>> > >>> And that is just the start of the problems. > >>> > >>> This patch has clearly never been tested. > >> Hmm... well this is not true... I have 1.2.4 running > >> on a number of machines and without a problem... > > > > I'm guessing you used --enable-libmount-mount which causes the patched code > > to not be compiled. So while you did test nfs-utils, I don't think you > > tested the patch :-( > Thats exactly what happen... With your latest patch I never even compiled > the code... darn! > > hmm... I wonder if its time to throw the enable-libmount-mount-on-by-default > switch... Dumb question, as I haven't been following the libmount stuff, but: how soon before we can throw out the non-libmount code? Looks like patching and maintaining both is a pain. Could we get away with saying "if you want a non-libmount-based mount, feel free to go checkout 1.2.4 or earlier, and backport security fixes to it yourself if you want", and just rip it out now? --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/30/2011 01:40 PM, J. Bruce Fields wrote: > On Thu, Jun 30, 2011 at 07:19:12AM -0400, Steve Dickson wrote: >> >> >> On 06/30/2011 07:10 AM, NeilBrown wrote: >>> On Thu, 30 Jun 2011 06:58:04 -0400 Steve Dickson <SteveD@redhat.com> wrote: >>> >>>> >>>> >>>> On 06/29/2011 07:09 PM, NeilBrown wrote: >>>>> On Wed, 29 Jun 2011 10:28:23 -0400 Steve Dickson <SteveD@redhat.com> wrote: >>>>> >>>>>> >>>>>> >>>>>> On 06/28/2011 06:41 AM, Prem Karat wrote: >>>>>>> >>>>>>> Currently souce and destination parameters should be passed as first and >>>>>>> second paramter while using mount.nfs. This patch allows them to be passed >>>>>>> anywhere while mounting. >>>>>>> >>>>>>> Current functionality is >>>>>>> mount.nfs source destn -o <options> >>>>>>> This patch will allow to do this >>>>>>> mount.nfs -o <options> source destn >>>>>>> or >>>>>>> mount.nfs -o <options> source -o <options> destn >>>>>>> >>>>>>> Signed-off-by: Prem Karat <prem.karat@linux.vnet.ibm.com> >>>>>> Committed... >>>>> >>>>> Uhmm... that's unfortunate because the patch is badly broken. >>>>> >>>>> With the patch in place, argv[2] gets destroyed. >>>>> i.e. mount cannot possibly work correctly now. >>>>> >>>>> And that is just the start of the problems. >>>>> >>>>> This patch has clearly never been tested. >>>> Hmm... well this is not true... I have 1.2.4 running >>>> on a number of machines and without a problem... >>> >>> I'm guessing you used --enable-libmount-mount which causes the patched code >>> to not be compiled. So while you did test nfs-utils, I don't think you >>> tested the patch :-( >> Thats exactly what happen... With your latest patch I never even compiled >> the code... darn! >> >> hmm... I wonder if its time to throw the enable-libmount-mount-on-by-default >> switch... > > Dumb question, as I haven't been following the libmount stuff, but: how > soon before we can throw out the non-libmount code? The switch has been throw in Fedora Rawhide/Fedora 16. > > Looks like patching and maintaining both is a pain. Likes like?? Boy, you have 20/20 8-) > > Could we get away with saying "if you want a non-libmount-based mount, > feel free to go checkout 1.2.4 or earlier, and backport security fixes > to it yourself if you want", and just rip it out now? Well we would not rip it out the code, we would just change the default to always use the libmount code and have a --disable-libmount config flag for people that don't have the libmount code.... I'm thinking that patch would fairly trivial... steved. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 01, 2011 at 04:04:28PM -0400, Steve Dickson wrote: > On 06/30/2011 01:40 PM, J. Bruce Fields wrote: > > Dumb question, as I haven't been following the libmount stuff, but: how > > soon before we can throw out the non-libmount code? > The switch has been throw in Fedora Rawhide/Fedora 16. > > > > > Looks like patching and maintaining both is a pain. > Likes like?? Boy, you have 20/20 8-) > > > > > Could we get away with saying "if you want a non-libmount-based mount, > > feel free to go checkout 1.2.4 or earlier, and backport security fixes > > to it yourself if you want", and just rip it out now? > Well we would not rip it out the code, we would just change the default to > always use the libmount code and have a --disable-libmount config flag > for people that don't have the libmount code.... I'm thinking that > patch would fairly trivial... Yeah, understood, that's why I was asking when we could actually rip out all the code. As long as it's still there, people may be tempted to use it, which means you're stuck maintaining it. --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/utils/mount/mount.c b/utils/mount/mount.c index f3f0a83..62115bb 100644 --- a/utils/mount/mount.c +++ b/utils/mount/mount.c @@ -374,7 +374,7 @@ static int try_mount(char *spec, char *mount_point, int flags, int main(int argc, char *argv[]) { int c, flags = 0, mnt_err = 1, fake = 0; - char *spec, *mount_point, *fs_type = "nfs"; + char *spec = NULL, *mount_point = NULL, *fs_type = "nfs"; char *extra_opts = NULL, *mount_opts = NULL; uid_t uid = getuid(); @@ -398,9 +398,6 @@ int main(int argc, char *argv[]) exit(EX_USAGE); } - spec = argv[1]; - mount_point = argv[2]; - mount_config_init(progname); argv[2] = argv[0]; /* so that getopt error messages are correct */ @@ -447,6 +444,14 @@ int main(int argc, char *argv[]) if (optind != argc - 2) { mount_usage(); goto out_usage; + } else { + while (optind < argc) { + if (!spec) + spec = argv[optind]; + else + mount_point = argv[optind]; + optind++; + } } if (strcmp(progname, "mount.nfs4") == 0)
Currently souce and destination parameters should be passed as first and second paramter while using mount.nfs. This patch allows them to be passed anywhere while mounting. Current functionality is mount.nfs source destn -o <options> This patch will allow to do this mount.nfs -o <options> source destn or mount.nfs -o <options> source -o <options> destn Signed-off-by: Prem Karat <prem.karat@linux.vnet.ibm.com> --- utils/mount/mount.c | 13 +++++++++---- 1 files changed, 9 insertions(+), 4 deletions(-)