Date: Fri, 21 Jul 2023 16:07:26 -0700 From: Rick Macklem <rick.macklem@gmail.com> To: Peter Eriksson <pen@lysator.liu.se> Cc: FreeBSD FS <freebsd-fs@freebsd.org>, kevans@freebsd.org Subject: Re: RFC: Patch for mountd to handle a database for exports Message-ID: <CAM5tNy5pFVKOxPT87eN4Cj2%2BiVVXjzc-vLA-5jov0uv=y8u2pQ@mail.gmail.com> In-Reply-To: <F3A27BD2-C093-4F3F-A750-F0A6727149E2@lysator.liu.se> References: <CAM5tNy7GkMLgy-wjonVu%2BgOMnexoA2W8vSZvMo4NDikrg3-A1A@mail.gmail.com> <2D8B626E-82BB-410C-B7C7-35B4D3834A44@lysator.liu.se> <CAM5tNy5FhOSKmxzmOxAo_yGk97N9N2eW1Aw=mj7Q32pj2NS71g@mail.gmail.com> <F3A27BD2-C093-4F3F-A750-F0A6727149E2@lysator.liu.se>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Jul 21, 2023 at 1:33=E2=80=AFPM Peter Eriksson <pen@lysator.liu.se>= wrote: > > > I did try to fix that but the code quickly got hairy so I didn=E2=80=99t = like it. If we really want/need that I=E2=80=99m thinking of creating a spe= cial case for the V4: handling, sort of like prefixing the database key wit= h 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=E2= =80=99s not a problem for the /etc/zfs/exports.db case. > > And converting /etc/exports to /etc/exports.db is not really necessary fr= om 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=E2=80=99t support it= either so no change from the current setup but it would be nice to have. > Ie, support for things like: > /export -sec=3Dkrb5 > /export -sec=3Dsys 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=E2=80=99t 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 tryi= ng to convert /etc/exports into a DB (if it uses NFSv4 / the =E2=80=9CV4:= =E2=80=9D 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 hav= e that here for now so that I can quickly disable the code) > > Regarding the patch to the zfs part - I=E2=80=99m not sure which way to g= o there - the current patch switches to always use the DB. But one could ar= gue 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 bo= th the old way and the new way, but it requires an empty /etc/zfs/exports.d= b 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. rick > > - Peter > > > On 21 Jul 2023, at 00:50, Rick Macklem <rick.macklem@gmail.com> wrote: > > Hi, > > Peter Eriksson has submitted an interesting patch that adds support > for a database file for exports. My understanding is that this improves > performance when used with the ZFS share property for exporting a > large number of file systems. > > There are a couple of user visible issues that I'd like feedback from > others. (Hopefully Peter can correct me if I get any of these wrong.) > > - The patch uses a single database file and a new "-D" option to > specify it on the command line. > --> I think it might be less confusing to just put the database file(s) > in the exports list and identify them via a ".db" filename suffix. > What do others think? > > The changes are #ifdef'd on USE_SHAREDB. I'm thinking that this > change will be always built, so maybe USE_SHAREDB is not needed? > (Obviously mountd(8)'s semantics will only changed if/when database > file(s) are provided.) > > Once I have feedback on the above, I will put a patch up on > phabricator. > > rick > ps: I believe kevans@ has volunteered to shepperd the ZFS share > property changes. > >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAM5tNy5pFVKOxPT87eN4Cj2%2BiVVXjzc-vLA-5jov0uv=y8u2pQ>