On Fri, Feb 22, 2013 at 05:34:47PM +0800, Miao Xie wrote:
> On fri, 22 Feb 2013 16:40:35 +0800, Liu Bo wrote:
> > On Fri, Feb 22, 2013 at 03:32:50AM -0500, Marios Titas wrote:
> >> Sorry, but the bug persists even with the above patch.
> >>
> >> touch test
> >> chattr +C test
> >> lsattr test
> >> mv test test2
> >> lsattr test2
> >>
> >> In the above scenario test2 will not have the C flag.
> >
> > What do you expect? IMO it's right that test2 does not have the C flag.
>
> No, it's not right.
> For the users, they expect the C flag is not lost because they just do
> a rename operation. but fixup_inode_flags() re-sets the flags by the
> parent directory's flag.
>
> I think we should inherit the flags from the parent just when we create
> a new file/directory, in the other cases, just give a option to the users.
> How do you think about?
Actually what's in my mind is that we just allow files to inherit the parent
flags across different directories. This can ensure a rename in the same
directory keeps the file's flags.
Sort of this:
fixup_inode_flags(new, old)
{
if (new == old)
return;
...
}
But I don't know if it'll bring new bugs.
Any ideas?
thanks,
liubo
>
> Thanks
> Miao
>
> >
> > This patch ensure that we get the same result after we remount, no more
> > the C flag coming back :)
> >
> > thanks,
> > liubo
> >
> >>
> >> On Fri, Feb 22, 2013 at 3:11 AM, Liu Bo <bo.li....@oracle.com> wrote:
> >>> A user reported some weird behaviours,
> >>> if we move a file with the noCow flag to a directory without the
> >>> noCow flag, the file is now without the flag, but after remount,
> >>> we'll find the file's noCow flag comes back.
> >>>
> >>> This is because we missed a proper inode update after inheriting
> >>> parent directory's flags,
> >>>
> >>> Reported-by: Marios Titas <redneb8...@gmail.com>
> >>> Signed-off-by: Liu Bo <bo.li....@oracle.com>
> >>> ---
> >>> fs/btrfs/inode.c | 7 +++++--
> >>> 1 files changed, 5 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> >>> index d9984fa..d2e3352 100644
> >>> --- a/fs/btrfs/inode.c
> >>> +++ b/fs/btrfs/inode.c
> >>> @@ -7478,8 +7478,6 @@ static int btrfs_rename(struct inode *old_dir,
> >>> struct dentry *old_dentry,
> >>> old_dentry->d_inode,
> >>> old_dentry->d_name.name,
> >>> old_dentry->d_name.len);
> >>> - if (!ret)
> >>> - ret = btrfs_update_inode(trans, root, old_inode);
> >>> }
> >>> if (ret) {
> >>> btrfs_abort_transaction(trans, root, ret);
> >>> @@ -7514,6 +7512,11 @@ static int btrfs_rename(struct inode *old_dir,
> >>> struct dentry *old_dentry,
> >>> }
> >>>
> >>> fixup_inode_flags(new_dir, old_inode);
> >>> + ret = btrfs_update_inode(trans, root, old_inode);
> >>> + if (ret) {
> >>> + btrfs_abort_transaction(trans, root, ret);
> >>> + goto out_fail;
> >>> + }
> >>>
> >>> ret = btrfs_add_link(trans, new_dir, old_inode,
> >>> new_dentry->d_name.name,
> >>> --
> >>> 1.7.7.6
> >>>
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
