* [PATCH 0/4] ovl: allow virtiofs as upper
@ 2020-01-31 11:50 Miklos Szeredi
2020-01-31 11:50 ` [PATCH 1/4] ovl: restructure dentry revalidation Miklos Szeredi
` (3 more replies)
0 siblings, 4 replies; 25+ messages in thread
From: Miklos Szeredi @ 2020-01-31 11:50 UTC (permalink / raw)
To: linux-unionfs; +Cc: linux-fsdevel, Vivek Goyal
The purpose of this mini-series is to allow virtiofs as upper layer of
overlayfs.
Applies on top of #overlayfs-next (commit 1a980b8cbf00 ("ovl: add splice
file read write helper")).
Git tree is:
git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git#ovl-remote-upper
Thanks,
Miklos
---
Miklos Szeredi (4):
ovl: restructure dentry revalidation
ovl: separate detection of remote upper layer from stacked overlay
ovl: decide if revalidate needed on a per-dentry bases
ovl: alllow remote upper
fs/overlayfs/dir.c | 3 ++
fs/overlayfs/export.c | 2 +
fs/overlayfs/namei.c | 5 ++-
fs/overlayfs/overlayfs.h | 3 +-
fs/overlayfs/super.c | 90 +++++++++++++++++++---------------------
fs/overlayfs/util.c | 18 ++++++--
6 files changed, 68 insertions(+), 53 deletions(-)
--
2.21.1
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH 1/4] ovl: restructure dentry revalidation 2020-01-31 11:50 [PATCH 0/4] ovl: allow virtiofs as upper Miklos Szeredi @ 2020-01-31 11:50 ` Miklos Szeredi 2020-01-31 11:50 ` [PATCH 2/4] ovl: separate detection of remote upper layer from stacked overlay Miklos Szeredi ` (2 subsequent siblings) 3 siblings, 0 replies; 25+ messages in thread From: Miklos Szeredi @ 2020-01-31 11:50 UTC (permalink / raw) To: linux-unionfs; +Cc: linux-fsdevel, Vivek Goyal Use a common loop for plain and weak revalidation. This will aid doing revalidation on upper layer. This patch doesn't change behavior. Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> --- fs/overlayfs/super.c | 51 ++++++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 319fe0d355b0..852a1816fea1 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -113,47 +113,48 @@ static struct dentry *ovl_d_real(struct dentry *dentry, return dentry; } -static int ovl_dentry_revalidate(struct dentry *dentry, unsigned int flags) +static int ovl_revalidate_real(struct dentry *d, unsigned int flags, bool weak) { - struct ovl_entry *oe = dentry->d_fsdata; - unsigned int i; int ret = 1; - for (i = 0; i < oe->numlower; i++) { - struct dentry *d = oe->lowerstack[i].dentry; - - if (d->d_flags & DCACHE_OP_REVALIDATE) { - ret = d->d_op->d_revalidate(d, flags); - if (ret < 0) - return ret; - if (!ret) { - if (!(flags & LOOKUP_RCU)) - d_invalidate(d); - return -ESTALE; - } + if (weak) { + if (d->d_flags & DCACHE_OP_WEAK_REVALIDATE) + ret = d->d_op->d_weak_revalidate(d, flags); + } else if (d->d_flags & DCACHE_OP_REVALIDATE) { + ret = d->d_op->d_revalidate(d, flags); + if (!ret) { + if (!(flags & LOOKUP_RCU)) + d_invalidate(d); + ret = -ESTALE; } } - return 1; + return ret; } -static int ovl_dentry_weak_revalidate(struct dentry *dentry, unsigned int flags) +static int ovl_dentry_revalidate_common(struct dentry *dentry, + unsigned int flags, bool weak) { struct ovl_entry *oe = dentry->d_fsdata; unsigned int i; int ret = 1; - for (i = 0; i < oe->numlower; i++) { - struct dentry *d = oe->lowerstack[i].dentry; - - if (d->d_flags & DCACHE_OP_WEAK_REVALIDATE) { - ret = d->d_op->d_weak_revalidate(d, flags); - if (ret <= 0) - break; - } + for (i = 0; ret > 0 && i < oe->numlower; i++) { + ret = ovl_revalidate_real(oe->lowerstack[i].dentry, flags, + weak); } return ret; } +static int ovl_dentry_revalidate(struct dentry *dentry, unsigned int flags) +{ + return ovl_dentry_revalidate_common(dentry, flags, false); +} + +static int ovl_dentry_weak_revalidate(struct dentry *dentry, unsigned int flags) +{ + return ovl_dentry_revalidate_common(dentry, flags, true); +} + static const struct dentry_operations ovl_dentry_operations = { .d_release = ovl_dentry_release, .d_real = ovl_d_real, -- 2.21.1 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 2/4] ovl: separate detection of remote upper layer from stacked overlay 2020-01-31 11:50 [PATCH 0/4] ovl: allow virtiofs as upper Miklos Szeredi 2020-01-31 11:50 ` [PATCH 1/4] ovl: restructure dentry revalidation Miklos Szeredi @ 2020-01-31 11:50 ` Miklos Szeredi 2020-01-31 11:50 ` [PATCH 3/4] ovl: decide if revalidate needed on a per-dentry bases Miklos Szeredi 2020-01-31 11:50 ` [PATCH 4/4] ovl: alllow remote upper Miklos Szeredi 3 siblings, 0 replies; 25+ messages in thread From: Miklos Szeredi @ 2020-01-31 11:50 UTC (permalink / raw) To: linux-unionfs; +Cc: linux-fsdevel, Vivek Goyal Following patch will allow remote as upper layer, but not overlay stacked on upper layer. Separate the two concepts. This patch is doesn't change behavior. Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> --- fs/overlayfs/namei.c | 3 ++- fs/overlayfs/super.c | 14 +++++++------- fs/overlayfs/util.c | 3 +-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c index ed9e129fae04..a5b998a93a24 100644 --- a/fs/overlayfs/namei.c +++ b/fs/overlayfs/namei.c @@ -845,7 +845,8 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, if (err) goto out; - if (upperdentry && unlikely(ovl_dentry_remote(upperdentry))) { + if (upperdentry && (upperdentry->d_flags & DCACHE_OP_REAL || + unlikely(ovl_dentry_remote(upperdentry)))) { dput(upperdentry); err = -EREMOTE; goto out; diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 852a1816fea1..7e294bf719ff 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -752,13 +752,13 @@ static int ovl_mount_dir(const char *name, struct path *path) ovl_unescape(tmp); err = ovl_mount_dir_noesc(tmp, path); - if (!err) - if (ovl_dentry_remote(path->dentry)) { - pr_err("filesystem on '%s' not supported as upperdir\n", - tmp); - path_put_init(path); - err = -EINVAL; - } + if (!err && (ovl_dentry_remote(path->dentry) || + path->dentry->d_flags & DCACHE_OP_REAL)) { + pr_err("filesystem on '%s' not supported as upperdir\n", + tmp); + path_put_init(path); + err = -EINVAL; + } kfree(tmp); } return err; diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index ea005085803f..67cd2866aaa2 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -93,8 +93,7 @@ struct ovl_entry *ovl_alloc_entry(unsigned int numlower) bool ovl_dentry_remote(struct dentry *dentry) { return dentry->d_flags & - (DCACHE_OP_REVALIDATE | DCACHE_OP_WEAK_REVALIDATE | - DCACHE_OP_REAL); + (DCACHE_OP_REVALIDATE | DCACHE_OP_WEAK_REVALIDATE); } bool ovl_dentry_weird(struct dentry *dentry) -- 2.21.1 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 3/4] ovl: decide if revalidate needed on a per-dentry bases 2020-01-31 11:50 [PATCH 0/4] ovl: allow virtiofs as upper Miklos Szeredi 2020-01-31 11:50 ` [PATCH 1/4] ovl: restructure dentry revalidation Miklos Szeredi 2020-01-31 11:50 ` [PATCH 2/4] ovl: separate detection of remote upper layer from stacked overlay Miklos Szeredi @ 2020-01-31 11:50 ` Miklos Szeredi 2020-01-31 14:53 ` Amir Goldstein 2020-01-31 11:50 ` [PATCH 4/4] ovl: alllow remote upper Miklos Szeredi 3 siblings, 1 reply; 25+ messages in thread From: Miklos Szeredi @ 2020-01-31 11:50 UTC (permalink / raw) To: linux-unionfs; +Cc: linux-fsdevel, Vivek Goyal Allow completely skipping ->revalidate() on a per-dentry bases, in case the underlying layers used for a dentry do not themselves have ->revalidate(). E.g. negative overlay dentry has no underlying layers, hence revalidate is unnecessary. Or if lower layer is remote but overlay dentry is pure-upper, then can skip revalidate. The following places need to update whether the dentry needs revalidate or not: - fill-super (root dentry) - lookup - create - fh_to_dentry Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> --- fs/overlayfs/dir.c | 3 +++ fs/overlayfs/export.c | 2 ++ fs/overlayfs/namei.c | 3 +++ fs/overlayfs/overlayfs.h | 3 ++- fs/overlayfs/super.c | 23 +++++++---------------- fs/overlayfs/util.c | 15 ++++++++++++--- 6 files changed, 29 insertions(+), 20 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 8e57d5372b8f..b3471ef51440 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -243,6 +243,9 @@ static int ovl_instantiate(struct dentry *dentry, struct inode *inode, ovl_dir_modified(dentry->d_parent, false); ovl_dentry_set_upper_alias(dentry); + ovl_dentry_update_reval(dentry, newdentry, + DCACHE_OP_REVALIDATE | DCACHE_OP_WEAK_REVALIDATE); + if (!hardlink) { /* * ovl_obtain_alias() can be called after ovl_create_real() diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c index 6f54d70cef27..a58b3d9b06b9 100644 --- a/fs/overlayfs/export.c +++ b/fs/overlayfs/export.c @@ -324,6 +324,8 @@ static struct dentry *ovl_obtain_alias(struct super_block *sb, if (upper_alias) ovl_dentry_set_upper_alias(dentry); } + ovl_dentry_update_reval(dentry, upper, + DCACHE_OP_REVALIDATE | DCACHE_OP_WEAK_REVALIDATE); return d_instantiate_anon(dentry, inode); diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c index a5b998a93a24..76e61cc27822 100644 --- a/fs/overlayfs/namei.c +++ b/fs/overlayfs/namei.c @@ -1077,6 +1077,9 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, goto out_free_oe; } + ovl_dentry_update_reval(dentry, upperdentry, + DCACHE_OP_REVALIDATE | DCACHE_OP_WEAK_REVALIDATE); + revert_creds(old_cred); if (origin_path) { dput(origin_path->dentry); diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index 3623d28aa4fa..68124a4f8f9b 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -229,7 +229,8 @@ struct dentry *ovl_indexdir(struct super_block *sb); bool ovl_index_all(struct super_block *sb); bool ovl_verify_lower(struct super_block *sb); struct ovl_entry *ovl_alloc_entry(unsigned int numlower); -bool ovl_dentry_remote(struct dentry *dentry); +void ovl_dentry_update_reval(struct dentry *dentry, struct dentry *upperdentry, + unsigned int mask); bool ovl_dentry_weird(struct dentry *dentry); enum ovl_path_type ovl_path_type(struct dentry *dentry); void ovl_path_upper(struct dentry *dentry, struct path *path); diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 7e294bf719ff..26d4153240a8 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -158,11 +158,6 @@ static int ovl_dentry_weak_revalidate(struct dentry *dentry, unsigned int flags) static const struct dentry_operations ovl_dentry_operations = { .d_release = ovl_dentry_release, .d_real = ovl_d_real, -}; - -static const struct dentry_operations ovl_reval_dentry_operations = { - .d_release = ovl_dentry_release, - .d_real = ovl_d_real, .d_revalidate = ovl_dentry_revalidate, .d_weak_revalidate = ovl_dentry_weak_revalidate, }; @@ -779,7 +774,7 @@ static int ovl_check_namelen(struct path *path, struct ovl_fs *ofs, } static int ovl_lower_dir(const char *name, struct path *path, - struct ovl_fs *ofs, int *stack_depth, bool *remote) + struct ovl_fs *ofs, int *stack_depth) { int fh_type; int err; @@ -794,9 +789,6 @@ static int ovl_lower_dir(const char *name, struct path *path, *stack_depth = max(*stack_depth, path->mnt->mnt_sb->s_stack_depth); - if (ovl_dentry_remote(path->dentry)) - *remote = true; - /* * The inodes index feature and NFS export need to encode and decode * file handles, so they require that all layers support them. @@ -1439,7 +1431,6 @@ static struct ovl_entry *ovl_get_lowerstack(struct super_block *sb, char *lowertmp, *lower; struct path *stack = NULL; unsigned int stacklen, numlower = 0, i; - bool remote = false; struct ovl_entry *oe; err = -ENOMEM; @@ -1471,7 +1462,7 @@ static struct ovl_entry *ovl_get_lowerstack(struct super_block *sb, lower = lowertmp; for (numlower = 0; numlower < stacklen; numlower++) { err = ovl_lower_dir(lower, &stack[numlower], ofs, - &sb->s_stack_depth, &remote); + &sb->s_stack_depth); if (err) goto out_err; @@ -1499,11 +1490,6 @@ static struct ovl_entry *ovl_get_lowerstack(struct super_block *sb, oe->lowerstack[i].layer = &ofs->layers[i+1]; } - if (remote) - sb->s_d_op = &ovl_reval_dentry_operations; - else - sb->s_d_op = &ovl_dentry_operations; - out: for (i = 0; i < numlower; i++) path_put(&stack[i]); @@ -1597,6 +1583,8 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) struct cred *cred; int err; + sb->s_d_op = &ovl_dentry_operations; + err = -ENOMEM; ofs = kzalloc(sizeof(struct ovl_fs), GFP_KERNEL); if (!ofs) @@ -1724,6 +1712,9 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) ovl_inode_init(d_inode(root_dentry), upperpath.dentry, ovl_dentry_lower(root_dentry), NULL); + ovl_dentry_update_reval(root_dentry, upperpath.dentry, + DCACHE_OP_WEAK_REVALIDATE); + sb->s_root = root_dentry; return 0; diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index 67cd2866aaa2..3ad8fb291f7d 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -90,10 +90,19 @@ struct ovl_entry *ovl_alloc_entry(unsigned int numlower) return oe; } -bool ovl_dentry_remote(struct dentry *dentry) +void ovl_dentry_update_reval(struct dentry *dentry, struct dentry *upperdentry, + unsigned int mask) { - return dentry->d_flags & - (DCACHE_OP_REVALIDATE | DCACHE_OP_WEAK_REVALIDATE); + struct ovl_entry *oe = OVL_E(dentry); + unsigned int i, flags = 0; + + for (i = 0; i < oe->numlower; i++) + flags |= oe->lowerstack[i].dentry->d_flags; + + spin_lock(&dentry->d_lock); + dentry->d_flags &= ~mask; + dentry->d_flags |= flags & mask; + spin_unlock(&dentry->d_lock); } bool ovl_dentry_weird(struct dentry *dentry) -- 2.21.1 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 3/4] ovl: decide if revalidate needed on a per-dentry bases 2020-01-31 11:50 ` [PATCH 3/4] ovl: decide if revalidate needed on a per-dentry bases Miklos Szeredi @ 2020-01-31 14:53 ` Amir Goldstein 2020-01-31 15:15 ` Miklos Szeredi 0 siblings, 1 reply; 25+ messages in thread From: Amir Goldstein @ 2020-01-31 14:53 UTC (permalink / raw) To: Miklos Szeredi; +Cc: overlayfs, linux-fsdevel, Vivek Goyal On Fri, Jan 31, 2020 at 1:51 PM Miklos Szeredi <mszeredi@redhat.com> wrote: > > Allow completely skipping ->revalidate() on a per-dentry bases, in case the > underlying layers used for a dentry do not themselves have ->revalidate(). > > E.g. negative overlay dentry has no underlying layers, hence revalidate is > unnecessary. Or if lower layer is remote but overlay dentry is pure-upper, > then can skip revalidate. > > The following places need to update whether the dentry needs revalidate or > not: > > - fill-super (root dentry) > - lookup > - create > - fh_to_dentry > > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> > --- > fs/overlayfs/dir.c | 3 +++ > fs/overlayfs/export.c | 2 ++ > fs/overlayfs/namei.c | 3 +++ > fs/overlayfs/overlayfs.h | 3 ++- > fs/overlayfs/super.c | 23 +++++++---------------- > fs/overlayfs/util.c | 15 ++++++++++++--- > 6 files changed, 29 insertions(+), 20 deletions(-) > > diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c > index 8e57d5372b8f..b3471ef51440 100644 > --- a/fs/overlayfs/dir.c > +++ b/fs/overlayfs/dir.c > @@ -243,6 +243,9 @@ static int ovl_instantiate(struct dentry *dentry, struct inode *inode, > > ovl_dir_modified(dentry->d_parent, false); > ovl_dentry_set_upper_alias(dentry); > + ovl_dentry_update_reval(dentry, newdentry, > + DCACHE_OP_REVALIDATE | DCACHE_OP_WEAK_REVALIDATE); > + > if (!hardlink) { > /* > * ovl_obtain_alias() can be called after ovl_create_real() > diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c > index 6f54d70cef27..a58b3d9b06b9 100644 > --- a/fs/overlayfs/export.c > +++ b/fs/overlayfs/export.c > @@ -324,6 +324,8 @@ static struct dentry *ovl_obtain_alias(struct super_block *sb, > if (upper_alias) > ovl_dentry_set_upper_alias(dentry); > } > + ovl_dentry_update_reval(dentry, upper, > + DCACHE_OP_REVALIDATE | DCACHE_OP_WEAK_REVALIDATE); > > return d_instantiate_anon(dentry, inode); > > diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c > index a5b998a93a24..76e61cc27822 100644 > --- a/fs/overlayfs/namei.c > +++ b/fs/overlayfs/namei.c > @@ -1077,6 +1077,9 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, > goto out_free_oe; > } > > + ovl_dentry_update_reval(dentry, upperdentry, > + DCACHE_OP_REVALIDATE | DCACHE_OP_WEAK_REVALIDATE); > + > revert_creds(old_cred); > if (origin_path) { > dput(origin_path->dentry); > diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h > index 3623d28aa4fa..68124a4f8f9b 100644 > --- a/fs/overlayfs/overlayfs.h > +++ b/fs/overlayfs/overlayfs.h > @@ -229,7 +229,8 @@ struct dentry *ovl_indexdir(struct super_block *sb); > bool ovl_index_all(struct super_block *sb); > bool ovl_verify_lower(struct super_block *sb); > struct ovl_entry *ovl_alloc_entry(unsigned int numlower); > -bool ovl_dentry_remote(struct dentry *dentry); > +void ovl_dentry_update_reval(struct dentry *dentry, struct dentry *upperdentry, > + unsigned int mask); > bool ovl_dentry_weird(struct dentry *dentry); > enum ovl_path_type ovl_path_type(struct dentry *dentry); > void ovl_path_upper(struct dentry *dentry, struct path *path); > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c > index 7e294bf719ff..26d4153240a8 100644 > --- a/fs/overlayfs/super.c > +++ b/fs/overlayfs/super.c > @@ -158,11 +158,6 @@ static int ovl_dentry_weak_revalidate(struct dentry *dentry, unsigned int flags) > static const struct dentry_operations ovl_dentry_operations = { > .d_release = ovl_dentry_release, > .d_real = ovl_d_real, > -}; > - > -static const struct dentry_operations ovl_reval_dentry_operations = { > - .d_release = ovl_dentry_release, > - .d_real = ovl_d_real, > .d_revalidate = ovl_dentry_revalidate, > .d_weak_revalidate = ovl_dentry_weak_revalidate, > }; > @@ -779,7 +774,7 @@ static int ovl_check_namelen(struct path *path, struct ovl_fs *ofs, > } > > static int ovl_lower_dir(const char *name, struct path *path, > - struct ovl_fs *ofs, int *stack_depth, bool *remote) > + struct ovl_fs *ofs, int *stack_depth) > { > int fh_type; > int err; > @@ -794,9 +789,6 @@ static int ovl_lower_dir(const char *name, struct path *path, > > *stack_depth = max(*stack_depth, path->mnt->mnt_sb->s_stack_depth); > > - if (ovl_dentry_remote(path->dentry)) > - *remote = true; > - > /* > * The inodes index feature and NFS export need to encode and decode > * file handles, so they require that all layers support them. > @@ -1439,7 +1431,6 @@ static struct ovl_entry *ovl_get_lowerstack(struct super_block *sb, > char *lowertmp, *lower; > struct path *stack = NULL; > unsigned int stacklen, numlower = 0, i; > - bool remote = false; > struct ovl_entry *oe; > > err = -ENOMEM; > @@ -1471,7 +1462,7 @@ static struct ovl_entry *ovl_get_lowerstack(struct super_block *sb, > lower = lowertmp; > for (numlower = 0; numlower < stacklen; numlower++) { > err = ovl_lower_dir(lower, &stack[numlower], ofs, > - &sb->s_stack_depth, &remote); > + &sb->s_stack_depth); > if (err) > goto out_err; > > @@ -1499,11 +1490,6 @@ static struct ovl_entry *ovl_get_lowerstack(struct super_block *sb, > oe->lowerstack[i].layer = &ofs->layers[i+1]; > } > > - if (remote) > - sb->s_d_op = &ovl_reval_dentry_operations; > - else > - sb->s_d_op = &ovl_dentry_operations; > - > out: > for (i = 0; i < numlower; i++) > path_put(&stack[i]); > @@ -1597,6 +1583,8 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) > struct cred *cred; > int err; > > + sb->s_d_op = &ovl_dentry_operations; > + > err = -ENOMEM; > ofs = kzalloc(sizeof(struct ovl_fs), GFP_KERNEL); > if (!ofs) > @@ -1724,6 +1712,9 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) > ovl_inode_init(d_inode(root_dentry), upperpath.dentry, > ovl_dentry_lower(root_dentry), NULL); > > + ovl_dentry_update_reval(root_dentry, upperpath.dentry, > + DCACHE_OP_WEAK_REVALIDATE); > + > sb->s_root = root_dentry; > > return 0; > diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c > index 67cd2866aaa2..3ad8fb291f7d 100644 > --- a/fs/overlayfs/util.c > +++ b/fs/overlayfs/util.c > @@ -90,10 +90,19 @@ struct ovl_entry *ovl_alloc_entry(unsigned int numlower) > return oe; > } > > -bool ovl_dentry_remote(struct dentry *dentry) Removed too early. It still has users. Otherwise looks ok. Thanks, Amir. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/4] ovl: decide if revalidate needed on a per-dentry bases 2020-01-31 14:53 ` Amir Goldstein @ 2020-01-31 15:15 ` Miklos Szeredi 0 siblings, 0 replies; 25+ messages in thread From: Miklos Szeredi @ 2020-01-31 15:15 UTC (permalink / raw) To: Amir Goldstein; +Cc: overlayfs, linux-fsdevel, Vivek Goyal > > diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c > > index 67cd2866aaa2..3ad8fb291f7d 100644 > > --- a/fs/overlayfs/util.c > > +++ b/fs/overlayfs/util.c > > @@ -90,10 +90,19 @@ struct ovl_entry *ovl_alloc_entry(unsigned int numlower) > > return oe; > > } > > > > -bool ovl_dentry_remote(struct dentry *dentry) > > Removed too early. It still has users. > Otherwise looks ok. Thanks, fixed ones pushed to @ovl-remote-upper-v2. Thanks, Miklos ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 4/4] ovl: alllow remote upper 2020-01-31 11:50 [PATCH 0/4] ovl: allow virtiofs as upper Miklos Szeredi ` (2 preceding siblings ...) 2020-01-31 11:50 ` [PATCH 3/4] ovl: decide if revalidate needed on a per-dentry bases Miklos Szeredi @ 2020-01-31 11:50 ` Miklos Szeredi 2020-01-31 15:29 ` Amir Goldstein 2020-02-04 14:59 ` Vivek Goyal 3 siblings, 2 replies; 25+ messages in thread From: Miklos Szeredi @ 2020-01-31 11:50 UTC (permalink / raw) To: linux-unionfs; +Cc: linux-fsdevel, Vivek Goyal No reason to prevent upper layer being a remote filesystem. Do the revalidation in that case, just as we already do for lower layers. This lets virtiofs be used as upper layer, which appears to be a real use case. Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> --- fs/overlayfs/namei.c | 3 +-- fs/overlayfs/super.c | 8 ++++++-- fs/overlayfs/util.c | 2 ++ 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c index 76e61cc27822..0db23baf98e7 100644 --- a/fs/overlayfs/namei.c +++ b/fs/overlayfs/namei.c @@ -845,8 +845,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, if (err) goto out; - if (upperdentry && (upperdentry->d_flags & DCACHE_OP_REAL || - unlikely(ovl_dentry_remote(upperdentry)))) { + if (upperdentry && upperdentry->d_flags & DCACHE_OP_REAL) { dput(upperdentry); err = -EREMOTE; goto out; diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 26d4153240a8..ed3a11db9039 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -135,9 +135,14 @@ static int ovl_dentry_revalidate_common(struct dentry *dentry, unsigned int flags, bool weak) { struct ovl_entry *oe = dentry->d_fsdata; + struct dentry *upper; unsigned int i; int ret = 1; + upper = ovl_dentry_upper(dentry); + if (upper) + ret = ovl_revalidate_real(upper, flags, weak); + for (i = 0; ret > 0 && i < oe->numlower; i++) { ret = ovl_revalidate_real(oe->lowerstack[i].dentry, flags, weak); @@ -747,8 +752,7 @@ static int ovl_mount_dir(const char *name, struct path *path) ovl_unescape(tmp); err = ovl_mount_dir_noesc(tmp, path); - if (!err && (ovl_dentry_remote(path->dentry) || - path->dentry->d_flags & DCACHE_OP_REAL)) { + if (!err && path->dentry->d_flags & DCACHE_OP_REAL) { pr_err("filesystem on '%s' not supported as upperdir\n", tmp); path_put_init(path); diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index 3ad8fb291f7d..c793722739e1 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -96,6 +96,8 @@ void ovl_dentry_update_reval(struct dentry *dentry, struct dentry *upperdentry, struct ovl_entry *oe = OVL_E(dentry); unsigned int i, flags = 0; + if (upperdentry) + flags |= upperdentry->d_flags; for (i = 0; i < oe->numlower; i++) flags |= oe->lowerstack[i].dentry->d_flags; -- 2.21.1 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] ovl: alllow remote upper 2020-01-31 11:50 ` [PATCH 4/4] ovl: alllow remote upper Miklos Szeredi @ 2020-01-31 15:29 ` Amir Goldstein 2020-01-31 15:38 ` Miklos Szeredi 2020-02-04 14:59 ` Vivek Goyal 1 sibling, 1 reply; 25+ messages in thread From: Amir Goldstein @ 2020-01-31 15:29 UTC (permalink / raw) To: Miklos Szeredi; +Cc: overlayfs, linux-fsdevel, Vivek Goyal On Fri, Jan 31, 2020 at 1:51 PM Miklos Szeredi <mszeredi@redhat.com> wrote: > > No reason to prevent upper layer being a remote filesystem. Do the > revalidation in that case, just as we already do for lower layers. > No reason to prevent upper layer from being a remote filesystem, but the !remote criteria for upper fs kept away a lot of filesystems from upper. Those filesystems have never been tested as upper and many of them are probably not fit for upper. The goal is to lift the !remote limitation, not to allow for lots of new types of upper fs's. What can we do to minimize damages? We can assert that is upper is remote, it must qualify for a more strict criteria as upper fs, that is: - support d_type - support xattr - support RENAME_EXCHANGE|RENAME_WHITEOUT I have a patch on branch ovl-strict which implements those restrictions. Now I know fuse doesn't support RENAME_WHITEOUT, but it does support RENAME_EXCHANGE, which already proves to be a very narrow filter for remote fs: afs, fuse, gfs2. Did not check if afs, gfs2 qualify for the rest of the criteria. Is it simple to implement RENAME_WHITEOUT for fuse/virtiofs? Is it not a problem to rely on an upper fs for modern systems that does not support RENAME_WHITEOUT? Thanks, Amir. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] ovl: alllow remote upper 2020-01-31 15:29 ` Amir Goldstein @ 2020-01-31 15:38 ` Miklos Szeredi 2020-01-31 15:50 ` Amir Goldstein 0 siblings, 1 reply; 25+ messages in thread From: Miklos Szeredi @ 2020-01-31 15:38 UTC (permalink / raw) To: Amir Goldstein; +Cc: Miklos Szeredi, overlayfs, linux-fsdevel, Vivek Goyal On Fri, Jan 31, 2020 at 4:30 PM Amir Goldstein <amir73il@gmail.com> wrote: > > On Fri, Jan 31, 2020 at 1:51 PM Miklos Szeredi <mszeredi@redhat.com> wrote: > > > > No reason to prevent upper layer being a remote filesystem. Do the > > revalidation in that case, just as we already do for lower layers. > > > > No reason to prevent upper layer from being a remote filesystem, but > the !remote criteria for upper fs kept away a lot of filesystems from > upper. Those filesystems have never been tested as upper and many > of them are probably not fit for upper. > > The goal is to lift the !remote limitation, not to allow for lots of new > types of upper fs's. > > What can we do to minimize damages? > > We can assert that is upper is remote, it must qualify for a more strict > criteria as upper fs, that is: > - support d_type > - support xattr > - support RENAME_EXCHANGE|RENAME_WHITEOUT > > I have a patch on branch ovl-strict which implements those restrictions. Sounds good. Not sure how much this is this going to be a compatibility headache. If it does, then we can conditionally enable this with a config/module option. > > Now I know fuse doesn't support RENAME_WHITEOUT, but it does > support RENAME_EXCHANGE, which already proves to be a very narrow > filter for remote fs: afs, fuse, gfs2. > Did not check if afs, gfs2 qualify for the rest of the criteria. > > Is it simple to implement RENAME_WHITEOUT for fuse/virtiofs? Trivial. > Is it not a problem to rely on an upper fs for modern systems > that does not support RENAME_WHITEOUT? Limited r/w overlay functionality is still available without whiteout/xattr support, so it could turn out to be something people already rely on. Can't tell without trying... Thanks, Miklos ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] ovl: alllow remote upper 2020-01-31 15:38 ` Miklos Szeredi @ 2020-01-31 15:50 ` Amir Goldstein 2020-01-31 16:05 ` Miklos Szeredi 0 siblings, 1 reply; 25+ messages in thread From: Amir Goldstein @ 2020-01-31 15:50 UTC (permalink / raw) To: Miklos Szeredi; +Cc: Miklos Szeredi, overlayfs, linux-fsdevel, Vivek Goyal On Fri, Jan 31, 2020 at 5:38 PM Miklos Szeredi <miklos@szeredi.hu> wrote: > > On Fri, Jan 31, 2020 at 4:30 PM Amir Goldstein <amir73il@gmail.com> wrote: > > > > On Fri, Jan 31, 2020 at 1:51 PM Miklos Szeredi <mszeredi@redhat.com> wrote: > > > > > > No reason to prevent upper layer being a remote filesystem. Do the > > > revalidation in that case, just as we already do for lower layers. > > > > > > > No reason to prevent upper layer from being a remote filesystem, but > > the !remote criteria for upper fs kept away a lot of filesystems from > > upper. Those filesystems have never been tested as upper and many > > of them are probably not fit for upper. > > > > The goal is to lift the !remote limitation, not to allow for lots of new > > types of upper fs's. > > > > What can we do to minimize damages? > > > > We can assert that is upper is remote, it must qualify for a more strict > > criteria as upper fs, that is: > > - support d_type > > - support xattr > > - support RENAME_EXCHANGE|RENAME_WHITEOUT > > > > I have a patch on branch ovl-strict which implements those restrictions. > > Sounds good. Not sure how much this is this going to be a > compatibility headache. If it does, then we can conditionally enable > this with a config/module option. > No headache at all: - For now, do not change criteria for !remote fs - Only remote fs needs to meet the most strict criteria - We can add the 'strict' config later if we want impose same criteria also for local fs > > > > Now I know fuse doesn't support RENAME_WHITEOUT, but it does > > support RENAME_EXCHANGE, which already proves to be a very narrow > > filter for remote fs: afs, fuse, gfs2. > > Did not check if afs, gfs2 qualify for the rest of the criteria. > > I checked - afs has d_automount and gfs2 is d_hash. They do not qualify as any layer. > > Is it simple to implement RENAME_WHITEOUT for fuse/virtiofs? > > Trivial. > So that leaves only fuse after implementing RENAME_WHITEOUT. We are back in control ;-) Thanks, Amir. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] ovl: alllow remote upper 2020-01-31 15:50 ` Amir Goldstein @ 2020-01-31 16:05 ` Miklos Szeredi 0 siblings, 0 replies; 25+ messages in thread From: Miklos Szeredi @ 2020-01-31 16:05 UTC (permalink / raw) To: Amir Goldstein; +Cc: Miklos Szeredi, overlayfs, linux-fsdevel, Vivek Goyal On Fri, Jan 31, 2020 at 4:50 PM Amir Goldstein <amir73il@gmail.com> wrote: > > I checked - afs has d_automount and gfs2 is d_hash. > They do not qualify as any layer. DCACHE_NEED_AUTOMOUNT doesn't work that way: it's set on specific automount-point dentries only. So AFS won't be disqualified based on that criteria. But afs also does not have RENAME_WHITEOUT, so that's fine. Thanks, MIklos ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] ovl: alllow remote upper 2020-01-31 11:50 ` [PATCH 4/4] ovl: alllow remote upper Miklos Szeredi 2020-01-31 15:29 ` Amir Goldstein @ 2020-02-04 14:59 ` Vivek Goyal 2020-02-04 16:16 ` Miklos Szeredi 1 sibling, 1 reply; 25+ messages in thread From: Vivek Goyal @ 2020-02-04 14:59 UTC (permalink / raw) To: Miklos Szeredi; +Cc: linux-unionfs, linux-fsdevel On Fri, Jan 31, 2020 at 12:50:04PM +0100, Miklos Szeredi wrote: > No reason to prevent upper layer being a remote filesystem. Do the > revalidation in that case, just as we already do for lower layers. > > This lets virtiofs be used as upper layer, which appears to be a real use > case. Hi Miklos, I have couple of very basic questions. - So with this change, we will allow NFS to be upper layer also? - What does revalidation on lower/upper mean? Does that mean that lower/upper can now change underneath overlayfs and overlayfs will cope with it. If we still expect underlying layers not to change, then what's the point of calling ->revalidate(). Thanks Vivek > > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> > --- > fs/overlayfs/namei.c | 3 +-- > fs/overlayfs/super.c | 8 ++++++-- > fs/overlayfs/util.c | 2 ++ > 3 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c > index 76e61cc27822..0db23baf98e7 100644 > --- a/fs/overlayfs/namei.c > +++ b/fs/overlayfs/namei.c > @@ -845,8 +845,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, > if (err) > goto out; > > - if (upperdentry && (upperdentry->d_flags & DCACHE_OP_REAL || > - unlikely(ovl_dentry_remote(upperdentry)))) { > + if (upperdentry && upperdentry->d_flags & DCACHE_OP_REAL) { > dput(upperdentry); > err = -EREMOTE; > goto out; > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c > index 26d4153240a8..ed3a11db9039 100644 > --- a/fs/overlayfs/super.c > +++ b/fs/overlayfs/super.c > @@ -135,9 +135,14 @@ static int ovl_dentry_revalidate_common(struct dentry *dentry, > unsigned int flags, bool weak) > { > struct ovl_entry *oe = dentry->d_fsdata; > + struct dentry *upper; > unsigned int i; > int ret = 1; > > + upper = ovl_dentry_upper(dentry); > + if (upper) > + ret = ovl_revalidate_real(upper, flags, weak); > + > for (i = 0; ret > 0 && i < oe->numlower; i++) { > ret = ovl_revalidate_real(oe->lowerstack[i].dentry, flags, > weak); > @@ -747,8 +752,7 @@ static int ovl_mount_dir(const char *name, struct path *path) > ovl_unescape(tmp); > err = ovl_mount_dir_noesc(tmp, path); > > - if (!err && (ovl_dentry_remote(path->dentry) || > - path->dentry->d_flags & DCACHE_OP_REAL)) { > + if (!err && path->dentry->d_flags & DCACHE_OP_REAL) { > pr_err("filesystem on '%s' not supported as upperdir\n", > tmp); > path_put_init(path); > diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c > index 3ad8fb291f7d..c793722739e1 100644 > --- a/fs/overlayfs/util.c > +++ b/fs/overlayfs/util.c > @@ -96,6 +96,8 @@ void ovl_dentry_update_reval(struct dentry *dentry, struct dentry *upperdentry, > struct ovl_entry *oe = OVL_E(dentry); > unsigned int i, flags = 0; > > + if (upperdentry) > + flags |= upperdentry->d_flags; > for (i = 0; i < oe->numlower; i++) > flags |= oe->lowerstack[i].dentry->d_flags; > > -- > 2.21.1 > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] ovl: alllow remote upper 2020-02-04 14:59 ` Vivek Goyal @ 2020-02-04 16:16 ` Miklos Szeredi 2020-02-04 17:02 ` Amir Goldstein 0 siblings, 1 reply; 25+ messages in thread From: Miklos Szeredi @ 2020-02-04 16:16 UTC (permalink / raw) To: Vivek Goyal; +Cc: Miklos Szeredi, overlayfs, linux-fsdevel On Tue, Feb 4, 2020 at 3:59 PM Vivek Goyal <vgoyal@redhat.com> wrote: > > On Fri, Jan 31, 2020 at 12:50:04PM +0100, Miklos Szeredi wrote: > > No reason to prevent upper layer being a remote filesystem. Do the > > revalidation in that case, just as we already do for lower layers. > > > > This lets virtiofs be used as upper layer, which appears to be a real use > > case. > > Hi Miklos, > > I have couple of very basic questions. > > - So with this change, we will allow NFS to be upper layer also? I haven't tested, but I think it will fail on the d_type test. > - What does revalidation on lower/upper mean? Does that mean that > lower/upper can now change underneath overlayfs and overlayfs will > cope with it. No, that's a more complicated thing. Especially with redirected layers (i.e. revalidating a redirect actually means revalidating all the path components of that redirect). > If we still expect underlying layers not to change, then > what's the point of calling ->revalidate(). That's a good question; I guess because that's what the filesystem expects. OTOH, it's probably unnecessary in most cases, since the path could come from an open file descriptor, in which case the vfs will not do any revalidation on that path. So this is basically done to be on the safe side, but it might not be necessary. Thanks, Miklos > Thanks > Vivek > > > > > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> > > --- > > fs/overlayfs/namei.c | 3 +-- > > fs/overlayfs/super.c | 8 ++++++-- > > fs/overlayfs/util.c | 2 ++ > > 3 files changed, 9 insertions(+), 4 deletions(-) > > > > diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c > > index 76e61cc27822..0db23baf98e7 100644 > > --- a/fs/overlayfs/namei.c > > +++ b/fs/overlayfs/namei.c > > @@ -845,8 +845,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, > > if (err) > > goto out; > > > > - if (upperdentry && (upperdentry->d_flags & DCACHE_OP_REAL || > > - unlikely(ovl_dentry_remote(upperdentry)))) { > > + if (upperdentry && upperdentry->d_flags & DCACHE_OP_REAL) { > > dput(upperdentry); > > err = -EREMOTE; > > goto out; > > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c > > index 26d4153240a8..ed3a11db9039 100644 > > --- a/fs/overlayfs/super.c > > +++ b/fs/overlayfs/super.c > > @@ -135,9 +135,14 @@ static int ovl_dentry_revalidate_common(struct dentry *dentry, > > unsigned int flags, bool weak) > > { > > struct ovl_entry *oe = dentry->d_fsdata; > > + struct dentry *upper; > > unsigned int i; > > int ret = 1; > > > > + upper = ovl_dentry_upper(dentry); > > + if (upper) > > + ret = ovl_revalidate_real(upper, flags, weak); > > + > > for (i = 0; ret > 0 && i < oe->numlower; i++) { > > ret = ovl_revalidate_real(oe->lowerstack[i].dentry, flags, > > weak); > > @@ -747,8 +752,7 @@ static int ovl_mount_dir(const char *name, struct path *path) > > ovl_unescape(tmp); > > err = ovl_mount_dir_noesc(tmp, path); > > > > - if (!err && (ovl_dentry_remote(path->dentry) || > > - path->dentry->d_flags & DCACHE_OP_REAL)) { > > + if (!err && path->dentry->d_flags & DCACHE_OP_REAL) { > > pr_err("filesystem on '%s' not supported as upperdir\n", > > tmp); > > path_put_init(path); > > diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c > > index 3ad8fb291f7d..c793722739e1 100644 > > --- a/fs/overlayfs/util.c > > +++ b/fs/overlayfs/util.c > > @@ -96,6 +96,8 @@ void ovl_dentry_update_reval(struct dentry *dentry, struct dentry *upperdentry, > > struct ovl_entry *oe = OVL_E(dentry); > > unsigned int i, flags = 0; > > > > + if (upperdentry) > > + flags |= upperdentry->d_flags; > > for (i = 0; i < oe->numlower; i++) > > flags |= oe->lowerstack[i].dentry->d_flags; > > > > -- > > 2.21.1 > > > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] ovl: alllow remote upper 2020-02-04 16:16 ` Miklos Szeredi @ 2020-02-04 17:02 ` Amir Goldstein 2020-02-04 18:42 ` Vivek Goyal 2020-02-20 7:52 ` Amir Goldstein 0 siblings, 2 replies; 25+ messages in thread From: Amir Goldstein @ 2020-02-04 17:02 UTC (permalink / raw) To: Miklos Szeredi; +Cc: Vivek Goyal, Miklos Szeredi, overlayfs, linux-fsdevel On Tue, Feb 4, 2020 at 6:17 PM Miklos Szeredi <miklos@szeredi.hu> wrote: > > On Tue, Feb 4, 2020 at 3:59 PM Vivek Goyal <vgoyal@redhat.com> wrote: > > > > On Fri, Jan 31, 2020 at 12:50:04PM +0100, Miklos Szeredi wrote: > > > No reason to prevent upper layer being a remote filesystem. Do the > > > revalidation in that case, just as we already do for lower layers. > > > > > > This lets virtiofs be used as upper layer, which appears to be a real use > > > case. > > > > Hi Miklos, > > > > I have couple of very basic questions. > > > > - So with this change, we will allow NFS to be upper layer also? > > I haven't tested, but I think it will fail on the d_type test. But we do not fail mount on no d_type support... Besides, I though you were going to add the RENAME_WHITEOUT test to avert untested network fs as upper. > > > - What does revalidation on lower/upper mean? Does that mean that > > lower/upper can now change underneath overlayfs and overlayfs will > > cope with it. > > No, that's a more complicated thing. Especially with redirected > layers (i.e. revalidating a redirect actually means revalidating all > the path components of that redirect). > > > If we still expect underlying layers not to change, then > > what's the point of calling ->revalidate(). > > That's a good question; I guess because that's what the filesystem > expects. OTOH, it's probably unnecessary in most cases, since the > path could come from an open file descriptor, in which case the vfs > will not do any revalidation on that path. > Note that ovl_dentry_revalidate() never returns 0 and therefore, vfs will never actually redo the lookup, but instead return -ESTALE to userspace. Right? This makes some sense considering that underlying layers are not expected to change. The question is, with virtiofs, can we know that the server/host will not invalidate entries? And if it does, should it cause a permanent error in overlayfs or a transient error? If we do not want a permanent error, then ->revalidate() needs to be called to invalidate the overlay dentry. No? Thanks, Amir. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] ovl: alllow remote upper 2020-02-04 17:02 ` Amir Goldstein @ 2020-02-04 18:42 ` Vivek Goyal 2020-02-04 19:11 ` Amir Goldstein 2020-02-20 7:52 ` Amir Goldstein 1 sibling, 1 reply; 25+ messages in thread From: Vivek Goyal @ 2020-02-04 18:42 UTC (permalink / raw) To: Amir Goldstein; +Cc: Miklos Szeredi, Miklos Szeredi, overlayfs, linux-fsdevel On Tue, Feb 04, 2020 at 07:02:05PM +0200, Amir Goldstein wrote: > On Tue, Feb 4, 2020 at 6:17 PM Miklos Szeredi <miklos@szeredi.hu> wrote: > > > > On Tue, Feb 4, 2020 at 3:59 PM Vivek Goyal <vgoyal@redhat.com> wrote: > > > > > > On Fri, Jan 31, 2020 at 12:50:04PM +0100, Miklos Szeredi wrote: > > > > No reason to prevent upper layer being a remote filesystem. Do the > > > > revalidation in that case, just as we already do for lower layers. > > > > > > > > This lets virtiofs be used as upper layer, which appears to be a real use > > > > case. > > > > > > Hi Miklos, > > > > > > I have couple of very basic questions. > > > > > > - So with this change, we will allow NFS to be upper layer also? > > > > I haven't tested, but I think it will fail on the d_type test. > > But we do not fail mount on no d_type support... > Besides, I though you were going to add the RENAME_WHITEOUT > test to avert untested network fs as upper. > > > > > > - What does revalidation on lower/upper mean? Does that mean that > > > lower/upper can now change underneath overlayfs and overlayfs will > > > cope with it. > > > > No, that's a more complicated thing. Especially with redirected > > layers (i.e. revalidating a redirect actually means revalidating all > > the path components of that redirect). > > > > > If we still expect underlying layers not to change, then > > > what's the point of calling ->revalidate(). > > > > That's a good question; I guess because that's what the filesystem > > expects. OTOH, it's probably unnecessary in most cases, since the > > path could come from an open file descriptor, in which case the vfs > > will not do any revalidation on that path. > > > > Note that ovl_dentry_revalidate() never returns 0 and therefore, vfs > will never actually redo the lookup, but instead return -ESTALE > to userspace. Right? This makes some sense considering that underlying > layers are not expected to change. > > The question is, with virtiofs, can we know that the server/host will not > invalidate entries? I don't think virtiofs will ensure that server/host will not invalidate entries. It will be more of a configuration thing where we will expect users to configure things in such a way that shared directory does not change. So that means, if user does not configure it properly and things change unexpectedly, then overlayfs should be able to detect it and flag error to user space? > And if it does, should it cause a permanent error > in overlayfs or a transient error? If we do not want a permanent error, > then ->revalidate() needs to be called to invalidate the overlay dentry. No? So as of now user space will get -ESTALE and that will get cleared when user space retries after corresponding ovl dentry has been dropped from cache (either dentry is evicted, cache is cleared forcibly or overlayfs is remounted)? If yes, that kind of makes sense. Overlay does not expect underlying layers to change and if a change it detected it is flagged to user space (and overlayfs does not try to fix it)? Thanks Vivek ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] ovl: alllow remote upper 2020-02-04 18:42 ` Vivek Goyal @ 2020-02-04 19:11 ` Amir Goldstein 2020-02-04 19:16 ` Miklos Szeredi 0 siblings, 1 reply; 25+ messages in thread From: Amir Goldstein @ 2020-02-04 19:11 UTC (permalink / raw) To: Vivek Goyal; +Cc: Miklos Szeredi, Miklos Szeredi, overlayfs, linux-fsdevel On Tue, Feb 4, 2020 at 8:42 PM Vivek Goyal <vgoyal@redhat.com> wrote: > > On Tue, Feb 04, 2020 at 07:02:05PM +0200, Amir Goldstein wrote: > > On Tue, Feb 4, 2020 at 6:17 PM Miklos Szeredi <miklos@szeredi.hu> wrote: > > > > > > On Tue, Feb 4, 2020 at 3:59 PM Vivek Goyal <vgoyal@redhat.com> wrote: > > > > > > > > On Fri, Jan 31, 2020 at 12:50:04PM +0100, Miklos Szeredi wrote: > > > > > No reason to prevent upper layer being a remote filesystem. Do the > > > > > revalidation in that case, just as we already do for lower layers. > > > > > > > > > > This lets virtiofs be used as upper layer, which appears to be a real use > > > > > case. > > > > > > > > Hi Miklos, > > > > > > > > I have couple of very basic questions. > > > > > > > > - So with this change, we will allow NFS to be upper layer also? > > > > > > I haven't tested, but I think it will fail on the d_type test. > > > > But we do not fail mount on no d_type support... > > Besides, I though you were going to add the RENAME_WHITEOUT > > test to avert untested network fs as upper. > > > > > > > > > - What does revalidation on lower/upper mean? Does that mean that > > > > lower/upper can now change underneath overlayfs and overlayfs will > > > > cope with it. > > > > > > No, that's a more complicated thing. Especially with redirected > > > layers (i.e. revalidating a redirect actually means revalidating all > > > the path components of that redirect). > > > > > > > If we still expect underlying layers not to change, then > > > > what's the point of calling ->revalidate(). > > > > > > That's a good question; I guess because that's what the filesystem > > > expects. OTOH, it's probably unnecessary in most cases, since the > > > path could come from an open file descriptor, in which case the vfs > > > will not do any revalidation on that path. > > > > > > > Note that ovl_dentry_revalidate() never returns 0 and therefore, vfs > > will never actually redo the lookup, but instead return -ESTALE > > to userspace. Right? This makes some sense considering that underlying > > layers are not expected to change. > > > > The question is, with virtiofs, can we know that the server/host will not > > invalidate entries? > > I don't think virtiofs will ensure that server/host will not invalidate > entries. It will be more of a configuration thing where we will expect > users to configure things in such a way that shared directory does not > change. > > So that means, if user does not configure it properly and things change > unexpectedly, then overlayfs should be able to detect it and flag error > to user space? > > > And if it does, should it cause a permanent error > > in overlayfs or a transient error? If we do not want a permanent error, > > then ->revalidate() needs to be called to invalidate the overlay dentry. No? > > So as of now user space will get -ESTALE and that will get cleared when > user space retries after corresponding ovl dentry has been dropped from > cache (either dentry is evicted, cache is cleared forcibly or overlayfs > is remounted)? If yes, that kind of makes sense. Overlay does not expect > underlying layers to change and if a change it detected it is flagged > to user space (and overlayfs does not try to fix it)? > I looks like it. I don't really understand why overlayfs shouldn't drop the dentry on failure to revalidate. Maybe I am missing something. Thanks, Amir. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] ovl: alllow remote upper 2020-02-04 19:11 ` Amir Goldstein @ 2020-02-04 19:16 ` Miklos Szeredi 0 siblings, 0 replies; 25+ messages in thread From: Miklos Szeredi @ 2020-02-04 19:16 UTC (permalink / raw) To: Amir Goldstein; +Cc: Vivek Goyal, Miklos Szeredi, overlayfs, linux-fsdevel On Tue, Feb 4, 2020 at 8:11 PM Amir Goldstein <amir73il@gmail.com> wrote: > > On Tue, Feb 4, 2020 at 8:42 PM Vivek Goyal <vgoyal@redhat.com> wrote: > > So as of now user space will get -ESTALE and that will get cleared when > > user space retries after corresponding ovl dentry has been dropped from > > cache (either dentry is evicted, cache is cleared forcibly or overlayfs > > is remounted)? If yes, that kind of makes sense. Overlay does not expect > > underlying layers to change and if a change it detected it is flagged > > to user space (and overlayfs does not try to fix it)? > > > > I looks like it. I don't really understand why overlayfs shouldn't drop > the dentry on failure to revalidate. Maybe I am missing something. I don't remember the exact reason. Maybe it's just that it makes little sense to redo the lookup on remote change, but not on local change... Note: I'm not against detection of changing underlying layers and redoing the lookup in that case. Maybe we can do it optionally (because it could be expensive), but first there needs to be a use case and we seem to lack that. Thanks, Miklos ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] ovl: alllow remote upper 2020-02-04 17:02 ` Amir Goldstein 2020-02-04 18:42 ` Vivek Goyal @ 2020-02-20 7:52 ` Amir Goldstein 2020-02-20 20:00 ` Amir Goldstein 1 sibling, 1 reply; 25+ messages in thread From: Amir Goldstein @ 2020-02-20 7:52 UTC (permalink / raw) To: Miklos Szeredi; +Cc: Vivek Goyal, Miklos Szeredi, overlayfs, linux-fsdevel On Tue, Feb 4, 2020 at 7:02 PM Amir Goldstein <amir73il@gmail.com> wrote: > > On Tue, Feb 4, 2020 at 6:17 PM Miklos Szeredi <miklos@szeredi.hu> wrote: > > > > On Tue, Feb 4, 2020 at 3:59 PM Vivek Goyal <vgoyal@redhat.com> wrote: > > > > > > On Fri, Jan 31, 2020 at 12:50:04PM +0100, Miklos Szeredi wrote: > > > > No reason to prevent upper layer being a remote filesystem. Do the > > > > revalidation in that case, just as we already do for lower layers. > > > > > > > > This lets virtiofs be used as upper layer, which appears to be a real use > > > > case. > > > > > > Hi Miklos, > > > > > > I have couple of very basic questions. > > > > > > - So with this change, we will allow NFS to be upper layer also? > > > > I haven't tested, but I think it will fail on the d_type test. > > But we do not fail mount on no d_type support... > Besides, I though you were going to add the RENAME_WHITEOUT > test to avert untested network fs as upper. > Pushed strict remote upper check to: https://github.com/amir73il/linux/commits/ovl-strict-upper FWIW, overlayfs-next+ovl-strict-upper passes the quick xfstests, except for overlay/031 - it fails because the RENAME_WHITEOUT check leaves behind a whiteout in workdir. I think it it is not worth to cleanup that whiteout leftover and easier to fix the test. Thanks, Amir. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] ovl: alllow remote upper 2020-02-20 7:52 ` Amir Goldstein @ 2020-02-20 20:00 ` Amir Goldstein 2020-03-14 13:16 ` Amir Goldstein 0 siblings, 1 reply; 25+ messages in thread From: Amir Goldstein @ 2020-02-20 20:00 UTC (permalink / raw) To: Miklos Szeredi; +Cc: Vivek Goyal, Miklos Szeredi, overlayfs, linux-fsdevel On Thu, Feb 20, 2020 at 9:52 AM Amir Goldstein <amir73il@gmail.com> wrote: > > On Tue, Feb 4, 2020 at 7:02 PM Amir Goldstein <amir73il@gmail.com> wrote: > > > > On Tue, Feb 4, 2020 at 6:17 PM Miklos Szeredi <miklos@szeredi.hu> wrote: > > > > > > On Tue, Feb 4, 2020 at 3:59 PM Vivek Goyal <vgoyal@redhat.com> wrote: > > > > > > > > On Fri, Jan 31, 2020 at 12:50:04PM +0100, Miklos Szeredi wrote: > > > > > No reason to prevent upper layer being a remote filesystem. Do the > > > > > revalidation in that case, just as we already do for lower layers. > > > > > > > > > > This lets virtiofs be used as upper layer, which appears to be a real use > > > > > case. > > > > > > > > Hi Miklos, > > > > > > > > I have couple of very basic questions. > > > > > > > > - So with this change, we will allow NFS to be upper layer also? > > > > > > I haven't tested, but I think it will fail on the d_type test. > > > > But we do not fail mount on no d_type support... > > Besides, I though you were going to add the RENAME_WHITEOUT > > test to avert untested network fs as upper. > > > > Pushed strict remote upper check to: > https://github.com/amir73il/linux/commits/ovl-strict-upper > > FWIW, overlayfs-next+ovl-strict-upper passes the quick xfstests, > except for overlay/031 - it fails because the RENAME_WHITEOUT check > leaves behind a whiteout in workdir. > I think it it is not worth to cleanup that whiteout leftover and > easier to fix the test. Nevermind. Fixed the whiteout cleanup and re-pushed. Thanks, Amir. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] ovl: alllow remote upper 2020-02-20 20:00 ` Amir Goldstein @ 2020-03-14 13:16 ` Amir Goldstein 2020-03-16 17:54 ` Vivek Goyal 0 siblings, 1 reply; 25+ messages in thread From: Amir Goldstein @ 2020-03-14 13:16 UTC (permalink / raw) To: Miklos Szeredi, Vivek Goyal; +Cc: Miklos Szeredi, overlayfs, linux-fsdevel On Thu, Feb 20, 2020 at 10:00 PM Amir Goldstein <amir73il@gmail.com> wrote: > > On Thu, Feb 20, 2020 at 9:52 AM Amir Goldstein <amir73il@gmail.com> wrote: > > > > On Tue, Feb 4, 2020 at 7:02 PM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > On Tue, Feb 4, 2020 at 6:17 PM Miklos Szeredi <miklos@szeredi.hu> wrote: > > > > > > > > On Tue, Feb 4, 2020 at 3:59 PM Vivek Goyal <vgoyal@redhat.com> wrote: > > > > > > > > > > On Fri, Jan 31, 2020 at 12:50:04PM +0100, Miklos Szeredi wrote: > > > > > > No reason to prevent upper layer being a remote filesystem. Do the > > > > > > revalidation in that case, just as we already do for lower layers. > > > > > > > > > > > > This lets virtiofs be used as upper layer, which appears to be a real use > > > > > > case. > > > > > > > > > > Hi Miklos, > > > > > > > > > > I have couple of very basic questions. > > > > > > > > > > - So with this change, we will allow NFS to be upper layer also? > > > > > > > > I haven't tested, but I think it will fail on the d_type test. > > > > > > But we do not fail mount on no d_type support... > > > Besides, I though you were going to add the RENAME_WHITEOUT > > > test to avert untested network fs as upper. > > > > > > > Pushed strict remote upper check to: > > https://github.com/amir73il/linux/commits/ovl-strict-upper > > Vivek, Could you please make sure that the code in ovl-strict-upper branch works as expected for virtio as upper fs? I have rebased it on latest overlayfs-next merge into current master. I would very much prefer that the code merged to v5.7-rc1 will be more restrictive than the current overlayfs-next. Thanks, Amir. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] ovl: alllow remote upper 2020-03-14 13:16 ` Amir Goldstein @ 2020-03-16 17:54 ` Vivek Goyal 2020-03-16 19:02 ` Amir Goldstein 0 siblings, 1 reply; 25+ messages in thread From: Vivek Goyal @ 2020-03-16 17:54 UTC (permalink / raw) To: Amir Goldstein; +Cc: Miklos Szeredi, Miklos Szeredi, overlayfs, linux-fsdevel On Sat, Mar 14, 2020 at 03:16:28PM +0200, Amir Goldstein wrote: > On Thu, Feb 20, 2020 at 10:00 PM Amir Goldstein <amir73il@gmail.com> wrote: > > > > On Thu, Feb 20, 2020 at 9:52 AM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > On Tue, Feb 4, 2020 at 7:02 PM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > > > On Tue, Feb 4, 2020 at 6:17 PM Miklos Szeredi <miklos@szeredi.hu> wrote: > > > > > > > > > > On Tue, Feb 4, 2020 at 3:59 PM Vivek Goyal <vgoyal@redhat.com> wrote: > > > > > > > > > > > > On Fri, Jan 31, 2020 at 12:50:04PM +0100, Miklos Szeredi wrote: > > > > > > > No reason to prevent upper layer being a remote filesystem. Do the > > > > > > > revalidation in that case, just as we already do for lower layers. > > > > > > > > > > > > > > This lets virtiofs be used as upper layer, which appears to be a real use > > > > > > > case. > > > > > > > > > > > > Hi Miklos, > > > > > > > > > > > > I have couple of very basic questions. > > > > > > > > > > > > - So with this change, we will allow NFS to be upper layer also? > > > > > > > > > > I haven't tested, but I think it will fail on the d_type test. > > > > > > > > But we do not fail mount on no d_type support... > > > > Besides, I though you were going to add the RENAME_WHITEOUT > > > > test to avert untested network fs as upper. > > > > > > > > > > Pushed strict remote upper check to: > > > https://github.com/amir73il/linux/commits/ovl-strict-upper > > > > > Vivek, > > Could you please make sure that the code in ovl-strict-upper branch > works as expected for virtio as upper fs? Hi Amir, Right now it fails becuase virtiofs doesn't seem to support tmpfile yet. overlayfs: upper fs does not support tmpfile overlayfs: upper fs missing required features. Will have to check what's required to support it. I also wanted to run either overlay xfstests or unionmount-testsuite. But none of these seem to give me enough flexibility where I can specify that overlayfs needs to be mounted on top of virtiofs. I feel that atleast for unionmount-testsuite, there should be an option where we can simply give a target directory and tests run on that directory and user mounts that directory as needed. > I have rebased it on latest overlayfs-next merge into current master. > > I would very much prefer that the code merged to v5.7-rc1 will be more > restrictive than the current overlayfs-next. In general I agree that if we want to not support some configuration with remote upper, this is the time to introduce that restriction otherwise we will later run into backward compatibility issue. Having said that, tmpfile support for upper sounds like a nice to have feature. Not sure why to make it mandatory. Vivek ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] ovl: alllow remote upper 2020-03-16 17:54 ` Vivek Goyal @ 2020-03-16 19:02 ` Amir Goldstein 2020-03-16 19:40 ` Vivek Goyal 2020-03-18 13:36 ` unionmount testsuite with upper virtiofs Amir Goldstein 0 siblings, 2 replies; 25+ messages in thread From: Amir Goldstein @ 2020-03-16 19:02 UTC (permalink / raw) To: Vivek Goyal; +Cc: Miklos Szeredi, Miklos Szeredi, overlayfs, linux-fsdevel On Mon, Mar 16, 2020 at 8:15 PM Vivek Goyal <vgoyal@redhat.com> wrote: > > On Sat, Mar 14, 2020 at 03:16:28PM +0200, Amir Goldstein wrote: > > On Thu, Feb 20, 2020 at 10:00 PM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > On Thu, Feb 20, 2020 at 9:52 AM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > > > On Tue, Feb 4, 2020 at 7:02 PM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > > > > > On Tue, Feb 4, 2020 at 6:17 PM Miklos Szeredi <miklos@szeredi.hu> wrote: > > > > > > > > > > > > On Tue, Feb 4, 2020 at 3:59 PM Vivek Goyal <vgoyal@redhat.com> wrote: > > > > > > > > > > > > > > On Fri, Jan 31, 2020 at 12:50:04PM +0100, Miklos Szeredi wrote: > > > > > > > > No reason to prevent upper layer being a remote filesystem. Do the > > > > > > > > revalidation in that case, just as we already do for lower layers. > > > > > > > > > > > > > > > > This lets virtiofs be used as upper layer, which appears to be a real use > > > > > > > > case. > > > > > > > > > > > > > > Hi Miklos, > > > > > > > > > > > > > > I have couple of very basic questions. > > > > > > > > > > > > > > - So with this change, we will allow NFS to be upper layer also? > > > > > > > > > > > > I haven't tested, but I think it will fail on the d_type test. > > > > > > > > > > But we do not fail mount on no d_type support... > > > > > Besides, I though you were going to add the RENAME_WHITEOUT > > > > > test to avert untested network fs as upper. > > > > > > > > > > > > > Pushed strict remote upper check to: > > > > https://github.com/amir73il/linux/commits/ovl-strict-upper > > > > > > > > Vivek, > > > > Could you please make sure that the code in ovl-strict-upper branch > > works as expected for virtio as upper fs? > > Hi Amir, > > Right now it fails becuase virtiofs doesn't seem to support tmpfile yet. > > overlayfs: upper fs does not support tmpfile > overlayfs: upper fs missing required features. > > Will have to check what's required to support it. > > I also wanted to run either overlay xfstests or unionmount-testsuite. But > none of these seem to give me enough flexibility where I can specify > that overlayfs needs to be mounted on top of virtiofs. > > I feel that atleast for unionmount-testsuite, there should be an > option where we can simply give a target directory and tests run > on that directory and user mounts that directory as needed. > Need to see how patches look. Don't want too much configuration complexity, but I agree that some flexibly is needed. Maybe the provided target directory should be the upper/work basedir? > > I have rebased it on latest overlayfs-next merge into current master. > > > > I would very much prefer that the code merged to v5.7-rc1 will be more > > restrictive than the current overlayfs-next. > > In general I agree that if we want to not support some configuration > with remote upper, this is the time to introduce that restriction > otherwise we will later run into backward compatibility issue. > > Having said that, tmpfile support for upper sounds like a nice to > have feature. Not sure why to make it mandatory. > Agreed, I just went automatic on all the warnings. tmpfile should not be a requirement for upper. Could you please verify that if dropping the tmpfile strict check, virtio can be used as upper. Thanks, Amir. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/4] ovl: alllow remote upper 2020-03-16 19:02 ` Amir Goldstein @ 2020-03-16 19:40 ` Vivek Goyal 2020-03-18 13:36 ` unionmount testsuite with upper virtiofs Amir Goldstein 1 sibling, 0 replies; 25+ messages in thread From: Vivek Goyal @ 2020-03-16 19:40 UTC (permalink / raw) To: Amir Goldstein; +Cc: Miklos Szeredi, Miklos Szeredi, overlayfs, linux-fsdevel On Mon, Mar 16, 2020 at 09:02:32PM +0200, Amir Goldstein wrote: [..] > > > Could you please make sure that the code in ovl-strict-upper branch > > > works as expected for virtio as upper fs? > > > > Hi Amir, > > > > Right now it fails becuase virtiofs doesn't seem to support tmpfile yet. > > > > overlayfs: upper fs does not support tmpfile > > overlayfs: upper fs missing required features. > > > > Will have to check what's required to support it. > > > > I also wanted to run either overlay xfstests or unionmount-testsuite. But > > none of these seem to give me enough flexibility where I can specify > > that overlayfs needs to be mounted on top of virtiofs. > > > > I feel that atleast for unionmount-testsuite, there should be an > > option where we can simply give a target directory and tests run > > on that directory and user mounts that directory as needed. > > > > Need to see how patches look. > Don't want too much configuration complexity, but I agree that some > flexibly is needed. > Maybe the provided target directory should be the upper/work basedir? > > > > I have rebased it on latest overlayfs-next merge into current master. > > > > > > I would very much prefer that the code merged to v5.7-rc1 will be more > > > restrictive than the current overlayfs-next. > > > > In general I agree that if we want to not support some configuration > > with remote upper, this is the time to introduce that restriction > > otherwise we will later run into backward compatibility issue. > > > > Having said that, tmpfile support for upper sounds like a nice to > > have feature. Not sure why to make it mandatory. > > > > Agreed, I just went automatic on all the warnings. > tmpfile should not be a requirement for upper. > Could you please verify that if dropping the tmpfile strict check, > virtio can be used as upper. I dropped tmpfile strict check and now I can mount overlayfs using virtiofs as upper. Tried few basic file operations and these are working. Vivek ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: unionmount testsuite with upper virtiofs 2020-03-16 19:02 ` Amir Goldstein 2020-03-16 19:40 ` Vivek Goyal @ 2020-03-18 13:36 ` Amir Goldstein 2020-03-19 21:40 ` Vivek Goyal 1 sibling, 1 reply; 25+ messages in thread From: Amir Goldstein @ 2020-03-18 13:36 UTC (permalink / raw) To: Vivek Goyal; +Cc: Miklos Szeredi, Miklos Szeredi, overlayfs, linux-fsdevel > > I also wanted to run either overlay xfstests or unionmount-testsuite. But > > none of these seem to give me enough flexibility where I can specify > > that overlayfs needs to be mounted on top of virtiofs. > > > > I feel that atleast for unionmount-testsuite, there should be an > > option where we can simply give a target directory and tests run > > on that directory and user mounts that directory as needed. > > > > Need to see how patches look. > Don't want too much configuration complexity, but I agree that some > flexibly is needed. > Maybe the provided target directory should be the upper/work basedir? > Vivek, I was going to see what's the best way to add the needed flexibility, but then I realized I had already implemented this undocumented feature. I have been using this to test overlay over XFS as documented here: https://github.com/amir73il/overlayfs/wiki/Overlayfs-testing#Setup_overlayfs_mount_over_XFS_with_reflink_support That's an example of how to configure a custom /base mount for --samefs to be xfs. Similar hidden feature exists for configuring a custom /lower and /upper mounts via fstab, but I don't think I ever tested those, so not sure if they work as expected. unionmount testsuite will first try to mount the entry from fstab and fallback to mounting tmpfs. I admit this a lousy configuration method, but we could make it official using env vars or something. I also never liked the fact that unionmount testsuite hard codes the /lower /upper /mnt paths. The reason we 'need' the instructions how to mount the fs as opposed to an already mounted dir is that unmounting the underlying fs exposes dentry/inode reference leaks by overlayfs. But it is nice to have and xfstests has support for configuring an already mounted overlayfs for the generic tests. So if you think that you cannot use the existing hack and that pointing to an already mounted /upper or mounted overlay is needed, I suggest that you experiment with patches to make that change. Thanks, Amir. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: unionmount testsuite with upper virtiofs 2020-03-18 13:36 ` unionmount testsuite with upper virtiofs Amir Goldstein @ 2020-03-19 21:40 ` Vivek Goyal 0 siblings, 0 replies; 25+ messages in thread From: Vivek Goyal @ 2020-03-19 21:40 UTC (permalink / raw) To: Amir Goldstein; +Cc: Miklos Szeredi, Miklos Szeredi, overlayfs, linux-fsdevel On Wed, Mar 18, 2020 at 03:36:44PM +0200, Amir Goldstein wrote: > > > I also wanted to run either overlay xfstests or unionmount-testsuite. But > > > none of these seem to give me enough flexibility where I can specify > > > that overlayfs needs to be mounted on top of virtiofs. > > > > > > I feel that atleast for unionmount-testsuite, there should be an > > > option where we can simply give a target directory and tests run > > > on that directory and user mounts that directory as needed. > > > > > > > Need to see how patches look. > > Don't want too much configuration complexity, but I agree that some > > flexibly is needed. > > Maybe the provided target directory should be the upper/work basedir? > > > > Vivek, > > I was going to see what's the best way to add the needed flexibility, > but then I realized I had already implemented this undocumented > feature. > > I have been using this to test overlay over XFS as documented here: > https://github.com/amir73il/overlayfs/wiki/Overlayfs-testing#Setup_overlayfs_mount_over_XFS_with_reflink_support > > That's an example of how to configure a custom /base mount for > --samefs to be xfs. Hi Amir, This seems to work for me. Thanks for the idea. I put following entries in /etc/fstab. myfs /mnt/virtiofs-lower_layer virtiofs defaults 0 0 /mnt/virtiofs-lower_layer /base none bind 0 0 After that tests seem to start but soon I hit failures. Now its time to debug the failures one at a time. Vivek ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2020-03-19 21:40 UTC | newest] Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-01-31 11:50 [PATCH 0/4] ovl: allow virtiofs as upper Miklos Szeredi 2020-01-31 11:50 ` [PATCH 1/4] ovl: restructure dentry revalidation Miklos Szeredi 2020-01-31 11:50 ` [PATCH 2/4] ovl: separate detection of remote upper layer from stacked overlay Miklos Szeredi 2020-01-31 11:50 ` [PATCH 3/4] ovl: decide if revalidate needed on a per-dentry bases Miklos Szeredi 2020-01-31 14:53 ` Amir Goldstein 2020-01-31 15:15 ` Miklos Szeredi 2020-01-31 11:50 ` [PATCH 4/4] ovl: alllow remote upper Miklos Szeredi 2020-01-31 15:29 ` Amir Goldstein 2020-01-31 15:38 ` Miklos Szeredi 2020-01-31 15:50 ` Amir Goldstein 2020-01-31 16:05 ` Miklos Szeredi 2020-02-04 14:59 ` Vivek Goyal 2020-02-04 16:16 ` Miklos Szeredi 2020-02-04 17:02 ` Amir Goldstein 2020-02-04 18:42 ` Vivek Goyal 2020-02-04 19:11 ` Amir Goldstein 2020-02-04 19:16 ` Miklos Szeredi 2020-02-20 7:52 ` Amir Goldstein 2020-02-20 20:00 ` Amir Goldstein 2020-03-14 13:16 ` Amir Goldstein 2020-03-16 17:54 ` Vivek Goyal 2020-03-16 19:02 ` Amir Goldstein 2020-03-16 19:40 ` Vivek Goyal 2020-03-18 13:36 ` unionmount testsuite with upper virtiofs Amir Goldstein 2020-03-19 21:40 ` Vivek Goyal
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).