On 3/18/25 10:58 PM, Theodore Ts'o wrote: > On Tue, Mar 18, 2025 at 07:58:05PM -0500, Eric Sandeen wrote: >> >> I shouldn't have said "failed" - you're seeing what I'm seeing, it's >> skipped (not failed) because it tries to test overlayfs idmapped layers >> using the ext4 default options, and acl mounts fail, skipping the test. > > Ah, OK. > > Hmm... I think the fundamental problem is that _overlay_mount_dirs() > shouldn't be using _common_dev_mount_options(). I'm not sure but I think that's still necessary to get to OVERLAY_MOUNT_OPTIONS for overlayfs mounts ... > Depending on the file > system configuration that we might be testing, MOUNT_OPTIONS might > include any number of things which overlayfs might not understand, > including things like: "nfsvers=4", "data=journal", "dax", > "test_dummy_encryption", "trans=virtio" ,"version=9p2000.L" > ,"posixacl", "prjquota", and probably quite a few others. Right. > After all, we'd want to test overlayfs on top of (for exaple) 9pfs > with generic/699, and in that case, MOUNT_OPTIONS will be set from > PLAN9_MOUNT_OPTIONS, and will contain options like "version=9p2000.L" > that overalyfs has no hope of handling. > > What I'm wondering is what mount options _overlay_mount_dirs() needs > from _common_dev_mount_options()? Why is it there in the first place? > If there are some specific mount options that we need to pass on to > overlatyfs, maybe _overlay_mount_dirs() should be grepping out > whatever mount options. it needs, instead of just blindly pulling all > of the mount options? Or, maybe _overlay_mount_dirs() sould avoid > calling _common_dev_mount_options if FSTYP != overlay? Again I think that _common_dev_mount_options is intended to pick up any OVERLAY_MOUNT_OPTIONS mount options the user might have set. In this case, though, it picks up the underlying fs options, because in this case FSTYP is the underlying fs type, not overlay. I asked Zorro offline if maybe this test should just move into overlayfs/ if that can work. I think it would solve the issue, though I'm not familiar enough with what this test is doing to know if it's ok to only get this coverage with an explicit ./check -overlay run ... -Eric > - Ted > >