On Tue, Oct 3, 2017 at 6:10 PM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
> On Mon, Oct 02, 2017 at 10:28:30PM +0300, Amir Goldstein wrote:
>> On Mon, Oct 2, 2017 at 4:40 PM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
>> > Presence of OVL_XATTR_METACOPY reflects that file has been copied up
>> > with metadata only and data needs to be copied up later from lower.
>> > So this xattr is set when a metadata copy takes place and cleared when
>> > data copy takes place.
>> >
>> > We also use a bit in ovl_inode->flags to cache OVL_METACOPY which reflects
>> > whether ovl inode has only metadata copied up.
>> >
>> > Signed-off-by: Vivek Goyal <vgoyal@xxxxxxxxxx>
>> > ---
>> > fs/overlayfs/copy_up.c | 42 ++++++++++++++++++++++++++++++++++++++++--
>> > fs/overlayfs/inode.c | 3 ++-
>> > fs/overlayfs/overlayfs.h | 5 ++++-
>> > fs/overlayfs/util.c | 21 +++++++++++++++++++--
>> > 4 files changed, 65 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
>> > index b2d9ed81e9ff..d76a4272b43e 100644
>> > --- a/fs/overlayfs/copy_up.c
>> > +++ b/fs/overlayfs/copy_up.c
>> > @@ -439,6 +439,26 @@ static int ovl_get_tmpfile(struct ovl_copy_up_ctx *c, struct dentry **tempp)
>> > goto out;
>> > }
>> >
>> > +static int ovl_copy_up_data_inode(struct ovl_copy_up_ctx *c)
>> > +{
>> > + struct path upperpath;
>> > + int err;
>> > +
>> > + ovl_path_upper(c->dentry, &upperpath);
>> > + BUG_ON(upperpath.dentry == NULL);
>> > +
>> > + err = ovl_copy_up_data(&c->lowerpath, &upperpath, c->stat.size);
>> > + if (err)
>> > + return err;
>> > +
>> > + err= vfs_removexattr(upperpath.dentry, OVL_XATTR_METACOPY);
>> > + if (err)
>> > + return err;
>> > +
>> > + ovl_clear_flag(OVL_METACOPY, d_inode(c->dentry));
>> > + return err;
>> > +}
>> > +
>> > static int ovl_copy_up_inode(struct ovl_copy_up_ctx *c, struct dentry *temp)
>> > {
>> > int err;
>> > @@ -482,6 +502,13 @@ static int ovl_copy_up_inode(struct ovl_copy_up_ctx *c, struct dentry *temp)
>> > return err;
>> > }
>> >
>> > + if (c->metadata_only) {
>> > + err = ovl_check_setxattr(c->dentry, temp, OVL_XATTR_METACOPY,
>> > + NULL, 0, -EOPNOTSUPP);
>> > + if (err)
>> > + return err;
>> > + }
>> > +
>> > return 0;
>> > }
>> >
>> > @@ -511,6 +538,9 @@ static int ovl_copy_up_locked(struct ovl_copy_up_ctx *c)
>> > goto out_cleanup;
>> >
>> > ovl_inode_update(d_inode(c->dentry), newdentry);
>> > + if (c->metadata_only) {
>> > + ovl_set_flag(OVL_METACOPY, d_inode(c->dentry));
>> > + }
>> > out:
>> > dput(temp);
>> > return err;
>> > @@ -638,7 +668,7 @@ static int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry,
>> > }
>> > ovl_do_check_copy_up(ctx.lowerpath.dentry);
>> >
>> > - err = ovl_copy_up_start(dentry);
>> > + err = ovl_copy_up_start(dentry, flags);
>> > /* err < 0: interrupted, err > 0: raced with another copy-up */
>> > if (unlikely(err)) {
>> > if (err > 0)
>> > @@ -648,6 +678,8 @@ static int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry,
>> > err = ovl_do_copy_up(&ctx);
>> > if (!err && !ovl_dentry_has_upper_alias(dentry))
>> > err = ovl_link_up(&ctx);
>> > + if (!err && ovl_dentry_needs_data_copy_up(dentry, flags))
>> > + err = ovl_copy_up_data_inode(&ctx);
>>
>> I think it would simplify life cycle of copy up if ovl_copy_up_data_inode()
>> is always before ovl_link_up() and that ovl_dentry_has_upper_alias()
>> means "has a concrete upper alias with data".
>
> Hmm..., I think I don't understand the suggestion. So will upper alias will
> not be created until and unless data copy up happens?
>
> IOW, say I have file lower-file.txt in lower/ and also a hard link, say
> lower-link.txt
>
> lower-file.txt
> lower-link.txt
>
> Now if do "chown vivek lower-file.txt", then lower-file.txt will be copied
> up metadata only and an alias will be created. So are you saying that we
> should not create an alias till actual data copy happens?
Yes, that is what I was suggesting.
There is no need for an upper alias for the purpose of copy up
from ovl_setattr(), ovl_xattr_set() and ovl_nlink_start().
However, the call sites ovl_copy_up(old) and ovl_copy_up(new) from
ovl_link() and ovl_rename() do need to create an upper alias,
so either they out-in to data copy (as they behave today) with
ovl_copy_up_flags(dentry, O_RWONLY) or they could be converted
to ovl_copy_up_flags(dentry, O_PATH) as special indication to opt-in
for metacopy + link_up with no data copy.
>
> What about regular files with no links.
>
When you chmod a regular file with or without links,
an index is created with its metadata.
Any future lookup/stat/permissions will see the i_mode/st_mode
of the upper index.
Amir.
--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
