2009-06-08 14:16:31

by Chuck Lever III

[permalink] [raw]
Subject: Re: should we make --enable-tirpc the default in current nfs-utils?


On Jun 6, 2009, at 4:02 PM, Jeff Layton wrote:

> On Sat, 6 Jun 2009 11:00:41 -0700
> "Muntz, Daniel" <[email protected]> wrote:
>
>>
>>
>>> -----Original Message-----
>>> From: Jeff Layton [mailto:[email protected]]
>>> Sent: Saturday, June 06, 2009 4:12 AM
>>> To: Mike Frysinger
>>> Cc: [email protected]
>>> Subject: Re: should we make --enable-tirpc the default in
>>> current nfs-utils?
>>>
>>> On Fri, 5 Jun 2009 16:50:41 -0400
>>> Mike Frysinger <[email protected]> wrote:
>>>
>>>> On Friday 05 June 2009 13:36:34 Jeff Layton wrote:
>>>>> On Fri, 5 Jun 2009 12:24:39 -0400 Mike Frysinger wrote:
>>>>>> On Friday 05 June 2009 07:36:48 Jeff Layton wrote:
>>>>>>> Doing this now would add wider testing exposure for these
>>>>>>> codepaths and help flush out bugs in TIRPC+IPV4
>>> codepaths. OTOH,
>>>>>>> it means adding a new library dependency for packagers, or
>>>>>>> they'll need to take the conscious step to
>>> --disable-tirpc when they configure.
>>>>>>
>>>>>> or have the configure script dump a warning whenever
>>> libtirpc is
>>>>>> not used ...
>>>>>
>>>>> The problem there is that these sorts of warnings tend to
>>> get lost
>>>>> in the noise. So then you have the situation where people aren't
>>>>> sure whether they built against libtirpc or not. Only running ldd
>>>>> against the binaries will tell you.
>>>>
>>>> the configure script knows whether it's going to be
>>> building against libtirpc.
>>>> it isnt going to happen randomly during `make`.
>>>> AC_MSG_WARNING([
>>>>
>>>> You really should think about switching to libtirpc
>>>>
>>>> ])
>>>>
>>>> maybe it's different in Gentoo, but people report configure
>>> warnings
>>>> all the time ;)
>>>>
>>>
>>> Well, Gentoo probably has a larger percentage of people
>>> compiling the sources. Other distros generally distribute the
>>> binaries. But to be fair, it's not unreasonable to expect
>>> people who are compiling from sources to know what they're doing.
>>>
>>>>>>> We could make it so that configure looks for libtirpc and if
>>>>>>> it's not available, configures the build against legacy RPC
>>>>>>> interfaces. I think this is a bad idea however. While
>>> it should
>>>>>>> "just work" either way, there are some small behavioral
>>>>>>> differences when TIRPC support is built in. I think it's
>>>>>>> probably better to make enabling and disabling TIRPC
>>> a conscious step.
>>>>>>
>>>>>> i think this is the correct behavior for unspecified configure
>>>>>> flags
>>>>>
>>>>> In general, yes. In this case though I think it's reasonable to
>>>>> force people compiling the package without tirpc
>>> installed to take
>>>>> the conscious step to either install the right libs and
>>> headers, or
>>>>> to add --disable-tirpc.
>>>>>
>>>>> I think doing so will lead to a more deterministic
>>> outcome in this
>>>>> situation. If that's a problem however, I'm willing to
>>> listen to the
>>>>> reasoning and reconsider...
>>>>
>>>> i just dont agree with having to re-run configure to "fix"
>>> a condition
>>>> that the configure script should already be able to handle.
>>> but i'm
>>>> speaking in general terms here, not specific to what you propose as
>>>> that isnt exactly the same thing. i dont feel too strongly here,
>>>> especially since it doesnt affect me in any realistic way.
>>>> -mike
>>>
>>> Ok, fair enough. I don't feel terribly strongly about this
>>> either and that is the the conventional way that configure
>>> options work (don't fail unless absolutely necessary). I'll
>>> see about coding up a patch that makes --enable-tirpc the
>>> default but falls back to legacy RPC code with a warning if
>>> TIRPC libs/headers aren't present.
>>
>> Changing the default because the code isn't sufficiently tested
>> strikes
>> me as a particularly bad idea. If Red Hat wants more testing,
>> distribute nfs-utils with TIRPC enabled in Fedora, and _then_
>> change the
>> default in nfs-utils after more testing has occurred. Delegating
>> testing to unsuspecting end-users (especially people who need to
>> rebuild
>> in production environments) seems like an ideal way to cause real
>> problems.
>
> If users have TIRPC installed on their systems, why would we want to
> avoid using it? Pieces of this code (mount.nfs, for instance) are
> pretty much complete and working. There's no real reason to build
> these
> apps against legacy RPC now if we can help it.

The reason to build without it is that libtirpc is largely untested
code (on Linux), and the nfs-utils support to use TI-RPC is also
largely untested. I think the default config settings should
configure a safe, known-working configuration, not the most advanced
configuration.

As much as I like the idea of wider testing, the idea that we happen
to be testing with live users is not inviting. But I guess it's all
we've got at this point.

>> And ffs, don't change the existing configure behavior. When nfs-
>> utils
>> is supposed to build with TIRPC (e.g., when TIRPC is the default),
>> the
>> configure should fail if TIRPC isn't installed. Perhaps the error
>> message on failure could suggest running configure with --disable-
>> tirpc.
>
> nfs-utils is already builds with TIRPC. It also builds with legacy
> RPC.
> So in this discussion the first question is, "Is there some reason to
> not build against TIRPC when it's available on the machine?"

Yes, there is, I would argue.

> Second question: "Should make configure bail out when TIRPC isn't
> available and force the user to specify --disable-tirpc on the command
> line, or should we make the build just fall back to legacy RPC when
> the
> right TIRPC libs/headers aren't present?"

In many other cases (libevent, libnfsidmap, libgssglue, and so on),
configure currently bails if the library is not present. If we make --
enable-tirpc drop back automatically, why wouldn't we also do this for
the others?

Frankly, I think dropping back automatically is not a good idea. The
torrent of messages that configure normally spits out means that
messages about a missing libtirpc are going to be missed in most
cases, and folks will think that because they specified --enable-tirpc
on the configure command line, that's the build they got.

> So far, I'm leaning toward "No" on the first question and to
> "automatically fall back" on the second question.

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


2009-06-08 14:23:57

by Mike Frysinger

[permalink] [raw]
Subject: Re: should we make --enable-tirpc the default in current nfs-utils?

On Monday 08 June 2009 10:16:07 Chuck Lever wrote:
> Frankly, I think dropping back automatically is not a good idea. The
> torrent of messages that configure normally spits out means that
> messages about a missing libtirpc are going to be missed in most
> cases, and folks will think that because they specified --enable-tirpc
> on the configure command line, that's the build they got.

the automatic fallback is when no tirpc option is specified. if --enable-
tirpc is specified, then it should fail (and that is what the proposed patch
does).
-mike

2009-06-08 15:36:54

by Muntz, Daniel

[permalink] [raw]
Subject: RE: should we make --enable-tirpc the default in current nfs-utils?



> -----Original Message-----
> From: Mike Frysinger [mailto:[email protected]]
> Sent: Monday, June 08, 2009 7:24 AM
> To: Chuck Lever
> Cc: Jeff Layton; Muntz, Daniel; [email protected]
> Subject: Re: should we make --enable-tirpc the default in
> current nfs-utils?
>
> On Monday 08 June 2009 10:16:07 Chuck Lever wrote:
> > Frankly, I think dropping back automatically is not a good
> idea. The
> > torrent of messages that configure normally spits out means that
> > messages about a missing libtirpc are going to be missed in most
> > cases, and folks will think that because they specified
> --enable-tirpc
> > on the configure command line, that's the build they got.
>
> the automatic fallback is when no tirpc option is specified.
> if --enable- tirpc is specified, then it should fail (and
> that is what the proposed patch does).
> -mike
>

When libnfsidmap, libgssglue, etc. are not present the build fails. The
builder didn't specify any particular --enable-X flag, and the build
doesn't just do something like fall back and build v3-only. Why would I
want the build to nondeterministically (to the extent that one might not
be aware of what libraries are installed) generate different code?

-Dan

2009-06-08 15:46:43

by Muntz, Daniel

[permalink] [raw]
Subject: RE: should we make --enable-tirpc the default in current nfs-utils?



> -----Original Message-----
> From: Chuck Lever [mailto:[email protected]]
> Sent: Monday, June 08, 2009 7:16 AM
> To: Jeff Layton
> Cc: Muntz, Daniel; Mike Frysinger; [email protected]
> Subject: Re: should we make --enable-tirpc the default in
> current nfs-utils?
>
>
> On Jun 6, 2009, at 4:02 PM, Jeff Layton wrote:
>
> > On Sat, 6 Jun 2009 11:00:41 -0700
> > "Muntz, Daniel" <[email protected]> wrote:
> >
> >>
> >>
> >>> -----Original Message-----
> >>> From: Jeff Layton [mailto:[email protected]]
> >>> Sent: Saturday, June 06, 2009 4:12 AM
> >>> To: Mike Frysinger
> >>> Cc: [email protected]
> >>> Subject: Re: should we make --enable-tirpc the default in current
> >>> nfs-utils?
> >>>
> >>> On Fri, 5 Jun 2009 16:50:41 -0400
> >>> Mike Frysinger <[email protected]> wrote:
> >>>
> >>>> On Friday 05 June 2009 13:36:34 Jeff Layton wrote:
> >>>>> On Fri, 5 Jun 2009 12:24:39 -0400 Mike Frysinger wrote:
> >>>>>> On Friday 05 June 2009 07:36:48 Jeff Layton wrote:
> >>>>>>> Doing this now would add wider testing exposure for these
> >>>>>>> codepaths and help flush out bugs in TIRPC+IPV4
> >>> codepaths. OTOH,
> >>>>>>> it means adding a new library dependency for packagers, or
> >>>>>>> they'll need to take the conscious step to
> >>> --disable-tirpc when they configure.
> >>>>>>
> >>>>>> or have the configure script dump a warning whenever
> >>> libtirpc is
> >>>>>> not used ...
> >>>>>
> >>>>> The problem there is that these sorts of warnings tend to
> >>> get lost
> >>>>> in the noise. So then you have the situation where
> people aren't
> >>>>> sure whether they built against libtirpc or not. Only
> running ldd
> >>>>> against the binaries will tell you.
> >>>>
> >>>> the configure script knows whether it's going to be
> >>> building against libtirpc.
> >>>> it isnt going to happen randomly during `make`.
> >>>> AC_MSG_WARNING([
> >>>>
> >>>> You really should think about switching to libtirpc
> >>>>
> >>>> ])
> >>>>
> >>>> maybe it's different in Gentoo, but people report configure
> >>> warnings
> >>>> all the time ;)
> >>>>
> >>>
> >>> Well, Gentoo probably has a larger percentage of people compiling
> >>> the sources. Other distros generally distribute the
> binaries. But to
> >>> be fair, it's not unreasonable to expect people who are compiling
> >>> from sources to know what they're doing.
> >>>
> >>>>>>> We could make it so that configure looks for libtirpc and if
> >>>>>>> it's not available, configures the build against legacy RPC
> >>>>>>> interfaces. I think this is a bad idea however. While
> >>> it should
> >>>>>>> "just work" either way, there are some small behavioral
> >>>>>>> differences when TIRPC support is built in. I think it's
> >>>>>>> probably better to make enabling and disabling TIRPC
> >>> a conscious step.
> >>>>>>
> >>>>>> i think this is the correct behavior for unspecified configure
> >>>>>> flags
> >>>>>
> >>>>> In general, yes. In this case though I think it's reasonable to
> >>>>> force people compiling the package without tirpc
> >>> installed to take
> >>>>> the conscious step to either install the right libs and
> >>> headers, or
> >>>>> to add --disable-tirpc.
> >>>>>
> >>>>> I think doing so will lead to a more deterministic
> >>> outcome in this
> >>>>> situation. If that's a problem however, I'm willing to
> >>> listen to the
> >>>>> reasoning and reconsider...
> >>>>
> >>>> i just dont agree with having to re-run configure to "fix"
> >>> a condition
> >>>> that the configure script should already be able to handle.
> >>> but i'm
> >>>> speaking in general terms here, not specific to what you
> propose as
> >>>> that isnt exactly the same thing. i dont feel too
> strongly here,
> >>>> especially since it doesnt affect me in any realistic way.
> >>>> -mike
> >>>
> >>> Ok, fair enough. I don't feel terribly strongly about this either
> >>> and that is the the conventional way that configure options work
> >>> (don't fail unless absolutely necessary). I'll see about
> coding up a
> >>> patch that makes --enable-tirpc the default but falls
> back to legacy
> >>> RPC code with a warning if TIRPC libs/headers aren't present.
> >>
> >> Changing the default because the code isn't sufficiently tested
> >> strikes me as a particularly bad idea. If Red Hat wants more
> >> testing, distribute nfs-utils with TIRPC enabled in Fedora, and
> >> _then_ change the default in nfs-utils after more testing has
> >> occurred. Delegating testing to unsuspecting end-users
> (especially
> >> people who need to rebuild in production environments)
> seems like an
> >> ideal way to cause real problems.
> >
> > If users have TIRPC installed on their systems, why would
> we want to
> > avoid using it? Pieces of this code (mount.nfs, for instance) are
> > pretty much complete and working. There's no real reason to build
> > these apps against legacy RPC now if we can help it.
>
> The reason to build without it is that libtirpc is largely
> untested code (on Linux), and the nfs-utils support to use
> TI-RPC is also largely untested. I think the default config
> settings should configure a safe, known-working
> configuration, not the most advanced configuration.
>
> As much as I like the idea of wider testing, the idea that we
> happen to be testing with live users is not inviting. But I
> guess it's all we've got at this point.

It would be nice if RH had a way of testing this with Fedora without
making it the default in the standard nfs-utils package until _after_
testing. Perhaps nfs-utils has evolved to the point where it could use
a release-candidate model. Then all distros could pull an RC build if
they want it, while production users could pull the last "stable"
release.

>
> >> And ffs, don't change the existing configure behavior. When nfs-
> >> utils is supposed to build with TIRPC (e.g., when TIRPC is the
> >> default), the configure should fail if TIRPC isn't installed.
> >> Perhaps the error message on failure could suggest running
> configure
> >> with --disable- tirpc.
> >
> > nfs-utils is already builds with TIRPC. It also builds with legacy
> > RPC.
> > So in this discussion the first question is, "Is there some
> reason to
> > not build against TIRPC when it's available on the machine?"
>
> Yes, there is, I would argue.
>
> > Second question: "Should make configure bail out when TIRPC isn't
> > available and force the user to specify --disable-tirpc on
> the command
> > line, or should we make the build just fall back to legacy RPC when
> > the right TIRPC libs/headers aren't present?"
>
> In many other cases (libevent, libnfsidmap, libgssglue, and
> so on), configure currently bails if the library is not
> present. If we make -- enable-tirpc drop back automatically,
> why wouldn't we also do this for the others?
>
> Frankly, I think dropping back automatically is not a good
> idea. The torrent of messages that configure normally spits
> out means that messages about a missing libtirpc are going to
> be missed in most cases, and folks will think that because
> they specified --enable-tirpc on the configure command line,
> that's the build they got.
>
> > So far, I'm leaning toward "No" on the first question and to
> > "automatically fall back" on the second question.
>
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
>