Re: [PATCH 4/4] ovl: alllow remote upper

[Miklos Szeredi <miklos@szeredi.hu>]

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