overlayfs: support more than one read-only layer

RFC PATCH In review Miklos Szeredi 4 participants 7 messages Nov 7, 2014 – Nov 17, 2014

Subsystems (3)

VFS Overlay filesystem Ecrypt file system

lore

Share & tools

lore

Miklos Szeredi

author maintainer

2014-11-07 17:02 UTC #0 lore raw

RFC PATCH 1/1 overlayfs: support more than one read-only layer

Apparently this is the top feature request, and is relateively easy to do.

This is the first iteration of the patch, and quite possibly buggy. Testing and review is welcome.

Usage: -olowerdirs=/top:/middle:/bottom...

Now the upperdir and workdir options can be omitted to get a read-only overlay.

Thanks,
Miklos
----

fs/overlayfs/copy_up.c +2 -2

index ea10a8719107..a5bfd60f4f6f 100644
@@ -385,7 +385,7 @@ int ovl_copy_up(struct dentry *dentry)
 		struct kstat stat;
 		enum ovl_path_type type = ovl_path_type(dentry);
 
-		if (type != OVL_PATH_LOWER)
+		if (OVL_TYPE_UPPER(type))
 			break;
 
 		next = dget(dentry);

@@ -394,7 +394,7 @@ int ovl_copy_up(struct dentry *dentry)
 			parent = dget_parent(next);
 
 			type = ovl_path_type(parent);
-			if (type != OVL_PATH_LOWER)
+			if (OVL_TYPE_UPPER(type))
 				break;
 
 			dput(next);

fs/overlayfs/dir.c +30 ~~-23~~

146 lines not on this page · raw message

fs/overlayfs/inode.c +23 ~~-11~~

index af2d18c9fcee..48492f1240ad 100644
@@ -235,26 +235,39 @@ out:
 	return err;
 }
 
+static bool ovl_need_xattr_filter(struct dentry *dentry,
+				  enum ovl_path_type type)
+{
+	if ((type & (__OVL_PATH_PURE | __OVL_PATH_UPPER)) == __OVL_PATH_UPPER)
+		return S_ISDIR(dentry->d_inode->i_mode);
+	else
+		return false;
+}
+
 ssize_t ovl_getxattr(struct dentry *dentry, const char *name,
 		     void *value, size_t size)
 {
-	if (ovl_path_type(dentry->d_parent) == OVL_PATH_MERGE &&
-	    ovl_is_private_xattr(name))
+	struct path realpath;
+	enum ovl_path_type type = ovl_path_real(dentry, &realpath);
+
+	if (ovl_need_xattr_filter(dentry, type) && ovl_is_private_xattr(name))
 		return -ENODATA;
 
-	return vfs_getxattr(ovl_dentry_real(dentry), name, value, size);
+	return vfs_getxattr(realpath.dentry, name, value, size);
 }
 
 ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size)
 {
+	struct path realpath;
+	enum ovl_path_type type = ovl_path_real(dentry, &realpath);
 	ssize_t res;
 	int off;
 
-	res = vfs_listxattr(ovl_dentry_real(dentry), list, size);
+	res = vfs_listxattr(realpath.dentry, list, size);
 	if (res <= 0 || size == 0)
 		return res;
 
-	if (ovl_path_type(dentry->d_parent) != OVL_PATH_MERGE)
+	if (!ovl_need_xattr_filter(dentry, type))
 		return res;
 
 	/* filter out private xattrs */

@@ -279,18 +292,17 @@ int ovl_removexattr(struct dentry *dentry, const char *name)
 {
 	int err;
 	struct path realpath;
-	enum ovl_path_type type;
+	enum ovl_path_type type = ovl_path_real(dentry, &realpath);
 
 	err = ovl_want_write(dentry);
 	if (err)
 		goto out;
 
-	if (ovl_path_type(dentry->d_parent) == OVL_PATH_MERGE &&
-	    ovl_is_private_xattr(name))
+	err = -ENODATA;
+	if (ovl_need_xattr_filter(dentry, type) && ovl_is_private_xattr(name))
 		goto out_drop_write;
 
-	type = ovl_path_real(dentry, &realpath);
-	if (type == OVL_PATH_LOWER) {
+	if (!OVL_TYPE_UPPER(type)) {
 		err = vfs_getxattr(realpath.dentry, name, NULL, 0);
 		if (err < 0)
 			goto out_drop_write;

@@ -312,7 +324,7 @@ out:
 static bool ovl_open_need_copy_up(int flags, enum ovl_path_type type,
 				  struct dentry *realdentry)
 {
-	if (type != OVL_PATH_LOWER)
+	if (OVL_TYPE_UPPER(type))
 		return false;
 
 	if (special_file(realdentry->d_inode->i_mode))

fs/overlayfs/overlayfs.h +10 -4

index 814bed33dd07..5b59e7d0a03a 100644
@@ -12,12 +12,17 @@
 struct ovl_entry;
 
 enum ovl_path_type {
-	OVL_PATH_PURE_UPPER,
-	OVL_PATH_UPPER,
-	OVL_PATH_MERGE,
-	OVL_PATH_LOWER,
+	__OVL_PATH_PURE		= (1 << 0),
+	__OVL_PATH_UPPER	= (1 << 1),
+	__OVL_PATH_MERGE	= (1 << 2),
 };
 
+#define OVL_TYPE_UPPER(type)	((type) & __OVL_PATH_UPPER)
+#define OVL_TYPE_MERGE(type)	((type) & __OVL_PATH_MERGE)
+#define OVL_TYPE_PURE_UPPER(type) ((type) & __OVL_PATH_PURE)
+#define OVL_TYPE_MERGE_OR_LOWER(type) \
+	(OVL_TYPE_MERGE(type) || !OVL_TYPE_UPPER(type))
+
 extern const char *ovl_opaque_xattr;
 
 static inline int ovl_do_rmdir(struct inode *dir, struct dentry *dentry)

@@ -130,6 +135,7 @@ void ovl_dentry_version_inc(struct dentry *dentry);
 void ovl_path_upper(struct dentry *dentry, struct path *path);
 void ovl_path_lower(struct dentry *dentry, struct path *path);
 enum ovl_path_type ovl_path_real(struct dentry *dentry, struct path *path);
+int ovl_path_next(int *idx, struct dentry *dentry, struct path *path);
 struct dentry *ovl_dentry_upper(struct dentry *dentry);
 struct dentry *ovl_dentry_lower(struct dentry *dentry);
 struct dentry *ovl_dentry_real(struct dentry *dentry);

fs/overlayfs/readdir.c +53 ~~-82~~

203 lines not on this page · raw message

fs/overlayfs/super.c +340 ~~-185~~

756 lines not on this page · raw message

my name is Patrick, I'm new on the list. A few days ago I asked Miklos
for the inclusion of multiple read-only branches in overlayfs and I promised to test, when code is available.
So, well, he's quick with the delivery:-) ... I applied the patch to kernel 3.18-rc3, everything works fine, I was able to mount a few directories (72 to be precise) together but failed to mount 311 directories due to length limitations of the mount command. I tried to avoid that by doing a kind of chaining recursion like so:

mount -t overlayfs -olowerdirs=/dir1 none /chain/1 mount -t overlayfs -olowerdirs=/dir2,/chain/1 none /chain/2
...

This doesn't work, i got the following kernel trace:

[  342.374984] CPU: 3 PID: 3090 Comm: mount Tainted: G      D 3.18.0-rc3 #1
[  342.385589] Hardware name: Packard Bell Easynote TE69BM/Easynote TE69BM, BIOS V2.05 12/25/2013
[  342.396324] task: ffff88016e312800 ti: ffff88003685c000 task.ti: ffff88003685c000
[  342.407201] RIP: 0010:[<ffffffff811f6ca5>]  [<ffffffff811f6ca5>] statfs_by_dentry+0x5/0xa0
[  342.407207] RSP: 0018:ffff88003685fb50  EFLAGS: 00010246
[  342.407212] RAX: 0000000000000000 RBX: ffff88003685fbf8 RCX: 0000000000000000
[  342.407216] RDX: ffff8800679e2f40 RSI: ffff88003685fbf8 RDI: 0000000000000000
[  342.407221] RBP: ffff88003685fb68 R08: 0000000000000000 R09: ffff88016e312800
[  342.407226] R10: 8080808080808080 R11: fefefefefefefeff R12: ffff88003685fb80
[  342.407230] R13: ffff88017812c260 R14: ffff88016f40f970 R15: ffff880064885d9e
[  342.407238] FS:  00007f3793d28780(0000) GS:ffff88017fd80000(0000) knlGS:0000000000000000
[  342.407243] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  342.407247] CR2: 0000000000000068 CR3: 00000000648a9000 CR4: 00000000001007e0
[  342.407250] Stack:
[  342.407263]  ffffffff811f6d5b ffff88003685fbf8 ffff88006a9dab40 ffff88003685fba8
[  342.407272]  ffffffffa0cb51c7 ffff88003685fba8 0000000000000000 0000000000000000
[  342.407282]  000000009e277ae4 ffff88003685fbf8 ffff8800666f70c0 ffff88003685fbc8
[  342.407285] Call Trace:
[  342.407299]  [<ffffffff811f6d5b>] ? vfs_statfs+0x1b/0xb0
[  342.407318]  [<ffffffffa0cb51c7>] ovl_statfs+0x57/0xa0 [overlayfs]
[  342.407329]  [<ffffffff811f6d12>] statfs_by_dentry+0x72/0xa0
[  342.407338]  [<ffffffff811f6d5b>] vfs_statfs+0x1b/0xb0
[  342.407352]  [<ffffffffa0cb54d3>] ovl_lower_dir.part.2+0x43/0xb0 [overlayfs]
[  342.407367]  [<ffffffffa0cb5576>] ovl_lower_dir+0x36/0x40 [overlayfs]
[  342.407381]  [<ffffffffa0cb5e10>] ovl_fill_super+0x890/0x960 [overlayfs]
[  342.407395]  [<ffffffff811c75c0>] ? sget+0x3b0/0x3f0
[  342.407405]  [<ffffffff811c68d0>] ? get_anon_bdev+0x120/0x120
[  342.407418]  [<ffffffffa0cb5580>] ? ovl_lower_dir+0x40/0x40 [overlayfs]
[  342.407429]  [<ffffffff811c770e>] mount_nodev+0x4e/0xa0
[  342.407443]  [<ffffffffa0cb5058>] ovl_mount+0x18/0x20 [overlayfs]
[  342.407454]  [<ffffffff811c8448>] mount_fs+0x38/0x1c0
[  342.407467]  [<ffffffff811e3c2b>] vfs_kern_mount+0x6b/0x120
[  342.407478]  [<ffffffff811e6bb4>] do_mount+0x224/0xb20
[  342.407488]  [<ffffffff811e77be>] SyS_mount+0x7e/0xe0
[  342.407503]  [<ffffffff8150c169>] system_call_fastpath+0x12/0x17
[  342.407607] Code: 01 19 c0 f7 d0 83 e0 f2 48 8b 4d f8 65 48 33 0c 25 28 00 00 00 75 02 c9 c3 e8 08 2d e7 ff 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 <48> 8b 47 68 48 8b 40 30 48 83 78 50 00 74 7c 55 48 89 f1 31 c0
[  342.407617] RIP  [<ffffffff811f6ca5>] statfs_by_dentry+0x5/0xa0
[  342.407620]  RSP <ffff88003685fb50>
[  342.407623] CR2: 0000000000000068
[  342.407720] ---[ end trace 8d18589ddc4cd155 ]---

Hope, this helps in any way.

As I already said to Miklos, thank you all for your hard work for the linux community, I'm just one of them.

bye,
P.

Jordi Pujol Palomer

2014-11-08 18:27 UTC #2 lore raw

In reply to Miklos Szeredi

EL Fri, 7 Nov 2014 18:02:42 +0100
Miklos Szeredi <[email protected]> escrigué:

Quoted 3 lines

This is the first iteration of the patch, and quite possibly buggy. Testing and review is welcome.

Hello,

I did light tests on this patch applied to kernel 3.18-rc3, it works.

Observations:

- option lowerdirs is an extended case of lowerdir, then we can use only a name for that option and discard the other, for compatibility with older versions may be lowerdir.

- overlayfs changes fast the mount options that we should specify, an overlayfs version number will help the implementer to control it.

- limit filesystem depth by a kernel config parameter, allowing nomax.

- print more detailed error message

Following patches are included to better explain some points, you can use the idea and make your best,

Regards,

Jordi Pujol

modinfo --field=version overlayfs
busybox modinfo -F version /lib/modules/$(uname -r)/kernel/fs/overlayfs.ko | awk '{print $NF}'

fs/overlayfs/super.c +1 -0

@@ -23,6 +23,7 @@
 MODULE_AUTHOR("Miklos Szeredi <[email protected]>");
 MODULE_DESCRIPTION("Overlay filesystem");
 MODULE_LICENSE("GPL");
+MODULE_VERSION(OVERLAYFS_VERSION);
 
 #define OVERLAYFS_SUPER_MAGIC 0x794c764f

fs/overlayfs/overlayfs.h +2 -0

@@ -9,6 +9,8 @@
 
 #include <linux/kernel.h>
 
+#define OVERLAYFS_VERSION   "25"
+
 struct ovl_entry;
 
 enum ovl_path_type {

fs/overlayfs/super.c +4 -1

@@ -728,7 +728,10 @@ static int ovl_fill_super(struct super_b
 	}
 	if (!ufs->config.upperdir &&
 	    !ufs->config.lowerdir && !ufs->config.lowerdirs) {
-		pr_err("overlayfs: no 'upperdir', 'lowerdir' or 'lowerdirs' specified\n");
+		pr_err("overlayfs: missing%s%s%s\n",
+			ufs->config.upperdir ? "" : " upperdir",
+			ufs->config.lowerdir ? "" : " lowerdir",
+			ufs->config.lowerdirs ? "" : " lowerdirs");
 		goto out_free_config;
 	}

include/linux/fs.h +0 -6

@@ -246,12 +246,6 @@ struct iattr {
  */
 #include <linux/quota.h>
 
-/*
- * Maximum number of layers of fs stack.  Needs to be limited to
- * prevent kernel stack overflow
- */
-#define FILESYSTEM_MAX_STACK_DEPTH 2
-
 /** 
  * enum positive_aop_returns - aop return codes with specific semantics
  *

init/Kconfig +6 -0

@@ -44,6 +44,12 @@ config INIT_ENV_ARG_LIMIT
 	  Maximum of each of the number of arguments and environment
 	  variables passed to init from the kernel command line.
 
+config FILESYSTEM_MAX_STACK_DEPTH
+	int "Maximum number of layers of fs stack"
+	default 2
+	help
+	  Maximum number of layers of fs stack.  Needs to be limited to
+	  prevent kernel stack overflow. Set to 0 for no limit.
 
 config CROSS_COMPILE
 	string "Cross-compiler tool prefix"

fs/ecryptfs/main.c +3 -2

@@ -568,8 +568,9 @@ static struct dentry *ecryptfs_mount(str
 	s->s_magic = ECRYPTFS_SUPER_MAGIC;
 	s->s_stack_depth = path.dentry->d_sb->s_stack_depth + 1;
 
-	rc = -EINVAL;
-	if (s->s_stack_depth > FILESYSTEM_MAX_STACK_DEPTH) {
+	if (CONFIG_FILESYSTEM_MAX_STACK_DEPTH > 0 &&
+	s->s_stack_depth > CONFIG_FILESYSTEM_MAX_STACK_DEPTH) {
+		rc = -EINVAL;
 		pr_err("eCryptfs: maximum fs stacking depth exceeded\n");
 		goto out_free;
 	}

fs/overlayfs/super.c +2 -1

@@ -818,7 +818,8 @@ static int ovl_fill_super(struct super_b
 	}
 
 	err = -EINVAL;
-	if (sb->s_stack_depth > FILESYSTEM_MAX_STACK_DEPTH) {
+	if (CONFIG_FILESYSTEM_MAX_STACK_DEPTH > 0 &&
+	sb->s_stack_depth > CONFIG_FILESYSTEM_MAX_STACK_DEPTH) {
 		pr_err("overlayfs: maximum fs stacking depth exceeded\n");
 		goto out_put_lowerpath;
 	}

Miklos Szeredi

author maintainer

2014-11-10 09:09 UTC #3 lore raw

Thanks to everyone for testing.

Here's an updated patch to fix the statfs Oops.

Maybe it wasn't clear, but the number of lower layers isn't limited by FILESYSTEM_MAX_STACK_DEPTH, only by the max size of the mount option buffer in the kernel (1 page, usually 4096bytes). So you could have a hundred read-only layers stacked in a single overlayfs mount.

As for changing the stacking while the overlayfs is mounted: currently this is not supported, the layers specified at the mount time remain there until the overlay is unmounted. Currently there's no possibility to add or remove layers in a dynamic way, and it is definitely more tricky to implement than the static configuration.

Thanks,
Miklos
----

fs/overlayfs/copy_up.c +2 -2

index ea10a8719107..a5bfd60f4f6f 100644
@@ -385,7 +385,7 @@ int ovl_copy_up(struct dentry *dentry)
 		struct kstat stat;
 		enum ovl_path_type type = ovl_path_type(dentry);
 
-		if (type != OVL_PATH_LOWER)
+		if (OVL_TYPE_UPPER(type))
 			break;
 
 		next = dget(dentry);

@@ -394,7 +394,7 @@ int ovl_copy_up(struct dentry *dentry)
 			parent = dget_parent(next);
 
 			type = ovl_path_type(parent);
-			if (type != OVL_PATH_LOWER)
+			if (OVL_TYPE_UPPER(type))
 				break;
 
 			dput(next);

fs/overlayfs/dir.c +30 ~~-23~~

146 lines not on this page · raw message

fs/overlayfs/inode.c +23 ~~-11~~

index af2d18c9fcee..48492f1240ad 100644
@@ -235,26 +235,39 @@ out:
 	return err;
 }
 
+static bool ovl_need_xattr_filter(struct dentry *dentry,
+				  enum ovl_path_type type)
+{
+	if ((type & (__OVL_PATH_PURE | __OVL_PATH_UPPER)) == __OVL_PATH_UPPER)
+		return S_ISDIR(dentry->d_inode->i_mode);
+	else
+		return false;
+}
+
 ssize_t ovl_getxattr(struct dentry *dentry, const char *name,
 		     void *value, size_t size)
 {
-	if (ovl_path_type(dentry->d_parent) == OVL_PATH_MERGE &&
-	    ovl_is_private_xattr(name))
+	struct path realpath;
+	enum ovl_path_type type = ovl_path_real(dentry, &realpath);
+
+	if (ovl_need_xattr_filter(dentry, type) && ovl_is_private_xattr(name))
 		return -ENODATA;
 
-	return vfs_getxattr(ovl_dentry_real(dentry), name, value, size);
+	return vfs_getxattr(realpath.dentry, name, value, size);
 }
 
 ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size)
 {
+	struct path realpath;
+	enum ovl_path_type type = ovl_path_real(dentry, &realpath);
 	ssize_t res;
 	int off;
 
-	res = vfs_listxattr(ovl_dentry_real(dentry), list, size);
+	res = vfs_listxattr(realpath.dentry, list, size);
 	if (res <= 0 || size == 0)
 		return res;
 
-	if (ovl_path_type(dentry->d_parent) != OVL_PATH_MERGE)
+	if (!ovl_need_xattr_filter(dentry, type))
 		return res;
 
 	/* filter out private xattrs */

@@ -279,18 +292,17 @@ int ovl_removexattr(struct dentry *dentry, const char *name)
 {
 	int err;
 	struct path realpath;
-	enum ovl_path_type type;
+	enum ovl_path_type type = ovl_path_real(dentry, &realpath);
 
 	err = ovl_want_write(dentry);
 	if (err)
 		goto out;
 
-	if (ovl_path_type(dentry->d_parent) == OVL_PATH_MERGE &&
-	    ovl_is_private_xattr(name))
+	err = -ENODATA;
+	if (ovl_need_xattr_filter(dentry, type) && ovl_is_private_xattr(name))
 		goto out_drop_write;
 
-	type = ovl_path_real(dentry, &realpath);
-	if (type == OVL_PATH_LOWER) {
+	if (!OVL_TYPE_UPPER(type)) {
 		err = vfs_getxattr(realpath.dentry, name, NULL, 0);
 		if (err < 0)
 			goto out_drop_write;

@@ -312,7 +324,7 @@ out:
 static bool ovl_open_need_copy_up(int flags, enum ovl_path_type type,
 				  struct dentry *realdentry)
 {
-	if (type != OVL_PATH_LOWER)
+	if (OVL_TYPE_UPPER(type))
 		return false;
 
 	if (special_file(realdentry->d_inode->i_mode))

fs/overlayfs/overlayfs.h +10 -4

index 814bed33dd07..5b59e7d0a03a 100644
@@ -12,12 +12,17 @@
 struct ovl_entry;
 
 enum ovl_path_type {
-	OVL_PATH_PURE_UPPER,
-	OVL_PATH_UPPER,
-	OVL_PATH_MERGE,
-	OVL_PATH_LOWER,
+	__OVL_PATH_PURE		= (1 << 0),
+	__OVL_PATH_UPPER	= (1 << 1),
+	__OVL_PATH_MERGE	= (1 << 2),
 };
 
+#define OVL_TYPE_UPPER(type)	((type) & __OVL_PATH_UPPER)
+#define OVL_TYPE_MERGE(type)	((type) & __OVL_PATH_MERGE)
+#define OVL_TYPE_PURE_UPPER(type) ((type) & __OVL_PATH_PURE)
+#define OVL_TYPE_MERGE_OR_LOWER(type) \
+	(OVL_TYPE_MERGE(type) || !OVL_TYPE_UPPER(type))
+
 extern const char *ovl_opaque_xattr;
 
 static inline int ovl_do_rmdir(struct inode *dir, struct dentry *dentry)

@@ -130,6 +135,7 @@ void ovl_dentry_version_inc(struct dentry *dentry);
 void ovl_path_upper(struct dentry *dentry, struct path *path);
 void ovl_path_lower(struct dentry *dentry, struct path *path);
 enum ovl_path_type ovl_path_real(struct dentry *dentry, struct path *path);
+int ovl_path_next(int *idx, struct dentry *dentry, struct path *path);
 struct dentry *ovl_dentry_upper(struct dentry *dentry);
 struct dentry *ovl_dentry_lower(struct dentry *dentry);
 struct dentry *ovl_dentry_real(struct dentry *dentry);

fs/overlayfs/readdir.c +53 ~~-82~~

203 lines not on this page · raw message

fs/overlayfs/super.c +341 ~~-186~~

764 lines not on this page · raw message

Erez Zadok

2014-11-10 17:08 UTC #4 lore raw

Indeed, Miklos, changing layers dynamically is VERY tricky. Unionfs supports that, but a good amount of its code all over is dedicated to handling dynamic layer changes. I advise you to avoid supporting this feature for a while, if ever: get a small, functional, working overlayfs first.

I personally never liked the idea of changing layers dynamically: it’s like asking a file system to remain consistent and detect all lower changes when someone goes and hand-edits disk blocks. Too messy. The pain of developing and supporting such a feature is much greater than the annoyance to users who have to unmount and remount back their layers to form a new structure.

Cheers,
Erez.

Quoted 10 lines · Miklos Szeredi

As for changing the stacking while the overlayfs is mounted: currently this is not supported, the layers specified at the mount time remain there until the overlay is unmounted. Currently there's no possibility to add or remove layers in a dynamic way, and it is definitely more tricky to implement than the static configuration.
Thanks,
Miklos

Jordi Pujol Palomer

2014-11-14 08:55 UTC #5 lore raw

In reply to Miklos Szeredi

EL Mon, 10 Nov 2014 10:09:54 +0100
Miklos Szeredi <[email protected]> escrigué:

Quoted 2 lines

Maybe it wasn't clear, but the number of lower layers isn't limited by FILESYSTEM_MAX_STACK_DEPTH,

sorry, you have been clear, it's me that have not explained the purpose of that patch. This idea is also valid for the main overlayfs development.
Consider that we have an encrypted partition and a live OS that uses overlayfs and works over it, when the OS is started already reaches the maximum stacking depth (2). Also, hardcoding parameters into the code may be annoying because a change implies creating and mantaining patches, otherwise when the parameter is configurable we can change it in a config value.

Also, I include a patch to explain better my first statement, would say lowerdirs has extended the functionality of lowerdir, therefore we can discard the lowerdir case that only works with one lower layer and rename lowerdirs to lowerdir. It's more simple and all the functionality is preserved.
Regards,

Jordi Pujol

fs/overlayfs/super.c +4 ~~-29~~

@@ -29,7 +29,6 @@ MODULE_VERSION(OVERLAYFS_VERSION);
 
 struct ovl_config {
 	char *lowerdir;
-	char *lowerdirs;
 	char *upperdir;
 	char *workdir;
 };

@@ -497,8 +496,6 @@ static int ovl_show_options(struct seq_f
 
 	if (ufs->config.lowerdir)
 		seq_printf(m, ",lowerdir=%s", ufs->config.lowerdir);
-	if (ufs->config.lowerdirs)
-		seq_printf(m, ",lowerdirs=%s", ufs->config.lowerdirs);
 	if (ufs->config.upperdir) {
 		seq_printf(m, ",upperdir=%s", ufs->config.upperdir);
 		seq_printf(m, ",workdir=%s", ufs->config.workdir);

@@ -522,7 +519,6 @@ enum {
 
 static const match_table_t ovl_tokens = {
 	{OPT_LOWERDIR,			"lowerdir=%s"},
-	{OPT_LOWERDIRS,			"lowerdirs=%s"},
 	{OPT_UPPERDIR,			"upperdir=%s"},
 	{OPT_WORKDIR,			"workdir=%s"},
 	{OPT_ERR,			NULL}

@@ -555,13 +551,6 @@ static int ovl_parse_opt(char *opt, stru
 				return -ENOMEM;
 			break;
 
-		case OPT_LOWERDIRS:
-			kfree(config->lowerdirs);
-			config->lowerdirs = match_strdup(&args[0]);
-			if (!config->lowerdirs)
-				return -ENOMEM;
-			break;
-
 		case OPT_WORKDIR:
 			kfree(config->workdir);
 			config->workdir = match_strdup(&args[0]);

@@ -718,24 +707,18 @@ static int ovl_fill_super(struct super_b
 		goto out_free_config;
 
 	err = -EINVAL;
-	if (ufs->config.lowerdir && ufs->config.lowerdirs) {
-		pr_err("overlayfs: both 'lowerdir' and 'lowerdirs' not allowed\n");
-		goto out_free_config;
-	}
 	if (ufs->config.upperdir && !ufs->config.workdir) {
 		pr_err("overlayfs: missing 'workdir'\n");
 		goto out_free_config;
 	}
 	if (!ufs->config.upperdir &&
-	    !ufs->config.lowerdir && !ufs->config.lowerdirs) {
-		pr_err("overlayfs: no 'upperdir', 'lowerdir' or 'lowerdirs' specified\n");
+	    !ufs->config.lowerdir) {
+		pr_err("overlayfs: no 'upperdir' or 'lowerdir' specified\n");
 		goto out_free_config;
 	}
 
 	if (ufs->config.lowerdir) {
-		stacklen = 1;
-	} else if (ufs->config.lowerdirs) {
-		char *p = ufs->config.lowerdirs;
+		char *p = ufs->config.lowerdir;
 
 		for (;;) {
 			stacklen++;

@@ -780,15 +763,7 @@ static int ovl_fill_super(struct super_b
 	}
 
 	if (ufs->config.lowerdir) {
-		BUG_ON(numlower >= stacklen);
-
-		err = ovl_lower_dir(ufs->config.lowerdir, &stack[numlower], &ufs->lower_namelen, &sb->s_stack_depth);
-		if (err)
-			goto out_put_workpath;
-
-		numlower++;
-	} else if (ufs->config.lowerdirs) {
-		char *p = ufs->config.lowerdirs;
+		char *p = ufs->config.lowerdir;
 
 		for (;;) {
 			char *next = strchr(p, ':');

Miklos Szeredi

author maintainer

2014-11-17 10:32 UTC #6 lore raw

Quoted 5 lines · Jordi Pujol Palomer

EL Mon, 10 Nov 2014 10:09:54 +0100
Miklos Szeredi <[email protected]> escrigué:
Quoted 2 lines
Maybe it wasn't clear, but the number of lower layers isn't limited by FILESYSTEM_MAX_STACK_DEPTH,

Quoted 9 lines

sorry, you have been clear, it's me that have not explained the purpose of that patch. This idea is also valid for the main overlayfs development.
Consider that we have an encrypted partition and a live OS that uses overlayfs and works over it, when the OS is started already reaches the maximum stacking depth (2). Also, hardcoding parameters into the code may be annoying because a change implies creating and mantaining patches, otherwise when the parameter is configurable we can change it in a config value.

Before allowing a change to FILESYSTEM_MAX_STACK_DEPTH we'd need to analyze the kernel stack usage of anything taking part in the filesystem stack. Not something that can be expected of users before they set a kernel config variable.

'Course we can change overlayfs and/or ecryptfs to use a new context for operations done on underlying layers. Not sure how much work that would be...

Quoted 5 lines

Also, I include a patch to explain better my first statement, would say lowerdirs has extended the functionality of lowerdir, therefore we can discard the lowerdir case that only works with one lower layer and rename lowerdirs to lowerdir. It's more simple and all the functionality is preserved.

Yeah. But we also need some escaping in 'lowerdir=' argument to allow filenames with comma and colon. Current patch doesn't yet have that, but will work on it.

Thanks,
Miklos
--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html