In D60044#1377567, @rmacklem wrote:

I suppose this is ok. It would be nice to note
somewhere that /etc/exports is always needed
for NFSv4. (A /etc/zfs/exports only setup can
be done for NFSv3 only.)

It might be better to require /etc/exports when either
nfsv4_server_enable or nfsv4_server_only are "yes".

In D60044#1377730, @rmacklem wrote:

In D60044#1377567, @rmacklem wrote:

I suppose this is ok. It would be nice to note
somewhere that /etc/exports is always needed
for NFSv4. (A /etc/zfs/exports only setup can
be done for NFSv3 only.)

It might be better to require /etc/exports when either
nfsv4_server_enable or nfsv4_server_only are "yes".

I'm not really experienced with NFS, so we could get more people to weigh in on this. @markj what do you think?

In D60044#1377996, @christos wrote:

In D60044#1377730, @rmacklem wrote:

In D60044#1377567, @rmacklem wrote:

I suppose this is ok. It would be nice to note
somewhere that /etc/exports is always needed
for NFSv4. (A /etc/zfs/exports only setup can
be done for NFSv3 only.)

It might be better to require /etc/exports when either
nfsv4_server_enable or nfsv4_server_only are "yes".

I'm not really experienced with NFS, so we could get more people to weigh in on this. @markj what do you think?

Just to clarify it. sharenfs in ZFS works for NFSv4, but it does not
generate a "V4:" line, so that has to be done manually by putting
it in /etc/exports.

Is there a reason we shouldn't instead ship an empty /etc/exports file as part of the distribution, just like we do with, say, /etc/sysctl.conf?

Or, make sure that zfs(1) creates a dummy /etc/exports when one configures a sharenfs property and /etc/exports doesn't already exist?

I'm not sure if those are better solutions than what's proposed here, but maybe they are preferable.

In D60044#1378989, @markj wrote:

Is there a reason we shouldn't instead ship an empty /etc/exports file as part of the distribution, just like we do with, say, /etc/sysctl.conf?

I thought there was such a thing installed for a while?
(If I recall correctly, it was about a screenfull of comment that tried to explain
what needed to be in /etc/exports, but maybe I don't recall correctly.)

I did once succeed in getting rid of "toor" from the password file for about
6 months (multiple entries for the same uid messes up nfsuserd(8)) before
someon put it back in.

My only problem with an empty /etc/exports is that someone will take it
out someday, arguing that it isn't useful.

Do whatever you think makes sense.

Or, make sure that zfs(1) creates a dummy /etc/exports when one configures a sharenfs property and /etc/exports doesn't already exist?

I'm not sure if those are better solutions than what's proposed here, but maybe they are preferable.

@rmacklem I see that you have many commits to NFS, so the question would better be what do you think is best here. I'm not sure what is best either. My patch looks like a hack on one hand, but I do agree about the empty file being mistaken for unnecessary. Could you document this with a comment inside the empty file?

If the mountd program can already tell if it's missing essential files, then perhaps there it makes no sense to set required_files="/etc/exports" at all. The error will show up in logs, it's just that the error will come from the mountd program instead of the mountd service. I think that this would be cleaner than a implementing a complicated if statement conditional that is expressing what's already present in mountd. Also, this approach would allow us to avoid shipping an empty /etc/exports file that might mess with existing automation scripts. Not the end of the world in itself, but also potentially unnecessary work for users.

Not to mention that required_files="/etc/exports" is probably wrong anyway, because alternative exports files can be specified via mountd_flags.

tl;dr: I'd suggest removing the required_files="/etc/exports" from the service script completely and let mountd handle its requirements on its own.

In D60044#1379367, @0mp wrote:

If the mountd program can already tell if it's missing essential files, then perhaps there it makes no sense to set required_files="/etc/exports" at all. The error will show up in logs, it's just that the error will come from the mountd program instead of the mountd service. I think that this would be cleaner than a implementing a complicated if statement conditional that is expressing what's already present in mountd. Also, this approach would allow us to avoid shipping an empty /etc/exports file that might mess with existing automation scripts. Not the end of the world in itself, but also potentially unnecessary work for users.

Not to mention that required_files="/etc/exports" is probably wrong anyway, because alternative exports files can be specified via mountd_flags.

tl;dr: I'd suggest removing the required_files="/etc/exports" from the service script completely and let mountd handle its requirements on its own.

This sounds like a reasonable suggestion to me. However, you'll need to test
the case where nfsv4_server_enable="YES" is set in /etc/rc.conf, to see if
mountd does throw an error w.r.t. there being no V4: line.
(Btw, I don't think it is practical for ZFS to generate the V4: line,
since sharenfs is meant to be applied to all file systems and
the V4: line is not related to any specific file system. It simply
pins where in the server's file system space the NFSv4 root is.)

Cool, less code that does more is always good ;)