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

From: "J. Bruce Fields" <bfields@fieldses.org>
To: Steve Dickson <SteveD@redhat.com>
Cc: NeilBrown <neilb@suse.de>,
	Prem Karat <prem.karat@linux.vnet.ibm.com>,
	linux-nfs@vger.kernel.org
Subject: Re: [PATCH 1/1] nfs-utils: Don't hard code source and destination args
Date: Thu, 30 Jun 2011 13:40:05 -0400	[thread overview]
Message-ID: <20110630174005.GA18713@fieldses.org> (raw)
In-Reply-To: <4E0C5BB0.5030004@RedHat.com>

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.