Re: [PATCH] mount.nfs: set the default family for lookups based on Defaultproto= setting

Jeff Layton
2010-02-05T16:30:57Z

On Fri, 05 Feb 2010 10:31:28 -0500
Chuck Lever <chuck.lever@oracle.com> wrote:

> On 02/05/2010 09:27 AM, Jeff Layton wrote:
> > When IPv6 is enabled, the Proto= config file option is treated as a
> > netid, and the address family for lookups is selected based on that
> > setting. The Defaultproto= option however still only affects the
> > protocol setting for the sockets (IPPROTO_*) and not the address family.
> >
> > This patch makes it so that if someone sets the "Defaultproto=" option
> > in the nfsmount.conf, it's used to determine the default address family
> > for lookups as well as the protocol type.
> >
> > This gives users a way to force a particular address family to be used
> > universally for mounts and brings the behavior of the Defaultproto=
> > option in line with the Proto= option.
> >
> > Signed-off-by: Jeff Layton<jlayton@redhat.com>
> > ---
> > utils/mount/configfile.c | 8 ++++++++
> > utils/mount/network.c | 18 ++++++++----------
> > 2 files changed, 16 insertions(+), 10 deletions(-)
> >
> > diff --git a/utils/mount/configfile.c b/utils/mount/configfile.c
> > index 6843098..71d3627 100644
> > --- a/utils/mount/configfile.c
> > +++ b/utils/mount/configfile.c
> > @@ -221,6 +221,8 @@ int inline check_vers(char *mopt, char *field)
> >
> > unsigned long config_default_vers;
> > unsigned long config_default_proto;
> > +extern sa_family_t config_default_family;
> > +
> > /*
> > * Check to see if a default value is being set.
> > * If so, set the appropriate global value which will
> > @@ -242,6 +244,12 @@ int inline default_value(char *mopt)
> > xlog_warn("Unable to set default protocol : %s",
> > strerror(errno));
> > }
> > +#ifdef IPV6_SUPPORTED
> > + if (!nfs_nfs_proto_family(options,&config_default_family)) {
> > + xlog_warn("Unable to set default family : %s",
> > + strerror(errno));
> > + }
> > +#endif
>
> Maybe you don't need the #ifdef here?
>
> Aside from making the code more readable, removing the #ifdef would also
> make it so the same code path is followed whether IPv6 is supported or
> not, which makes testing easier.
>
> Just a thought.
>

They probably aren't essential. It all comes down to what we want the
behavior to be in the following situation:

If someone had a TIRPC enabled nfs-utils but IPv6 support is disabled,
and then set Defaultproto=tcp6.

With the ifdef's in place, then the address family part of it would
just be ignored. Without those in place, IPv6 will be forced for the
lookup but we won't actually be able to make use of the addresses
returned.

Granted, it's a bit of a pathological setup, but I don't think the
#ifdef's here really take away anything and may help prevent confusion
with the union of all these different options.

> > } else {
> > xlog_warn("Unable to alloc memory for default protocol");
> > }
> > diff --git a/utils/mount/network.c b/utils/mount/network.c
> > index 92bba2d..0ab3bb1 100644
> > --- a/utils/mount/network.c
> > +++ b/utils/mount/network.c
> > @@ -1331,6 +1331,12 @@ nfs_nfs_port(struct mount_options *options, unsigned long *port)
> > return 1;
> > }
> >
> > +#ifdef IPV6_SUPPORTED
> > +sa_family_t config_default_family = AF_UNSPEC;
> > +#else
> > +sa_family_t config_default_family = AF_INET;
> > +#endif
> > +
> > /*
> > * Returns TRUE and fills in @family if a valid NFS protocol option
> > * is found, or FALSE if the option was specified with an invalid value.
> > @@ -1341,11 +1347,7 @@ int nfs_nfs_proto_family(struct mount_options *options,
> > unsigned long protocol;
> > char *option;
> >
> > -#ifdef IPV6_SUPPORTED
> > - *family = AF_UNSPEC;
> > -#else
> > - *family = AF_INET;
> > -#endif
> > + *family = config_default_family;
> >
> > switch (po_rightmost(options, nfs_transport_opttbl)) {
> > case 0: /* udp */
> > @@ -1488,11 +1490,7 @@ int nfs_mount_proto_family(struct mount_options *options,
> > unsigned long protocol;
> > char *option;
> >
> > -#ifdef HAVE_LIBTIRPC
> > - *family = AF_UNSPEC;
> > -#else
> > - *family = AF_INET;
> > -#endif
> > + *family = config_default_family;
> >
> > option = po_get(options, "mountproto");
> > if (option != NULL)
>
>

--
Jeff Layton <jlayton@redhat.com>