Re: [PATCH v3] common: allow to run all tests on idmapped mounts

From: Amir Goldstein <amir73il@gmail.com>
To: Christian Brauner <brauner@kernel.org>
Cc: "Miklos Szeredi" <mszeredi@redhat.com>,
	"Eryu Guan" <guan@eryu.me>, fstests <fstests@vger.kernel.org>,
	"Christoph Hellwig" <hch@lst.de>,
	"Aleksa Sarai" <cyphar@cyphar.com>,
	"Giuseppe Scrivano" <gscrivan@redhat.com>,
	"Rodrigo Campos Catelin" <rodrigo@sdfg.com.ar>,
	"Seth Forshee" <sforshee@digitalocean.com>,
	"Luca Bocassi" <luca.boccassi@microsoft.com>,
	"Lennart Poettering" <mzxreary@0pointer.de>,
	"Stéphane Graber" <stgraber@ubuntu.com>,
	"Eryu Guan" <guaneryu@gmail.com>
Subject: Re: [PATCH v3] common: allow to run all tests on idmapped mounts
Date: Fri, 1 Apr 2022 10:16:39 +0300	[thread overview]
Message-ID: <CAOQ4uxj5uPhXcKk09_ZC2UVq6p3z2Zt0PjTE4AxKg=tqMuQqww@mail.gmail.com> (raw)
In-Reply-To: <20220331114128.6j7lnp2h6f5ip4rf@wittgenstein>

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.