[PATCH 1/1] nfs-utils: Don't hard code source and destination args

[PATCH 1/1] nfs-utils: Don't hard code source and destination args

[Prem Karat]

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(-)

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

Re: [PATCH 1/1] nfs-utils: Don't hard code source and destination args

[Chuck Lever]

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

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH 1/1] nfs-utils: Don't hard code source and destination args

[Prem Karat]

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

Re: [PATCH 1/1] nfs-utils: Don't hard code source and destination args

[Steve Dickson]

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. 

Re: [PATCH 1/1] nfs-utils: Don't hard code source and destination args

[Chuck Lever]

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.

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH 1/1] nfs-utils: Don't hard code source and destination args

[Luk Claes]

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

Re: [PATCH 1/1] nfs-utils: Don't hard code source and destination args

[Chuck Lever]

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?

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH 1/1] nfs-utils: Don't hard code source and destination args

[Steve Dickson]

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)

Re: [PATCH 1/1] nfs-utils: Don't hard code source and destination args

[NeilBrown]

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.

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

Re: [PATCH 1/1] nfs-utils: Don't hard code source and destination args

[Steve Dickson]

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
> 

Re: [PATCH 1/1] nfs-utils: Don't hard code source and destination args

[NeilBrown]

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 :-(

> 
> But I will looking what you are saying...
> 
Thanks,

NeilBrown

Re: [PATCH 1/1] nfs-utils: Don't hard code source and destination args

[Steve Dickson]

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... 

Well Let me figure out what to do... I'll probably just reissue 1.2.4 

steved.

Re: [PATCH 1/1] nfs-utils: Don't hard code source and destination args

[J. Bruce Fields]

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.

Re: [PATCH 1/1] nfs-utils: Don't hard code source and destination args

[Steve Dickson]

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.

Re: [PATCH 1/1] nfs-utils: Don't hard code source and destination args

[J. Bruce Fields]

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.