Date: Fri, 21 Jul 2023 22:33:17 +0200 From: Peter Eriksson <pen@lysator.liu.se> To: FreeBSD FS <freebsd-fs@freebsd.org> Cc: kevans@freebsd.org, Rick Macklem <rick.macklem@gmail.com> Subject: Re: RFC: Patch for mountd to handle a database for exports Message-ID: <F3A27BD2-C093-4F3F-A750-F0A6727149E2@lysator.liu.se> In-Reply-To: <CAM5tNy5FhOSKmxzmOxAo_yGk97N9N2eW1Aw=mj7Q32pj2NS71g@mail.gmail.com> References: <CAM5tNy7GkMLgy-wjonVu%2BgOMnexoA2W8vSZvMo4NDikrg3-A1A@mail.gmail.com> <2D8B626E-82BB-410C-B7C7-35B4D3834A44@lysator.liu.se> <CAM5tNy5FhOSKmxzmOxAo_yGk97N9N2eW1Aw=mj7Q32pj2NS71g@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--Apple-Mail=_F528DA9E-C185-410F-9675-2F557F67BE84 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 >> I did try to fix that but the code quickly got hairy so I didn=E2=80=99= t like it. If we really want/need that I=E2=80=99m 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). >>=20 > 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. >=20 > 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 = from a performance point since I doubt it ever will be very long.=20 However I bet some enthusiastic fellow is bound to try to do it sometime = in the future just because it can be done :-) >>=20 >> 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 >>=20 > 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. >=20 > 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? >=20 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 = trying to convert /etc/exports into a DB (if it uses NFSv4 / the = =E2=80=9CV4:=E2=80=9D line(s)).=20 Or should we just special-case /etc/exports and forbid the check for = /etc/exports.db?=20 - Peter > rick >=20 >>=20 >>=20 >>=20 >>=20 >>=20 >> (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) >>=20 >> Regarding the patch to the zfs part - I=E2=80=99m 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. >>=20 >> - Peter >>=20 >>=20 >>> On 21 Jul 2023, at 00:50, Rick Macklem <rick.macklem@gmail.com> = wrote: >>>=20 >>> Hi, >>>=20 >>> 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. >>>=20 >>> 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.) >>>=20 >>> - 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? >>>=20 >>> 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.) >>>=20 >>> Once I have feedback on the above, I will put a patch up on >>> phabricator. >>>=20 >>> rick >>> ps: I believe kevans@ has volunteered to shepperd the ZFS share >>> property changes. --Apple-Mail=_F528DA9E-C185-410F-9675-2F557F67BE84 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 <html><head><meta http-equiv=3D"content-type" content=3D"text/html; = charset=3Dutf-8"></head><body style=3D"overflow-wrap: break-word; = -webkit-nbsp-mode: space; line-break: = after-white-space;"><br><div><blockquote type=3D"cite"><div><blockquote = type=3D"cite" style=3D"font-family: Helvetica; font-size: 12px; = font-style: normal; font-variant-caps: normal; font-weight: 400; = letter-spacing: normal; orphans: auto; text-align: start; text-indent: = 0px; text-transform: none; white-space: normal; widows: auto; = word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: = none;">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 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).<br><br></blockquote><span style=3D"caret-color: rgb(0, 0, 0); = font-family: Helvetica; font-size: 12px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none; float: none; display: inline !important;">Does = the ZFS share property generate a V4: line?</span><br = style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: = 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; = letter-spacing: normal; text-align: start; text-indent: 0px; = text-transform: none; white-space: normal; word-spacing: 0px; = -webkit-text-stroke-width: 0px; text-decoration: none;"><span = style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: = 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; = letter-spacing: normal; text-align: start; text-indent: 0px; = text-transform: none; white-space: normal; word-spacing: 0px; = -webkit-text-stroke-width: 0px; text-decoration: none; float: none; = display: inline !important;">I doubt anyone will convert a non-ZFS = /etc/exports file to a DB file,</span><br style=3D"caret-color: rgb(0, = 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none;"><span style=3D"caret-color: rgb(0, 0, 0); = font-family: Helvetica; font-size: 12px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none; float: none; display: inline !important;">so = support of that does not seem to be needed.</span><br = style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: = 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; = letter-spacing: normal; text-align: start; text-indent: 0px; = text-transform: none; white-space: normal; word-spacing: 0px; = -webkit-text-stroke-width: 0px; text-decoration: none;"><br = style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: = 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; = letter-spacing: normal; text-align: start; text-indent: 0px; = text-transform: none; white-space: normal; word-spacing: 0px; = -webkit-text-stroke-width: 0px; text-decoration: none;"><span = style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: = 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; = letter-spacing: normal; text-align: start; text-indent: 0px; = text-transform: none; white-space: normal; word-spacing: 0px; = -webkit-text-stroke-width: 0px; text-decoration: none; float: none; = display: inline !important;">I see this as useful for the ZFS share = property case,</span><br style=3D"caret-color: rgb(0, 0, 0); = font-family: Helvetica; font-size: 12px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none;"><span style=3D"caret-color: rgb(0, 0, 0); = font-family: Helvetica; font-size: 12px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none; float: none; display: inline !important;">so if = that works correctly, I do not see the above as a concern.</span><br = style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: = 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; = letter-spacing: normal; text-align: start; text-indent: 0px; = text-transform: none; white-space: normal; word-spacing: 0px; = -webkit-text-stroke-width: 0px; text-decoration: = none;"></div></blockquote><div><br></div>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.</div><div><br></div><div>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. </div><div>However I bet some enthusiastic fellow is bound to = try to do it sometime in the future just because it can be done = :-)</div><div><br><br><blockquote type=3D"cite"><div><blockquote = type=3D"cite" style=3D"font-family: Helvetica; font-size: 12px; = font-style: normal; font-variant-caps: normal; font-weight: 400; = letter-spacing: normal; orphans: auto; text-align: start; text-indent: = 0px; text-transform: none; white-space: normal; widows: auto; = word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: = none;"><br>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.<br> Ie, support for things = like:<br> /export = -sec=3Dkrb5<br> /export -sec=3Dsys = ro<br><br></blockquote><span style=3D"caret-color: rgb(0, 0, 0); = font-family: Helvetica; font-size: 12px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none; float: none; display: inline !important;">Yes. = There is at least one PR that requests that the ZFS share = property</span><br style=3D"caret-color: rgb(0, 0, 0); font-family: = Helvetica; font-size: 12px; font-style: normal; font-variant-caps: = normal; font-weight: 400; letter-spacing: normal; text-align: start; = text-indent: 0px; text-transform: none; white-space: normal; = word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: = none;"><span style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; = font-size: 12px; font-style: normal; font-variant-caps: normal; = font-weight: 400; letter-spacing: normal; text-align: start; = text-indent: 0px; text-transform: none; white-space: normal; = word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: = none; float: none; display: inline !important;">be enhanced to do this. = Part of the reason for getting this patch in main</span><br = style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: = 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; = letter-spacing: normal; text-align: start; text-indent: 0px; = text-transform: none; white-space: normal; word-spacing: 0px; = -webkit-text-stroke-width: 0px; text-decoration: none;"><span = style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: = 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; = letter-spacing: normal; text-align: start; text-indent: 0px; = text-transform: none; white-space: normal; word-spacing: 0px; = -webkit-text-stroke-width: 0px; text-decoration: none; float: none; = display: inline !important;">is so that this can be pursued.</span><br = style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: = 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; = letter-spacing: normal; text-align: start; text-indent: 0px; = text-transform: none; white-space: normal; word-spacing: 0px; = -webkit-text-stroke-width: 0px; text-decoration: none;"><br = style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: = 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; = letter-spacing: normal; text-align: start; text-indent: 0px; = text-transform: none; white-space: normal; word-spacing: 0px; = -webkit-text-stroke-width: 0px; text-decoration: none;"><span = style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: = 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; = letter-spacing: normal; text-align: start; text-indent: 0px; = text-transform: none; white-space: normal; word-spacing: 0px; = -webkit-text-stroke-width: 0px; text-decoration: none; float: none; = display: inline !important;">Again, I only see this as a ZFS share = property issue because I do not</span><br style=3D"caret-color: rgb(0, = 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none;"><span style=3D"caret-color: rgb(0, 0, 0); = font-family: Helvetica; font-size: 12px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none; float: none; display: inline !important;">see any = reason to convert /etc/exports flat files to db files?</span><br = style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: = 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; = letter-spacing: normal; text-align: start; text-indent: 0px; = text-transform: none; white-space: normal; word-spacing: 0px; = -webkit-text-stroke-width: 0px; text-decoration: none;"><br = style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: = 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; = letter-spacing: normal; text-align: start; text-indent: 0px; = text-transform: none; white-space: normal; word-spacing: 0px; = -webkit-text-stroke-width: 0px; text-decoration: = none;"></div></blockquote><div><br></div>I agree that the need probably = isn=E2=80=99t there.</div><div><br></div><div>The current DB code will = generate a syslog(LOG_ERR) warning if it detects anything not starting = with / in the keys (and ignores it).</div><div><br></div><div>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 =E2=80=9CV4:=E2=80=9D= line(s)). </div><div>Or should we just special-case /etc/exports = and forbid the check for = /etc/exports.db? </div><div><br></div><div>- = Peter</div><div><br><blockquote type=3D"cite"><div><span = style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: = 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; = letter-spacing: normal; text-align: start; text-indent: 0px; = text-transform: none; white-space: normal; word-spacing: 0px; = -webkit-text-stroke-width: 0px; text-decoration: none; float: none; = display: inline !important;">rick</span><br style=3D"caret-color: rgb(0, = 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none;"><br style=3D"caret-color: rgb(0, 0, 0); = font-family: Helvetica; font-size: 12px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none;"><blockquote type=3D"cite" style=3D"font-family: = Helvetica; font-size: 12px; font-style: normal; font-variant-caps: = normal; font-weight: 400; letter-spacing: normal; orphans: auto; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none;"><br><br><br><br><br>(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)<br><br>Regarding the patch to the = zfs part - I=E2=80=99m 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.<br><br>- Peter<br><br><br><blockquote type=3D"cite">On = 21 Jul 2023, at 00:50, Rick Macklem <rick.macklem@gmail.com> = wrote:<br><br>Hi,<br><br>Peter Eriksson has submitted an interesting = patch that adds support<br>for a database file for exports. My = understanding is that this improves<br>performance when used with the = ZFS share property for exporting a<br>large number of file = systems.<br><br>There are a couple of user visible issues that I'd like = feedback from<br>others. (Hopefully Peter can correct me if I get any of = these wrong.)<br><br>- The patch uses a single database file and a new = "-D" option to<br>specify it on the command line.<br>--> I think it = might be less confusing to just put the database = file(s)<br> in the exports list and = identify them via a ".db" filename suffix.<br>What do others = think?<br><br>The changes are #ifdef'd on USE_SHAREDB. I'm thinking that = this<br>change will be always built, so maybe USE_SHAREDB is not = needed?<br>(Obviously mountd(8)'s semantics will only changed if/when = database<br>file(s) are provided.)<br><br>Once I have feedback on the = above, I will put a patch up on<br>phabricator.<br><br>rick<br>ps: I = believe kevans@ has volunteered to shepperd the ZFS = share<br> property = changes.</blockquote></blockquote></div></blockquote></div><br></body></ht= ml>= --Apple-Mail=_F528DA9E-C185-410F-9675-2F557F67BE84--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?F3A27BD2-C093-4F3F-A750-F0A6727149E2>