Re: [PATCH v5 2/2] ovl: introduce new "uuid=off" option for inodes index feature

From: Amir Goldstein
Date: Tue Oct 13 2020 - 12:23:51 EST


On Tue, Oct 13, 2020 at 6:00 PM Pavel Tikhomirov
<ptikhomirov@xxxxxxxxxxxxx> wrote:
>
> This replaces uuid with null in overlayfs 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 unique. 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.
>
> 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
> create the copy of container and we require fs uuid to be unique for a new
> container.
>
> CC: Amir Goldstein <amir73il@xxxxxxxxx>
> CC: Vivek Goyal <vgoyal@xxxxxxxxxx>
> CC: Miklos Szeredi <miklos@xxxxxxxxxx>
> CC: linux-unionfs@xxxxxxxxxxxxxxx
> CC: linux-kernel@xxxxxxxxxxxxxxx
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@xxxxxxxxxxxxx>
>

Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>


> ---
> 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
> misprint in kernel config name
> v5: fix typos; remove config option, module param, ovl_uuid_def and the
> corresponding notes
>
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@xxxxxxxxxxxxx>
> ---
> Documentation/filesystems/overlayfs.rst | 5 +++++
> fs/overlayfs/copy_up.c | 3 ++-
> fs/overlayfs/namei.c | 4 +++-
> fs/overlayfs/ovl_entry.h | 1 +
> fs/overlayfs/super.c | 20 ++++++++++++++++++++
> 5 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
> index 580ab9a0fe31..a3e588dc8437 100644
> --- a/Documentation/filesystems/overlayfs.rst
> +++ b/Documentation/filesystems/overlayfs.rst
> @@ -563,6 +563,11 @@ 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 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/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..4717244e7d7a 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -356,6 +356,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)
> + seq_puts(m, ",uuid=off");
> if (ofs->config.nfs_export != ovl_nfs_export_def)
> seq_printf(m, ",nfs_export=%s", ofs->config.nfs_export ?
> "on" : "off");
> @@ -410,6 +412,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 +433,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 +555,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 +1891,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 = true;
> ofs->config.nfs_export = ovl_nfs_export_def;
> ofs->config.xino = ovl_xino_def();
> ofs->config.metacopy = ovl_metacopy_def;
> @@ -1956,6 +1971,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
>