| Message ID | 20220331112318.1377494-21-brauner@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] common: allow to run all tests on idmapped mounts | expand |
On Thu, Mar 31, 2022 at 2:25 PM Christian Brauner <brauner@kernel.org> wrote: > > In addition to the generic and filesystem-specific idmapped mount > testsuites that already exist upstream today add simple infrastructure > so any test can be run on idmapped mounts simply by setting > IDMAPPED_MOUNTS=true in the config file or section. The main user for > now will be overlay to verify it works correctly on idmapped mounts. > > Note that the infrastructure is completely generic so every filesystem that > supports idmapped mounts can simply run all of their tests idmapped. But > note that not all ways to create a mount have been converted yet. That > includes e.g. _dmthin_mount and direct calls to _mount in various tests. > > In addition, there will be corner-cases. For example, xfs doesn't allow > bulkstat on idmapped mounts because it is a filesystem wide operation, > i.e. you can retrieve information for any inode in the filesystem so the > operation cannot be scoped reasonably under a single mount. So xfstests > testing bulkstat will fail as it's blocked. Similar for some btrfs > ioctl()s. > > While we could of course restrict this testmode to -overlay for which we > know things work correctly we should not do this. It would mean that > people won't start using it and so we won't see issues unless someone > sits down and goes through more than 1000 tests and figures out for each > individual one whether it needs to be skipped or not. > > So instead allow this mode but for all filesystems so people can start > running and reporting failures and we can fix them up or block them as > we detect them. > > Cc: Amir Goldstein <amir73il@gmail.com> > Cc: Eryu Guan <guaneryu@gmail.com> > Cc: Christoph Hellwig <hch@lst.de> > Cc: <fstests@vger.kernel.org> > Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org> > --- > /* v2 */ > unchanged > > /* v3 */ > - Amir Goldstein <amir73il@gmail.com>: > - Add more detailed explanation about the current state and > expectations of the newly added IDMAPPED_MOUNTS support. Please document the new envvar in README with a *short* disclaimer about expectations with and without -overlay and please add an [idmapped] section to the multi section config example in README.overlay. Sorry if I wasn't clear about what better documentation of expectations the text in the commit message is important but is no replacement for documentation. Thanks, Amir.
On Thu, Mar 31, 2022 at 02:35:55PM +0300, Amir Goldstein wrote: > On Thu, Mar 31, 2022 at 2:25 PM Christian Brauner <brauner@kernel.org> wrote: > > > > In addition to the generic and filesystem-specific idmapped mount > > testsuites that already exist upstream today add simple infrastructure > > so any test can be run on idmapped mounts simply by setting > > IDMAPPED_MOUNTS=true in the config file or section. The main user for > > now will be overlay to verify it works correctly on idmapped mounts. > > > > Note that the infrastructure is completely generic so every filesystem that > > supports idmapped mounts can simply run all of their tests idmapped. But > > note that not all ways to create a mount have been converted yet. That > > includes e.g. _dmthin_mount and direct calls to _mount in various tests. > > > > In addition, there will be corner-cases. For example, xfs doesn't allow > > bulkstat on idmapped mounts because it is a filesystem wide operation, > > i.e. you can retrieve information for any inode in the filesystem so the > > operation cannot be scoped reasonably under a single mount. So xfstests > > testing bulkstat will fail as it's blocked. Similar for some btrfs > > ioctl()s. > > > > While we could of course restrict this testmode to -overlay for which we > > know things work correctly we should not do this. It would mean that > > people won't start using it and so we won't see issues unless someone > > sits down and goes through more than 1000 tests and figures out for each > > individual one whether it needs to be skipped or not. > > > > So instead allow this mode but for all filesystems so people can start > > running and reporting failures and we can fix them up or block them as > > we detect them. > > > > Cc: Amir Goldstein <amir73il@gmail.com> > > Cc: Eryu Guan <guaneryu@gmail.com> > > Cc: Christoph Hellwig <hch@lst.de> > > Cc: <fstests@vger.kernel.org> > > Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org> > > --- > > /* v2 */ > > unchanged > > > > /* v3 */ > > - Amir Goldstein <amir73il@gmail.com>: > > - Add more detailed explanation about the current state and > > expectations of the newly added IDMAPPED_MOUNTS support. > > Please document the new envvar in README with a *short* disclaimer > about expectations with and without -overlay and please add an [idmapped] > section to the multi section config example in README.overlay. > > Sorry if I wasn't clear about what better documentation of expectations > the text in the commit message is important but is no replacement for > documentation. Will do. Since I'm upstreaming the patch separately I will hold off on resending because of this. Instead I'll start upstreaming this in parallel. Thanks!
On Thu, Mar 31, 2022 at 2:41 PM Christian Brauner <brauner@kernel.org> wrote: > > On Thu, Mar 31, 2022 at 02:35:55PM +0300, Amir Goldstein wrote: > > On Thu, Mar 31, 2022 at 2:25 PM Christian Brauner <brauner@kernel.org> wrote: > > > > > > In addition to the generic and filesystem-specific idmapped mount > > > testsuites that already exist upstream today add simple infrastructure > > > so any test can be run on idmapped mounts simply by setting > > > IDMAPPED_MOUNTS=true in the config file or section. The main user for > > > now will be overlay to verify it works correctly on idmapped mounts. > > > > > > Note that the infrastructure is completely generic so every filesystem that > > > supports idmapped mounts can simply run all of their tests idmapped. But > > > note that not all ways to create a mount have been converted yet. That > > > includes e.g. _dmthin_mount and direct calls to _mount in various tests. > > > > > > In addition, there will be corner-cases. For example, xfs doesn't allow > > > bulkstat on idmapped mounts because it is a filesystem wide operation, > > > i.e. you can retrieve information for any inode in the filesystem so the > > > operation cannot be scoped reasonably under a single mount. So xfstests > > > testing bulkstat will fail as it's blocked. Similar for some btrfs > > > ioctl()s. > > > > > > While we could of course restrict this testmode to -overlay for which we > > > know things work correctly we should not do this. It would mean that > > > people won't start using it and so we won't see issues unless someone > > > sits down and goes through more than 1000 tests and figures out for each > > > individual one whether it needs to be skipped or not. > > > > > > So instead allow this mode but for all filesystems so people can start > > > running and reporting failures and we can fix them up or block them as > > > we detect them. > > > > > > Cc: Amir Goldstein <amir73il@gmail.com> > > > Cc: Eryu Guan <guaneryu@gmail.com> > > > Cc: Christoph Hellwig <hch@lst.de> > > > Cc: <fstests@vger.kernel.org> > > > Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org> > > > --- > > > /* v2 */ > > > unchanged > > > > > > /* v3 */ > > > - Amir Goldstein <amir73il@gmail.com>: > > > - Add more detailed explanation about the current state and > > > expectations of the newly added IDMAPPED_MOUNTS support. > > > > Please document the new envvar in README with a *short* disclaimer > > about expectations with and without -overlay and please add an [idmapped] > > section to the multi section config example in README.overlay. > > > > Sorry if I wasn't clear about what better documentation of expectations > > the text in the commit message is important but is no replacement for > > documentation. > > Will do. Since I'm upstreaming the patch separately I will hold off on > resending because of this. Instead I'll start upstreaming this in > parallel. > Excellent! Thanks, Amir.
diff --git a/common/config b/common/config index 479e50d1..1033b890 100644 --- a/common/config +++ b/common/config @@ -647,6 +647,7 @@ _overlay_config_override() # Set fsck options, use default if user not set directly. export FSCK_OPTIONS="$OVERLAY_FSCK_OPTIONS" [ -z "$FSCK_OPTIONS" ] && _fsck_opts + export IDMAPPED_MOUNTS="$IDMAPPED_MOUNTS" } _overlay_config_restore() diff --git a/common/overlay b/common/overlay index 1ca37e29..fff67ba1 100644 --- a/common/overlay +++ b/common/overlay @@ -73,6 +73,7 @@ _overlay_base_mount() if [ -z "$dev" -o -z "$mnt" ] || \ _check_mounted_on $devname $dev $mntname $mnt; then + _idmapped_mount $dev $mnt # no base fs or already mounted return 0 elif [ $? -ne 1 ]; then @@ -81,6 +82,7 @@ _overlay_base_mount() fi _mount $* $dev $mnt + _idmapped_mount $dev $mnt } _overlay_base_test_mount() diff --git a/common/rc b/common/rc index faf54ef9..5090cbf8 100644 --- a/common/rc +++ b/common/rc @@ -334,6 +334,7 @@ _try_scratch_mount() return $? fi _mount -t $FSTYP `_scratch_mount_options $*` + _idmapped_mount $SCRATCH_DEV $SCRATCH_MNT } # mount scratch device with given options and _fail if mount fails @@ -444,6 +445,53 @@ _scratch_shutdown_handle() fi } +_move_mount() +{ + local mnt=$1 + local tmp=$2 + + # Replace $mnt with $tmp. Use a temporary bind-mount because + # mount --move will fail with certain mount propagation layouts. + $UMOUNT_PROG $mnt || _fail "Failed to unmount $mnt" + _mount --bind $tmp $mnt || _fail "Failed to bind-mount $tmp to $mnt" + $UMOUNT_PROG $tmp || _fail "Failed to unmount $tmp" + rmdir $tmp +} + +_idmapped_mount() +{ + [ "$IDMAPPED_MOUNTS" = "true" ] || return 0 + + local dev=$1 + local mnt=$2 + local status=0 + local tmp=`mktemp -d` + + local mount_rec=`findmnt -rncv -S $dev -o OPTIONS` + if [[ "$mount_rec" == *"idmapped"* ]]; then + return 0 + fi + + # We create an idmapped mount where {g,u}id 0 writes to disk as + # {g,u}id 10000000 and $(id -u fsgqa) + 10000000. We change ownership + # of $mnt so {g,u} id 0 can actually create objects in there. + chown 10000000:10000000 $mnt || return 1 + $here/src/idmapped-mounts/mount-idmapped \ + --map-mount b:10000000:0:100000000000 \ + $mnt $tmp + if [ $? -ne 0 ]; then + rmdir $tmp + return 1 + fi + + # The next call ensures we don't end up stacking an idmapped mount on + # top of the original mount. Instead we fully replace the original + # mount with the idmapped mount. This will not just allow a clean mount + # layout it also makes unmount and remounting way simpler. + _move_mount $mnt $tmp + return $? +} + _test_mount() { if [ "$FSTYP" == "overlay" ]; then @@ -452,6 +500,7 @@ _test_mount() fi _test_options mount _mount -t $FSTYP $TEST_OPTIONS $TEST_FS_MOUNT_OPTS $SELINUX_MOUNT_OPTIONS $* $TEST_DEV $TEST_DIR + _idmapped_mount $TEST_DEV $TEST_DIR } _test_unmount() @@ -3007,6 +3056,7 @@ _mount_or_remount_rw() if [ $USE_REMOUNT -eq 0 ]; then if [ "$FSTYP" != "overlay" ]; then _mount -t $FSTYP $mount_opts $device $mountpoint + _idmapped_mount $device $mountpoint else _overlay_mount $device $mountpoint fi
In addition to the generic and filesystem-specific idmapped mount testsuites that already exist upstream today add simple infrastructure so any test can be run on idmapped mounts simply by setting IDMAPPED_MOUNTS=true in the config file or section. The main user for now will be overlay to verify it works correctly on idmapped mounts. Note that the infrastructure is completely generic so every filesystem that supports idmapped mounts can simply run all of their tests idmapped. But note that not all ways to create a mount have been converted yet. That includes e.g. _dmthin_mount and direct calls to _mount in various tests. In addition, there will be corner-cases. For example, xfs doesn't allow bulkstat on idmapped mounts because it is a filesystem wide operation, i.e. you can retrieve information for any inode in the filesystem so the operation cannot be scoped reasonably under a single mount. So xfstests testing bulkstat will fail as it's blocked. Similar for some btrfs ioctl()s. While we could of course restrict this testmode to -overlay for which we know things work correctly we should not do this. It would mean that people won't start using it and so we won't see issues unless someone sits down and goes through more than 1000 tests and figures out for each individual one whether it needs to be skipped or not. So instead allow this mode but for all filesystems so people can start running and reporting failures and we can fix them up or block them as we detect them. Cc: Amir Goldstein <amir73il@gmail.com> Cc: Eryu Guan <guaneryu@gmail.com> Cc: Christoph Hellwig <hch@lst.de> Cc: <fstests@vger.kernel.org> Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org> --- /* v2 */ unchanged /* v3 */ - Amir Goldstein <amir73il@gmail.com>: - Add more detailed explanation about the current state and expectations of the newly added IDMAPPED_MOUNTS support. --- common/config | 1 + common/overlay | 2 ++ common/rc | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+)