Re: RFC: Patch for mountd to handle a database for exports

From: Kyle Evans <kevans_at_FreeBSD.org>
Date: Sat, 22 Jul 2023 03:50:42 UTC
On 7/21/23 18:07, Rick Macklem wrote:
> On Fri, Jul 21, 2023 at 1:33 PM Peter Eriksson <pen@lysator.liu.se> wrote:
>>
>>
>> I did try to fix that but the code quickly got hairy so I didn’t like it. If we really want/need that I’m thinking of creating a special case for the V4: handling, sort of like prefixing the database key with a NUL byte or something (so that it will be sorted first).
>>
>> Does the ZFS share property generate a V4: line?
>> I doubt anyone will convert a non-ZFS /etc/exports file to a DB file,
>> so support of that does not seem to be needed.
>>
>> I see this as useful for the ZFS share property case,
>> so if that works correctly, I do not see the above as a concern.
>>
>>
>> No, the ZFS share property stuff never generates the V4: line(s) so it’s not a problem for the /etc/zfs/exports.db case.
>>
>> And converting /etc/exports to /etc/exports.db is not really necessary from a performance point since I doubt it ever will be very long.
>> However I bet some enthusiastic fellow is bound to try to do it sometime in the future just because it can be done :-)
>>
> Yep. Capturing this in a man page update when the time comes needs
> to be done, I think? (That the V4: line doesn't work in the DB.)
> 
>>
>>
>> Multiline options - well, the current ZFS code doesn’t support it either so no change from the current setup but it would be nice to have.
>>   Ie, support for things like:
>>      /export -sec=krb5
>>      /export -sec=sys ro
>>
>> Yes. There is at least one PR that requests that the ZFS share property
>> be enhanced to do this. Part of the reason for getting this patch in main
>> is so that this can be pursued.
>>
>> Again, I only see this as a ZFS share property issue because I do not
>> see any reason to convert /etc/exports flat files to db files?
>>
>>
>> I agree that the need probably isn’t there.
>>
>> The current DB code will generate a syslog(LOG_ERR) warning if it detects anything not starting with / in the keys (and ignores it).
>>
>> Perhaps there should be a warning in the manual for mountd about not trying to convert /etc/exports into a DB (if it uses NFSv4 / the “V4:” line(s)).
>> Or should we just special-case /etc/exports and forbid the check for /etc/exports.db?
>>
>> - Peter
>>
>> rick
>>
>>
>>
>>
>>
>>
>> (I also agree that the USE_SHAREDB probably should be removed, I just have that here for now so that I can quickly disable the code)
>>
>> Regarding the patch to the zfs part - I’m not sure which way to go there - the current patch switches to always use the DB. But one could argue that the code could check for an existing /etc/zfs/exports.db and only use the DB-writing code if that already exists. That way it will support both the old way and the new way, but it requires an empty /etc/zfs/exports.db to be pre-created at initial boot time for it to start using it.
> 
> That's up to the ZFS folk. Hopefully kevans@ can figure it out.
> 

I was going to massage it a bit before passing it up for review; my 
preference (that I think will make it fairly easy to accept upstream) is 
that we simply prefer exports.db if it exists but otherwise don't change 
anything for the folks happy with their current setup -- at least for an 
interim period, at a minimum.

Thanks,

Kyle Evans