linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/2] ovl introduce "uuid=off"
@ 2020-09-25  8:35 Pavel Tikhomirov
  2020-09-25  8:35 ` [PATCH v4 1/2] ovl: propagate ovl_fs to ovl_decode_real_fh and ovl_encode_real_fh Pavel Tikhomirov
  2020-09-25  8:35 ` [PATCH v4 2/2] ovl: introduce new "uuid=off" option for inodes index feature Pavel Tikhomirov
  0 siblings, 2 replies; 8+ messages in thread
From: Pavel Tikhomirov @ 2020-09-25  8:35 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: Pavel Tikhomirov, Amir Goldstein, Vivek Goyal, linux-unionfs,
	linux-kernel

This is a v4 of:
ovl: introduce new "index=nouuid" option for inodes index feature

Changes in v3: rebase to overlayfs-next, replace uuid with null in file
handles, propagate ovl_fs to needed functions in a separate patch, add
separate bool "uuid=on/off" option, fix numfs check fallback, add a note
to docs.

Changes in v4: get rid of double negatives, remove nouuid leftower
comment, fix missprint in kernel config name.

CC: Amir Goldstein <amir73il@gmail.com>
CC: Vivek Goyal <vgoyal@redhat.com>
CC: Miklos Szeredi <miklos@szeredi.hu>
CC: linux-unionfs@vger.kernel.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>

Pavel Tikhomirov (2):
  ovl: propagate ovl_fs to ovl_decode_real_fh and ovl_encode_real_fh
  ovl: introduce new "uuid=off" option for inodes index feature

 Documentation/filesystems/overlayfs.rst |  6 ++++++
 fs/overlayfs/Kconfig                    | 19 +++++++++++++++++++
 fs/overlayfs/copy_up.c                  | 25 ++++++++++++++-----------
 fs/overlayfs/export.c                   | 10 ++++++----
 fs/overlayfs/namei.c                    | 23 +++++++++++++----------
 fs/overlayfs/overlayfs.h                | 14 ++++++++------
 fs/overlayfs/ovl_entry.h                |  1 +
 fs/overlayfs/super.c                    | 25 +++++++++++++++++++++++++
 fs/overlayfs/util.c                     |  3 ++-
 9 files changed, 94 insertions(+), 32 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4 1/2] ovl: propagate ovl_fs to ovl_decode_real_fh and ovl_encode_real_fh
  2020-09-25  8:35 [PATCH v4 0/2] ovl introduce "uuid=off" Pavel Tikhomirov
@ 2020-09-25  8:35 ` Pavel Tikhomirov
  2020-09-25 16:37   ` Amir Goldstein
  2020-09-25  8:35 ` [PATCH v4 2/2] ovl: introduce new "uuid=off" option for inodes index feature Pavel Tikhomirov
  1 sibling, 1 reply; 8+ messages in thread
From: Pavel Tikhomirov @ 2020-09-25  8:35 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: Pavel Tikhomirov, Amir Goldstein, Vivek Goyal, linux-unionfs,
	linux-kernel

This will be used in next patch to be able to change uuid checks and
add uuid nullification based on ofs->config.index for a new "uuid=off"
mode.

CC: Amir Goldstein <amir73il@gmail.com>
CC: Vivek Goyal <vgoyal@redhat.com>
CC: Miklos Szeredi <miklos@szeredi.hu>
CC: linux-unionfs@vger.kernel.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
 fs/overlayfs/copy_up.c   | 22 ++++++++++++----------
 fs/overlayfs/export.c    | 10 ++++++----
 fs/overlayfs/namei.c     | 19 ++++++++++---------
 fs/overlayfs/overlayfs.h | 14 ++++++++------
 fs/overlayfs/util.c      |  3 ++-
 5 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 955ecd4030f0..3380039036d6 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -275,7 +275,8 @@ int ovl_set_attr(struct dentry *upperdentry, struct kstat *stat)
 	return err;
 }
 
-struct ovl_fh *ovl_encode_real_fh(struct dentry *real, bool is_upper)
+struct ovl_fh *ovl_encode_real_fh(struct ovl_fs *ofs, struct dentry *real,
+				  bool is_upper)
 {
 	struct ovl_fh *fh;
 	int fh_type, dwords;
@@ -328,8 +329,8 @@ struct ovl_fh *ovl_encode_real_fh(struct dentry *real, bool is_upper)
 	return ERR_PTR(err);
 }
 
-int ovl_set_origin(struct dentry *dentry, struct dentry *lower,
-		   struct dentry *upper)
+int ovl_set_origin(struct ovl_fs *ofs, struct dentry *dentry,
+		   struct dentry *lower, struct dentry *upper)
 {
 	const struct ovl_fh *fh = NULL;
 	int err;
@@ -340,7 +341,7 @@ int ovl_set_origin(struct dentry *dentry, struct dentry *lower,
 	 * up and a pure upper inode.
 	 */
 	if (ovl_can_decode_fh(lower->d_sb)) {
-		fh = ovl_encode_real_fh(lower, false);
+		fh = ovl_encode_real_fh(ofs, lower, false);
 		if (IS_ERR(fh))
 			return PTR_ERR(fh);
 	}
@@ -362,7 +363,7 @@ static int ovl_set_upper_fh(struct ovl_fs *ofs, struct dentry *upper,
 	const struct ovl_fh *fh;
 	int err;
 
-	fh = ovl_encode_real_fh(upper, true);
+	fh = ovl_encode_real_fh(ofs, upper, true);
 	if (IS_ERR(fh))
 		return PTR_ERR(fh);
 
@@ -380,6 +381,7 @@ static int ovl_set_upper_fh(struct ovl_fs *ofs, struct dentry *upper,
 static int ovl_create_index(struct dentry *dentry, struct dentry *origin,
 			    struct dentry *upper)
 {
+	struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
 	struct dentry *indexdir = ovl_indexdir(dentry->d_sb);
 	struct inode *dir = d_inode(indexdir);
 	struct dentry *index = NULL;
@@ -402,7 +404,7 @@ static int ovl_create_index(struct dentry *dentry, struct dentry *origin,
 	if (WARN_ON(ovl_test_flag(OVL_INDEX, d_inode(dentry))))
 		return -EIO;
 
-	err = ovl_get_index_name(origin, &name);
+	err = ovl_get_index_name(ofs, origin, &name);
 	if (err)
 		return err;
 
@@ -411,7 +413,7 @@ static int ovl_create_index(struct dentry *dentry, struct dentry *origin,
 	if (IS_ERR(temp))
 		goto free_name;
 
-	err = ovl_set_upper_fh(OVL_FS(dentry->d_sb), upper, temp);
+	err = ovl_set_upper_fh(ofs, upper, temp);
 	if (err)
 		goto out;
 
@@ -521,7 +523,7 @@ static int ovl_copy_up_inode(struct ovl_copy_up_ctx *c, struct dentry *temp)
 	 * hard link.
 	 */
 	if (c->origin) {
-		err = ovl_set_origin(c->dentry, c->lowerpath.dentry, temp);
+		err = ovl_set_origin(ofs, c->dentry, c->lowerpath.dentry, temp);
 		if (err)
 			return err;
 	}
@@ -700,7 +702,7 @@ static int ovl_copy_up_tmpfile(struct ovl_copy_up_ctx *c)
 static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
 {
 	int err;
-	struct ovl_fs *ofs = c->dentry->d_sb->s_fs_info;
+	struct ovl_fs *ofs = OVL_FS(c->dentry->d_sb);
 	bool to_index = false;
 
 	/*
@@ -722,7 +724,7 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
 
 	if (to_index) {
 		c->destdir = ovl_indexdir(c->dentry->d_sb);
-		err = ovl_get_index_name(c->lowerpath.dentry, &c->destname);
+		err = ovl_get_index_name(ofs, c->lowerpath.dentry, &c->destname);
 		if (err)
 			return err;
 	} else if (WARN_ON(!c->parent)) {
diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c
index ed35be3fafc6..41ebf52f1bbc 100644
--- a/fs/overlayfs/export.c
+++ b/fs/overlayfs/export.c
@@ -211,7 +211,8 @@ static int ovl_check_encode_origin(struct dentry *dentry)
 	return 1;
 }
 
-static int ovl_dentry_to_fid(struct dentry *dentry, u32 *fid, int buflen)
+static int ovl_dentry_to_fid(struct ovl_fs *ofs, struct dentry *dentry,
+			     u32 *fid, int buflen)
 {
 	struct ovl_fh *fh = NULL;
 	int err, enc_lower;
@@ -226,7 +227,7 @@ static int ovl_dentry_to_fid(struct dentry *dentry, u32 *fid, int buflen)
 		goto fail;
 
 	/* Encode an upper or lower file handle */
-	fh = ovl_encode_real_fh(enc_lower ? ovl_dentry_lower(dentry) :
+	fh = ovl_encode_real_fh(ofs, enc_lower ? ovl_dentry_lower(dentry) :
 				ovl_dentry_upper(dentry), !enc_lower);
 	if (IS_ERR(fh))
 		return PTR_ERR(fh);
@@ -249,6 +250,7 @@ static int ovl_dentry_to_fid(struct dentry *dentry, u32 *fid, int buflen)
 static int ovl_encode_fh(struct inode *inode, u32 *fid, int *max_len,
 			 struct inode *parent)
 {
+	struct ovl_fs *ofs = OVL_FS(inode->i_sb);
 	struct dentry *dentry;
 	int bytes, buflen = *max_len << 2;
 
@@ -260,7 +262,7 @@ static int ovl_encode_fh(struct inode *inode, u32 *fid, int *max_len,
 	if (WARN_ON(!dentry))
 		return FILEID_INVALID;
 
-	bytes = ovl_dentry_to_fid(dentry, fid, buflen);
+	bytes = ovl_dentry_to_fid(ofs, dentry, fid, buflen);
 	dput(dentry);
 	if (bytes <= 0)
 		return FILEID_INVALID;
@@ -680,7 +682,7 @@ static struct dentry *ovl_upper_fh_to_d(struct super_block *sb,
 	if (!ovl_upper_mnt(ofs))
 		return ERR_PTR(-EACCES);
 
-	upper = ovl_decode_real_fh(fh, ovl_upper_mnt(ofs), true);
+	upper = ovl_decode_real_fh(ofs, fh, ovl_upper_mnt(ofs), true);
 	if (IS_ERR_OR_NULL(upper))
 		return upper;
 
diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index a6162c4076db..f058bf8e8b87 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -150,8 +150,8 @@ static struct ovl_fh *ovl_get_fh(struct ovl_fs *ofs, struct dentry *dentry,
 	goto out;
 }
 
-struct dentry *ovl_decode_real_fh(struct ovl_fh *fh, struct vfsmount *mnt,
-				  bool connected)
+struct dentry *ovl_decode_real_fh(struct ovl_fs *ofs, struct ovl_fh *fh,
+				  struct vfsmount *mnt, bool connected)
 {
 	struct dentry *real;
 	int bytes;
@@ -354,7 +354,7 @@ int ovl_check_origin_fh(struct ovl_fs *ofs, struct ovl_fh *fh, bool connected,
 		    ofs->layers[i].fs->bad_uuid)
 			continue;
 
-		origin = ovl_decode_real_fh(fh, ofs->layers[i].mnt,
+		origin = ovl_decode_real_fh(ofs, fh, ofs->layers[i].mnt,
 					    connected);
 		if (origin)
 			break;
@@ -450,7 +450,7 @@ int ovl_verify_set_fh(struct ovl_fs *ofs, struct dentry *dentry,
 	struct ovl_fh *fh;
 	int err;
 
-	fh = ovl_encode_real_fh(real, is_upper);
+	fh = ovl_encode_real_fh(ofs, real, is_upper);
 	err = PTR_ERR(fh);
 	if (IS_ERR(fh)) {
 		fh = NULL;
@@ -488,7 +488,7 @@ struct dentry *ovl_index_upper(struct ovl_fs *ofs, struct dentry *index)
 	if (IS_ERR_OR_NULL(fh))
 		return ERR_CAST(fh);
 
-	upper = ovl_decode_real_fh(fh, ovl_upper_mnt(ofs), true);
+	upper = ovl_decode_real_fh(ofs, fh, ovl_upper_mnt(ofs), true);
 	kfree(fh);
 
 	if (IS_ERR_OR_NULL(upper))
@@ -640,12 +640,13 @@ static int ovl_get_index_name_fh(struct ovl_fh *fh, struct qstr *name)
  * index dir was cleared. Either way, that index cannot be used to indentify
  * the overlay inode.
  */
-int ovl_get_index_name(struct dentry *origin, struct qstr *name)
+int ovl_get_index_name(struct ovl_fs *ofs, struct dentry *origin,
+		       struct qstr *name)
 {
 	struct ovl_fh *fh;
 	int err;
 
-	fh = ovl_encode_real_fh(origin, false);
+	fh = ovl_encode_real_fh(ofs, origin, false);
 	if (IS_ERR(fh))
 		return PTR_ERR(fh);
 
@@ -694,7 +695,7 @@ struct dentry *ovl_lookup_index(struct ovl_fs *ofs, struct dentry *upper,
 	bool is_dir = d_is_dir(origin);
 	int err;
 
-	err = ovl_get_index_name(origin, &name);
+	err = ovl_get_index_name(ofs, origin, &name);
 	if (err)
 		return ERR_PTR(err);
 
@@ -805,7 +806,7 @@ static int ovl_fix_origin(struct ovl_fs *ofs, struct dentry *dentry,
 	if (err)
 		return err;
 
-	err = ovl_set_origin(dentry, lower, upper);
+	err = ovl_set_origin(ofs, dentry, lower, upper);
 	if (!err)
 		err = ovl_set_impure(dentry->d_parent, upper->d_parent);
 
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 7bce2469fe55..b56b5f46f224 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -383,8 +383,8 @@ static inline int ovl_check_fh_len(struct ovl_fh *fh, int fh_len)
 	return ovl_check_fb_len(&fh->fb, fh_len - OVL_FH_WIRE_OFFSET);
 }
 
-struct dentry *ovl_decode_real_fh(struct ovl_fh *fh, struct vfsmount *mnt,
-				  bool connected);
+struct dentry *ovl_decode_real_fh(struct ovl_fs *ofs, struct ovl_fh *fh,
+				  struct vfsmount *mnt, bool connected);
 int ovl_check_origin_fh(struct ovl_fs *ofs, struct ovl_fh *fh, bool connected,
 			struct dentry *upperdentry, struct ovl_path **stackp);
 int ovl_verify_set_fh(struct ovl_fs *ofs, struct dentry *dentry,
@@ -392,7 +392,8 @@ int ovl_verify_set_fh(struct ovl_fs *ofs, struct dentry *dentry,
 		      bool set);
 struct dentry *ovl_index_upper(struct ovl_fs *ofs, struct dentry *index);
 int ovl_verify_index(struct ovl_fs *ofs, struct dentry *index);
-int ovl_get_index_name(struct dentry *origin, struct qstr *name);
+int ovl_get_index_name(struct ovl_fs *ofs, struct dentry *origin,
+		       struct qstr *name);
 struct dentry *ovl_get_index_fh(struct ovl_fs *ofs, struct ovl_fh *fh);
 struct dentry *ovl_lookup_index(struct ovl_fs *ofs, struct dentry *upper,
 				struct dentry *origin, bool verify);
@@ -511,9 +512,10 @@ int ovl_maybe_copy_up(struct dentry *dentry, int flags);
 int ovl_copy_xattr(struct super_block *sb, struct dentry *old,
 		   struct dentry *new);
 int ovl_set_attr(struct dentry *upper, struct kstat *stat);
-struct ovl_fh *ovl_encode_real_fh(struct dentry *real, bool is_upper);
-int ovl_set_origin(struct dentry *dentry, struct dentry *lower,
-		   struct dentry *upper);
+struct ovl_fh *ovl_encode_real_fh(struct ovl_fs *ofs, struct dentry *real,
+				  bool is_upper);
+int ovl_set_origin(struct ovl_fs *ofs, struct dentry *dentry,
+		   struct dentry *lower, struct dentry *upper);
 
 /* export.c */
 extern const struct export_operations ovl_export_operations;
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 23f475627d07..44b4b62a8ac8 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -716,6 +716,7 @@ bool ovl_need_index(struct dentry *dentry)
 /* Caller must hold OVL_I(inode)->lock */
 static void ovl_cleanup_index(struct dentry *dentry)
 {
+	struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
 	struct dentry *indexdir = ovl_indexdir(dentry->d_sb);
 	struct inode *dir = indexdir->d_inode;
 	struct dentry *lowerdentry = ovl_dentry_lower(dentry);
@@ -725,7 +726,7 @@ static void ovl_cleanup_index(struct dentry *dentry)
 	struct qstr name = { };
 	int err;
 
-	err = ovl_get_index_name(lowerdentry, &name);
+	err = ovl_get_index_name(ofs, lowerdentry, &name);
 	if (err)
 		goto fail;
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 2/2] ovl: introduce new "uuid=off" option for inodes index feature
  2020-09-25  8:35 [PATCH v4 0/2] ovl introduce "uuid=off" Pavel Tikhomirov
  2020-09-25  8:35 ` [PATCH v4 1/2] ovl: propagate ovl_fs to ovl_decode_real_fh and ovl_encode_real_fh Pavel Tikhomirov
@ 2020-09-25  8:35 ` Pavel Tikhomirov
  2020-09-25 16:42   ` Amir Goldstein
  2020-10-06 15:13   ` Miklos Szeredi
  1 sibling, 2 replies; 8+ messages in thread
From: Pavel Tikhomirov @ 2020-09-25  8:35 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: Pavel Tikhomirov, Amir Goldstein, Vivek Goyal, linux-unionfs,
	linux-kernel

This replaces uuid with null in overelayfs file handles and thus relaxes
uuid checks for overlay index feature. It is only possible in case there
is only one filesystem for all the work/upper/lower directories and bare
file handles from this backing filesystem are uniq. In other case when
we have multiple filesystems lets just fallback to "uuid=on" which is
and equivalent of how it worked before with all uuid checks.

This is needed when overlayfs is/was mounted in a container with index
enabled (e.g.: to be able to resolve inotify watch file handles on it to
paths in CRIU), and this container is copied and started alongside with
the original one. This way the "copy" container can't have the same uuid
on the superblock and mounting the overlayfs from it later would fail.

Note: In our (Virtuozzo) use case users inside a container can create
"regular" overlayfs mounts without any "index=" option, but we still
want to migrate this containers with CRIU so we set "index=on" as kernel
default so that all the container overlayfs mounts get support of file
handles automatically. With "uuid=off" we want the same thing (to be
able to "copy" container with uuid change) - we would set kernel default
so that all the container overlayfs mounts get "uuid=off" automatically.

That is an example of the problem on top of loop+ext4:

dd if=/dev/zero of=loopbackfile.img bs=100M count=10
losetup -fP loopbackfile.img
losetup -a
  #/dev/loop0: [64768]:35 (/loop-test/loopbackfile.img)
mkfs.ext4 loopbackfile.img
mkdir loop-mp
mount -o loop /dev/loop0 loop-mp
mkdir loop-mp/{lower,upper,work,merged}
mount -t overlay overlay -oindex=on,lowerdir=loop-mp/lower,\
upperdir=loop-mp/upper,workdir=loop-mp/work loop-mp/merged
umount loop-mp/merged
umount loop-mp
e2fsck -f /dev/loop0
tune2fs -U random /dev/loop0

mount -o loop /dev/loop0 loop-mp
mount -t overlay overlay -oindex=on,lowerdir=loop-mp/lower,\
upperdir=loop-mp/upper,workdir=loop-mp/work loop-mp/merged
  #mount: /loop-test/loop-mp/merged:
  #mount(2) system call failed: Stale file handle.

If you just change the uuid of the backing filesystem, overlay is not
mounting any more. In Virtuozzo we copy container disks (ploops) when
crate the copy of container and we require fs uuid to be uniq for a new
container.

CC: Amir Goldstein <amir73il@gmail.com>
CC: Vivek Goyal <vgoyal@redhat.com>
CC: Miklos Szeredi <miklos@szeredi.hu>
CC: linux-unionfs@vger.kernel.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>

---
v2: in v1 I missed actual uuid check skip
v3: rebase to overlayfs-next, replace uuid with null in file handles,
split ovl_fs propagation to function arguments to separate patch, add
separate bool "uuid=on/off" option, move numfs check up, add doc note.
v4: get rid of double negatives, remove nouuid leftower comment, fix
missprint in kernel config name

 Documentation/filesystems/overlayfs.rst |  6 ++++++
 fs/overlayfs/Kconfig                    | 19 +++++++++++++++++++
 fs/overlayfs/copy_up.c                  |  3 ++-
 fs/overlayfs/namei.c                    |  4 +++-
 fs/overlayfs/ovl_entry.h                |  1 +
 fs/overlayfs/super.c                    | 25 +++++++++++++++++++++++++
 6 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
index 580ab9a0fe31..4f9cc20f255c 100644
--- a/Documentation/filesystems/overlayfs.rst
+++ b/Documentation/filesystems/overlayfs.rst
@@ -563,6 +563,12 @@ This verification may cause significant overhead in some cases.
 Note: the mount options index=off,nfs_export=on are conflicting for a
 read-write mount and will result in an error.
 
+Note: the mount option uuid=off (or corresponding module param, or kernel
+config) can be used to replace UUID of the underlying filesystem in file
+handles with null, and effectively disable UUID checks. This can be useful in
+case the underlying disk is copied and the UUID of this copy is changed. This
+is only applicable if all lower/upper/work directories are on the same
+filesystem, otherwise it will fallback to normal behaviour.
 
 Volatile mount
 --------------
diff --git a/fs/overlayfs/Kconfig b/fs/overlayfs/Kconfig
index dd188c7996b3..c21abdb43206 100644
--- a/fs/overlayfs/Kconfig
+++ b/fs/overlayfs/Kconfig
@@ -61,6 +61,25 @@ config OVERLAY_FS_INDEX
 
 	  If unsure, say N.
 
+config OVERLAY_FS_INDEX_UUID
+	bool "Overlayfs: export uuid in file handles"
+	default y
+	depends on OVERLAY_FS
+	help
+	  If this config option is disabled then overlay will replace uuid with
+	  null in overlayfs file handles, effectively disabling uuid checks for
+	  them. This affects overlayfs mounted with "index=on". This only can be
+	  done if all upper and lower directories are on the same filesystem
+	  where basic fhandles are uniq. In case the latter is not true
+	  overlayfs would fallback to normal uuid checking mode.
+
+	  Disabling it is needed to overcome possible change of uuid on
+	  superblock of the backing filesystem, e.g. when you copied the
+	  virtual disk and mount both the copy of the disk and the original one
+	  at the same time.
+
+	  If unsure, say Y.
+
 config OVERLAY_FS_NFS_EXPORT
 	bool "Overlayfs: turn on NFS export feature by default"
 	depends on OVERLAY_FS
diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 3380039036d6..0b7e7a90a435 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -320,7 +320,8 @@ struct ovl_fh *ovl_encode_real_fh(struct ovl_fs *ofs, struct dentry *real,
 	if (is_upper)
 		fh->fb.flags |= OVL_FH_FLAG_PATH_UPPER;
 	fh->fb.len = sizeof(fh->fb) + buflen;
-	fh->fb.uuid = *uuid;
+	if (ofs->config.uuid)
+		fh->fb.uuid = *uuid;
 
 	return fh;
 
diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index f058bf8e8b87..f731eb4d35f9 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -159,8 +159,10 @@ struct dentry *ovl_decode_real_fh(struct ovl_fs *ofs, struct ovl_fh *fh,
 	/*
 	 * Make sure that the stored uuid matches the uuid of the lower
 	 * layer where file handle will be decoded.
+	 * In case of uuid=off option just make sure that stored uuid is null.
 	 */
-	if (!uuid_equal(&fh->fb.uuid, &mnt->mnt_sb->s_uuid))
+	if (ofs->config.uuid ? !uuid_equal(&fh->fb.uuid, &mnt->mnt_sb->s_uuid) :
+			      !uuid_is_null(&fh->fb.uuid))
 		return NULL;
 
 	bytes = (fh->fb.len - offsetof(struct ovl_fb, fid));
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index 1b5a2094df8e..b7a73ea147b8 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -14,6 +14,7 @@ struct ovl_config {
 	bool redirect_follow;
 	const char *redirect_mode;
 	bool index;
+	bool uuid;
 	bool nfs_export;
 	int xino;
 	bool metacopy;
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 290983bcfbb3..a37995138b0d 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -43,6 +43,11 @@ module_param_named(index, ovl_index_def, bool, 0644);
 MODULE_PARM_DESC(index,
 		 "Default to on or off for the inodes index feature");
 
+static bool ovl_uuid_def = IS_ENABLED(CONFIG_OVERLAY_FS_INDEX_UUID);
+module_param_named(uuid, ovl_uuid_def, bool, 0644);
+MODULE_PARM_DESC(uuid,
+		 "Export null uuid in file handles of inodes index feature");
+
 static bool ovl_nfs_export_def = IS_ENABLED(CONFIG_OVERLAY_FS_NFS_EXPORT);
 module_param_named(nfs_export, ovl_nfs_export_def, bool, 0644);
 MODULE_PARM_DESC(nfs_export,
@@ -356,6 +361,8 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
 		seq_printf(m, ",redirect_dir=%s", ofs->config.redirect_mode);
 	if (ofs->config.index != ovl_index_def)
 		seq_printf(m, ",index=%s", ofs->config.index ? "on" : "off");
+	if (ofs->config.uuid != ovl_uuid_def)
+		seq_printf(m, ",uuid=%s", ofs->config.uuid ? "on" : "off");
 	if (ofs->config.nfs_export != ovl_nfs_export_def)
 		seq_printf(m, ",nfs_export=%s", ofs->config.nfs_export ?
 						"on" : "off");
@@ -410,6 +417,8 @@ enum {
 	OPT_REDIRECT_DIR,
 	OPT_INDEX_ON,
 	OPT_INDEX_OFF,
+	OPT_UUID_ON,
+	OPT_UUID_OFF,
 	OPT_NFS_EXPORT_ON,
 	OPT_NFS_EXPORT_OFF,
 	OPT_XINO_ON,
@@ -429,6 +438,8 @@ static const match_table_t ovl_tokens = {
 	{OPT_REDIRECT_DIR,		"redirect_dir=%s"},
 	{OPT_INDEX_ON,			"index=on"},
 	{OPT_INDEX_OFF,			"index=off"},
+	{OPT_UUID_ON,			"uuid=on"},
+	{OPT_UUID_OFF,			"uuid=off"},
 	{OPT_NFS_EXPORT_ON,		"nfs_export=on"},
 	{OPT_NFS_EXPORT_OFF,		"nfs_export=off"},
 	{OPT_XINO_ON,			"xino=on"},
@@ -549,6 +560,14 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
 			index_opt = true;
 			break;
 
+		case OPT_UUID_ON:
+			config->uuid = true;
+			break;
+
+		case OPT_UUID_OFF:
+			config->uuid = false;
+			break;
+
 		case OPT_NFS_EXPORT_ON:
 			config->nfs_export = true;
 			nfs_export_opt = true;
@@ -1877,6 +1896,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 	ofs->share_whiteout = true;
 
 	ofs->config.index = ovl_index_def;
+	ofs->config.uuid = ovl_uuid_def;
 	ofs->config.nfs_export = ovl_nfs_export_def;
 	ofs->config.xino = ovl_xino_def();
 	ofs->config.metacopy = ovl_metacopy_def;
@@ -1956,6 +1976,11 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 	if (!ovl_upper_mnt(ofs))
 		sb->s_flags |= SB_RDONLY;
 
+	if (!ofs->config.uuid && ofs->numfs > 1) {
+		pr_warn("The uuid=off requires a single fs for lower and upper, falling back to uuid=on.\n");
+		ofs->config.uuid = true;
+	}
+
 	if (!ovl_force_readonly(ofs) && ofs->config.index) {
 		err = ovl_get_indexdir(sb, ofs, oe, &upperpath);
 		if (err)
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 1/2] ovl: propagate ovl_fs to ovl_decode_real_fh and ovl_encode_real_fh
  2020-09-25  8:35 ` [PATCH v4 1/2] ovl: propagate ovl_fs to ovl_decode_real_fh and ovl_encode_real_fh Pavel Tikhomirov
@ 2020-09-25 16:37   ` Amir Goldstein
  0 siblings, 0 replies; 8+ messages in thread
From: Amir Goldstein @ 2020-09-25 16:37 UTC (permalink / raw)
  To: Pavel Tikhomirov; +Cc: Miklos Szeredi, Vivek Goyal, overlayfs, linux-kernel

On Fri, Sep 25, 2020 at 11:35 AM Pavel Tikhomirov
<ptikhomirov@virtuozzo.com> wrote:
>
> This will be used in next patch to be able to change uuid checks and
> add uuid nullification based on ofs->config.index for a new "uuid=off"
> mode.
>
> CC: Amir Goldstein <amir73il@gmail.com>
> CC: Vivek Goyal <vgoyal@redhat.com>
> CC: Miklos Szeredi <miklos@szeredi.hu>
> CC: linux-unionfs@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>

> ---
>  fs/overlayfs/copy_up.c   | 22 ++++++++++++----------
>  fs/overlayfs/export.c    | 10 ++++++----
>  fs/overlayfs/namei.c     | 19 ++++++++++---------
>  fs/overlayfs/overlayfs.h | 14 ++++++++------
>  fs/overlayfs/util.c      |  3 ++-
>  5 files changed, 38 insertions(+), 30 deletions(-)
>
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index 955ecd4030f0..3380039036d6 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -275,7 +275,8 @@ int ovl_set_attr(struct dentry *upperdentry, struct kstat *stat)
>         return err;
>  }
>
> -struct ovl_fh *ovl_encode_real_fh(struct dentry *real, bool is_upper)
> +struct ovl_fh *ovl_encode_real_fh(struct ovl_fs *ofs, struct dentry *real,
> +                                 bool is_upper)
>  {
>         struct ovl_fh *fh;
>         int fh_type, dwords;
> @@ -328,8 +329,8 @@ struct ovl_fh *ovl_encode_real_fh(struct dentry *real, bool is_upper)
>         return ERR_PTR(err);
>  }
>
> -int ovl_set_origin(struct dentry *dentry, struct dentry *lower,
> -                  struct dentry *upper)
> +int ovl_set_origin(struct ovl_fs *ofs, struct dentry *dentry,
> +                  struct dentry *lower, struct dentry *upper)
>  {
>         const struct ovl_fh *fh = NULL;
>         int err;
> @@ -340,7 +341,7 @@ int ovl_set_origin(struct dentry *dentry, struct dentry *lower,
>          * up and a pure upper inode.
>          */
>         if (ovl_can_decode_fh(lower->d_sb)) {
> -               fh = ovl_encode_real_fh(lower, false);
> +               fh = ovl_encode_real_fh(ofs, lower, false);
>                 if (IS_ERR(fh))
>                         return PTR_ERR(fh);
>         }
> @@ -362,7 +363,7 @@ static int ovl_set_upper_fh(struct ovl_fs *ofs, struct dentry *upper,
>         const struct ovl_fh *fh;
>         int err;
>
> -       fh = ovl_encode_real_fh(upper, true);
> +       fh = ovl_encode_real_fh(ofs, upper, true);
>         if (IS_ERR(fh))
>                 return PTR_ERR(fh);
>
> @@ -380,6 +381,7 @@ static int ovl_set_upper_fh(struct ovl_fs *ofs, struct dentry *upper,
>  static int ovl_create_index(struct dentry *dentry, struct dentry *origin,
>                             struct dentry *upper)
>  {
> +       struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
>         struct dentry *indexdir = ovl_indexdir(dentry->d_sb);
>         struct inode *dir = d_inode(indexdir);
>         struct dentry *index = NULL;
> @@ -402,7 +404,7 @@ static int ovl_create_index(struct dentry *dentry, struct dentry *origin,
>         if (WARN_ON(ovl_test_flag(OVL_INDEX, d_inode(dentry))))
>                 return -EIO;
>
> -       err = ovl_get_index_name(origin, &name);
> +       err = ovl_get_index_name(ofs, origin, &name);
>         if (err)
>                 return err;
>
> @@ -411,7 +413,7 @@ static int ovl_create_index(struct dentry *dentry, struct dentry *origin,
>         if (IS_ERR(temp))
>                 goto free_name;
>
> -       err = ovl_set_upper_fh(OVL_FS(dentry->d_sb), upper, temp);
> +       err = ovl_set_upper_fh(ofs, upper, temp);
>         if (err)
>                 goto out;
>
> @@ -521,7 +523,7 @@ static int ovl_copy_up_inode(struct ovl_copy_up_ctx *c, struct dentry *temp)
>          * hard link.
>          */
>         if (c->origin) {
> -               err = ovl_set_origin(c->dentry, c->lowerpath.dentry, temp);
> +               err = ovl_set_origin(ofs, c->dentry, c->lowerpath.dentry, temp);
>                 if (err)
>                         return err;
>         }
> @@ -700,7 +702,7 @@ static int ovl_copy_up_tmpfile(struct ovl_copy_up_ctx *c)
>  static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
>  {
>         int err;
> -       struct ovl_fs *ofs = c->dentry->d_sb->s_fs_info;
> +       struct ovl_fs *ofs = OVL_FS(c->dentry->d_sb);
>         bool to_index = false;
>
>         /*
> @@ -722,7 +724,7 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
>
>         if (to_index) {
>                 c->destdir = ovl_indexdir(c->dentry->d_sb);
> -               err = ovl_get_index_name(c->lowerpath.dentry, &c->destname);
> +               err = ovl_get_index_name(ofs, c->lowerpath.dentry, &c->destname);
>                 if (err)
>                         return err;
>         } else if (WARN_ON(!c->parent)) {
> diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c
> index ed35be3fafc6..41ebf52f1bbc 100644
> --- a/fs/overlayfs/export.c
> +++ b/fs/overlayfs/export.c
> @@ -211,7 +211,8 @@ static int ovl_check_encode_origin(struct dentry *dentry)
>         return 1;
>  }
>
> -static int ovl_dentry_to_fid(struct dentry *dentry, u32 *fid, int buflen)
> +static int ovl_dentry_to_fid(struct ovl_fs *ofs, struct dentry *dentry,
> +                            u32 *fid, int buflen)
>  {
>         struct ovl_fh *fh = NULL;
>         int err, enc_lower;
> @@ -226,7 +227,7 @@ static int ovl_dentry_to_fid(struct dentry *dentry, u32 *fid, int buflen)
>                 goto fail;
>
>         /* Encode an upper or lower file handle */
> -       fh = ovl_encode_real_fh(enc_lower ? ovl_dentry_lower(dentry) :
> +       fh = ovl_encode_real_fh(ofs, enc_lower ? ovl_dentry_lower(dentry) :
>                                 ovl_dentry_upper(dentry), !enc_lower);
>         if (IS_ERR(fh))
>                 return PTR_ERR(fh);
> @@ -249,6 +250,7 @@ static int ovl_dentry_to_fid(struct dentry *dentry, u32 *fid, int buflen)
>  static int ovl_encode_fh(struct inode *inode, u32 *fid, int *max_len,
>                          struct inode *parent)
>  {
> +       struct ovl_fs *ofs = OVL_FS(inode->i_sb);
>         struct dentry *dentry;
>         int bytes, buflen = *max_len << 2;
>
> @@ -260,7 +262,7 @@ static int ovl_encode_fh(struct inode *inode, u32 *fid, int *max_len,
>         if (WARN_ON(!dentry))
>                 return FILEID_INVALID;
>
> -       bytes = ovl_dentry_to_fid(dentry, fid, buflen);
> +       bytes = ovl_dentry_to_fid(ofs, dentry, fid, buflen);
>         dput(dentry);
>         if (bytes <= 0)
>                 return FILEID_INVALID;
> @@ -680,7 +682,7 @@ static struct dentry *ovl_upper_fh_to_d(struct super_block *sb,
>         if (!ovl_upper_mnt(ofs))
>                 return ERR_PTR(-EACCES);
>
> -       upper = ovl_decode_real_fh(fh, ovl_upper_mnt(ofs), true);
> +       upper = ovl_decode_real_fh(ofs, fh, ovl_upper_mnt(ofs), true);
>         if (IS_ERR_OR_NULL(upper))
>                 return upper;
>
> diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
> index a6162c4076db..f058bf8e8b87 100644
> --- a/fs/overlayfs/namei.c
> +++ b/fs/overlayfs/namei.c
> @@ -150,8 +150,8 @@ static struct ovl_fh *ovl_get_fh(struct ovl_fs *ofs, struct dentry *dentry,
>         goto out;
>  }
>
> -struct dentry *ovl_decode_real_fh(struct ovl_fh *fh, struct vfsmount *mnt,
> -                                 bool connected)
> +struct dentry *ovl_decode_real_fh(struct ovl_fs *ofs, struct ovl_fh *fh,
> +                                 struct vfsmount *mnt, bool connected)
>  {
>         struct dentry *real;
>         int bytes;
> @@ -354,7 +354,7 @@ int ovl_check_origin_fh(struct ovl_fs *ofs, struct ovl_fh *fh, bool connected,
>                     ofs->layers[i].fs->bad_uuid)
>                         continue;
>
> -               origin = ovl_decode_real_fh(fh, ofs->layers[i].mnt,
> +               origin = ovl_decode_real_fh(ofs, fh, ofs->layers[i].mnt,
>                                             connected);
>                 if (origin)
>                         break;
> @@ -450,7 +450,7 @@ int ovl_verify_set_fh(struct ovl_fs *ofs, struct dentry *dentry,
>         struct ovl_fh *fh;
>         int err;
>
> -       fh = ovl_encode_real_fh(real, is_upper);
> +       fh = ovl_encode_real_fh(ofs, real, is_upper);
>         err = PTR_ERR(fh);
>         if (IS_ERR(fh)) {
>                 fh = NULL;
> @@ -488,7 +488,7 @@ struct dentry *ovl_index_upper(struct ovl_fs *ofs, struct dentry *index)
>         if (IS_ERR_OR_NULL(fh))
>                 return ERR_CAST(fh);
>
> -       upper = ovl_decode_real_fh(fh, ovl_upper_mnt(ofs), true);
> +       upper = ovl_decode_real_fh(ofs, fh, ovl_upper_mnt(ofs), true);
>         kfree(fh);
>
>         if (IS_ERR_OR_NULL(upper))
> @@ -640,12 +640,13 @@ static int ovl_get_index_name_fh(struct ovl_fh *fh, struct qstr *name)
>   * index dir was cleared. Either way, that index cannot be used to indentify
>   * the overlay inode.
>   */
> -int ovl_get_index_name(struct dentry *origin, struct qstr *name)
> +int ovl_get_index_name(struct ovl_fs *ofs, struct dentry *origin,
> +                      struct qstr *name)
>  {
>         struct ovl_fh *fh;
>         int err;
>
> -       fh = ovl_encode_real_fh(origin, false);
> +       fh = ovl_encode_real_fh(ofs, origin, false);
>         if (IS_ERR(fh))
>                 return PTR_ERR(fh);
>
> @@ -694,7 +695,7 @@ struct dentry *ovl_lookup_index(struct ovl_fs *ofs, struct dentry *upper,
>         bool is_dir = d_is_dir(origin);
>         int err;
>
> -       err = ovl_get_index_name(origin, &name);
> +       err = ovl_get_index_name(ofs, origin, &name);
>         if (err)
>                 return ERR_PTR(err);
>
> @@ -805,7 +806,7 @@ static int ovl_fix_origin(struct ovl_fs *ofs, struct dentry *dentry,
>         if (err)
>                 return err;
>
> -       err = ovl_set_origin(dentry, lower, upper);
> +       err = ovl_set_origin(ofs, dentry, lower, upper);
>         if (!err)
>                 err = ovl_set_impure(dentry->d_parent, upper->d_parent);
>
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 7bce2469fe55..b56b5f46f224 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -383,8 +383,8 @@ static inline int ovl_check_fh_len(struct ovl_fh *fh, int fh_len)
>         return ovl_check_fb_len(&fh->fb, fh_len - OVL_FH_WIRE_OFFSET);
>  }
>
> -struct dentry *ovl_decode_real_fh(struct ovl_fh *fh, struct vfsmount *mnt,
> -                                 bool connected);
> +struct dentry *ovl_decode_real_fh(struct ovl_fs *ofs, struct ovl_fh *fh,
> +                                 struct vfsmount *mnt, bool connected);
>  int ovl_check_origin_fh(struct ovl_fs *ofs, struct ovl_fh *fh, bool connected,
>                         struct dentry *upperdentry, struct ovl_path **stackp);
>  int ovl_verify_set_fh(struct ovl_fs *ofs, struct dentry *dentry,
> @@ -392,7 +392,8 @@ int ovl_verify_set_fh(struct ovl_fs *ofs, struct dentry *dentry,
>                       bool set);
>  struct dentry *ovl_index_upper(struct ovl_fs *ofs, struct dentry *index);
>  int ovl_verify_index(struct ovl_fs *ofs, struct dentry *index);
> -int ovl_get_index_name(struct dentry *origin, struct qstr *name);
> +int ovl_get_index_name(struct ovl_fs *ofs, struct dentry *origin,
> +                      struct qstr *name);
>  struct dentry *ovl_get_index_fh(struct ovl_fs *ofs, struct ovl_fh *fh);
>  struct dentry *ovl_lookup_index(struct ovl_fs *ofs, struct dentry *upper,
>                                 struct dentry *origin, bool verify);
> @@ -511,9 +512,10 @@ int ovl_maybe_copy_up(struct dentry *dentry, int flags);
>  int ovl_copy_xattr(struct super_block *sb, struct dentry *old,
>                    struct dentry *new);
>  int ovl_set_attr(struct dentry *upper, struct kstat *stat);
> -struct ovl_fh *ovl_encode_real_fh(struct dentry *real, bool is_upper);
> -int ovl_set_origin(struct dentry *dentry, struct dentry *lower,
> -                  struct dentry *upper);
> +struct ovl_fh *ovl_encode_real_fh(struct ovl_fs *ofs, struct dentry *real,
> +                                 bool is_upper);
> +int ovl_set_origin(struct ovl_fs *ofs, struct dentry *dentry,
> +                  struct dentry *lower, struct dentry *upper);
>
>  /* export.c */
>  extern const struct export_operations ovl_export_operations;
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index 23f475627d07..44b4b62a8ac8 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -716,6 +716,7 @@ bool ovl_need_index(struct dentry *dentry)
>  /* Caller must hold OVL_I(inode)->lock */
>  static void ovl_cleanup_index(struct dentry *dentry)
>  {
> +       struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
>         struct dentry *indexdir = ovl_indexdir(dentry->d_sb);
>         struct inode *dir = indexdir->d_inode;
>         struct dentry *lowerdentry = ovl_dentry_lower(dentry);
> @@ -725,7 +726,7 @@ static void ovl_cleanup_index(struct dentry *dentry)
>         struct qstr name = { };
>         int err;
>
> -       err = ovl_get_index_name(lowerdentry, &name);
> +       err = ovl_get_index_name(ofs, lowerdentry, &name);
>         if (err)
>                 goto fail;
>
> --
> 2.26.2
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 2/2] ovl: introduce new "uuid=off" option for inodes index feature
  2020-09-25  8:35 ` [PATCH v4 2/2] ovl: introduce new "uuid=off" option for inodes index feature Pavel Tikhomirov
@ 2020-09-25 16:42   ` Amir Goldstein
  2020-09-28  7:22     ` Pavel Tikhomirov
  2020-10-06 15:13   ` Miklos Szeredi
  1 sibling, 1 reply; 8+ messages in thread
From: Amir Goldstein @ 2020-09-25 16:42 UTC (permalink / raw)
  To: Pavel Tikhomirov; +Cc: Miklos Szeredi, Vivek Goyal, overlayfs, linux-kernel

On Fri, Sep 25, 2020 at 11:35 AM Pavel Tikhomirov
<ptikhomirov@virtuozzo.com> wrote:
>
> This replaces uuid with null in overelayfs file handles and thus relaxes

typo: overelayfs

> uuid checks for overlay index feature. It is only possible in case there
> is only one filesystem for all the work/upper/lower directories and bare
> file handles from this backing filesystem are uniq. In other case when
> we have multiple filesystems lets just fallback to "uuid=on" which is
> and equivalent of how it worked before with all uuid checks.
>

typo: uniq

> This is needed when overlayfs is/was mounted in a container with index
> enabled (e.g.: to be able to resolve inotify watch file handles on it to
> paths in CRIU), and this container is copied and started alongside with
> the original one. This way the "copy" container can't have the same uuid
> on the superblock and mounting the overlayfs from it later would fail.
>
> Note: In our (Virtuozzo) use case users inside a container can create
> "regular" overlayfs mounts without any "index=" option, but we still
> want to migrate this containers with CRIU so we set "index=on" as kernel

this container

> default so that all the container overlayfs mounts get support of file
> handles automatically. With "uuid=off" we want the same thing (to be
> able to "copy" container with uuid change) - we would set kernel default
> so that all the container overlayfs mounts get "uuid=off" automatically.
>
> That is an example of the problem on top of loop+ext4:
>
> dd if=/dev/zero of=loopbackfile.img bs=100M count=10
> losetup -fP loopbackfile.img
> losetup -a
>   #/dev/loop0: [64768]:35 (/loop-test/loopbackfile.img)
> mkfs.ext4 loopbackfile.img
> mkdir loop-mp
> mount -o loop /dev/loop0 loop-mp
> mkdir loop-mp/{lower,upper,work,merged}
> mount -t overlay overlay -oindex=on,lowerdir=loop-mp/lower,\
> upperdir=loop-mp/upper,workdir=loop-mp/work loop-mp/merged
> umount loop-mp/merged
> umount loop-mp
> e2fsck -f /dev/loop0
> tune2fs -U random /dev/loop0
>
> mount -o loop /dev/loop0 loop-mp
> mount -t overlay overlay -oindex=on,lowerdir=loop-mp/lower,\
> upperdir=loop-mp/upper,workdir=loop-mp/work loop-mp/merged
>   #mount: /loop-test/loop-mp/merged:
>   #mount(2) system call failed: Stale file handle.
>
> If you just change the uuid of the backing filesystem, overlay is not
> mounting any more. In Virtuozzo we copy container disks (ploops) when
> crate the copy of container and we require fs uuid to be uniq for a new

typos: crat, uniq

> container.
>
> CC: Amir Goldstein <amir73il@gmail.com>
> CC: Vivek Goyal <vgoyal@redhat.com>
> CC: Miklos Szeredi <miklos@szeredi.hu>
> CC: linux-unionfs@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
>

Apart from some typos, looks good to me.

You may add:
Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Please do not post v5. you should wait for more feedback from others.

> ---
> v2: in v1 I missed actual uuid check skip
> v3: rebase to overlayfs-next, replace uuid with null in file handles,
> split ovl_fs propagation to function arguments to separate patch, add
> separate bool "uuid=on/off" option, move numfs check up, add doc note.
> v4: get rid of double negatives, remove nouuid leftower comment, fix
> missprint in kernel config name
>
>  Documentation/filesystems/overlayfs.rst |  6 ++++++
>  fs/overlayfs/Kconfig                    | 19 +++++++++++++++++++
>  fs/overlayfs/copy_up.c                  |  3 ++-
>  fs/overlayfs/namei.c                    |  4 +++-
>  fs/overlayfs/ovl_entry.h                |  1 +
>  fs/overlayfs/super.c                    | 25 +++++++++++++++++++++++++
>  6 files changed, 56 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
> index 580ab9a0fe31..4f9cc20f255c 100644
> --- a/Documentation/filesystems/overlayfs.rst
> +++ b/Documentation/filesystems/overlayfs.rst
> @@ -563,6 +563,12 @@ This verification may cause significant overhead in some cases.
>  Note: the mount options index=off,nfs_export=on are conflicting for a
>  read-write mount and will result in an error.
>
> +Note: the mount option uuid=off (or corresponding module param, or kernel
> +config) can be used to replace UUID of the underlying filesystem in file
> +handles with null, and effectively disable UUID checks. This can be useful in
> +case the underlying disk is copied and the UUID of this copy is changed. This
> +is only applicable if all lower/upper/work directories are on the same
> +filesystem, otherwise it will fallback to normal behaviour.
>
>  Volatile mount
>  --------------
> diff --git a/fs/overlayfs/Kconfig b/fs/overlayfs/Kconfig
> index dd188c7996b3..c21abdb43206 100644
> --- a/fs/overlayfs/Kconfig
> +++ b/fs/overlayfs/Kconfig
> @@ -61,6 +61,25 @@ config OVERLAY_FS_INDEX
>
>           If unsure, say N.
>
> +config OVERLAY_FS_INDEX_UUID
> +       bool "Overlayfs: export uuid in file handles"
> +       default y
> +       depends on OVERLAY_FS
> +       help
> +         If this config option is disabled then overlay will replace uuid with
> +         null in overlayfs file handles, effectively disabling uuid checks for
> +         them. This affects overlayfs mounted with "index=on". This only can be
> +         done if all upper and lower directories are on the same filesystem
> +         where basic fhandles are uniq. In case the latter is not true

There are some typos in it, but I think this phrase can be dropped:
"where basic fhandles are uniq"

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 2/2] ovl: introduce new "uuid=off" option for inodes index feature
  2020-09-25 16:42   ` Amir Goldstein
@ 2020-09-28  7:22     ` Pavel Tikhomirov
  0 siblings, 0 replies; 8+ messages in thread
From: Pavel Tikhomirov @ 2020-09-28  7:22 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: Miklos Szeredi, Vivek Goyal, overlayfs, linux-kernel

On 9/25/20 7:42 PM, Amir Goldstein wrote:
> Apart from some typos, looks good to me.

Amir, Thanks a lot for your review!

 > you should wait for more feedback from others

Sure, will wait.

-- 
Best regards, Tikhomirov Pavel
Software Developer, Virtuozzo.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 2/2] ovl: introduce new "uuid=off" option for inodes index feature
  2020-09-25  8:35 ` [PATCH v4 2/2] ovl: introduce new "uuid=off" option for inodes index feature Pavel Tikhomirov
  2020-09-25 16:42   ` Amir Goldstein
@ 2020-10-06 15:13   ` Miklos Szeredi
  2020-10-13 13:54     ` Pavel Tikhomirov
  1 sibling, 1 reply; 8+ messages in thread
From: Miklos Szeredi @ 2020-10-06 15:13 UTC (permalink / raw)
  To: Pavel Tikhomirov; +Cc: Amir Goldstein, Vivek Goyal, overlayfs, linux-kernel

On Fri, Sep 25, 2020 at 10:35 AM Pavel Tikhomirov
<ptikhomirov@virtuozzo.com> wrote:

> Note: In our (Virtuozzo) use case users inside a container can create
> "regular" overlayfs mounts without any "index=" option, but we still
> want to migrate this containers with CRIU so we set "index=on" as kernel
> default so that all the container overlayfs mounts get support of file
> handles automatically. With "uuid=off" we want the same thing (to be
> able to "copy" container with uuid change) - we would set kernel default
> so that all the container overlayfs mounts get "uuid=off" automatically.

I'm not sure I buy that argument for a kernel option.   It should
rather be a "container" option in that case, but AFAIK the kernel
doesn't have a concept of a container.  I think this needs to be
discussed on the relevant mailing lists.

As of now mainline kernel doesn't support unprivileged overlay mounts,
so I guess this is not an issue.  Let's just merge this without the
kernel and the module options.

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 2/2] ovl: introduce new "uuid=off" option for inodes index feature
  2020-10-06 15:13   ` Miklos Szeredi
@ 2020-10-13 13:54     ` Pavel Tikhomirov
  0 siblings, 0 replies; 8+ messages in thread
From: Pavel Tikhomirov @ 2020-10-13 13:54 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: Amir Goldstein, Vivek Goyal, overlayfs, linux-kernel



On 10/6/20 6:13 PM, Miklos Szeredi wrote:
> On Fri, Sep 25, 2020 at 10:35 AM Pavel Tikhomirov
> <ptikhomirov@virtuozzo.com> wrote:
> 
>> Note: In our (Virtuozzo) use case users inside a container can create
>> "regular" overlayfs mounts without any "index=" option, but we still
>> want to migrate this containers with CRIU so we set "index=on" as kernel
>> default so that all the container overlayfs mounts get support of file
>> handles automatically. With "uuid=off" we want the same thing (to be
>> able to "copy" container with uuid change) - we would set kernel default
>> so that all the container overlayfs mounts get "uuid=off" automatically.
> 
> I'm not sure I buy that argument for a kernel option.   It should
> rather be a "container" option in that case, but AFAIK the kernel
> doesn't have a concept of a container.  I think this needs to be
> discussed on the relevant mailing lists.
> 
> As of now mainline kernel doesn't support unprivileged overlay mounts,
> so I guess this is not an issue.  Let's just merge this without the
> kernel and the module options.

Virtuozzo kernel does have a "container" concept and we do have 
unprivileged overlay mounts to support docker inside Virtuozzo 
containers. We don't face any major issues with it. But you are right 
it's not mainstream.

Probably a normal user of mainstream kernel also might want to set 
index=on+uuid=off by default, so that all their docker containters 
automatically support inotifies and survive backing disk uuid change 
automaticaly.

I will prepare next patchset version without default.

> 
> Thanks,
> Miklos
> 

-- 
Best regards, Tikhomirov Pavel
Software Developer, Virtuozzo.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-10-13 13:54 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-25  8:35 [PATCH v4 0/2] ovl introduce "uuid=off" Pavel Tikhomirov
2020-09-25  8:35 ` [PATCH v4 1/2] ovl: propagate ovl_fs to ovl_decode_real_fh and ovl_encode_real_fh Pavel Tikhomirov
2020-09-25 16:37   ` Amir Goldstein
2020-09-25  8:35 ` [PATCH v4 2/2] ovl: introduce new "uuid=off" option for inodes index feature Pavel Tikhomirov
2020-09-25 16:42   ` Amir Goldstein
2020-09-28  7:22     ` Pavel Tikhomirov
2020-10-06 15:13   ` Miklos Szeredi
2020-10-13 13:54     ` Pavel Tikhomirov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).