Re: [PATCH 2/2] Configure custom layers via environment variables

[Vivek Goyal <vgoyal@redhat.com>]

On Thu, Apr 16, 2020 at 10:10:35AM +0300, Amir Goldstein wrote:
> On Wed, Apr 15, 2020 at 10:42 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> >
> > On Wed, Apr 15, 2020 at 07:27:43PM +0300, Amir Goldstein wrote:
> > > On Wed, Apr 15, 2020 at 6:30 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> > > >
> > > > On Wed, Apr 15, 2020 at 03:01:34PM +0300, Amir Goldstein wrote:
> > > > > The following environment variables are supported:
> > > > >
> > > > >  UNIONMOUNT_BASEDIR  - base dir for --samefs (default: /base)
> > > > >  UNIONMOUNT_UPPERDIR - upper layer root path (default: /upper)
> > > > >  UNIONMOUNT_LOWERDIR - lower layer root path (default: /lower)
> > > > >  UNIONMOUNT_MNTPOINT - mount point for tests (default: /mnt)
> > > > >
> > > > > User provided paths for base/lower/upper should point at a pre-mounted
> > > > > filesystem, whereas tmpfs instances will be created on default paths.
> > > > >
> > > > > This is going to be used for running unionmount tests from xfstests.
> > > >
> > > > Hi Amir,
> > > >
> > > > I don't understand this testsuite code. So I will ask.
> > > >
> > > > What's base dir?
> > >
> > > Before these changes, with option --samefs, tmpfs is mounted
> > > at /base, overlay lowerdir is /base/lower and upperdir is /base/upper.
> > > After these changes, /base can be substituted with any pre mounted
> > > filesystem path.
> >
> > If I can specify lower and upper root, then why do I need to specify
> > base directory. User can put lower, upper either on samefs or different
> > fs as need be.
> >
> > IOW, either I need to specify base dir, so that lower and upper can
> > be setup by testsuite automatically. Or I need to specify lower and
> > upper and then base should not matter.
> >
> > What am I missing.
> >
> 
> Not much, as it stands, with option --samefs, UNIONMOUNT_BASEDIR
> is used and without --samefs UNIONMOUNT_LOWERDIR and
> UNIONMOUNT_UPPERDIR used instead.
> 
> I agree that this a bit lame and non intuitive way to configure.
> The reason for explicit --samefs option (vs. providing upper and lower
> root from same fs) is, again, for the test sanity checks which differ
> for is_samefs() case.
> 
> I think what I will do is I will get rid of UNIONMOUNT_UPPERDIR
> because this name is a bit confusing. It is not the overlay upperdir,
> it is the grandfather of upperdir/workdir. So I might as well call
> this config UNIONMOUNT_BASEDIR and use it also as the parent
> of lowerdir in --samefs tests.

How about calling upper/work root as UNIONMOUNT_UPPER_WORK_ROOT instead.
That's more intuitive as oppsed to BASEDIR. But I understand that due 
to legacy reasons there must be many other assumptions in the code so
it might not be trivial.

What will help though, that document these options well, so that
those who don't read the code and still understand use different
config options.

> 
> The config UNIONMOUNT_LOWERDIR will remain, but it will only
> be relevant to tests without --samefs.
> 
> IOW, you won't need the fstab bind mount trick and you won't need
> to use the magic suffix "lower_layer" anymore. You could set:
>   UNIONMOUNT_BASEDIR=/mnt/virtiofs
> 
> to run --ov --samefs --verify tests.

If I specify UNIONMOUNT_BASEDIR, then --samefs should be implied?

Thanks
Vivek

> 
> If you want to run test with virtio fs as upper (single or multi layers)
> and lower as another fs, you could additionally set:
>   UNIONMOUNT_LOWERDIR=/mnt/anotherfs
> 
> and run --ov --verify tests.
> 
> Thanks for the feedback!
> Amir.
>