Allow ZFS sharenfs to generate multiple export(5) lines

[rmacklem]

Summary

There has been a bugzilla PR#147881 requesting this
for a long time (14 years!). It extends the syntax of
the ZFS shanenfs property (for FreeBSD only) to allow
multiple sets of options for different hosts/nets,
separated by ';'s.

This patch will need to be submitted to OpenZFS as
a pull request, but I thought I'd try and get it reviewed
here first.

Test Plan

I tries assorted ZFS shanenfs settings and checked the
resultant /etc/zfs/exports.

The weakest part is that multiple adjacent ';'s
(or separated only by whitespace) result in multiple
lines with the same "default" (as in none) options.
This seems to work, but is ugly.

If I scan for the above and do not allow it, it is impossible
to use ';'s for the default case. (This might be ok, since the
default case exports the file system to the world and probably
either should not be done, or done by itself and not with other
sets of options.)
If others think that multiple ';'s with only whitespace between them
shouldn't be allowed, I will update the patch accordingly.

[rmacklem]

This version ignores whitespace only semicolon separated
option sets. I decided that allowing the "only whitespace
case" which generates a line with default options (which
is read/write to the world) did not make sense when other
option sets are being specified for other hosts/networks.

The default export can still be done if it is the only option
set in sharenfs (ie. no semicolons).

[mav]

Have you looked on similar Linux code? It would be good to be consistent or at least similar. I haven't looked deep, but foreach_nfs_host_cb() seems to support multiple hosts.

sys/contrib/openzfs/lib/libshare/os/freebsd/nfs.c

111

I am not sure I like this OPTSSIZE, both here and in existing translate_opts(). It seems to be crafted to match ZFS_MAXPROPLEN value, but it does only on FreeBSD, and IIRC there was recent activity to increase ZFS_MAXPROPLEN, that would affect this code. May be strdup() could be cleaner?

[rmacklem]

Used strdup() to copy the options into a malloc'd area,
as suggested by mav@. This makes sense, since a sharenfs
with many semicolons could be much larger than an
option list.

Removed the redundant copying to oldopts, since it is
already copied from the const string.
Since newopts can be slightly larger than oldopts, I
malloc'd it to strlen(oldopts) + 1024. 1024 is way overkill,
but since the exact increase cannot be estimated unless you
do two passes through the options (one to calculate the size
and a second to copy the options), I figured I'd make it really
large. This allowed OPTSSIZE to be deleted.

[rmacklem]

I did take a quick look at the Linux code and it
appears to be completely different to me, so I
don't think trying to convert it makes sense, at
least for now.

[mav]

Looks fine to me, just couple nits.

sys/contrib/openzfs/lib/libshare/os/freebsd/nfs.c

80

I can't say that I like 1024. It maybe impossible to overflow now, but in general, searching for worst possible cases, I can see expansion of 50% for "ro" and 33% for "ro,ro,ro,ro.". I'm thinking if strlen(oldopts) * 3 / 2 + 1 would be better, or too specific/complicated?

118

Would be nice to check for error.

[mav]

sys/contrib/openzfs/lib/libshare/os/freebsd/nfs.c

80

I actually wanted to say strlen(oldopts) * 4 / 3 + 1 + 1, but that is even more tangled.

[rmacklem]

If you are agreeable to my version of the length formula,
I will update the patch with that and checks for NULL
return for both strdup() and malloc().

Thanks for the review, rick

sys/contrib/openzfs/lib/libshare/os/freebsd/nfs.c

80

Well, if we are going to calculate it accurately...
Assuming the worst case of a single character option,
I come up with:

(strlen(oldopts) + 1) * 3 / 2 + 1

I add one to strlen(oldopts) first, so that the trailing nul in
oldopts is counted as the last separator.
Then I do something similar to you, but assuming a single
character option, which expands from 2 chars (option character +
separator) to 3 chars ('-' + 'option character + ' ').
I add one for the trailing nul in newopts, needed for certain cases,
such as just one single character option or an odd number of two
character options.
I tried some examples using one, two and three character options
once and repeated and it always seems to work.
Sound ok to you?

Actually, I thought mountd would complain about repeated
options, but I just tried it and it does not.

118

Yes, good point. I should check for a NULL reply.
I should also check for a NULL reply to the malloc()
I added to translate_opts() above.