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

[Vivek Goyal <vgoyal@redhat.com>]

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