Bug 255061 - [PATCH] Proof of concept nested mounts for automounter -hosts map
Summary: [PATCH] Proof of concept nested mounts for automounter -hosts map
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 12.2-RELEASE
Hardware: Any Any
: --- Affects Only Me
Assignee: Cy Schubert
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2021-04-14 18:33 UTC by Richard M Kreuter
Modified: 2024-11-14 08:45 UTC (History)
1 user (show)

See Also:


Attachments
Extend automountd, autounmountd, and autofs to understand nested automounted mount points. (15.82 KB, application/x-gzip)
2021-04-14 18:33 UTC, Richard M Kreuter
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Richard M Kreuter 2021-04-14 18:33:42 UTC
Created attachment 224112 [details]
Extend automountd, autounmountd, and autofs to understand nested automounted mount points.

(This is perhaps a duplicate of bug #195564, but I don't know whether the project prefers reopening things vs. new reports.)

I'm interested in enhancing the automounter to be able to handle nested/hierarchical keys in the -hosts map (mostly as a complement to zfs, which makes it convenient to organize data sets in nested hierarchies), so I'm attaching a working proof-of-concept implementation of the capability.

This picture shows the net result on a test machine that exports some zfs shares rooted at /t to itself:

# mount | grep /net
map -hosts on /net (autofs)
localhost:/t/a on /net/localhost/t/a (nfs, nosuid, automounted)
map -hosts on /net/localhost/t/a/b (autofs, automounted)
localhost:/t/a/b on /net/localhost/t/a/b (nfs, nosuid, automounted)
map -hosts on /net/localhost/t/a/b/c (autofs, automounted)

That is, I've added the idea of "automounted autofs" mount points. Because they're automounted, they are created dynamically by automountd and destroyed by autounmountd. Because they're autofs mounts, they eventually cause something else to get mounted.

The attachment is a tar file, here's a description of its contents:

report.txt -- this PR writeup.

notes.txt -- detailed description of the approach, limitations, future work

automountd.diff -- add the notion of automounted autofs mounts

autounmountd.diff -- teach autounmountd about unmount order dependencies.
		  
autofs.diff -- have an automounted autofs pass getattr calls through.

These diffs apply against the releng-12.2 branch (it's what I was running when I started), but should be pretty cleanly applicable to CURRENT or HEAD, since not much has changed in the automounter.

Note that the diffs are named "logically"; in fact the first 2 diffs touch multiple files in usr.sbin/autofs, and and must be applied in the order I've mentioned them.

This set of changes is lightly tested, but does what I want it to so far under repeated, but manual, testing. That said, the approach I've taken isn't the only conceivable way to do things; it was just the shortest path I could think of from what was there to the behavior I wanted to see.

Anyhow, there's work to do before this might be merge-worthy (e.g., I haven't gotten to "automount -u" yet.) If there's an interest in incorporating this approach, I'd be very happy to iterate on this effort, ideally with some guidance/help from those who know this stuff.
Comment 1 Edward Tomasz Napierala freebsd_committer freebsd_triage 2021-04-30 20:04:13 UTC
Hello, and thank you!  I'm really happy to see work being done on what's probably the main piece of functionality missing from our current automounter.

I like the approach you've taken.  Regarding the unmounting problem (notes.txt, #1): I agree about it being the primary missing piece.  I'm not sure I like the idea of recursive unmounts (or recursive anything) in the kernel.  I wonder, though, using your example:

/net              (autofs)
/net/foo/a        (nfs, automounted)
/net/foo/a/b      (autofs, automounted)
/net/foo/a/x      (autofs, automounted)
/net/foo/a/b/c    (nfs, automounted)

It should be technically possible (using 'umount -f') to forcibly unmount /net/foo/a despite /net/foo/a/b and its siblings still being mounted, and unmount the (now unaccessible) submounts afterwards.  Thus, it might be possible to add a flag to the unmount(2) syscall to make it to fail the unmount(2) syscall with EBUSY if there are still vnodes open, except the ones with autofs submounts mounted over it?

Regarding notes.txt, #4: I think autofs(5) doesn't stop just the initial thread that triggered the mount, but also all other attempts to access the same mountpoint - the threads should "queue up" on the automountd request structure, and get unpaused after automountd signals the mount is done.

As for all the other points, I generally agree, or just have nothing to add at the moment.  I've only skimmed through the source for now, but don't have any major suggestions yet apart from a fairly general ones, eg that it might make sense to use tree(3) macros, or not check for NULL before free(3).

I wonder, should we perhaps move this discussion to http://reviews.FreeBSD.org?

Thanks again!
Comment 2 Richard M Kreuter 2021-05-05 16:08:47 UTC
Hi Edward,

Thank you for taking the time to review this work! I may not have time to return to it till mid-next week, but plan to get back to it by about Wednesday the 11th. First order of business will be reworking things using the tree(3) macros, and finishing off 'automount -u'.

Happy to move the conversation to reviews.freebsd.org. I've just applied for an account there, I believe it's pending approval.

Regards,
Richard
Comment 3 Graham Perrin freebsd_committer freebsd_triage 2022-10-17 12:39:17 UTC
Keyword: 

    patch
or  patch-ready

– in lieu of summary line prefix: 

    [patch]

* bulk change for the keyword
* summary lines may be edited manually (not in bulk). 

Keyword descriptions and search interface: 

    <https://bugs.freebsd.org/bugzilla/describekeywords.cgi>
Comment 4 Cy Schubert freebsd_committer freebsd_triage 2023-11-18 04:20:05 UTC
I'm certainly willing to test this out and move it forward in any way to get this into the tree.
Comment 5 Cy Schubert freebsd_committer freebsd_triage 2023-11-22 03:46:10 UTC
I've tested the patch on 15-CURRENT. Unfortunately it doesn't work. The patch does mount subdirectories but I'm unable to see any files within those subdirectories (same as without the patch).

A workaround might be that, until this patch or another can be made to work, users can install sysutils/am-utils and use it to manage /net only. I've reverted sysutils/am-utils deprecation until this bug has been resolved.
Comment 6 Richard M Kreuter 2023-11-23 14:59:53 UTC
(In reply to Cy Schubert from comment #5)

Hi Cy, Curious timing! I've just picked this up again last week. (After a hardware failure in May 2021, I ended up repurposing my FreeBSD system as a file server, so didn't end up running the automounter on FreeBSD again till a couple weeks ago.)

The automountd changes (the first patch of the 3 I sent in 2021) work as I intended on my 15.0 system, but I've got a theory about how it could be failing for you: I suspect "showmount -E <nfs-server>" in your environment is listing the server's exports in a non-sorted order.

If you're willing to retest, could you modify /etc/autofs/special_hosts to sort the export list from your NFS server? Specifically, change line 12 to read:

out=`showmount -E "$1"|sort`

Explanation: the changed automountd depends on the ordering of the exports out of special_hosts. I forgot to include the sorting in the patch I sent in 2021. Without enforced sorting, automountd will mount things in whatever order 'showmount -E' produces, and so could mount server:/a/b before server:/a. The net effect would be consistent with what you describe: there will be a path at a/b, but it'll be the (probably empty) "undermount" from the NFS server. 

The autofs change (the second patch) is trivial, and orthogonal to the rest.

The autounmountd changes (the 3rd patch) are orthogonal to whether automounting works, and anyway are not an optimal way to handle unmounting (basically because they're not atomic). I need to figure out how to do what Edward suggested, i.e., to teach the unmount system call to behave specially when unmounting an autofs that has one or more subsidiary autofses stacked on top.
Comment 7 Mark Linimon freebsd_committer freebsd_triage 2024-09-25 10:45:12 UTC
^Triage: committer's bit was taken in for safekeeping.
Comment 8 Cy Schubert freebsd_committer freebsd_triage 2024-09-25 16:20:18 UTC
I'll pick this up. I haven't done much with this because of the MIT KRB5 project I'm working on and have been using ports/sysutils/am-utils for hosts maps. But we certainly need this fixed so that I can finally deprecate sysutils/am-utils.

To Richard's point, yes autofs needs to detect multi-level mounts.
Comment 9 commit-hook freebsd_committer freebsd_triage 2024-11-14 08:45:35 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=ea27aad16ca1ac30fb5ba9f03fb467dad5351d26

commit ea27aad16ca1ac30fb5ba9f03fb467dad5351d26
Author:     Jesús Daniel Colmenares Oviedo <DtxdF@disroot.org>
AuthorDate: 2024-11-09 22:48:16 +0000
Commit:     Robert Clausecker <fuz@FreeBSD.org>
CommitDate: 2024-11-14 08:43:23 +0000

    graphics/vhs: Update to 0.8.0

    ChangeLog: https://github.com/charmbracelet/vhs/releases/tag/v0.8.0

    PR:             255061

 graphics/vhs/Makefile |  3 +--
 graphics/vhs/distinfo | 10 +++++-----
 2 files changed, 6 insertions(+), 7 deletions(-)