[PATCH 0/2] Prepare for running unionmount testssuite from

[PATCH 0/2] Prepare for running unionmount testssuite from

[Amir Goldstein]

Vivek,

You mentioned the need to run unionmount tests on custom layers
(virtiofs in your case) and I provided you the /etc/fstab solution,
which you said works for you.

I now added a new way to run unionmount tests on custom layers,
which I am going to use for xfstests integration [1].

I'd be interested to know if this method serves your use case as well.

Note that I overloaded the meaning of configuring base/lower/upper path:
  1. Use path different than the default
  2. Do not mount tmpfs nor unmount this path

I realize that it might have been better to split the two meanings into
different config options.  However, I am not fond of maintaining config
permutations that nobody is using.  So unless a user comes forward with
a use case, that's the way I intend to leave it.

Would love to get comments from anyone else of course.

Would love to get testing feedback from people that use the --fuse option,
because I am not testing it regularly.

Thanks,
Amir.

[1] https://github.com/amir73il/xfstests/commits/unionmount

Amir Goldstein (2):
  Stop using bind mounts for --samefs
  Configure custom layers via environment variables

 README           | 11 +++++++
 mount_union.py   |  8 ++---
 run              |  3 +-
 set_up.py        | 84 +++++++++++++++++++++++++++---------------------
 settings.py      | 61 +++++++++++++++++++++++++----------
 unmount_union.py | 19 ++++++-----
 6 files changed, 117 insertions(+), 69 deletions(-)

-- 
2.17.1

[PATCH 1/2] Stop using bind mounts for --samefs

[Amir Goldstein]

The implementation of --samefs uses bind mount to mount
/base/lower at /lower and /base/upper at /upper.

Re-assign the value of cfg.{lower,upper}_mntroot to the path
under base dir instead of using bind mounts.

Also stop the habit of remounting lowerdir ro in set_up(), just to
remount it again rw in mount_union() in case of --samefs.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 mount_union.py   |  8 ++---
 set_up.py        | 84 +++++++++++++++++++++++++++---------------------
 settings.py      | 18 +++++++++++
 unmount_union.py | 19 ++++++-----
 4 files changed, 78 insertions(+), 51 deletions(-)

diff --git a/mount_union.py b/mount_union.py
index 55515af..0b3316a 100644
--- a/mount_union.py
+++ b/mount_union.py
@@ -6,7 +6,6 @@ def mount_union(ctx):
     testdir = cfg.testdir()
     if cfg.testing_none():
         lower_mntroot = cfg.lower_mntroot()
-        system("mount -o remount,rw " + lower_mntroot)
         system("mount -o bind " + lower_mntroot + " " + union_mntroot)
         ctx.note_upper_fs(lower_mntroot, testdir, testdir)
 
@@ -14,14 +13,11 @@ def mount_union(ctx):
         lower_mntroot = cfg.lower_mntroot()
         upper_mntroot = cfg.upper_mntroot()
         if cfg.is_samefs():
-            base_mntroot = cfg.base_mntroot()
-            system("mount -o remount,rw " + base_mntroot)
             try:
-                os.mkdir(base_mntroot + upper_mntroot)
+                os.mkdir(upper_mntroot)
             except OSError:
                 pass
-            system("mount -o bind " + base_mntroot + upper_mntroot + " " + upper_mntroot)
-        else:
+        elif cfg.should_mount_upper():
             system("mount " + upper_mntroot + " 2>/dev/null"
                     " || mount -t tmpfs upper_layer " + upper_mntroot)
         layer_mntroot = upper_mntroot + "/" + ctx.curr_layer()
diff --git a/set_up.py b/set_up.py
index c3fad21..afdf5ba 100644
--- a/set_up.py
+++ b/set_up.py
@@ -12,9 +12,7 @@ def create_file(name, content):
 def set_up(ctx):
     cfg = ctx.config()
     lower_mntroot = cfg.lower_mntroot()
-    lowerdir = cfg.lowerdir()
-    lowerimg = cfg.lowerimg()
-    testdir = cfg.testdir()
+    upper_mntroot = cfg.upper_mntroot()
 
     os.sync()
 
@@ -26,12 +24,13 @@ def set_up(ctx):
         except RuntimeError:
             pass
 
-        try:
-            while system("grep -q 'lower_layer " + lower_mntroot + "' /proc/mounts" +
-                         " && umount " + lower_mntroot):
+        if cfg.should_mount_lower():
+            try:
+                while system("grep -q 'lower_layer " + lower_mntroot + "' /proc/mounts" +
+                             " && umount " + lower_mntroot):
+                    pass
+            except RuntimeError:
                 pass
-        except RuntimeError:
-            pass
 
     if cfg.testing_overlayfs():
         try:
@@ -41,59 +40,70 @@ def set_up(ctx):
         except RuntimeError:
             pass
 
-        try:
-            while system("grep -q 'lower_layer " + cfg.base_mntroot() + "' /proc/mounts" +
-                         " && umount " + cfg.base_mntroot()):
+        if cfg.should_mount_base():
+            try:
+                while system("grep -q 'lower_layer " + cfg.base_mntroot() + "' /proc/mounts" +
+                             " && umount " + cfg.base_mntroot()):
+                    pass
+            except RuntimeError:
                 pass
-        except RuntimeError:
-            pass
 
-        try:
-            while system("grep -q 'lower_layer " + lower_mntroot + "' /proc/mounts" +
-                         " && umount " + lower_mntroot):
+        if cfg.should_mount_lower():
+            try:
+                while system("grep -q 'lower_layer " + lower_mntroot + "' /proc/mounts" +
+                             " && umount " + lower_mntroot):
+                    pass
+            except RuntimeError:
                 pass
-        except RuntimeError:
-            pass
+
+        # Adjust lower/upper path in case of --samefs
+        if cfg.is_samefs():
+            base_mntroot = cfg.base_mntroot()
+            lower_mntroot = base_mntroot + "/lower"
+            cfg.set_lower_mntroot(lower_mntroot)
+            upper_mntroot = base_mntroot + "/upper"
+            cfg.set_upper_mntroot(upper_mntroot)
 
         try:
-            # grep filter to catch <lower|upper|N>_layer, in case upper and lower are on same fs
+            # grep filter to catch <lower|upper|N>_layer, in case of --ovov --samefs
             # and in case different layers are on different fs
-            while system("grep -q '_layer " + cfg.upper_mntroot() + "/' /proc/mounts" +
-                         " && umount " + cfg.upper_mntroot() + "/* 2>/dev/null"):
+            while system("grep -q '_layer " + upper_mntroot + "/' /proc/mounts" +
+                         " && umount " + upper_mntroot + "/* 2>/dev/null"):
                 pass
         except RuntimeError:
             pass
 
-        try:
-            # grep filter to catch <low|upp>er_layer, in case upper and lower are on same fs
-            while system("grep -q 'er_layer " + cfg.upper_mntroot() + "' /proc/mounts" +
-                         " && umount " + cfg.upper_mntroot()):
+        if cfg.should_mount_upper():
+            try:
+                while system("grep -q 'upper_layer " + cfg.upper_mntroot() + "' /proc/mounts" +
+                             " && umount " + cfg.upper_mntroot()):
+                    pass
+            except RuntimeError:
                 pass
-        except RuntimeError:
-            pass
 
-    if cfg.is_samefs() and cfg.testing_overlayfs():
+    if cfg.should_mount_base():
         # Create base fs for both lower and upper
-        base_mntroot = cfg.base_mntroot()
         system("mount " + base_mntroot + " 2>/dev/null"
                 " || mount -t tmpfs lower_layer " + base_mntroot)
         system("mount --make-private " + base_mntroot)
+
+    if cfg.is_samefs():
         try:
-            os.mkdir(base_mntroot + lower_mntroot)
+            os.mkdir(lower_mntroot)
         except OSError:
             pass
-        system("mount -o bind " + base_mntroot + lower_mntroot + " " + lower_mntroot)
-    else:
+    elif cfg.should_mount_lower():
         # Create a lower layer to union over
         system("mount " + lower_mntroot + " 2>/dev/null"
                 " || mount -t tmpfs lower_layer " + lower_mntroot)
-
-    # Systemd has weird ideas about things
-    system("mount --make-private " + lower_mntroot)
+        system("mount --make-private " + lower_mntroot)
 
     #
     # Create a few test files we can use in the lower layer
     #
+    lowerdir = cfg.lowerdir()
+    lowerimg = cfg.lowerimg()
+    testdir = cfg.testdir()
     try:
         os.mkdir(lowerdir)
     except OSError:
@@ -176,7 +186,7 @@ def set_up(ctx):
         system("mksquashfs " + lowerdir + " " + lowerimg + " -keep-as-directory > /dev/null");
         system("mount -o loop,ro " + lowerimg + " " + lower_mntroot)
         system("mount --make-private " + lower_mntroot)
-    else:
-        # The mount has to be read-only for us to make use of it
+    elif cfg.should_mount_lower_ro():
+        # Make overlay lower layer read-only
         system("mount -o remount,ro " + lower_mntroot)
     ctx.note_lower_fs(lowerdir)
diff --git a/settings.py b/settings.py
index 12015c8..ced9cae 100644
--- a/settings.py
+++ b/settings.py
@@ -61,10 +61,28 @@ class config:
         self.__union_mntroot = "/mnt"
         self.__testing_overlayfs = True
 
+    # base dir is mounted only for --ov --samefs
+    def should_mount_base(self):
+        return self.testing_overlayfs() and self.is_samefs()
     def base_mntroot(self):
         return self.__base_mntroot
+    # lower dir is mounted ro for --ov (without --samefs) ...
+    def should_mount_lower_ro(self):
+        return self.testing_overlayfs() and not self.is_samefs()
+    # ... and mounted rw for --no
+    def should_mount_lower_rw(self):
+        return self.testing_none()
+    def should_mount_lower(self):
+        return self.should_mount_lower_ro() or self.should_mount_lower_rw()
+    def set_lower_mntroot(self, path):
+        self.__lower_mntroot = path
     def lower_mntroot(self):
         return self.__lower_mntroot
+    # upper dir is mounted for --ov (without --samefs)
+    def should_mount_upper(self):
+        return self.testing_overlayfs() and not self.is_samefs()
+    def set_upper_mntroot(self, path):
+        self.__upper_mntroot = path
     def upper_mntroot(self):
         return self.__upper_mntroot
     def union_mntroot(self):
diff --git a/unmount_union.py b/unmount_union.py
index a83d457..f8f38af 100644
--- a/unmount_union.py
+++ b/unmount_union.py
@@ -6,18 +6,21 @@ def unmount_union(ctx):
     check_not_tainted()
 
     if cfg.testing_overlayfs():
-        if cfg.is_samefs():
-            system("umount " + cfg.base_mntroot())
-            check_not_tainted()
         # unmount individual layers with maxfs > 0
         if cfg.maxfs() > 0 or cfg.is_nested:
             try:
                 system("umount " + cfg.upper_mntroot() + "/* 2>/dev/null")
             except RuntimeError:
                 pass
-        check_not_tainted()
-        system("umount " + cfg.upper_mntroot())
-        check_not_tainted()
+            check_not_tainted()
 
-    system("umount " + cfg.lower_mntroot())
-    check_not_tainted()
+        if cfg.should_mount_base():
+            system("umount " + cfg.base_mntroot())
+            check_not_tainted()
+        elif cfg.should_mount_upper():
+            system("umount " + cfg.upper_mntroot())
+            check_not_tainted()
+
+    if cfg.should_mount_lower():
+        system("umount " + cfg.lower_mntroot())
+        check_not_tainted()
-- 
2.17.1

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

[Amir Goldstein]

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.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 README      | 11 +++++++++++
 run         |  3 ++-
 settings.py | 51 ++++++++++++++++++++++++++++++---------------------
 3 files changed, 43 insertions(+), 22 deletions(-)

diff --git a/README b/README
index c352878..616135f 100644
--- a/README
+++ b/README
@@ -47,5 +47,16 @@ To run these tests:
 	./run --ov --fuse=<subfs-type>
 
 
+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.
+
+
 For more advanced overlayfs test options and more examples, see:
      https://github.com/amir73il/overlayfs/wiki/Overlayfs-testing
diff --git a/run b/run
index e6262b8..60d5d0d 100755
--- a/run
+++ b/run
@@ -20,10 +20,11 @@ def show_format(why):
     print("\t", sys.argv[0], "--<fsop> <file> [<args>*] [-aLlv] [-R <content>] [-B] [-E <err>]")
     sys.exit(2)
 
+cfg = config(sys.argv[0])
+
 if len(sys.argv) < 2:
     show_format("Insufficient arguments")
 
-cfg = config(sys.argv[0])
 args = sys.argv[1:]
 
 ###############################################################################
diff --git a/settings.py b/settings.py
index ced9cae..f065494 100644
--- a/settings.py
+++ b/settings.py
@@ -20,15 +20,27 @@ along with this program; if not, write to the Free Software
 Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 """
 
+import os
+
 class config:
     def __init__(self, progname):
         self.__progname = progname
         self.__testing_overlayfs = False
         self.__testing_none = False
-        self.__base_mntroot = None
-        self.__lower_mntroot = None
-        self.__upper_mntroot = None
-        self.__union_mntroot = None
+        self.__base_mntroot = os.getenv('UNIONMOUNT_BASEDIR')
+        self.__lower_mntroot = os.getenv('UNIONMOUNT_LOWERDIR')
+        self.__upper_mntroot = os.getenv('UNIONMOUNT_UPPERDIR')
+        self.__union_mntroot = os.getenv('UNIONMOUNT_MNTPOINT')
+        print("Environment variables:")
+        if self.__base_mntroot:
+            print("UNIONMOUNT_BASEDIR=" + self.__base_mntroot)
+        if self.__lower_mntroot:
+            print("UNIONMOUNT_LOWERDIR=" + self.__lower_mntroot)
+        if self.__upper_mntroot:
+            print("UNIONMOUNT_UPPERDIR=" + self.__upper_mntroot)
+        if self.__union_mntroot:
+            print("UNIONMOUNT_MNTPOINT=" + self.__union_mntroot)
+        print()
         self.__verbose = False
         self.__verify = False
         self.__maxfs = 0
@@ -50,49 +62,46 @@ class config:
         return self.__testing_overlayfs
 
     def set_testing_none(self):
-        self.__lower_mntroot = "/lower"
-        self.__union_mntroot = "/mnt"
         self.__testing_none = True
 
     def set_testing_overlayfs(self):
-        self.__base_mntroot = "/base"
-        self.__lower_mntroot = "/lower"
-        self.__upper_mntroot = "/upper"
-        self.__union_mntroot = "/mnt"
         self.__testing_overlayfs = True
 
     # base dir is mounted only for --ov --samefs
+    # A user provided base dir should already be mounted
     def should_mount_base(self):
-        return self.testing_overlayfs() and self.is_samefs()
+        return self.__base_mntroot is None and self.testing_overlayfs() and self.is_samefs()
     def base_mntroot(self):
-        return self.__base_mntroot
+        return self.__base_mntroot or "/base"
     # lower dir is mounted ro for --ov (without --samefs) ...
     def should_mount_lower_ro(self):
-        return self.testing_overlayfs() and not self.is_samefs()
+        return self.__lower_mntroot is None and self.testing_overlayfs() and not self.is_samefs()
     # ... and mounted rw for --no
+    # A user provided lower dir should already be mounted
     def should_mount_lower_rw(self):
-        return self.testing_none()
+        return self.__lower_mntroot is None and self.testing_none()
     def should_mount_lower(self):
         return self.should_mount_lower_ro() or self.should_mount_lower_rw()
     def set_lower_mntroot(self, path):
         self.__lower_mntroot = path
     def lower_mntroot(self):
-        return self.__lower_mntroot
+        return self.__lower_mntroot or "/lower"
     # upper dir is mounted for --ov (without --samefs)
+    # A user provided upper dir should already be mounted
     def should_mount_upper(self):
-        return self.testing_overlayfs() and not self.is_samefs()
+        return self.__upper_mntroot is None and self.testing_overlayfs() and not self.is_samefs()
     def set_upper_mntroot(self, path):
         self.__upper_mntroot = path
     def upper_mntroot(self):
-        return self.__upper_mntroot
+        return self.__upper_mntroot or "/upper"
     def union_mntroot(self):
-        return self.__union_mntroot
+        return self.__union_mntroot or "/mnt"
     def lowerdir(self):
-        return self.__lower_mntroot + "/a"
+        return self.lower_mntroot() + "/a"
     def lowerimg(self):
-        return self.__lower_mntroot + "/a.img"
+        return self.lower_mntroot() + "/a.img"
     def testdir(self):
-        return self.__union_mntroot + "/a"
+        return self.union_mntroot() + "/a"
 
     def set_verbose(self, to=True):
         self.__verbose = to
-- 
2.17.1

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

[Vivek Goyal]

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?

So these options will allow me to specify lower directory, upper directory
and overlay mount point. User can specify these and testsuite will
mount overlay accordingly?

What about overlay mount options. Should there be one option for that too.

Assuming workdir is automatically determined.

Vivek

> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  README      | 11 +++++++++++
>  run         |  3 ++-
>  settings.py | 51 ++++++++++++++++++++++++++++++---------------------
>  3 files changed, 43 insertions(+), 22 deletions(-)
> 
> diff --git a/README b/README
> index c352878..616135f 100644
> --- a/README
> +++ b/README
> @@ -47,5 +47,16 @@ To run these tests:
>  	./run --ov --fuse=<subfs-type>
>  
>  
> +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.
> +
> +
>  For more advanced overlayfs test options and more examples, see:
>       https://github.com/amir73il/overlayfs/wiki/Overlayfs-testing
> diff --git a/run b/run
> index e6262b8..60d5d0d 100755
> --- a/run
> +++ b/run
> @@ -20,10 +20,11 @@ def show_format(why):
>      print("\t", sys.argv[0], "--<fsop> <file> [<args>*] [-aLlv] [-R <content>] [-B] [-E <err>]")
>      sys.exit(2)
>  
> +cfg = config(sys.argv[0])
> +
>  if len(sys.argv) < 2:
>      show_format("Insufficient arguments")
>  
> -cfg = config(sys.argv[0])
>  args = sys.argv[1:]
>  
>  ###############################################################################
> diff --git a/settings.py b/settings.py
> index ced9cae..f065494 100644
> --- a/settings.py
> +++ b/settings.py
> @@ -20,15 +20,27 @@ along with this program; if not, write to the Free Software
>  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>  """
>  
> +import os
> +
>  class config:
>      def __init__(self, progname):
>          self.__progname = progname
>          self.__testing_overlayfs = False
>          self.__testing_none = False
> -        self.__base_mntroot = None
> -        self.__lower_mntroot = None
> -        self.__upper_mntroot = None
> -        self.__union_mntroot = None
> +        self.__base_mntroot = os.getenv('UNIONMOUNT_BASEDIR')
> +        self.__lower_mntroot = os.getenv('UNIONMOUNT_LOWERDIR')
> +        self.__upper_mntroot = os.getenv('UNIONMOUNT_UPPERDIR')
> +        self.__union_mntroot = os.getenv('UNIONMOUNT_MNTPOINT')
> +        print("Environment variables:")
> +        if self.__base_mntroot:
> +            print("UNIONMOUNT_BASEDIR=" + self.__base_mntroot)
> +        if self.__lower_mntroot:
> +            print("UNIONMOUNT_LOWERDIR=" + self.__lower_mntroot)
> +        if self.__upper_mntroot:
> +            print("UNIONMOUNT_UPPERDIR=" + self.__upper_mntroot)
> +        if self.__union_mntroot:
> +            print("UNIONMOUNT_MNTPOINT=" + self.__union_mntroot)
> +        print()
>          self.__verbose = False
>          self.__verify = False
>          self.__maxfs = 0
> @@ -50,49 +62,46 @@ class config:
>          return self.__testing_overlayfs
>  
>      def set_testing_none(self):
> -        self.__lower_mntroot = "/lower"
> -        self.__union_mntroot = "/mnt"
>          self.__testing_none = True
>  
>      def set_testing_overlayfs(self):
> -        self.__base_mntroot = "/base"
> -        self.__lower_mntroot = "/lower"
> -        self.__upper_mntroot = "/upper"
> -        self.__union_mntroot = "/mnt"
>          self.__testing_overlayfs = True
>  
>      # base dir is mounted only for --ov --samefs
> +    # A user provided base dir should already be mounted
>      def should_mount_base(self):
> -        return self.testing_overlayfs() and self.is_samefs()
> +        return self.__base_mntroot is None and self.testing_overlayfs() and self.is_samefs()
>      def base_mntroot(self):
> -        return self.__base_mntroot
> +        return self.__base_mntroot or "/base"
>      # lower dir is mounted ro for --ov (without --samefs) ...
>      def should_mount_lower_ro(self):
> -        return self.testing_overlayfs() and not self.is_samefs()
> +        return self.__lower_mntroot is None and self.testing_overlayfs() and not self.is_samefs()
>      # ... and mounted rw for --no
> +    # A user provided lower dir should already be mounted
>      def should_mount_lower_rw(self):
> -        return self.testing_none()
> +        return self.__lower_mntroot is None and self.testing_none()
>      def should_mount_lower(self):
>          return self.should_mount_lower_ro() or self.should_mount_lower_rw()
>      def set_lower_mntroot(self, path):
>          self.__lower_mntroot = path
>      def lower_mntroot(self):
> -        return self.__lower_mntroot
> +        return self.__lower_mntroot or "/lower"
>      # upper dir is mounted for --ov (without --samefs)
> +    # A user provided upper dir should already be mounted
>      def should_mount_upper(self):
> -        return self.testing_overlayfs() and not self.is_samefs()
> +        return self.__upper_mntroot is None and self.testing_overlayfs() and not self.is_samefs()
>      def set_upper_mntroot(self, path):
>          self.__upper_mntroot = path
>      def upper_mntroot(self):
> -        return self.__upper_mntroot
> +        return self.__upper_mntroot or "/upper"
>      def union_mntroot(self):
> -        return self.__union_mntroot
> +        return self.__union_mntroot or "/mnt"
>      def lowerdir(self):
> -        return self.__lower_mntroot + "/a"
> +        return self.lower_mntroot() + "/a"
>      def lowerimg(self):
> -        return self.__lower_mntroot + "/a.img"
> +        return self.lower_mntroot() + "/a.img"
>      def testdir(self):
> -        return self.__union_mntroot + "/a"
> +        return self.union_mntroot() + "/a"
>  
>      def set_verbose(self, to=True):
>          self.__verbose = to
> -- 
> 2.17.1
> 

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

[Amir Goldstein]

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.

>
> So these options will allow me to specify lower directory, upper directory

Almost.

They let you specify lower fs root and upper fs root.
Before these changes, tmpfs is mounted at /lower and this is used
as overlay lowerdir.
After these changes, /lower can be substituted with any pre mounted
filesystem path.

Situation for upperdir/workdir is a bit different (see below).

> and overlay mount point. User can specify these and testsuite will
> mount overlay accordingly?
>
> What about overlay mount options. Should there be one option for that too.

Maybe. Currently the overlay mount options are determined by the various
command line arguments, like --xino --meta --verify.
The reason that testsuite does not let you use any combination of mount
options is because the options (e.g. --meta) determine both how overlay is
mounted and also how verification is done (see for example the is_metacopy
case in check_copy_up()).

Do you have any requirement for specific overlay mount options you
would like to test?

>
> Assuming workdir is automatically determined.
>

Yes. Before these changes, tmpfs is mounted at /upper.
The directories /upper/0/u /upper/0/w are used as upperdir/workdir with
single layer.
With multiple layers (e.g. ov=2) more directories can be created, like
/upper/1/{u,w}.
After these changes, tmpfs /upper can be substituted with any pre
mounted filesystem path.

Hope this is clear now.
See example is how xfstests make use of those variable to use
the test and scratch partitions for lower and upper fs:
https://github.com/amir73il/xfstests/commit/a36f476a04c5af5100141c3ff9938d0c2f93018d#diff-7892ac2dd3f989038dfaa2e708ab12e2R386

Thanks,
Amir.

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

[Vivek Goyal]

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.

> 
> >
> > So these options will allow me to specify lower directory, upper directory
> 
> Almost.
> 
> They let you specify lower fs root and upper fs root.
> Before these changes, tmpfs is mounted at /lower and this is used
> as overlay lowerdir.
> After these changes, /lower can be substituted with any pre mounted
> filesystem path.

Ok.

> 
> Situation for upperdir/workdir is a bit different (see below).
> 
> > and overlay mount point. User can specify these and testsuite will
> > mount overlay accordingly?
> >
> > What about overlay mount options. Should there be one option for that too.
> 
> Maybe. Currently the overlay mount options are determined by the various
> command line arguments, like --xino --meta --verify.
> The reason that testsuite does not let you use any combination of mount
> options is because the options (e.g. --meta) determine both how overlay is
> mounted and also how verification is done (see for example the is_metacopy
> case in check_copy_up()).
> 
> Do you have any requirement for specific overlay mount options you
> would like to test?

No, I don't have any requirements yet. Just thought that providing
extra flexibility will be good. At the same time I understand that
tests are tied to specific config, hence mount options. If we allow
override, suddenly many tests will start failing.

> 
> >
> > Assuming workdir is automatically determined.
> >
> 
> Yes. Before these changes, tmpfs is mounted at /upper.
> The directories /upper/0/u /upper/0/w are used as upperdir/workdir with
> single layer.
> With multiple layers (e.g. ov=2) more directories can be created, like
> /upper/1/{u,w}.
> After these changes, tmpfs /upper can be substituted with any pre
> mounted filesystem path.
> 
> Hope this is clear now.

Yes. Thanks.

Vivek

> See example is how xfstests make use of those variable to use
> the test and scratch partitions for lower and upper fs:
> https://github.com/amir73il/xfstests/commit/a36f476a04c5af5100141c3ff9938d0c2f93018d#diff-7892ac2dd3f989038dfaa2e708ab12e2R386
> 
> Thanks,
> Amir.
> 

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

[Amir Goldstein]

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.

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 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.

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

[Vivek Goyal]

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.
> 

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

[Amir Goldstein]

> > > 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.
>

This naming might have made sense with existing meaning of
UNIONMOUNT_UPPERDIR, but the new idea is to throw that a way and have
UNIONMOUNT_BASEDIR as the base dir for lower/upper/work.
This is similar to OVL_BASE_TEST_DIR and OVL_BASE_SCRATCH_MNT
vars in xfstests.

> 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.
>

Sure, I added those to README, will try to elaborate.

> >
> > 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?
>

This might have made sense with the meaning of UNIONMOUNT_BASEDIR
as it is in current posting, but with intended change, I suppose an empty
UNIONMOUNT_LOWERDIR could mean --samefs.
When both --samefs and UNIONMOUNT_LOWERDIR are specified, I'll
throw a warning that UNIONMOUNT_LOWERDIR is ignored.

Thanks,
Amir.

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

[Amir Goldstein]

> > If I specify UNIONMOUNT_BASEDIR, then --samefs should be implied?
> >
>
> This might have made sense with the meaning of UNIONMOUNT_BASEDIR
> as it is in current posting, but with intended change, I suppose an empty
> UNIONMOUNT_LOWERDIR could mean --samefs.
> When both --samefs and UNIONMOUNT_LOWERDIR are specified, I'll
> throw a warning that UNIONMOUNT_LOWERDIR is ignored.
>

Vivek,

I updated the logic per some of your suggestions and push to:
  https://github.com/amir73il/unionmount-testsuite/commits/overlayfs-devel
The example of how xfstests uses it is at:
  https://github.com/amir73il/xfstests/commits/unionmount

Since I am mostly interested in feedback on config interface, I'll just
paste the commit message here (same text is also in README).

In short: if you set UNIONMOUNT_BASEDIR to virtiofs path and
execute run --ov, all layers will be created under that virtiofs path.

Let me know if this works for you.
Thanks,
Amir.

commit 8c2ac6e0cd9d4b01e421375e0b9c3703e774cd9f
Author: Amir Goldstein <amir73il@gmail.com>
Date:   Sun Apr 12 19:22:19 2020 +0300

    Configure custom layers via environment variables

    The following environment variables are supported:

     UNIONMOUNT_BASEDIR  - parent dir of all samefs layers (default: /base)
     UNIONMOUNT_LOWERDIR - lower layer path for non samefs (default: /lower)
     UNIONMOUNT_MNTPOINT - mount point for executing tests (default: /mnt)

    When user provides paths for base/lower dir, they should point at
    existing directories and their content will be deleted.
    When the default base/lower paths are used, tmpfs instances are created.

    UNIONMOUNT_LOWERDIR is meaningless and will be ignored with --samefs.
    Empty UNIONMOUNT_LOWERDIR with non-empty UNIONMOUNT_BASEDIR imply --samefs,
    unless user explicitly requested non samefs setup with maxfs=<M>.

    This is going to be used for running unionmount tests from xfstests.

    Signed-off-by: Amir Goldstein <amir73il@gmail.com>

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

[Vivek Goyal]

On Sat, Apr 18, 2020 at 12:57:27PM +0300, Amir Goldstein wrote:
> > > If I specify UNIONMOUNT_BASEDIR, then --samefs should be implied?
> > >
> >
> > This might have made sense with the meaning of UNIONMOUNT_BASEDIR
> > as it is in current posting, but with intended change, I suppose an empty
> > UNIONMOUNT_LOWERDIR could mean --samefs.
> > When both --samefs and UNIONMOUNT_LOWERDIR are specified, I'll
> > throw a warning that UNIONMOUNT_LOWERDIR is ignored.
> >
> 
> Vivek,
> 
> I updated the logic per some of your suggestions and push to:
>   https://github.com/amir73il/unionmount-testsuite/commits/overlayfs-devel
> The example of how xfstests uses it is at:
>   https://github.com/amir73il/xfstests/commits/unionmount
> 
> Since I am mostly interested in feedback on config interface, I'll just
> paste the commit message here (same text is also in README).
> 
> In short: if you set UNIONMOUNT_BASEDIR to virtiofs path and
> execute run --ov, all layers will be created under that virtiofs path.

This is nice. I tried following and it seems to work.

UNIONMOUNT_BASEDIR=/mnt/virtiofs/overlayfs-tests
UNIONMOUNT_MNTPOINT=/mnt/virtiofs/mnt

Got a question though. If somebody specifies a BASEDIR, whey not
automatically select mount point also inside that basedir. Is it because
of existing structure where basedir and mnt directory are separate and
defaults are different. Anway, I don't mind overlay mountpoint with
a separate environment variable.

> 
> Let me know if this works for you.
> Thanks,
> Amir.
> 
> commit 8c2ac6e0cd9d4b01e421375e0b9c3703e774cd9f
> Author: Amir Goldstein <amir73il@gmail.com>
> Date:   Sun Apr 12 19:22:19 2020 +0300
> 
>     Configure custom layers via environment variables
> 
>     The following environment variables are supported:
> 
>      UNIONMOUNT_BASEDIR  - parent dir of all samefs layers (default: /base)
>      UNIONMOUNT_LOWERDIR - lower layer path for non samefs (default: /lower)
>      UNIONMOUNT_MNTPOINT - mount point for executing tests (default: /mnt)
> 
>     When user provides paths for base/lower dir, they should point at
>     existing directories and their content will be deleted.
>     When the default base/lower paths are used, tmpfs instances are created.
> 
>     UNIONMOUNT_LOWERDIR is meaningless and will be ignored with --samefs.
>     Empty UNIONMOUNT_LOWERDIR with non-empty UNIONMOUNT_BASEDIR imply --samefs,

What happens if I specify both UNIONMOUNT_LOWERDIR as well as
UNIONMOUNT_BASEDIR. Does that mean only upper and work will be setup in
UNIONMOUNT_BASEDIR.

>     unless user explicitly requested non samefs setup with maxfs=<M>.

So if UNIONMOUNT_LOWERDIR is empty and I specify a UNIONMOUNT_BASEDIR and
use maxfs=<M>. All layers will still come from under UNIONMOUNT_BASEDIR,
right?

What's most intuitive to me is this.

- If user only specifies UNIONMOUNT_BASEDIR, all layers (lower, upper,
  work and even mount point) comes from that directory.

- If user specifies both UNIONMOUNT_LOWERDIR and UNIONMOUNT_BASEDIR, then
  lower layer path comes from UNIONMOUNT_LOWERDIR and rest of the layers
  come from UNIONMOUNT_BASEDIR.

- If user specifies UNIONMOUNT_MNTPOINT, it is used as overlay mount
  point. Otherwise one is selected from UNIONMOUNT_BASEDIR if user
  specified one. Otherwise "/mnt" is the default.

Thanks
Vivek

> 
>     This is going to be used for running unionmount tests from xfstests.
> 
>     Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> 

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

[Amir Goldstein]

On Mon, Apr 20, 2020 at 10:15 PM Vivek Goyal <vgoyal@redhat.com> wrote:
>
> On Sat, Apr 18, 2020 at 12:57:27PM +0300, Amir Goldstein wrote:
> > > > If I specify UNIONMOUNT_BASEDIR, then --samefs should be implied?
> > > >
> > >
> > > This might have made sense with the meaning of UNIONMOUNT_BASEDIR
> > > as it is in current posting, but with intended change, I suppose an empty
> > > UNIONMOUNT_LOWERDIR could mean --samefs.
> > > When both --samefs and UNIONMOUNT_LOWERDIR are specified, I'll
> > > throw a warning that UNIONMOUNT_LOWERDIR is ignored.
> > >
> >
> > Vivek,
> >
> > I updated the logic per some of your suggestions and push to:
> >   https://github.com/amir73il/unionmount-testsuite/commits/overlayfs-devel
> > The example of how xfstests uses it is at:
> >   https://github.com/amir73il/xfstests/commits/unionmount
> >
> > Since I am mostly interested in feedback on config interface, I'll just
> > paste the commit message here (same text is also in README).
> >
> > In short: if you set UNIONMOUNT_BASEDIR to virtiofs path and
> > execute run --ov, all layers will be created under that virtiofs path.
>
> This is nice. I tried following and it seems to work.
>
> UNIONMOUNT_BASEDIR=/mnt/virtiofs/overlayfs-tests
> UNIONMOUNT_MNTPOINT=/mnt/virtiofs/mnt
>
> Got a question though. If somebody specifies a BASEDIR, whey not
> automatically select mount point also inside that basedir. Is it because
> of existing structure where basedir and mnt directory are separate and
> defaults are different. Anway, I don't mind overlay mountpoint with
> a separate environment variable.
>

No reason. I think it's a good idea. I'll do it.

> >
> > Let me know if this works for you.
> > Thanks,
> > Amir.
> >
> > commit 8c2ac6e0cd9d4b01e421375e0b9c3703e774cd9f
> > Author: Amir Goldstein <amir73il@gmail.com>
> > Date:   Sun Apr 12 19:22:19 2020 +0300
> >
> >     Configure custom layers via environment variables
> >
> >     The following environment variables are supported:
> >
> >      UNIONMOUNT_BASEDIR  - parent dir of all samefs layers (default: /base)
> >      UNIONMOUNT_LOWERDIR - lower layer path for non samefs (default: /lower)
> >      UNIONMOUNT_MNTPOINT - mount point for executing tests (default: /mnt)
> >
> >     When user provides paths for base/lower dir, they should point at
> >     existing directories and their content will be deleted.
> >     When the default base/lower paths are used, tmpfs instances are created.
> >
> >     UNIONMOUNT_LOWERDIR is meaningless and will be ignored with --samefs.
> >     Empty UNIONMOUNT_LOWERDIR with non-empty UNIONMOUNT_BASEDIR imply --samefs,
>
> What happens if I specify both UNIONMOUNT_LOWERDIR as well as
> UNIONMOUNT_BASEDIR. Does that mean only upper and work will be setup in
> UNIONMOUNT_BASEDIR.
>

Yes, work and upper and with --ov=N also middle layers.

> >     unless user explicitly requested non samefs setup with maxfs=<M>.
>
> So if UNIONMOUNT_LOWERDIR is empty and I specify a UNIONMOUNT_BASEDIR and
> use maxfs=<M>. All layers will still come from under UNIONMOUNT_BASEDIR,
> right?

Yes, but note that while the *path* of all layers is under UNIONMOUNT_BASEDIR,
namely $UNIONMOUNT_BASEDIR/$N/{u,w}, some layers will use the basedir fs,
while others will have a tmpfs instance mounted at $UNIONMOUNT_BASEDIR/$N/.
M from maxfs=<M> determines the number of middle layers using tmpfs instances.

Now that I think about it, this setup should use $UNIONMOUNT_BASEDIR/lower
as lowerdir path and it uses /lower.
I'll need to fix that.

>
> What's most intuitive to me is this.
>
> - If user only specifies UNIONMOUNT_BASEDIR, all layers (lower, upper,
>   work and even mount point) comes from that directory.

OK.

>
> - If user specifies both UNIONMOUNT_LOWERDIR and UNIONMOUNT_BASEDIR, then
>   lower layer path comes from UNIONMOUNT_LOWERDIR and rest of the layers
>   come from UNIONMOUNT_BASEDIR.

DONE.

>
> - If user specifies UNIONMOUNT_MNTPOINT, it is used as overlay mount
>   point. Otherwise one is selected from UNIONMOUNT_BASEDIR if user
>   specified one. Otherwise "/mnt" is the default.
>

OK.

Thanks for valuable feedback,
Amir.

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

[Amir Goldstein]

> >
> > What's most intuitive to me is this.
> >
> > - If user only specifies UNIONMOUNT_BASEDIR, all layers (lower, upper,
> >   work and even mount point) comes from that directory.
>
> OK.
>
> >
> > - If user specifies both UNIONMOUNT_LOWERDIR and UNIONMOUNT_BASEDIR, then
> >   lower layer path comes from UNIONMOUNT_LOWERDIR and rest of the layers
> >   come from UNIONMOUNT_BASEDIR.
>
> DONE.
>
> >
> > - If user specifies UNIONMOUNT_MNTPOINT, it is used as overlay mount
> >   point. Otherwise one is selected from UNIONMOUNT_BASEDIR if user
> >   specified one. Otherwise "/mnt" is the default.
> >
>
> OK.
>

Vivek,

I finally got around to implementing your suggestion (see [1]).

Quoting from README:

     When user provides UNIONMOUNT_LOWERDIR:

     1) Path should be an existing directory whose content will be deleted.
     2) Path is assumed to be on a different filesystem than base dir, so
        --samefs setup is not supported.

     When user provides UNIONMOUNT_BASEDIR:

     1) Path should be an existing directory whose content will be deleted.
     2) If UNIONMOUNT_MNTPOINT is not provided, the overlay mount point will
        be created under base dir.
     3) If UNIONMOUNT_LOWERDIR is not provided, the lower layer dir will be
        created under base dir.
     4) If UNIONMOUNT_LOWERDIR is not provided, the test setup defaults to
        --samefs (i.e. lower and upper are on the same base fs).  However,
        if --maxfs=<M> is specified, a tmpfs instance will be created for
        the lower layer dir.
----

I realize this last item (4) is a bit tricky.
Let me know if you think it needs further clarification.

Thanks,
Amir.


[1] https://github.com/amir73il/unionmount-testsuite/commits/envvars

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

[Vivek Goyal]

On Sun, May 17, 2020 at 11:45:59AM +0300, Amir Goldstein wrote:
> > >
> > > What's most intuitive to me is this.
> > >
> > > - If user only specifies UNIONMOUNT_BASEDIR, all layers (lower, upper,
> > >   work and even mount point) comes from that directory.
> >
> > OK.
> >
> > >
> > > - If user specifies both UNIONMOUNT_LOWERDIR and UNIONMOUNT_BASEDIR, then
> > >   lower layer path comes from UNIONMOUNT_LOWERDIR and rest of the layers
> > >   come from UNIONMOUNT_BASEDIR.
> >
> > DONE.
> >
> > >
> > > - If user specifies UNIONMOUNT_MNTPOINT, it is used as overlay mount
> > >   point. Otherwise one is selected from UNIONMOUNT_BASEDIR if user
> > >   specified one. Otherwise "/mnt" is the default.
> > >
> >
> > OK.
> >
> 
> Vivek,
> 
> I finally got around to implementing your suggestion (see [1]).
> 
> Quoting from README:
> 
>      When user provides UNIONMOUNT_LOWERDIR:
> 
>      1) Path should be an existing directory whose content will be deleted.
>      2) Path is assumed to be on a different filesystem than base dir, so
>         --samefs setup is not supported.
> 
>      When user provides UNIONMOUNT_BASEDIR:
> 
>      1) Path should be an existing directory whose content will be deleted.
>      2) If UNIONMOUNT_MNTPOINT is not provided, the overlay mount point will
>         be created under base dir.
>      3) If UNIONMOUNT_LOWERDIR is not provided, the lower layer dir will be
>         created under base dir.
>      4) If UNIONMOUNT_LOWERDIR is not provided, the test setup defaults to
>         --samefs (i.e. lower and upper are on the same base fs).  However,
>         if --maxfs=<M> is specified, a tmpfs instance will be created for
>         the lower layer dir.

Hi Amir,

Do you want to mention a word upper dir also when UNIONMOUNT_BASEDIR. That
is upperdir is also created under UNIONMOUNT_BASEDIR. IOW, all directories
lower, upper and mount point are under UNIONMOUNT_BASEDIR (until and
unless overridden by other environment variables).

For point 4, I understand that we will mount multiple instances of
tmpfs because maxfs tests on multiple different filessytems. I am
assuming that we will be creating lowerdir mount points under
UNIONMOUNT_BASEDIR for --maxfs.

I think this looks pretty good. Just one more thing. Is there a way to
specify multiple lowerdirs as well. If not, may be in future we can
add it once somebody needs to specify multiple lowerdirs.

Thanks
Vivek

> ----
> 
> I realize this last item (4) is a bit tricky.
> Let me know if you think it needs further clarification.
> 
> Thanks,
> Amir.
> 
> 
> [1] https://github.com/amir73il/unionmount-testsuite/commits/envvars
> 

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

[Amir Goldstein]

> > Vivek,
> >
> > I finally got around to implementing your suggestion (see [1]).
> >
> > Quoting from README:
> >
> >      When user provides UNIONMOUNT_LOWERDIR:
> >
> >      1) Path should be an existing directory whose content will be deleted.
> >      2) Path is assumed to be on a different filesystem than base dir, so
> >         --samefs setup is not supported.
> >
> >      When user provides UNIONMOUNT_BASEDIR:
> >
> >      1) Path should be an existing directory whose content will be deleted.
> >      2) If UNIONMOUNT_MNTPOINT is not provided, the overlay mount point will
> >         be created under base dir.
> >      3) If UNIONMOUNT_LOWERDIR is not provided, the lower layer dir will be
> >         created under base dir.
> >      4) If UNIONMOUNT_LOWERDIR is not provided, the test setup defaults to
> >         --samefs (i.e. lower and upper are on the same base fs).  However,
> >         if --maxfs=<M> is specified, a tmpfs instance will be created for
> >         the lower layer dir.
>
> Hi Amir,
>
> Do you want to mention a word upper dir also when UNIONMOUNT_BASEDIR. That
> is upperdir is also created under UNIONMOUNT_BASEDIR. IOW, all directories
> lower, upper and mount point are under UNIONMOUNT_BASEDIR (until and
> unless overridden by other environment variables).

Sure. I will add:

2) Upper layer and middle layers will be created under base dir

>
> For point 4, I understand that we will mount multiple instances of
> tmpfs because maxfs tests on multiple different filessytems. I am
> assuming that we will be creating lowerdir mount points under
> UNIONMOUNT_BASEDIR for --maxfs.
>

Yap.

> I think this looks pretty good. Just one more thing. Is there a way to
> specify multiple lowerdirs as well. If not, may be in future we can
> add it once somebody needs to specify multiple lowerdirs.
>

You mean a way to specify different paths/fs to lower dirs?
It doesn't make much sense in the test.
The test rotates the upper layer 0 to be middle dir 0 and creates
upper dir 1. Most of the tests never rotate upper.

Currently, the most complex configuration you can run tests with
is rotating layers that end up with:
- Lowermost layer on user configurable lowerfs (*)
- Mid layers 0..M-1 on unique tmpfs instanced
- Layers M..N on same user configurable basefs.

(*) there is also the --squashfs option which formats the readonly
lowerfs with the test files.

Would you like a way to define a user configurable path
for upper dir 0? something else?

Which real life use case is this supposed to be simulating anyway?
for me this looks too much for no good reason.
If anyone comes forward with a reason and patches we can
consider that.

Thanks,
Amir.

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

[Amir Goldstein]

> > Hi Amir,
> >
> > Do you want to mention a word upper dir also when UNIONMOUNT_BASEDIR. That
> > is upperdir is also created under UNIONMOUNT_BASEDIR. IOW, all directories
> > lower, upper and mount point are under UNIONMOUNT_BASEDIR (until and
> > unless overridden by other environment variables).
>

Hi Vivek,

Please approve this text before I update master.
Pushed this work to branch 'envvars'

Thanks,
Amir.

-----
The following environment variables are supported:

     UNIONMOUNT_BASEDIR  - parent dir of all samefs layers (default: /base)
     UNIONMOUNT_LOWERDIR - lower layer path for non samefs (default: /lower)
     UNIONMOUNT_MNTPOINT - mount point for executing tests (default: /mnt)

     When user provides UNIONMOUNT_LOWERDIR:

     1) Path should be an existing directory whose content will be deleted.
     2) Path is assumed to be on a different filesystem than base dir, so
        --samefs setup is not supported.

     When user provides UNIONMOUNT_BASEDIR:

     1) Path should be an existing directory whose content will be deleted.
     2) Upper layer and middle layers will be created under base dir.
     3) If UNIONMOUNT_MNTPOINT is not provided, the overlay mount point will
        be created under base dir.
     4) If UNIONMOUNT_LOWERDIR is not provided, the lower layer dir will be
        created under base dir.
     5) If UNIONMOUNT_LOWERDIR is not provided, the test setup defaults to
        --samefs (i.e. lower and upper layers are on the same base fs).
        However, if --maxfs=<M> is specified, a tmpfs instance will be mounted
        on the lower layer dir that was created under base dir.

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

[Vivek Goyal]

On Sun, May 24, 2020 at 01:28:44PM +0300, Amir Goldstein wrote:
> > > Hi Amir,
> > >
> > > Do you want to mention a word upper dir also when UNIONMOUNT_BASEDIR. That
> > > is upperdir is also created under UNIONMOUNT_BASEDIR. IOW, all directories
> > > lower, upper and mount point are under UNIONMOUNT_BASEDIR (until and
> > > unless overridden by other environment variables).
> >
> 
> Hi Vivek,
> 
> Please approve this text before I update master.
> Pushed this work to branch 'envvars'

Hi Amir,

This looks good to me. Thank you for providing these environment
variables. Helps me run these tests on top of virtiofs now.

Vivek

> 
> Thanks,
> Amir.
> 
> -----
> The following environment variables are supported:
> 
>      UNIONMOUNT_BASEDIR  - parent dir of all samefs layers (default: /base)
>      UNIONMOUNT_LOWERDIR - lower layer path for non samefs (default: /lower)
>      UNIONMOUNT_MNTPOINT - mount point for executing tests (default: /mnt)
> 
>      When user provides UNIONMOUNT_LOWERDIR:
> 
>      1) Path should be an existing directory whose content will be deleted.
>      2) Path is assumed to be on a different filesystem than base dir, so
>         --samefs setup is not supported.
> 
>      When user provides UNIONMOUNT_BASEDIR:
> 
>      1) Path should be an existing directory whose content will be deleted.
>      2) Upper layer and middle layers will be created under base dir.
>      3) If UNIONMOUNT_MNTPOINT is not provided, the overlay mount point will
>         be created under base dir.
>      4) If UNIONMOUNT_LOWERDIR is not provided, the lower layer dir will be
>         created under base dir.
>      5) If UNIONMOUNT_LOWERDIR is not provided, the test setup defaults to
>         --samefs (i.e. lower and upper layers are on the same base fs).
>         However, if --maxfs=<M> is specified, a tmpfs instance will be mounted
>         on the lower layer dir that was created under base dir.
>