On Wed, 6 Apr 2016, Chuck Lever wrote:
>
> > On Apr 6, 2016, at 11:30 AM, Benjamin Coddington <bcodding@xxxxxxxxxx> wrote:
> >
> > A remount might fail if name resolution returns a different server address,
> > as might occur if there are multiple name records for the server. Since we
> > cannot change the server's address on a remount anyway, skip the lookup and
> > remove any set addresses in the options.
>
> That looks better.
>
> But I wonder, is it necessary to remove the old addr= option
> when doing a remount?
If we're not going to validate the value, better not pass in whatever the
user sent. Maybe it would be better to return EINVAL instead?
> Have you checked what the output of /proc/mounts looks like
> after a remount?
Yes, the addr= is unchanged if not provided.
Ben
> > Signed-off-by: Benjamin Coddington <bcodding@xxxxxxxxxx>
> > ---
> > utils/mount/stropts.c | 31 ++++++++++++++++++++-----------
> > 1 files changed, 20 insertions(+), 11 deletions(-)
> >
> > diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> > index 86829a9..634b58c 100644
> > --- a/utils/mount/stropts.c
> > +++ b/utils/mount/stropts.c
> > @@ -383,13 +383,26 @@ static int nfs_validate_options(struct nfsmount_info *mi)
> > if (!nfs_nfs_proto_family(mi->options, &family))
> > return 0;
> >
> > - hint.ai_family = (int)family;
> > - error = getaddrinfo(mi->hostname, NULL, &hint, &mi->address);
> > - if (error != 0) {
> > - nfs_error(_("%s: Failed to resolve server %s: %s"),
> > - progname, mi->hostname, gai_strerror(error));
> > - mi->address = NULL;
> > - return 0;
> > + /*
> > + * A remount is not going to be able to change the server's address,
> > + * nor should we try to resolve another address for the server as we
> > + * may end up with a different address.
> > + */
> > + if (mi->flags & MS_REMOUNT) {
> > + po_remove_all(mi->options, "addr");
> > + } else {
> > + hint.ai_family = (int)family;
> > + error = getaddrinfo(mi->hostname, NULL, &hint, &mi->address);
> > + if (error != 0) {
> > + nfs_error(_("%s: Failed to resolve server %s: %s"),
> > + progname, mi->hostname, gai_strerror(error));
> > + mi->address = NULL;
> > + return 0;
> > + }
> > +
> > + if (!nfs_append_addr_option(mi->address->ai_addr,
> > + mi->address->ai_addrlen, mi->options))
> > + return 0;
> > }
> >
> > if (!nfs_set_version(mi))
> > @@ -398,10 +411,6 @@ static int nfs_validate_options(struct nfsmount_info *mi)
> > if (!nfs_append_sloppy_option(mi->options))
> > return 0;
> >
> > - if (!nfs_append_addr_option(mi->address->ai_addr,
> > - mi->address->ai_addrlen, mi->options))
> > - return 0;
> > -
> > return 1;
> > }
> >
> > --
> > 1.7.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> Chuck Lever
>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
