Re: [nfs-utils PATCH] statd: make statd_canonical_name emit a warning if the presentation address maps to an empty hostname

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> On Dec 7, 2015, at 12:04 PM, Scott Mayhew <smayhew@xxxxxxxxxx> wrote:
> 
> On Mon, 07 Dec 2015, Chuck Lever wrote:
> 
>> 
>>> On Dec 7, 2015, at 11:42 AM, Scott Mayhew <smayhew@xxxxxxxxxx> wrote:
>>> 
>>> Certain name resolution misconfigurations (for example, a hosts file
>>> entry with an ip address but no hostnames) can cause get_nameinfo() to
>>> return an empty string in buf, which will lead to this cryptic failure:
>>> 
>>> Dec  7 09:37:44 hostname rpc.statd[8024]: Failed to insert: creating
>>> /var/lib/nfs/statd/sm/: Is a directory
>>> Dec  7 09:37:44 hostname rpc.statd[8024]: STAT_FAIL to
>>> hostname.example.com for SM_MON of 192.168.1.2
>>> Dec  7 09:37:44 hostname kernel: lockd: cannot monitor 192.168.1.2
>>> 
>>> It's better in that case to just go ahead and use the presentation
>>> address and log a more helpful warning:
>> 
>> Doesn???t this change wire behavior? Some peers do not appreciate
>> receiving a presentation address, which is why statd/sm_notify
>> avoid sending them.
> 
> I was just trying to keep it in line with the behavior you had added in
> e22f5a9c (statd: statd fails to monitor if no reverse mapping of mon_name
> exists).  As it stands now, if get_nameinfo() fails outright then the
> presentation address is used and we're able to monitor the peer just
> fine... but get_nameinfo() succeeds and returns an empty hostname then
> we ultimately wind up failing to monitor the peer.

OK, I see the “bug-for-bug compatible” red flag in the
description of e22f5a9c. I guess this shouldn’t even
fire a warning then, it should just return the address.

We should correct the documenting comment for
statd_canonical_name(). Just delete the sentence that
begins “We won’t monitor peers”.

Reviewed-by: Chuck Lever <chuck.lever@xxxxxxxxxx>


>> I think it would be better to _skip_ this address and print the
>> warning? IMO.
>> 
> That's actually what I had at first and if you think that's better then
> that's fine too.  The rest of the errors would still get logged then,
> but at least the reason would be a little more apparent.
> 
> -Scott
>> 
>>> Dec  7 10:24:52 hostname rpc.statd[14224]: get_nameinfo returned empty
>>> hostname for 10.10.183.219.  Fix your name resolution!
>>> 
>>> Signed-off-by: Scott Mayhew <smayhew@xxxxxxxxxx>
>>> ---
>>> utils/statd/hostname.c | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>> 
>>> diff --git a/utils/statd/hostname.c b/utils/statd/hostname.c
>>> index c61087c..86d11ca 100644
>>> --- a/utils/statd/hostname.c
>>> +++ b/utils/statd/hostname.c
>>> @@ -211,6 +211,12 @@ statd_canonical_name(const char *hostname)
>>> 			/* OK to use presentation address,
>>> 			 * if no reverse map exists */
>>> 			return strdup(hostname);
>>> +		else if (buf[0] == '\0') {
>>> +			xlog_warn("get_nameinfo returned empty hostname for "
>>> +					"%s. Fix your name resolution!",
>>> +					hostname);
>>> +			return strdup(hostname);
>>> +		}
>>> 		return strdup(buf);
>>> 	}
>>> 
>>> -- 
>>> 2.4.3
>>> 
>>> --
>>> 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

--
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



[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux