* [PATCH nfs-utils] mount.nfs: skip server address resolution on remount
@ 2016-04-06 18:30 Benjamin Coddington
2016-04-06 18:51 ` Chuck Lever
2016-04-28 12:16 ` Steve Dickson
0 siblings, 2 replies; 4+ messages in thread
From: Benjamin Coddington @ 2016-04-06 18:30 UTC (permalink / raw)
To: steved; +Cc: Chuck Lever, linux-nfs
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.
Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH nfs-utils] mount.nfs: skip server address resolution on remount
2016-04-06 18:30 [PATCH nfs-utils] mount.nfs: skip server address resolution on remount Benjamin Coddington
@ 2016-04-06 18:51 ` Chuck Lever
2016-04-07 8:46 ` Benjamin Coddington
2016-04-28 12:16 ` Steve Dickson
1 sibling, 1 reply; 4+ messages in thread
From: Chuck Lever @ 2016-04-06 18:51 UTC (permalink / raw)
To: Benjamin Coddington; +Cc: Steve Dickson, Linux NFS Mailing List
> On Apr 6, 2016, at 11:30 AM, Benjamin Coddington <bcodding@redhat.com> =
wrote:
>=20
> 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=3D option
when doing a remount?
Have you checked what the output of /proc/mounts looks like
after a remount?
> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
> ---
> utils/mount/stropts.c | 31 ++++++++++++++++++++-----------
> 1 files changed, 20 insertions(+), 11 deletions(-)
>=20
> 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;
>=20
> - hint.ai_family =3D (int)family;
> - error =3D getaddrinfo(mi->hostname, NULL, &hint, &mi->address);
> - if (error !=3D 0) {
> - nfs_error(_("%s: Failed to resolve server %s: %s"),
> - progname, mi->hostname, gai_strerror(error));
> - mi->address =3D 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 =3D (int)family;
> + error =3D getaddrinfo(mi->hostname, NULL, &hint, =
&mi->address);
> + if (error !=3D 0) {
> + nfs_error(_("%s: Failed to resolve server %s: =
%s"),
> + progname, mi->hostname, =
gai_strerror(error));
> + mi->address =3D NULL;
> + return 0;
> + }
> +
> + if (!nfs_append_addr_option(mi->address->ai_addr,
> + mi->address->ai_addrlen, =
mi->options))
> + return 0;
> }
>=20
> 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;
>=20
> - if (!nfs_append_addr_option(mi->address->ai_addr,
> - mi->address->ai_addrlen, =
mi->options))
> - return 0;
> -
> return 1;
> }
>=20
> --=20
> 1.7.1
>=20
> --
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH nfs-utils] mount.nfs: skip server address resolution on remount
2016-04-06 18:51 ` Chuck Lever
@ 2016-04-07 8:46 ` Benjamin Coddington
0 siblings, 0 replies; 4+ messages in thread
From: Benjamin Coddington @ 2016-04-07 8:46 UTC (permalink / raw)
To: Chuck Lever; +Cc: Steve Dickson, Linux NFS Mailing List
On Wed, 6 Apr 2016, Chuck Lever wrote:
>
> > On Apr 6, 2016, at 11:30 AM, Benjamin Coddington <bcodding@redhat.com> 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@redhat.com>
> > ---
> > 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@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> Chuck Lever
>
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH nfs-utils] mount.nfs: skip server address resolution on remount
2016-04-06 18:30 [PATCH nfs-utils] mount.nfs: skip server address resolution on remount Benjamin Coddington
2016-04-06 18:51 ` Chuck Lever
@ 2016-04-28 12:16 ` Steve Dickson
1 sibling, 0 replies; 4+ messages in thread
From: Steve Dickson @ 2016-04-28 12:16 UTC (permalink / raw)
To: Benjamin Coddington; +Cc: Chuck Lever, linux-nfs
On 04/06/2016 02:30 PM, Benjamin Coddington 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.
>
> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
Committed...
steved.
> ---
> 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;
> }
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-04-28 12:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-06 18:30 [PATCH nfs-utils] mount.nfs: skip server address resolution on remount Benjamin Coddington
2016-04-06 18:51 ` Chuck Lever
2016-04-07 8:46 ` Benjamin Coddington
2016-04-28 12:16 ` Steve Dickson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).