On Wed, Mar 30, 2022 at 1:26 PM Christian Brauner <brauner@xxxxxxxxxx> 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.
>
It is not clear from this message how IDMAPPED_MOUNTS=true affects
non -overlay runs and the answer is complicated:
You handled the majority of cases using _test_mount and _scratch_mount
but there are still many tests that open code _mount and some helpers
like _dmthin_mount that are used quite often.
So either document that IDMAPPED_MOUNTS=true will idmapp the
fs mounts whenever it can as best effort, or restrict the use of
IDMAPPED_MOUNTS=true with OVERLAY=true for now.
> Cc: Eryu Guan <guaneryu@xxxxxxxxx>
> Cc: Christoph Hellwig <hch@xxxxxx>
> Cc: <fstests@xxxxxxxxxxxxxxx>
> Signed-off-by: Christian Brauner (Microsoft) <brauner@xxxxxxxxxx>
> ---
> common/config | 1 +
> common/overlay | 2 ++
> common/rc | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 53 insertions(+)
>
> 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"
This is odd.
The purpose of _overlay_config_override() and _overlay_config_restore()
is to allow different configurations for the base layers and for the overlay.
This will be relevant when you implement idmapping for the overlay itself -
you would want to test all combinations of idmapped layers/overlay.
So this should be:
export OVERLAY_IDMAPPED_LAYERS="$IDMAPPED_MOUNTS"
export IDMAPPED_MOUNTS="$OVERLAY_IDMAPPED_MOUNTS"
And I think it would be simpler and better understood to open code the check
if the var in callers like this:
[ $OVERLAY_IDMAPPED_LAYERS ] || _idmapped_mount $dev $mnt
and not inside _idmapped_mount()
It will may it easier for you to implement _overlay_mount() in the future.
To clarify, your test setup to test idmapped overlay layers would not
change, you would still set IDMAPPED_MOUNTS=true in config file
as described in commit message because as README.overlay tries to
explain, with ./check -overlay
MOUNT_OPTIONS= refers to the layers and
OVERLAY_MOUNT_OPTIONS= refers to the overlay
I guess adding the IDMAPPED_MOUNTS=true to README.overlay
multi section config example wouldn't hurt.
Thanks,
Amir.
