Date: Fri, 21 Jul 2023 08:07:50 -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: <CAM5tNy5FhOSKmxzmOxAo_yGk97N9N2eW1Aw=mj7Q32pj2NS71g@mail.gmail.com> In-Reply-To: <2D8B626E-82BB-410C-B7C7-35B4D3834A44@lysator.liu.se> References: <CAM5tNy7GkMLgy-wjonVu%2BgOMnexoA2W8vSZvMo4NDikrg3-A1A@mail.gmail.com> <2D8B626E-82BB-410C-B7C7-35B4D3834A44@lysator.liu.se>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Jul 21, 2023 at 1:22=E2=80=AFAM Peter Eriksson <pen@lysator.liu.se>= wrote: > > Hi! > > Here=E2=80=99s a couple of updated patches with some bug fixes. I=E2=80= =99ve also moved the database location for ZFS to /etc/zfs/exports.db to mi= rror the current location. > > I also changed the patch for mountd so that for each =E2=80=9Cexports=E2= =80=9D source specified it first tries to open <path>.db which I think make= s it easier to use (no need for the =E2=80=9C-D=E2=80=9D option and it supp= orts multiple sources). Cleaner I think. > > > Btw, you can create the database manually from /etc/zfs/exports using =E2= =80=9Cmakemap=E2=80=9D like this: > > makemap -f btree /etc/zfs/exports.db </etc/zfs/exports > > > Known =E2=80=9Cbugs=E2=80=9D: > =E2=80=9CV4:=E2=80=9D > Even though you could create an /etc/exports.db with the contents of /etc= /exports it will fail with the =E2=80=9CV4:=E2=80=9D lines since the BTree = database will sort the exported entries so that the V4: key will appear las= t and then mountd will complain when scanning the DB. > > Also when using the file /etc/exports you can either write =E2=80=9CV4:/e= xport -sec=3Dsys=E2=80=9D or =E2=80=9CV4: /export -sec=3Dsys=E2=80=9D (with= a space between V4: and the path) so this probably needs some special hand= ling (can=E2=80=99t simply use =E2=80=9Cmakemap -f btree /etc/exports.db </= etc/exports=E2=80=9D to create it. > > 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. > > 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? 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. > > - 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 improve= s > > 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 suffi= x. > > 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?CAM5tNy5FhOSKmxzmOxAo_yGk97N9N2eW1Aw=mj7Q32pj2NS71g>