Page MenuHomeFreeBSD

Add support for a virtual hostname to nfsd
ClosedPublic

Authored by sef on Feb 14 2019, 6:08 AM.

Details

Summary

Another one we've had at iXsystems for a while -- although the code there uses a file in /etc, which did not strike me as the best way to do it. This allows us to switch between active and passive servers.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

sef created this revision.Feb 14 2019, 6:08 AM
0mp accepted this revision.Feb 14 2019, 8:50 AM
0mp added a subscriber: 0mp.

OK from manpages.

Please remember to bump the date in the manual before committing.

This revision is now accepted and ready to land.Feb 14 2019, 8:50 AM
rmacklem accepted this revision.Feb 15 2019, 12:02 AM

Looks ok to me. (I'll admit I don't understand when it is useful, but that's ok.;-)
The one change I might suggest is a sanity check for the strlen() of the "-V" optarg
being <= MAXHOSTNAMELEN.

I see that you use strlcpy(), so the code is safe w.r.t. not overflowing the buffer on
the stack, but it would result in a truncated hostname and confusion later, for the
case where "strlen(optarg) > MAXHOSTNAMELEN.

sef updated this revision to Diff 53951.Feb 15 2019, 7:18 AM

Fixed the date in the man page (although I guess it's already wrong :)), and don't set the virtual host if it's too long.

This revision now requires review to proceed.Feb 15 2019, 7:18 AM

Thanks for adding the strlen() check. I am going to be nitpicky and suggest an
error message be printed (not sure if the nfsd should fail or just log an error?),
since silently ignoring the "-V" argument could cause confusion too, I think?

sef added a comment.Feb 15 2019, 5:32 PM

Thanks for adding the strlen() check. I am going to be nitpicky and suggest an
error message be printed (not sure if the nfsd should fail or just log an error?),
since silently ignoring the "-V" argument could cause confusion too, I think?

I didn't think it should fail, and since it's a daemon I'd have to have logging set up
to really have a meaningful error; that's why I just let it silently ignore it. I can
put out a warnx call easily enough.

sef updated this revision to Diff 53973.Feb 15 2019, 6:44 PM

Warn if the virtual hostname argument is too long. (Note that this is only a warning.)

rmacklem accepted this revision.Feb 15 2019, 11:51 PM

Looks fine to me now. Thanks, rick

This revision is now accepted and ready to land.Feb 15 2019, 11:51 PM
This revision was automatically updated to reflect the committed changes.