Re: Stable inode numbers

[Vito Caputo <vito.caputo@coreos.com>]

Miklos,

This most recent patch looks ok to me, and it works fine in my limited testing.

Thanks,
Vito Caputo

On Tue, Jul 26, 2016 at 4:09 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Tue, Jul 26, 2016 at 12:02:13PM +0200, Miklos Szeredi wrote:
>> On Mon, Jul 25, 2016 at 11:25 PM, Vito Caputo <vito.caputo@coreos.com> wrote:
>> > I think this strategy is an improvement, but I'm a bit apprehensive
>> > about how specific it is to this particular style of untar-like
>> > directory metadata preservation failure in only providing stability to
>> > directory inode numbers.
>> >
>> > Additionally it introduces a userspace-visible mount option, for
>> > something which /feels/ like a stop-gap kludge to make some things
>> > work better in the short-term.  Maybe it's acceptable, since a more
>> > general solution could be implemented later which ignores the added
>> > mount option without conflict.
>
> Here's another patch that achieves stability without compromising
> performance/disk usage.
>
> Again not perfect, but perhaps good enough?
>
> Thanks,
> Miklos
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index 12bcd07b9e32..5b7de7d489e0 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -143,13 +143,24 @@ static int ovl_dir_getattr(struct vfsmount *mnt, struct dentry *dentry,
>         type = ovl_path_real(dentry, &realpath);
>         old_cred = ovl_override_creds(dentry->d_sb);
>         err = vfs_getattr(&realpath, stat);
> +       /*
> +        * Use lower ino/dev for merged dir, so they are stable across copy up.
> +        */
> +       if (!err && OVL_TYPE_MERGE(type) && OVL_TYPE_UPPER(type)) {
> +               struct path lowerpath;
> +               struct kstat lowerstat;
> +
> +               ovl_path_lower(dentry, &lowerpath);
> +               err = vfs_getattr(&lowerpath, &lowerstat);
> +               if (!err) {
> +                       stat->dev = lowerstat.dev;
> +                       stat->ino = lowerstat.ino;
> +               }
> +       }
>         revert_creds(old_cred);
>         if (err)
>                 return err;
>
> -       stat->dev = dentry->d_sb->s_dev;
> -       stat->ino = dentry->d_inode->i_ino;
> -
>         /*
>          * It's probably not worth it to count subdirs to get the
>          * correct link count.  nlink=1 seems to pacify 'find' and