On Sat, May 30, 2020 at 12:30 AM Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
>
> overlayfs can keep index of copied up files and directories and it
> seems to serve two primary puroposes. For regular files, it avoids
> breaking lower hardlinks over copy up. For directories it seems to
> be used for various error checks.
>
> During ovl_lookup(), we lookup for index using lower dentry in many
> a cases. That lower dentry is called "origin" and following is a summary
> of current logic.
>
> If there is no upperdentry, always lookup for index using lower dentry.
> For regular files it helps avoiding breaking hard links over copyup
> and for directories it seems to be just error checks.
>
> If there is an upperdentry, then there are 3 possible cases.
>
> - For directories, lower dentry is found using two ways. One is regular
> path based lookup in lower layers and second is using ORIGIN xattr
> on upper dentry. First verify that path based lookup lower dentry
> matches the one pointed by upper ORIGIN xattr. If yes, use this
> verified origin for index lookup.
>
> - For regular files (non-metacopy), there is no path based lookup in
> lower layers as lookup stops once we find upper dentry. So there
> is no origin verification. If there is ORIGIN xattr present on upper,
> use that to lookup index otherwise don't.
>
> - For regular metacopy files, again lower dentry is found using
> path based lookup as well as ORIGIN xattr on upper. Path based lookup
> is continued in this case to find lower data dentry for metacopy
> upper. So like directories we only use verified origin. If ORIGIN
> xattr is not present (Either because lower did not support file
> handles or because this is hardlink copied up with index=off), then
> don't use path lookup based lower dentry as origin. This is same
> as regular non-metacopy file case.
>
Very good summary.
You may add:
Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
But see one improvement below.
Also, please make sure to run unionmount setups:
./run --ov=10 --verify
./run --ov=10 --meta --verify
--verify will enable index and check st_dev;st_ino are not broken
on copy up. --ov=10 will cause lower hardlink copy up, because
after hardlink is creates by some test, upper is rotated to mid layer
and next modifying operation will trigger the hardlink copy up.
> Suggested-by: Amir Goldstein <amir73il@xxxxxxxxx>
> Signed-off-by: Vivek Goyal <vgoyal@xxxxxxxxxx>
> ---
> fs/overlayfs/namei.c | 29 +++++++++++++++++------------
> 1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
> index 0db23baf98e7..5d80d8cc0063 100644
> --- a/fs/overlayfs/namei.c
> +++ b/fs/overlayfs/namei.c
> @@ -1005,25 +1005,30 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
> }
> stack = origin_path;
> ctr = 1;
> + origin = origin_path->dentry;
> origin_path = NULL;
> }
>
[...]
> - if (ctr && (!upperdentry || (!d.is_dir && !metacopy)))
> + if (!origin && ctr && !upperdentry)
> origin = stack[0].dentry;
>
No need to understand the long story to verify this change is correct.
This is true simply because the conditions to set stack = origin_path are:
if (!metacopy && !d.is_dir && upperdentry && !ctr && origin_path)
And after getting there and setting ctr = 1, the complex conditions to
setting origin are met for certain:
if (ctr && (!upperdentry || (!d.is_dir && !metacopy)))
Therefore, it is logically equivalent (and makes much more sense)
to assign origin near stack = origin_path.
Further, thanks to Vivek's explanation, it is now clear to me that after
setting origin above, all that is left to do here is:
/* Always lookup index of non-upper */
if (!upperdentry)
origin = stack[0].dentry;
All the upperdentry cases described in the commit message have already
been dealt with by the time we get here.
Thanks,
Amir.
