On Tue, 16 Mar 2010 00:12:01 -0400
Chuck Lever <chuck.lever@xxxxxxxxxx> wrote:
> On 03/15/2010 06:25 PM, Jeff Layton wrote:
> > On Mon, 15 Mar 2010 13:51:37 -0400
> > Chuck Lever<chuck.lever@xxxxxxxxxx> wrote:
> >
> >> The mon_name argument of an SM_NOTIFY request is a string that
> >> identifies the rebooting host.
> >>
> >> sm-notify should send the my_name provided by the local lockd at the
> >> time the remote was monitored, rather than cocking up a mon_name
> >> argument based on the present return value of gethostname(3). If the
> >> local system's hostname happened to change after the last reboot, then
> >> the string returned by gethostname(3) will not be recognized by the
> >> remote. Thus the remote will never initiate lock recovery for the
> >> original named host, possibly leaving stale locks.
> >>
> >> The existing behavior of using the -v command line option as the
> >> mon_name argument is preserved, but we now prevent sending an IP
> >> presentation address, as some non-Linux implementations don't
> >> recognize addresses as valid mon_names.
> >>
> >> Signed-off-by: Chuck Lever<chuck.lever@xxxxxxxxxx>
> >> ---
> >>
> >> utils/statd/sm-notify.c | 41 ++++++++++++++++++++++++++++++++---------
> >> utils/statd/sm-notify.man | 28 +++++++++++++---------------
> >> utils/statd/statd.man | 14 ++------------
> >> 3 files changed, 47 insertions(+), 36 deletions(-)
> >>
> >> diff --git a/utils/statd/sm-notify.c b/utils/statd/sm-notify.c
> >> index 3259a3e..2d14668 100644
> >> --- a/utils/statd/sm-notify.c
> >> +++ b/utils/statd/sm-notify.c
> >> @@ -54,7 +54,7 @@ struct nsm_host {
> >> uint32_t xid;
> >> };
> >>
> >> -static char nsm_hostname[256];
> >> +static char nsm_hostname[SM_MAXSTRLEN + 1];
> >> static int nsm_state;
> >> static int nsm_family = AF_INET;
> >> static int opt_debug = 0;
> >> @@ -412,12 +412,33 @@ usage: fprintf(stderr,
> >> }
> >> }
> >>
> >> - if (opt_srcaddr) {
> >> - strncpy(nsm_hostname, opt_srcaddr, sizeof(nsm_hostname)-1);
> >> - } else
> >> - if (gethostname(nsm_hostname, sizeof(nsm_hostname))< 0) {
> >> - xlog(L_ERROR, "Failed to obtain name of local host: %m");
> >> - exit(1);
> >> + if (opt_srcaddr != NULL) {
> >> + struct addrinfo *ai = NULL;
> >> + struct addrinfo hint = {
> >> + .ai_family = AF_UNSPEC,
> >> + .ai_flags = AI_NUMERICHOST,
> >> + };
> >> +
> >> + if (getaddrinfo(opt_srcaddr, NULL,&hint,&ai))
> >> + /* not a presentation address - use it */
> >> + strncpy(nsm_hostname, opt_srcaddr, sizeof(nsm_hostname));
> >> + else {
> >> + /* was a presentation address - look it up in
> >> + * /etc/hosts, so it can be used for my_name */
> >> + int error;
> >> +
> >> + freeaddrinfo(ai);
> >> + hint.ai_flags = AI_CANONNAME;
> >> + error = getaddrinfo(opt_srcaddr, NULL,&hint,&ai);
> >> + if (error != 0) {
> >> + xlog(L_ERROR, "Bind address %s is unusable: %s",
> >> + opt_srcaddr, gai_strerror(error));
> >> + exit(1);
> >> + }
> >> + strncpy(nsm_hostname, ai->ai_canonname,
> >> + sizeof(nsm_hostname));
> >> + freeaddrinfo(ai);
> >> + }
> >
> > From what I can tell, lockd always uses the utsname()->nodename for the
> > "caller" field. Am I correct that that's what the server generally
> > expects to get from the client for my_name on a NSM notification?
>
> Yes.
>
> > If so, wouldn't the above patch pretty much ensure that the kernel and
> > statd are sending different info here?
>
> No, it guarantees they send the same info.
>
> lockd sends the same "caller" string to statd in an SM_MON request with
> the "my_name" argument. Statd records that value in the monitor record
> for this peer.
>
> That value is what I'm telling sm-notify to use here, rather than using
> gethostname(3)... precisely because the return value of gethostname(3)
> during this boot is not always the same as the value of
> utsname()->nodename during the previous boot.
>
> >> }
> >>
> >> (void)nsm_retire_monitored_hosts();
> >> @@ -535,6 +556,8 @@ notify(const int sock)
> >> static int
> >> notify_host(int sock, struct nsm_host *host)
> >> {
> >> + const char *my_name = (opt_srcaddr != NULL ?
> >> + nsm_hostname : host->my_name);
> >> struct sockaddr *sap;
> >> socklen_t salen;
> >>
> >> @@ -580,8 +603,8 @@ notify_host(int sock, struct nsm_host *host)
> >> host->xid = nsm_xmit_rpcbind(sock, sap, SM_PROG, SM_VERS);
> >> else
> >> host->xid = nsm_xmit_notify(sock, sap, salen,
> >> - SM_PROG, nsm_hostname, nsm_state);
> >> -
> >> + SM_PROG, my_name, nsm_state);
> >> +
> >> return 0;
> >> }
> >>
> >> diff --git a/utils/statd/sm-notify.man b/utils/statd/sm-notify.man
> >> index 163713e..7a1cbfa 100644
> >> --- a/utils/statd/sm-notify.man
> >> +++ b/utils/statd/sm-notify.man
> >> @@ -97,11 +97,9 @@ It uses the
> >> string as the destination.
> >> To identify which host has rebooted, the
> >> .B sm-notify
> >> -command normally sends the results of
> >> -.BR gethostname (3)
> >> -as the
> >> +command normally sends
> >> .I my_name
> >> -string.
> >> +string recorded when that remote was monitored.
> >> The remote
> >> .B rpc.statd
> >> matches incoming SM_NOTIFY requests using this string,
> >> @@ -202,15 +200,22 @@ argument to use when sending SM_NOTIFY requests.
> >> If this option is not specified,
> >> .B sm-notify
> >> uses a wildcard address as the transport bind address,
> >> -and uses the results of
> >> -.BR gethostname (3)
> >> -as the
> >> +and uses the
> >> +.I my_name
> >> +recorded when the remote was monitored as the
> >> .I mon_name
> >> -argument.
> >> +argument when sending SM_NOTIFY requests.
> >> .IP
> >> The
> >> .I ipaddr
> >> form can be expressed as either an IPv4 or an IPv6 presentation address.
> >> +If the
> >> +.I ipaddr
> >> +form is used, the
> >> +.B sm-notify
> >> +command converts this address to a hostname for use as the
> >> +.I mon_name
> >> +argument when sending SM_NOTIFY requests.
> >> .IP
> >> This option can be useful in multi-homed configurations where
> >> the remote requires notification from a specific network address.
> >> @@ -252,13 +257,6 @@ consistent
> >> The hostname the client uses to mount the server should match the server's
> >> .I mon_name
> >> in SM_NOTIFY requests it sends
> >> -.IP
> >> -The use of network addresses as a
> >> -.I mon_name
> >> -or a
> >> -.I my_name
> >> -string should be avoided when
> >> -interoperating with non-Linux NFS implementations.
> >> .PP
> >> Unmounting an NFS file system does not necessarily stop
> >> either the NFS client or server from monitoring each other.
> >> diff --git a/utils/statd/statd.man b/utils/statd/statd.man
> >> index ffc5e95..ca00e24 100644
> >> --- a/utils/statd/statd.man
> >> +++ b/utils/statd/statd.man
> >> @@ -100,11 +100,9 @@ It uses the
> >> string as the destination.
> >> To identify which host has rebooted, the
> >> .B sm-notify
> >> -command normally sends the results of
> >> -.BR gethostname (3)
> >> -as the
> >> +command sends the
> >> .I my_name
> >> -string.
> >> +string recorded when that remote was monitored.
> >> The remote
> >> .B rpc.statd
> >> matches incoming SM_NOTIFY requests using this string,
> >> @@ -292,7 +290,6 @@ man pages.
> >> .SH ADDITIONAL NOTES
> >> Lock recovery after a reboot is critical to maintaining data integrity
> >> and preventing unnecessary application hangs.
> >> -.PP
> >> To help
> >> .B rpc.statd
> >> match SM_NOTIFY requests to NLM requests, a number of best practices
> >> @@ -309,13 +306,6 @@ consistent
> >> The hostname the client uses to mount the server should match the server's
> >> .I mon_name
> >> in SM_NOTIFY requests it sends
> >> -.IP
> >> -The use of network addresses as a
> >> -.I mon_name
> >> -or a
> >> -.I my_name
> >> -string should be avoided when
> >> -interoperating with non-Linux NFS implementations.
> >> .PP
> >> Unmounting an NFS file system does not necessarily stop
> >> either the NFS client or server from monitoring each other.
> >>
> >> --
> >> 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
> >>
> >
> >
>
>
Thanks, that makes sense. This patch looks sane to me, but I think it
does sort of point out that using '-v' is heavily reliant on being able
to reverse-resolve that address to the nodename of the server. Ahh
well, not much we can do about that but warn people off from using that
option unless they know what they're doing...
Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
--
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
