| Message ID | 20220502085045.13038-1-richard@nod.at (mailing list archive) |
|---|---|
| Headers | show |
| Series | nfs-utils: Improving NFS re-exports | expand |
On Mon, May 02, 2022 at 10:50:40AM +0200, Richard Weinberger wrote: > This is the first non-RFC iteration of the NFS re-export > improvement series for nfs-utils. > While the kernel side[0] didn't change at all and is still small, > the userspace side saw much more changes. > > The core idea is adding new export option: reeport= > Using reexport= it is possible to mark an export entry in the exports > file explicitly as NFS re-export and select a strategy how unique > identifiers should be provided. > Currently two strategies are supported, "auto-fsidnum" and > "predefined-fsidnum", both use a SQLite database as backend to keep > track of generated ids. > For a more detailed description see patch "exports: Implement new export option reexport=". > I choose SQLite because nfs-utils already uses it and using SQL ids can nicely > generated and maintained. It will also scale for large setups where the amount > of subvolumes is high. > > Beside of id generation this series also addresses the reboot problem. > If the re-exporting NFS server reboots, uncovered NFS subvolumes are not yet > mounted and file handles become stale. > Now mountd/exportd keeps track of uncovered subvolumes and makes sure they get > uncovered while nfsd starts. > > The whole set of features is currently opt-in via --enable-reexport. Can we remove that option before upstreaming? For testing purposes it may makes sense to be able to turn the new code on and off quickly. But for something we're really going to support, it's just another hurdle for users to jump through, and another case we probably won't remember to test. The export options themselves should be enough configuration. Anyway, basically sounds reasonable to me. I'll try to give it a proper review sometime, feel free to bug me if I don't get to it in a week or so. --b. > I'm also not sure about the rearrangement of the reexport code, > currently it is a helper library. > > A typical export entry on a re-exporting server looks like: > /nfs *(rw,no_root_squash,no_subtree_check,crossmnt,reexport=auto-fsidnum) > reexport=auto-fsidnum will automatically assign an fsid= to /nfs and all > uncovered subvolumes. > > Richard Weinberger (5): > Implement reexport helper library > exports: Implement new export option reexport= > export: Implement logic behind reexport= > export: Avoid fsid= conflicts > reexport: Make state database location configurable > > [0] https://git.kernel.org/pub/scm/linux/kernel/git/rw/misc.git/log/?h=nfs_reexport_clean > > configure.ac | 12 ++ > nfs.conf | 3 + > support/Makefile.am | 4 + > support/export/Makefile.am | 2 + > support/export/cache.c | 71 ++++++- > support/export/export.c | 27 ++- > support/include/nfslib.h | 1 + > support/nfs/Makefile.am | 1 + > support/nfs/exports.c | 68 +++++++ > support/reexport/Makefile.am | 6 + > support/reexport/reexport.c | 354 +++++++++++++++++++++++++++++++++ > support/reexport/reexport.h | 39 ++++ > systemd/Makefile.am | 4 + > systemd/nfs-server-generator.c | 14 +- > systemd/nfs.conf.man | 6 + > utils/exportd/Makefile.am | 8 +- > utils/exportd/exportd.c | 5 + > utils/exportfs/Makefile.am | 6 + > utils/exportfs/exportfs.c | 21 +- > utils/exportfs/exports.man | 31 +++ > utils/mount/Makefile.am | 7 + > utils/mountd/Makefile.am | 6 + > utils/mountd/mountd.c | 1 + > utils/mountd/svc_run.c | 6 + > 24 files changed, 690 insertions(+), 13 deletions(-) > create mode 100644 support/reexport/Makefile.am > create mode 100644 support/reexport/reexport.c > create mode 100644 support/reexport/reexport.h > > -- > 2.31.1
On 5/2/22 12:17 PM, J. Bruce Fields wrote: > On Mon, May 02, 2022 at 10:50:40AM +0200, Richard Weinberger wrote: >> This is the first non-RFC iteration of the NFS re-export >> improvement series for nfs-utils. >> While the kernel side[0] didn't change at all and is still small, >> the userspace side saw much more changes. >> >> The core idea is adding new export option: reeport= >> Using reexport= it is possible to mark an export entry in the exports >> file explicitly as NFS re-export and select a strategy how unique >> identifiers should be provided. >> Currently two strategies are supported, "auto-fsidnum" and >> "predefined-fsidnum", both use a SQLite database as backend to keep >> track of generated ids. >> For a more detailed description see patch "exports: Implement new export option reexport=". >> I choose SQLite because nfs-utils already uses it and using SQL ids can nicely >> generated and maintained. It will also scale for large setups where the amount >> of subvolumes is high. >> >> Beside of id generation this series also addresses the reboot problem. >> If the re-exporting NFS server reboots, uncovered NFS subvolumes are not yet >> mounted and file handles become stale. >> Now mountd/exportd keeps track of uncovered subvolumes and makes sure they get >> uncovered while nfsd starts. >> >> The whole set of features is currently opt-in via --enable-reexport. > > Can we remove that option before upstreaming? > > For testing purposes it may makes sense to be able to turn the new code > on and off quickly. But for something we're really going to support, > it's just another hurdle for users to jump through, and another case we > probably won't remember to test. The export options themselves should > be enough configuration. > > Anyway, basically sounds reasonable to me. I'll try to give it a proper > review sometime, feel free to bug me if I don't get to it in a week or > so. How about --disable-reexport? So it is on by default to help testing but if there are issues we can turn it off... steved. > > --b. > > >> I'm also not sure about the rearrangement of the reexport code, >> currently it is a helper library. >> >> A typical export entry on a re-exporting server looks like: >> /nfs *(rw,no_root_squash,no_subtree_check,crossmnt,reexport=auto-fsidnum) >> reexport=auto-fsidnum will automatically assign an fsid= to /nfs and all >> uncovered subvolumes. >> >> Richard Weinberger (5): >> Implement reexport helper library >> exports: Implement new export option reexport= >> export: Implement logic behind reexport= >> export: Avoid fsid= conflicts >> reexport: Make state database location configurable >> >> [0] https://git.kernel.org/pub/scm/linux/kernel/git/rw/misc.git/log/?h=nfs_reexport_clean >> >> configure.ac | 12 ++ >> nfs.conf | 3 + >> support/Makefile.am | 4 + >> support/export/Makefile.am | 2 + >> support/export/cache.c | 71 ++++++- >> support/export/export.c | 27 ++- >> support/include/nfslib.h | 1 + >> support/nfs/Makefile.am | 1 + >> support/nfs/exports.c | 68 +++++++ >> support/reexport/Makefile.am | 6 + >> support/reexport/reexport.c | 354 +++++++++++++++++++++++++++++++++ >> support/reexport/reexport.h | 39 ++++ >> systemd/Makefile.am | 4 + >> systemd/nfs-server-generator.c | 14 +- >> systemd/nfs.conf.man | 6 + >> utils/exportd/Makefile.am | 8 +- >> utils/exportd/exportd.c | 5 + >> utils/exportfs/Makefile.am | 6 + >> utils/exportfs/exportfs.c | 21 +- >> utils/exportfs/exports.man | 31 +++ >> utils/mount/Makefile.am | 7 + >> utils/mountd/Makefile.am | 6 + >> utils/mountd/mountd.c | 1 + >> utils/mountd/svc_run.c | 6 + >> 24 files changed, 690 insertions(+), 13 deletions(-) >> create mode 100644 support/reexport/Makefile.am >> create mode 100644 support/reexport/reexport.c >> create mode 100644 support/reexport/reexport.h >> >> -- >> 2.31.1 >
> On May 2, 2022, at 6:46 PM, Steve Dickson <steved@redhat.com> wrote: > > > > On 5/2/22 12:17 PM, J. Bruce Fields wrote: >> On Mon, May 02, 2022 at 10:50:40AM +0200, Richard Weinberger wrote: >>> This is the first non-RFC iteration of the NFS re-export >>> improvement series for nfs-utils. >>> While the kernel side[0] didn't change at all and is still small, >>> the userspace side saw much more changes. >>> >>> The core idea is adding new export option: reeport= >>> Using reexport= it is possible to mark an export entry in the exports >>> file explicitly as NFS re-export and select a strategy how unique >>> identifiers should be provided. >>> Currently two strategies are supported, "auto-fsidnum" and >>> "predefined-fsidnum", both use a SQLite database as backend to keep >>> track of generated ids. >>> For a more detailed description see patch "exports: Implement new export option reexport=". >>> I choose SQLite because nfs-utils already uses it and using SQL ids can nicely >>> generated and maintained. It will also scale for large setups where the amount >>> of subvolumes is high. >>> >>> Beside of id generation this series also addresses the reboot problem. >>> If the re-exporting NFS server reboots, uncovered NFS subvolumes are not yet >>> mounted and file handles become stale. >>> Now mountd/exportd keeps track of uncovered subvolumes and makes sure they get >>> uncovered while nfsd starts. >>> >>> The whole set of features is currently opt-in via --enable-reexport. >> Can we remove that option before upstreaming? Somehow I missed Bruce's comment, but I had the same thought. The cover letter does not explain why someone would want to build nfs-utils without re-export support. If there isn't a good reason for needing this switch, IMO it can be removed. >> For testing purposes it may makes sense to be able to turn the new code >> on and off quickly. But for something we're really going to support, >> it's just another hurdle for users to jump through, and another case we >> probably won't remember to test. The export options themselves should >> be enough configuration. >> Anyway, basically sounds reasonable to me. I'll try to give it a proper >> review sometime, feel free to bug me if I don't get to it in a week or >> so. > How about --disable-reexport? So it is on by default to help testing > but if there are issues we can turn it off... > > steved. > >> --b. >>> I'm also not sure about the rearrangement of the reexport code, >>> currently it is a helper library. >>> >>> A typical export entry on a re-exporting server looks like: >>> /nfs *(rw,no_root_squash,no_subtree_check,crossmnt,reexport=auto-fsidnum) >>> reexport=auto-fsidnum will automatically assign an fsid= to /nfs and all >>> uncovered subvolumes. >>> >>> Richard Weinberger (5): >>> Implement reexport helper library >>> exports: Implement new export option reexport= >>> export: Implement logic behind reexport= >>> export: Avoid fsid= conflicts >>> reexport: Make state database location configurable >>> >>> [0] https://git.kernel.org/pub/scm/linux/kernel/git/rw/misc.git/log/?h=nfs_reexport_clean >>> >>> configure.ac | 12 ++ >>> nfs.conf | 3 + >>> support/Makefile.am | 4 + >>> support/export/Makefile.am | 2 + >>> support/export/cache.c | 71 ++++++- >>> support/export/export.c | 27 ++- >>> support/include/nfslib.h | 1 + >>> support/nfs/Makefile.am | 1 + >>> support/nfs/exports.c | 68 +++++++ >>> support/reexport/Makefile.am | 6 + >>> support/reexport/reexport.c | 354 +++++++++++++++++++++++++++++++++ >>> support/reexport/reexport.h | 39 ++++ >>> systemd/Makefile.am | 4 + >>> systemd/nfs-server-generator.c | 14 +- >>> systemd/nfs.conf.man | 6 + >>> utils/exportd/Makefile.am | 8 +- >>> utils/exportd/exportd.c | 5 + >>> utils/exportfs/Makefile.am | 6 + >>> utils/exportfs/exportfs.c | 21 +- >>> utils/exportfs/exports.man | 31 +++ >>> utils/mount/Makefile.am | 7 + >>> utils/mountd/Makefile.am | 6 + >>> utils/mountd/mountd.c | 1 + >>> utils/mountd/svc_run.c | 6 + >>> 24 files changed, 690 insertions(+), 13 deletions(-) >>> create mode 100644 support/reexport/Makefile.am >>> create mode 100644 support/reexport/reexport.c >>> create mode 100644 support/reexport/reexport.h >>> >>> -- >>> 2.31.1 > -- Chuck Lever
Bruce, ----- Ursprüngliche Mail ----- > Von: "bfields" <bfields@fieldses.org> >> The whole set of features is currently opt-in via --enable-reexport. > > Can we remove that option before upstreaming? What is the final resolution regarding this option? I can think of embedded/memory constraint systems where the dependency on SQLite is not wanted. On the other hand, with my latest patch, the plugin interface, the could be solved too. > For testing purposes it may makes sense to be able to turn the new code > on and off quickly. But for something we're really going to support, > it's just another hurdle for users to jump through, and another case we > probably won't remember to test. The export options themselves should > be enough configuration. > > Anyway, basically sounds reasonable to me. I'll try to give it a proper > review sometime, feel free to bug me if I don't get to it in a week or > so. *kind ping* :-) Please also don't forget the kernel side of this work. It is small but still needed. See: https://lore.kernel.org/linux-nfs/20220110184419.27665-1-richard@nod.at/ or https://git.kernel.org/pub/scm/linux/kernel/git/rw/misc.git/log/?h=nfs_reexport_clean Since the kernel changes don't change the ABI, it should not really matter which part is merged first. Kernel or userspace. The only important part is stating the right kernel version in the nfs-utils manpages. Thanks, //richard
> On May 23, 2022, at 3:53 AM, Richard Weinberger <richard@nod.at> wrote: > > Bruce, > > ----- Ursprüngliche Mail ----- >> Von: "bfields" <bfields@fieldses.org> >>> The whole set of features is currently opt-in via --enable-reexport. >> >> Can we remove that option before upstreaming? > > What is the final resolution regarding this option? > I can think of embedded/memory constraint systems where the dependency on SQLite > is not wanted. > > On the other hand, with my latest patch, the plugin interface, the could be solved > too. > >> For testing purposes it may makes sense to be able to turn the new code >> on and off quickly. But for something we're really going to support, >> it's just another hurdle for users to jump through, and another case we >> probably won't remember to test. The export options themselves should >> be enough configuration. >> >> Anyway, basically sounds reasonable to me. I'll try to give it a proper >> review sometime, feel free to bug me if I don't get to it in a week or >> so. > > *kind ping* :-) > > Please also don't forget the kernel side of this work. It is small but still needed. > See: https://lore.kernel.org/linux-nfs/20220110184419.27665-1-richard@nod.at/ > or > https://git.kernel.org/pub/scm/linux/kernel/git/rw/misc.git/log/?h=nfs_reexport_clean In his review comments, Bruce asked if testing with NFSv4 has been done. Also, can an idle client allow the re-export server to unmount an automounted export? Once you have NFSv4 results, the kernel patches need to be posted again with Cc: linux-fsdevel@vger.kernel.org and autofs@vger.kernel.org > Since the kernel changes don't change the ABI, it should not really matter which part > is merged first. Kernel or userspace. The only important part is stating the right > kernel version in the nfs-utils manpages. > > Thanks, > //richard -- Chuck Lever
On Mon, May 23, 2022 at 09:53:45AM +0200, Richard Weinberger wrote: > Please also don't forget the kernel side of this work. It is small but > still needed. See: > https://lore.kernel.org/linux-nfs/20220110184419.27665-1-richard@nod.at/ > or > https://git.kernel.org/pub/scm/linux/kernel/git/rw/misc.git/log/?h=nfs_reexport_clean The follow_down() code, at least, needs vfs review, maybe resend with a cc to Al Viro and linux-fsdevel. I'm not really familiar with how the vfs automounting code works. > Since the kernel changes don't change the ABI, it should not really > matter which part is merged first. Kernel or userspace. The only > important part is stating the right kernel version in the nfs-utils > manpages. OK, as long as we make sure nothing annoying happens in the old-kernel/new-nfs-utils case. (Looks to me like it should be OK.) --b.
On Mon, May 23, 2022 at 09:53:45AM +0200, Richard Weinberger wrote: > > Von: "bfields" <bfields@fieldses.org> > > Anyway, basically sounds reasonable to me. I'll try to give it a proper > > review sometime, feel free to bug me if I don't get to it in a week or > > so. > > *kind ping* :-) Anyway, yeah, still planning to, but I have a few other things I wanted to get out of the way first. --b.