Re: overlayfs NFS export

Re: overlayfs NFS export

[Amir Goldstein]

[changing the subject and adding more NFS guys so they can shoot my
idea down if it is too dumb to live]

On Fri, Apr 7, 2017 at 4:03 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Fri, Apr 7, 2017 at 12:47 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>
>> Come to think about it, NFS export of regular file don't need to
>> follow renames at all:
>> - The handle for a regular file is always the handle for the real
>> lower or upper inode
>> - To decode a handle, create an O_TMPFILE style overlay dentry, which
>> is not linked
>>   to any path in overlay, but has the _upperdentry/lowerstack setup
>
> I don't think nfs will allow such a scheme.  NFS3 server is stateless,
> which means there's no open/close in the protocol.   Hence we can't
> copy-up on open(O_WR*) and return a different file handle for writing.
> If client looks up a file currently on lower and we return file handle
> based on lower file, then we must be able to decode that handle after
> the file has been copied up and even after rename.  And this must work
> reliably even if the overlay dentry is no longer in the dcache.
>
> So there's no option, other than to have a reverse mapping somewhere.
>

Either I am missing something or you are.

Consider this scenario:

On server:
- touch a
- ln a b

On NFS client:
- rofd = open("a", O_RDONLY)

On server
- rm a
- reboot

NFS client must be able to continue to work with rofd
even after reboot and even after original file was unlinked.

Furthermore, on server:
- rm b

NFS client must continue to work with rofd even though
NFS server is stateless and even though inode is now
nlink = 0.
That is possible because fs will instantiate a disconnected
dentry when decoding the file handle is there is no dentry
already in cache.

So what I am saying is that when nfsd tries to decode
a handle from overlay mount, and there is no mathcing
overlay dentry in cache (with the lower ino of course)
then we instantiate a new disconnected dentry without
lookup and set its _upperdenry or lowerstack according
to the knowledge that we found the handle in underlying
fs and we checked if it is a decedent of lower_mnt[i] or
upper_mnt.

When NFS client opens a new rwfd it WILL get a different
handle, but it WILL really be a different file then the rofd,
so that sounds like a good thing?

I realize it may sound complicated, but redirect_fh patch
has already all the needed parts in place for this, so the
proof if what I am saying is right or wrong will be in whether
or not I am able to present a working POC...

Amir.

Re: overlayfs NFS export

[Jeff Layton]

On Fri, 2017-04-07 at 17:29 +0300, Amir Goldstein wrote:
> [changing the subject and adding more NFS guys so they can shoot my
> idea down if it is too dumb to live]
> 
> On Fri, Apr 7, 2017 at 4:03 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> > On Fri, Apr 7, 2017 at 12:47 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> > 
> > > Come to think about it, NFS export of regular file don't need to
> > > follow renames at all:
> > > - The handle for a regular file is always the handle for the real
> > > lower or upper inode
> > > - To decode a handle, create an O_TMPFILE style overlay dentry, which
> > > is not linked
> > >   to any path in overlay, but has the _upperdentry/lowerstack setup
> > 
> > I don't think nfs will allow such a scheme.  NFS3 server is stateless,
> > which means there's no open/close in the protocol.   Hence we can't
> > copy-up on open(O_WR*) and return a different file handle for writing.
> > If client looks up a file currently on lower and we return file handle
> > based on lower file, then we must be able to decode that handle after
> > the file has been copied up and even after rename.  And this must work
> > reliably even if the overlay dentry is no longer in the dcache.
> > 
> > So there's no option, other than to have a reverse mapping somewhere.
> > 
> 
> Either I am missing something or you are.
> 
> Consider this scenario:
> 
> On server:
> - touch a
> - ln a b
> 
> On NFS client:
> - rofd = open("a", O_RDONLY)
> 
> On server
> - rm a
> - reboot
> 
> NFS client must be able to continue to work with rofd
> even after reboot and even after original file was unlinked.
> 

Sure, it'll still work here. The inode still exists because of "b".

> Furthermore, on server:
> - rm b
> 
> NFS client must continue to work with rofd even though
> NFS server is stateless and even though inode is now
> nlink = 0.

No, at that point the server will probably just kill the inode and
you'll start getting back -ESTALE when you try to use that filehandle.
With NFSv2/3, there is no guarantee that the server will have bothered
to open it after reboot but before you removed it here.

> That is possible because fs will instantiate a disconnected
> dentry when decoding the file handle is there is no dentry
> already in cache.
> 
> So what I am saying is that when nfsd tries to decode
> a handle from overlay mount, and there is no mathcing
> overlay dentry in cache (with the lower ino of course)
> then we instantiate a new disconnected dentry without
> lookup and set its _upperdenry or lowerstack according
> to the knowledge that we found the handle in underlying
> fs and we checked if it is a decedent of lower_mnt[i] or
> upper_mnt.
> 
> When NFS client opens a new rwfd it WILL get a different
> handle, but it WILL really be a different file then the rofd,
> so that sounds like a good thing?
> 

An "open" with NFSv3 is basically a NFS LOOKUP or maybe a GETATTR call.
Again, the protocol is stateless, so in practice we open the inode each
time we service a READ or WRITE RPC.

How is knfsd to know that you want to access the r/o or r/w inode there
and thus give you the right filehandle?

> I realize it may sound complicated, but redirect_fh patch
> has already all the needed parts in place for this, so the
> proof if what I am saying is right or wrong will be in whether
> or not I am able to present a working POC...
> 

Sounds like an interesting experiment either way. It may be simpler to
think about generating some sort of synthetic filehandles that can
dispatch requests to the right inode depending on the context of the
call?

Not sure I understand overlayfs well enough to have that make sense.
-- 
Jeff Layton <jlayton@poochiereds.net>

Re: overlayfs NFS export

[Amir Goldstein]

On Fri, Apr 7, 2017 at 5:53 PM, Jeff Layton <jlayton@poochiereds.net> wrote:
> On Fri, 2017-04-07 at 17:29 +0300, Amir Goldstein wrote:
>> [changing the subject and adding more NFS guys so they can shoot my
>> idea down if it is too dumb to live]
>>
>> On Fri, Apr 7, 2017 at 4:03 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>> > On Fri, Apr 7, 2017 at 12:47 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>> >
>> > > Come to think about it, NFS export of regular file don't need to
>> > > follow renames at all:
>> > > - The handle for a regular file is always the handle for the real
>> > > lower or upper inode
>> > > - To decode a handle, create an O_TMPFILE style overlay dentry, which
>> > > is not linked
>> > >   to any path in overlay, but has the _upperdentry/lowerstack setup
>> >
>> > I don't think nfs will allow such a scheme.  NFS3 server is stateless,
>> > which means there's no open/close in the protocol.   Hence we can't
>> > copy-up on open(O_WR*) and return a different file handle for writing.
>> > If client looks up a file currently on lower and we return file handle
>> > based on lower file, then we must be able to decode that handle after
>> > the file has been copied up and even after rename.  And this must work
>> > reliably even if the overlay dentry is no longer in the dcache.
>> >
>> > So there's no option, other than to have a reverse mapping somewhere.
>> >
>>
>> Either I am missing something or you are.
>>
>> Consider this scenario:
>>
>> On server:
>> - touch a
>> - ln a b
>>
>> On NFS client:
>> - rofd = open("a", O_RDONLY)
>>
>> On server
>> - rm a
>> - reboot
>>
>> NFS client must be able to continue to work with rofd
>> even after reboot and even after original file was unlinked.
>>
>
> Sure, it'll still work here. The inode still exists because of "b".
>
>> Furthermore, on server:
>> - rm b
>>
>> NFS client must continue to work with rofd even though
>> NFS server is stateless and even though inode is now
>> nlink = 0.
>
> No, at that point the server will probably just kill the inode and
> you'll start getting back -ESTALE when you try to use that filehandle.
> With NFSv2/3, there is no guarantee that the server will have bothered
> to open it after reboot but before you removed it here.
>

Yes, I forgot to say that another process on server keeps the
inode alive.
Just stressing the point that if overlay provides a disconected
dentry to nfsd its not going to be a new thing.

>> That is possible because fs will instantiate a disconnected
>> dentry when decoding the file handle is there is no dentry
>> already in cache.
>>
>> So what I am saying is that when nfsd tries to decode
>> a handle from overlay mount, and there is no mathcing
>> overlay dentry in cache (with the lower ino of course)
>> then we instantiate a new disconnected dentry without
>> lookup and set its _upperdenry or lowerstack according
>> to the knowledge that we found the handle in underlying
>> fs and we checked if it is a decedent of lower_mnt[i] or
>> upper_mnt.
>>
>> When NFS client opens a new rwfd it WILL get a different
>> handle, but it WILL really be a different file then the rofd,
>> so that sounds like a good thing?
>>
>
> An "open" with NFSv3 is basically a NFS LOOKUP or maybe a GETATTR call.
> Again, the protocol is stateless, so in practice we open the inode each
> time we service a READ or WRITE RPC.

Yes, but you open the inode using a filehandle from the client right?
otherwise whats exportfs for?

>
> How is knfsd to know that you want to access the r/o or r/w inode there
> and thus give you the right filehandle?
>

Its not knfsd business to know that.
With overlayfs the first ro handle will have the lower inode
and second rw handle will have the upper inode
trying to open by handle from ro handle will get you
a disconnected overlay dentry pointing to lower inode
and trying to open by handle from rw handle will
get you a disconnected overlay dentry pointing to
upper inode.

Again, with overlayfs those are 2 different files
whose data may not be consistent at all.

This trick does circumvent the locks_inode() helper
that was added to make sure that ro and rw files
lock the same (overlay) object... hmm

>> I realize it may sound complicated, but redirect_fh patch
>> has already all the needed parts in place for this, so the
>> proof if what I am saying is right or wrong will be in whether
>> or not I am able to present a working POC...
>>
>
> Sounds like an interesting experiment either way. It may be simpler to
> think about generating some sort of synthetic filehandles that can
> dispatch requests to the right inode depending on the context of the
> call?

parent ino info is enough to find which layer the handle came from,
but as I understood in LSF, knfsd does not want to get the parent
ino is encoding (for uniqueness of hardlinks), so i need to see about that.

>
> Not sure I understand overlayfs well enough to have that make sense.
> --

Not sure I understand NFS well enough ;-)
trying to bring the two parties a bit closer to understand what
it will take for overlayfs to play well with NFS.

Suppose that NFSv3 is a hard problem for overlayfs,
Is that an option to play well only with NFSv4.
Is there a way for nfsd to query fs exportfs capabilities
and export v4 only? is that a thing?

Amir.

Re: overlayfs NFS export

[Trond Myklebust]

On Fri, 2017-04-07 at 17:29 +0300, Amir Goldstein wrote:
> [changing the subject and adding more NFS guys so they can shoot my
> idea down if it is too dumb to live]
> 
> On Fri, Apr 7, 2017 at 4:03 PM, Miklos Szeredi <miklos@szeredi.hu>
> wrote:
> > On Fri, Apr 7, 2017 at 12:47 PM, Amir Goldstein <amir73il@gmail.com
> > > wrote:
> > 
> > > Come to think about it, NFS export of regular file don't need to
> > > follow renames at all:
> > > - The handle for a regular file is always the handle for the real
> > > lower or upper inode
> > > - To decode a handle, create an O_TMPFILE style overlay dentry,
> > > which
> > > is not linked
> > >   to any path in overlay, but has the _upperdentry/lowerstack
> > > setup
> > 
> > I don't think nfs will allow such a scheme.  NFS3 server is
> > stateless,
> > which means there's no open/close in the protocol.   Hence we can't
> > copy-up on open(O_WR*) and return a different file handle for
> > writing.
> > If client looks up a file currently on lower and we return file
> > handle
> > based on lower file, then we must be able to decode that handle
> > after
> > the file has been copied up and even after rename.  And this must
> > work
> > reliably even if the overlay dentry is no longer in the dcache.
> > 
> > So there's no option, other than to have a reverse mapping
> > somewhere.
> > 
> 
> Either I am missing something or you are.
> 
> Consider this scenario:
> 
> On server:
> - touch a
> - ln a b
> 
> On NFS client:
> - rofd = open("a", O_RDONLY)
> 
> On server
> - rm a
> - reboot
> 
> NFS client must be able to continue to work with rofd
> even after reboot and even after original file was unlinked.
> 
> Furthermore, on server:
> - rm b
> 
> NFS client must continue to work with rofd even though
> NFS server is stateless and even though inode is now
> nlink = 0.
> That is possible because fs will instantiate a disconnected
> dentry when decoding the file handle is there is no dentry
> already in cache.
> 
> So what I am saying is that when nfsd tries to decode
> a handle from overlay mount, and there is no mathcing
> overlay dentry in cache (with the lower ino of course)
> then we instantiate a new disconnected dentry without
> lookup and set its _upperdenry or lowerstack according
> to the knowledge that we found the handle in underlying
> fs and we checked if it is a decedent of lower_mnt[i] or
> upper_mnt.
> 
> When NFS client opens a new rwfd it WILL get a different
> handle, but it WILL really be a different file then the rofd,
> so that sounds like a good thing?
> 
> I realize it may sound complicated, but redirect_fh patch
> has already all the needed parts in place for this, so the
> proof if what I am saying is right or wrong will be in whether
> or not I am able to present a working POC...
> 

What is the problem you are trying to solve?

As far as the NFS protocol is concerned, if 2 filehandles that
originate from the same server are equal (i.e. a byte-by-byte
comparison of their contents shows a match), then they MUST refer to
the same file.

On the other hand, if the filehandles differ, then they are _treated_
by both the client and the server as if they were pointing to different
files (even if that is not the case). That principle is even encoded in
the NFSv4 protocol, where it is a requirement that file state is
attached to the filehandle.

-- 
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com

Re: overlayfs NFS export

[Miklos Szeredi]

On Fri, Apr 7, 2017 at 4:57 PM, Trond Myklebust <trondmy@primarydata.com> wrote:

> What is the problem you are trying to solve?

The problem is getting a persistent file handle for overlayfs files.

One idea suggested by Viro is to create a dummy inode on the upper
layer whenever we look up a dentry in the overlay filesystem.  Then we
have an inode number reserved for the file if it needs to be copied
up. This solves the file handle problem, since we can generate a path
from the file handle and from there get the original lower layer file
(assumes the file handle has the parent handle encoded as well).  If
the file is copied up, the file is no longer assiciated with the lower
layer, we just need to use the upper inode, this works too.  And also
files created on the upper work fine.

The only little problem is that we are creating lots of inodes on disk
and memory that until now we haven't.  Currently overlayfs only
modifies upper layer if there's a good reason to believe that there is
really going to be modification (e.g. when file is opened for write).

The alternative is generate file handle from lower file (if on lower)
and from upper file (if on upper).   The issue is if the file is
copied up and goes from lower to upper.  In that case we need to find
the upper file from the handle generated from the lower file.   This
works as long as the upper file hasn't been renamed, same way we found
the lower from the upper Viro's scheme: generate path from lower file
handle and look up in overlay.  When copied up file is renamed, this
doesn't work anymore, so we need a mapping from lower-handle to
upper-handle.

Thanks,
Miklos

Re: overlayfs NFS export

[Amir Goldstein]

On Fri, Apr 7, 2017 at 6:28 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Fri, Apr 7, 2017 at 4:57 PM, Trond Myklebust <trondmy@primarydata.com> wrote:
>
>> What is the problem you are trying to solve?
>
> The problem is getting a persistent file handle for overlayfs files.

That is only part of the problem and the point I was trying to
explore is that we don't need to solve it at all (see below).

The other part of the problem is getting a persistent handle for
overlayfs directories.

Why this second problem is hard is too difficult to explain to
non-overlayfs folks, but Miklos and I started playing around with an idea.

>
> One idea suggested by Viro is to create a dummy inode on the upper
> layer whenever we look up a dentry in the overlay filesystem.  Then we

So that idea is not relevant for directories (I think)

> have an inode number reserved for the file if it needs to be copied
> up. This solves the file handle problem, since we can generate a path
> from the file handle and from there get the original lower layer file
> (assumes the file handle has the parent handle encoded as well).  If

Apparently, that is not the case with knfsd, but it doesn't matter
for directory handles which can always be reconnceted.

> the file is copied up, the file is no longer assiciated with the lower
> layer, we just need to use the upper inode, this works too.  And also
> files created on the upper work fine.
>
> The only little problem is that we are creating lots of inodes on disk
> and memory that until now we haven't.  Currently overlayfs only
> modifies upper layer if there's a good reason to believe that there is
> really going to be modification (e.g. when file is opened for write).
>
> The alternative is generate file handle from lower file (if on lower)
> and from upper file (if on upper).   The issue is if the file is
> copied up and goes from lower to upper.  In that case we need to find
> the upper file from the handle generated from the lower file.   This

So why do we really need to find the upper in that case?
If we follow my idea, then NFS read request with lower handle
may be served from lower inode and NFS write request with a
lower handle will get ESTALE and will try to lookup by path
(I suppose?).
That's probably more consistent then what local processes get
from overlayfs.

I realize I don't have all the pieces in my head yet... still trying
to collect them on the go.

Thanks for your patience guys,
Amir.

Re: overlayfs NFS export

[Trond Myklebust]

On Fri, 2017-04-07 at 18:45 +0300, Amir Goldstein wrote:
> On Fri, Apr 7, 2017 at 6:28 PM, Miklos Szeredi <miklos@szeredi.hu>
> wrote:
> > On Fri, Apr 7, 2017 at 4:57 PM, Trond Myklebust <trondmy@primarydat
> > a.com> wrote:
> > 
> > > What is the problem you are trying to solve?
> > 
> > The problem is getting a persistent file handle for overlayfs
> > files.
> 
> That is only part of the problem and the point I was trying to
> explore is that we don't need to solve it at all (see below).

You don't, if you are willing to live with non-POSIX semantics.
Otherwise you do.

> 
> The other part of the problem is getting a persistent handle for
> overlayfs directories.
> 
> Why this second problem is hard is too difficult to explain to
> non-overlayfs folks, but Miklos and I started playing around with an
> idea.
> 
> > 
> > One idea suggested by Viro is to create a dummy inode on the upper
> > layer whenever we look up a dentry in the overlay filesystem.  Then
> > we
> 
> So that idea is not relevant for directories (I think)
> 
> > have an inode number reserved for the file if it needs to be copied
> > up. This solves the file handle problem, since we can generate a
> > path
> > from the file handle and from there get the original lower layer
> > file
> > (assumes the file handle has the parent handle encoded as
> > well).  If
> 
> Apparently, that is not the case with knfsd, but it doesn't matter
> for directory handles which can always be reconnceted.
> 
> > the file is copied up, the file is no longer assiciated with the
> > lower
> > layer, we just need to use the upper inode, this works too.  And
> > also
> > files created on the upper work fine.
> > 
> > The only little problem is that we are creating lots of inodes on
> > disk
> > and memory that until now we haven't.  Currently overlayfs only
> > modifies upper layer if there's a good reason to believe that there
> > is
> > really going to be modification (e.g. when file is opened for
> > write).
> > 
> > The alternative is generate file handle from lower file (if on
> > lower)
> > and from upper file (if on upper).   The issue is if the file is
> > copied up and goes from lower to upper.  In that case we need to
> > find
> > the upper file from the handle generated from the lower
> > file.   This
> 
> So why do we really need to find the upper in that case?
> If we follow my idea, then NFS read request with lower handle
> may be served from lower inode and NFS write request with a
> lower handle will get ESTALE and will try to lookup by path
> (I suppose?).
> 

The client will never try to recover from an ESTALE error that is
returned on a file it has already opened. That would cause data
corruption if the user were to do something like 'rm foo; touch foo' on
the server; writes that were intended for the old file would suddenly
be written to the new one in violation of POSIX I/O rules.


IOW: In the case where WRITE returns ESTALE, that error will result in
the client returning EIO to the application on the next write() or
fsync() or close(). That error will persist; a retry will not clear
it.

-- 
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com

Re: overlayfs NFS export

[Amir Goldstein]

On Fri, Apr 7, 2017 at 6:58 PM, Trond Myklebust <trondmy@primarydata.com> wrote:
> On Fri, 2017-04-07 at 18:45 +0300, Amir Goldstein wrote:
>> On Fri, Apr 7, 2017 at 6:28 PM, Miklos Szeredi <miklos@szeredi.hu>
>> wrote:
>> > On Fri, Apr 7, 2017 at 4:57 PM, Trond Myklebust <trondmy@primarydat
>> > a.com> wrote:
>> >
>> > > What is the problem you are trying to solve?
>> >
>> > The problem is getting a persistent file handle for overlayfs
>> > files.
>>
>> That is only part of the problem and the point I was trying to
>> explore is that we don't need to solve it at all (see below).
>
> You don't, if you are willing to live with non-POSIX semantics.
> Otherwise you do.
>
>>
>> The other part of the problem is getting a persistent handle for
>> overlayfs directories.
>>
>> Why this second problem is hard is too difficult to explain to
>> non-overlayfs folks, but Miklos and I started playing around with an
>> idea.
>>
>> >
>> > One idea suggested by Viro is to create a dummy inode on the upper
>> > layer whenever we look up a dentry in the overlay filesystem.  Then
>> > we
>>
>> So that idea is not relevant for directories (I think)
>>
>> > have an inode number reserved for the file if it needs to be copied
>> > up. This solves the file handle problem, since we can generate a
>> > path
>> > from the file handle and from there get the original lower layer
>> > file
>> > (assumes the file handle has the parent handle encoded as
>> > well).  If
>>
>> Apparently, that is not the case with knfsd, but it doesn't matter
>> for directory handles which can always be reconnceted.
>>
>> > the file is copied up, the file is no longer assiciated with the
>> > lower
>> > layer, we just need to use the upper inode, this works too.  And
>> > also
>> > files created on the upper work fine.
>> >
>> > The only little problem is that we are creating lots of inodes on
>> > disk
>> > and memory that until now we haven't.  Currently overlayfs only
>> > modifies upper layer if there's a good reason to believe that there
>> > is
>> > really going to be modification (e.g. when file is opened for
>> > write).
>> >
>> > The alternative is generate file handle from lower file (if on
>> > lower)
>> > and from upper file (if on upper).   The issue is if the file is
>> > copied up and goes from lower to upper.  In that case we need to
>> > find
>> > the upper file from the handle generated from the lower
>> > file.   This
>>
>> So why do we really need to find the upper in that case?
>> If we follow my idea, then NFS read request with lower handle
>> may be served from lower inode and NFS write request with a
>> lower handle will get ESTALE and will try to lookup by path
>> (I suppose?).
>>
>
> The client will never try to recover from an ESTALE error that is
> returned on a file it has already opened. That would cause data
> corruption if the user were to do something like 'rm foo; touch foo' on
> the server; writes that were intended for the old file would suddenly
> be written to the new one in violation of POSIX I/O rules.
>
>
> IOW: In the case where WRITE returns ESTALE, that error will result in
> the client returning EIO to the application on the next write() or
> fsync() or close(). That error will persist; a retry will not clear
> it.
>

The most important point to understand is this:

If server opens a file for write it will trigger a copy up
and the file handle returned will be persistent and final.

The only problem is that when server opens a file for
read *before* it opens the same file for write, the returned
handle would be different, because first open for write
creates a new file and the old file remains a zombie
(as far as nfsd is concerned) only nfsd is able to to access
the old file and only for read.

Re: overlayfs NFS export

[Trond Myklebust]

On Fri, 2017-04-07 at 19:10 +0300, Amir Goldstein wrote
> 
> The most important point to understand is this:
> 
> If server opens a file for write it will trigger a copy up
> and the file handle returned will be persistent and final.
> 
> The only problem is that when server opens a file for
> read *before* it opens the same file for write, the returned
> handle would be different, because first open for write
> creates a new file and the old file remains a zombie
> (as far as nfsd is concerned) only nfsd is able to to access
> the old file and only for read.

That will break the client. As Miklos already told you, NFSv3 is
stateless, so the client won't be sending you an OPEN, and won't
actually hit a problem until it tries to send a WRITE, in which case
the resulting ESTALE error will be propagated to the application.

-- 
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com

Re: overlayfs NFS export

[Amir Goldstein]

On Fri, Apr 7, 2017 at 7:21 PM, Trond Myklebust <trondmy@primarydata.com> wrote:
> On Fri, 2017-04-07 at 19:10 +0300, Amir Goldstein wrote
>>
>> The most important point to understand is this:
>>
>> If server opens a file for write it will trigger a copy up
>> and the file handle returned will be persistent and final.
>>
>> The only problem is that when server opens a file for
>> read *before* it opens the same file for write, the returned
>> handle would be different, because first open for write
>> creates a new file and the old file remains a zombie
>> (as far as nfsd is concerned) only nfsd is able to to access
>> the old file and only for read.
>
> That will break the client. As Miklos already told you, NFSv3 is
> stateless, so the client won't be sending you an OPEN, and won't
> actually hit a problem until it tries to send a WRITE, in which case
> the resulting ESTALE error will be propagated to the application.
>


Yes, I should have listened to Miklos..
Thanks for taking the time to explain.

Amir.

Re: overlayfs NFS export

[Jeff Layton]

On Fri, 2017-04-07 at 19:10 +0300, Amir Goldstein wrote:
> On Fri, Apr 7, 2017 at 6:58 PM, Trond Myklebust <trondmy@primarydata.com> wrote:
> > On Fri, 2017-04-07 at 18:45 +0300, Amir Goldstein wrote:
> > > On Fri, Apr 7, 2017 at 6:28 PM, Miklos Szeredi <miklos@szeredi.hu>
> > > wrote:
> > > > On Fri, Apr 7, 2017 at 4:57 PM, Trond Myklebust <trondmy@primarydat
> > > > a.com> wrote:
> > > > 
> > > > > What is the problem you are trying to solve?
> > > > 
> > > > The problem is getting a persistent file handle for overlayfs
> > > > files.
> > > 
> > > That is only part of the problem and the point I was trying to
> > > explore is that we don't need to solve it at all (see below).
> > 
> > You don't, if you are willing to live with non-POSIX semantics.
> > Otherwise you do.
> > 
> > > 
> > > The other part of the problem is getting a persistent handle for
> > > overlayfs directories.
> > > 
> > > Why this second problem is hard is too difficult to explain to
> > > non-overlayfs folks, but Miklos and I started playing around with an
> > > idea.
> > > 
> > > > 
> > > > One idea suggested by Viro is to create a dummy inode on the upper
> > > > layer whenever we look up a dentry in the overlay filesystem.  Then
> > > > we
> > > 
> > > So that idea is not relevant for directories (I think)
> > > 
> > > > have an inode number reserved for the file if it needs to be copied
> > > > up. This solves the file handle problem, since we can generate a
> > > > path
> > > > from the file handle and from there get the original lower layer
> > > > file
> > > > (assumes the file handle has the parent handle encoded as
> > > > well).  If
> > > 
> > > Apparently, that is not the case with knfsd, but it doesn't matter
> > > for directory handles which can always be reconnceted.
> > > 
> > > > the file is copied up, the file is no longer assiciated with the
> > > > lower
> > > > layer, we just need to use the upper inode, this works too.  And
> > > > also
> > > > files created on the upper work fine.
> > > > 
> > > > The only little problem is that we are creating lots of inodes on
> > > > disk
> > > > and memory that until now we haven't.  Currently overlayfs only
> > > > modifies upper layer if there's a good reason to believe that there
> > > > is
> > > > really going to be modification (e.g. when file is opened for
> > > > write).
> > > > 
> > > > The alternative is generate file handle from lower file (if on
> > > > lower)
> > > > and from upper file (if on upper).   The issue is if the file is
> > > > copied up and goes from lower to upper.  In that case we need to
> > > > find
> > > > the upper file from the handle generated from the lower
> > > > file.   This
> > > 
> > > So why do we really need to find the upper in that case?
> > > If we follow my idea, then NFS read request with lower handle
> > > may be served from lower inode and NFS write request with a
> > > lower handle will get ESTALE and will try to lookup by path
> > > (I suppose?).
> > > 
> > 
> > The client will never try to recover from an ESTALE error that is
> > returned on a file it has already opened. That would cause data
> > corruption if the user were to do something like 'rm foo; touch foo' on
> > the server; writes that were intended for the old file would suddenly
> > be written to the new one in violation of POSIX I/O rules.
> > 
> > 
> > IOW: In the case where WRITE returns ESTALE, that error will result in
> > the client returning EIO to the application on the next write() or
> > fsync() or close(). That error will persist; a retry will not clear
> > it.
> > 
> 
> The most important point to understand is this:
> 
> If server opens a file for write it will trigger a copy up
> and the file handle returned will be persistent and final.
> 
> The only problem is that when server opens a file for
> read *before* it opens the same file for write, the returned
> handle would be different, because first open for write
> creates a new file and the old file remains a zombie
> (as far as nfsd is concerned) only nfsd is able to to access
> the old file and only for read.

Once a copy-up occurs, then I expect it'd look to the client like the
file had been renamed-over. You're getting back a different dentry/inode
pair on lookup, right? Eventually the client will revalidate the parent
directory inode, see that something has changed and redo the lookup for
the thing. New opens would go to the copied-up inode after that point.

In any case, nfsd will usually only hold the r/o file open if some
client was holding that file open. So, it sounds like you'll end up
projecting that weird overlayfs "read open before write open" corner
case across the wire, but it would otherwise "work".
-- 
Jeff Layton <jlayton@poochiereds.net>

[1] Side question: does the parent directory's mtime get updated when
there is a copy-up? The client might not notice that its dentries are
now invalid afterward unless it does.

Re: overlayfs NFS export

[Amir Goldstein]

On Fri, Apr 7, 2017 at 7:47 PM, Jeff Layton <jlayton@poochiereds.net> wrote:
> On Fri, 2017-04-07 at 19:10 +0300, Amir Goldstein wrote:
>> On Fri, Apr 7, 2017 at 6:58 PM, Trond Myklebust <trondmy@primarydata.com> wrote:
>> > On Fri, 2017-04-07 at 18:45 +0300, Amir Goldstein wrote:
>> > > On Fri, Apr 7, 2017 at 6:28 PM, Miklos Szeredi <miklos@szeredi.hu>
...
>> > >
>> > > So why do we really need to find the upper in that case?
>> > > If we follow my idea, then NFS read request with lower handle
>> > > may be served from lower inode and NFS write request with a
>> > > lower handle will get ESTALE and will try to lookup by path
>> > > (I suppose?).
>> > >
>> >
>> > The client will never try to recover from an ESTALE error that is
>> > returned on a file it has already opened. That would cause data
>> > corruption if the user were to do something like 'rm foo; touch foo' on
>> > the server; writes that were intended for the old file would suddenly
>> > be written to the new one in violation of POSIX I/O rules.
>> >
>> >
>> > IOW: In the case where WRITE returns ESTALE, that error will result in
>> > the client returning EIO to the application on the next write() or
>> > fsync() or close(). That error will persist; a retry will not clear
>> > it.
>> >
>>
>> The most important point to understand is this:
>>
>> If server opens a file for write it will trigger a copy up
>> and the file handle returned will be persistent and final.
>>
>> The only problem is that when server opens a file for
>> read *before* it opens the same file for write, the returned
>> handle would be different, because first open for write
>> creates a new file and the old file remains a zombie
>> (as far as nfsd is concerned) only nfsd is able to to access
>> the old file and only for read.
>
> Once a copy-up occurs, then I expect it'd look to the client like the
> file had been renamed-over. You're getting back a different dentry/inode
> pair on lookup, right? Eventually the client will revalidate the parent
> directory inode, see that something has changed and redo the lookup for
> the thing. New opens would go to the copied-up inode after that point.
>

That is correct. And makes me wonder how bad applications over nfs
client behave in case of file having been renamed over? which seems
to be an equivalent case to copy up.

But it's time to look for real solutions and not try to work around them...

> In any case, nfsd will usually only hold the r/o file open if some
> client was holding that file open. So, it sounds like you'll end up
> projecting that weird overlayfs "read open before write open" corner
> case across the wire, but it would otherwise "work".
> --
> Jeff Layton <jlayton@poochiereds.net>
>
> [1] Side question: does the parent directory's mtime get updated when
> there is a copy-up? The client might not notice that its dentries are
> now invalid afterward unless it does.
>

Yes, because the parent directory is the upper directory where the
upper file gets copied to.

Thanks,
Amir.

Re: overlayfs NFS export

[Trond Myklebust]

On Fri, 2017-04-07 at 17:28 +0200, Miklos Szeredi wrote:
> On Fri, Apr 7, 2017 at 4:57 PM, Trond Myklebust <trondmy@primarydata.
> com> wrote:
> 
> > What is the problem you are trying to solve?
> 
> The problem is getting a persistent file handle for overlayfs files.
> 
> One idea suggested by Viro is to create a dummy inode on the upper
> layer whenever we look up a dentry in the overlay filesystem.  Then
> we
> have an inode number reserved for the file if it needs to be copied
> up. This solves the file handle problem, since we can generate a path
> from the file handle and from there get the original lower layer file
> (assumes the file handle has the parent handle encoded as well).  If
> the file is copied up, the file is no longer assiciated with the
> lower
> layer, we just need to use the upper inode, this works too.  And also
> files created on the upper work fine.
> 

Why do you need the parent directory to be encoded as well? Doesn't
fh_to_dentry() suffice?

The thing is that for all namespace related operations (mkdir(),
rmdir(), open(O_CREATE), unlink,...) the client will supply the parent
filehandle, so those operations should not normally suffer.

The only operation that would be a problem is when you have to copy up
a regular file, but for that case overlayfs needs to generate dentries
for _all_ hardlinks to that file, in the upper layer, no?

> The only little problem is that we are creating lots of inodes on
> disk
> and memory that until now we haven't.  Currently overlayfs only
> modifies upper layer if there's a good reason to believe that there
> is
> really going to be modification (e.g. when file is opened for write).
> 
> The alternative is generate file handle from lower file (if on lower)
> and from upper file (if on upper).   The issue is if the file is
> copied up and goes from lower to upper.  In that case we need to find
> the upper file from the handle generated from the lower file.   This
> works as long as the upper file hasn't been renamed, same way we
> found
> the lower from the upper Viro's scheme: generate path from lower file
> handle and look up in overlay.  When copied up file is renamed, this
> doesn't work anymore, so we need a mapping from lower-handle to
> upper-handle.
> 
Generating filehandles from the namespace is a nightmare. It only works
if you have strict non-POSIX-compatible rules about not being able to
modify the parent of a subtree if there is a file in use in that
subtree. You can therefore make it work if the underlying filesystem is
something like NTFS, but not if you want POSIX semantics.

-- 
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com

Re: overlayfs NFS export

[Amir Goldstein]

On Fri, Apr 7, 2017 at 6:46 PM, Trond Myklebust <trondmy@primarydata.com> wrote:
> On Fri, 2017-04-07 at 17:28 +0200, Miklos Szeredi wrote:
>> On Fri, Apr 7, 2017 at 4:57 PM, Trond Myklebust <trondmy@primarydata.
>> com> wrote:
>>
>> > What is the problem you are trying to solve?
>>
>> The problem is getting a persistent file handle for overlayfs files.
>>
>> One idea suggested by Viro is to create a dummy inode on the upper
>> layer whenever we look up a dentry in the overlay filesystem.  Then
>> we
>> have an inode number reserved for the file if it needs to be copied
>> up. This solves the file handle problem, since we can generate a path
>> from the file handle and from there get the original lower layer file
>> (assumes the file handle has the parent handle encoded as well).  If
>> the file is copied up, the file is no longer assiciated with the
>> lower
>> layer, we just need to use the upper inode, this works too.  And also
>> files created on the upper work fine.
>>
>
> Why do you need the parent directory to be encoded as well? Doesn't
> fh_to_dentry() suffice?
>

Put it simply, you encoded a file /mnt/A/b which is found at
/lower/A/b and when you later come with this handle, you need to
check not only that the original lower inode is not stale (trivial),
but also that there is no new file or whiteout at path /upper/A/b.
so you can do this by converting handle to path and then looking
up same in /upper. everything breaks when upper dirs get renamed...


> The thing is that for all namespace related operations (mkdir(),
> rmdir(), open(O_CREATE), unlink,...) the client will supply the parent
> filehandle, so those operations should not normally suffer.
>
> The only operation that would be a problem is when you have to copy up
> a regular file, but for that case overlayfs needs to generate dentries
> for _all_ hardlinks to that file, in the upper layer, no?

Needs to... but it bloody well doesn't. That's one of the hardest issues.

>
>> The only little problem is that we are creating lots of inodes on
>> disk
>> and memory that until now we haven't.  Currently overlayfs only
>> modifies upper layer if there's a good reason to believe that there
>> is
>> really going to be modification (e.g. when file is opened for write).
>>
>> The alternative is generate file handle from lower file (if on lower)
>> and from upper file (if on upper).   The issue is if the file is
>> copied up and goes from lower to upper.  In that case we need to find
>> the upper file from the handle generated from the lower file.   This
>> works as long as the upper file hasn't been renamed, same way we
>> found
>> the lower from the upper Viro's scheme: generate path from lower file
>> handle and look up in overlay.  When copied up file is renamed, this
>> doesn't work anymore, so we need a mapping from lower-handle to
>> upper-handle.
>>
> Generating filehandles from the namespace is a nightmare. It only works

You mean generating a path from filehandle? or generating a filehandle
with parent so it can be reconnected?

> if you have strict non-POSIX-compatible rules about not being able to
> modify the parent of a subtree if there is a file in use in that
> subtree. You can therefore make it work if the underlying filesystem is
> something like NTFS, but not if you want POSIX semantics.
>

This was overlayfs prior to v4.10. too bad we fixed it ;-)

Amir.

Re: overlayfs NFS export

[Trond Myklebust]

On Fri, 2017-04-07 at 18:58 +0300, Amir Goldstein wrote:
> On Fri, Apr 7, 2017 at 6:46 PM, Trond Myklebust <trondmy@primarydata.
> com> wrote:
> > On Fri, 2017-04-07 at 17:28 +0200, Miklos Szeredi wrote:
> > > On Fri, Apr 7, 2017 at 4:57 PM, Trond Myklebust <trondmy@primaryd
> > > ata.
> > > com> wrote:
> > > 
> > > > What is the problem you are trying to solve?
> > > 
> > > The problem is getting a persistent file handle for overlayfs
> > > files.
> > > 
> > > One idea suggested by Viro is to create a dummy inode on the
> > > upper
> > > layer whenever we look up a dentry in the overlay
> > > filesystem.  Then
> > > we
> > > have an inode number reserved for the file if it needs to be
> > > copied
> > > up. This solves the file handle problem, since we can generate a
> > > path
> > > from the file handle and from there get the original lower layer
> > > file
> > > (assumes the file handle has the parent handle encoded as
> > > well).  If
> > > the file is copied up, the file is no longer assiciated with the
> > > lower
> > > layer, we just need to use the upper inode, this works too.  And
> > > also
> > > files created on the upper work fine.
> > > 
> > 
> > Why do you need the parent directory to be encoded as well? Doesn't
> > fh_to_dentry() suffice?
> > 
> 
> Put it simply, you encoded a file /mnt/A/b which is found at
> /lower/A/b and when you later come with this handle, you need to
> check not only that the original lower inode is not stale (trivial),
> but also that there is no new file or whiteout at path /upper/A/b.
> so you can do this by converting handle to path and then looking
> up same in /upper. everything breaks when upper dirs get renamed...

Only if the client asks you to lookup that file in that directory, in
which case it will supply you with a filehandle for that directory.

> 
> > The thing is that for all namespace related operations (mkdir(),
> > rmdir(), open(O_CREATE), unlink,...) the client will supply the
> > parent
> > filehandle, so those operations should not normally suffer.
> > 
> > The only operation that would be a problem is when you have to copy
> > up
> > a regular file, but for that case overlayfs needs to generate
> > dentries
> > for _all_ hardlinks to that file, in the upper layer, no?
> 
> Needs to... but it bloody well doesn't. That's one of the hardest
> issues.
> 
> > 
> > > The only little problem is that we are creating lots of inodes on
> > > disk
> > > and memory that until now we haven't.  Currently overlayfs only
> > > modifies upper layer if there's a good reason to believe that
> > > there
> > > is
> > > really going to be modification (e.g. when file is opened for
> > > write).
> > > 
> > > The alternative is generate file handle from lower file (if on
> > > lower)
> > > and from upper file (if on upper).   The issue is if the file is
> > > copied up and goes from lower to upper.  In that case we need to
> > > find
> > > the upper file from the handle generated from the lower
> > > file.   This
> > > works as long as the upper file hasn't been renamed, same way we
> > > found
> > > the lower from the upper Viro's scheme: generate path from lower
> > > file
> > > handle and look up in overlay.  When copied up file is renamed,
> > > this
> > > doesn't work anymore, so we need a mapping from lower-handle to
> > > upper-handle.
> > > 
> > 
> > Generating filehandles from the namespace is a nightmare. It only
> > works
> 
> You mean generating a path from filehandle? or generating a
> filehandle
> with parent so it can be reconnected?

I mean if you encode the path in the filehandle, you're borked. The 
whole purpose of filehandles is to provide something that is
independent of the path so that POSIX semantics can be provided.

> > if you have strict non-POSIX-compatible rules about not being able
> > to
> > modify the parent of a subtree if there is a file in use in that
> > subtree. You can therefore make it work if the underlying
> > filesystem is
> > something like NTFS, but not if you want POSIX semantics.
> > 
> 
> This was overlayfs prior to v4.10. too bad we fixed it ;-)
> 
> Amir.
> 
-- 
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com