| Message ID | 1361765082-6331-1-git-send-email-bo.li.liu@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Feb 25, 2013 at 12:04:42PM +0800, Liu Bo wrote: > Before we forced to change a file's NOCOW and COMPRESS flag due to > the parent directory's, but this ends up a bad idea, because it > confuses end users a lot about file's NOCOW status, eg. if someone > change a file to NOCOW via 'chattr' and then rename it in the current > directory which is without NOCOW attribute, the file will lose the > NOCOW flag silently. > > This diables 'change flags in rename', so from now on we'll only > inherit flags from the parent directory on creation stage while in > other places we can use 'chattr' to set NOCOW or COMPRESS flags. > > Reported-by: Marios Titas <redneb8888@gmail.com> > Signed-off-by: Liu Bo <bo.li.liu@oracle.com> Thank you! Reviewed-by: David Sterba <dsterba@suse.cz> -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
(add Miao to CC) On Mon, Feb 25, 2013 at 12:04:42PM +0800, Liu Bo wrote: > Before we forced to change a file's NOCOW and COMPRESS flag due to > the parent directory's, but this ends up a bad idea, because it > confuses end users a lot about file's NOCOW status, eg. if someone > change a file to NOCOW via 'chattr' and then rename it in the current > directory which is without NOCOW attribute, the file will lose the > NOCOW flag silently. > > This diables 'change flags in rename', so from now on we'll only > inherit flags from the parent directory on creation stage while in > other places we can use 'chattr' to set NOCOW or COMPRESS flags. > > Reported-by: Marios Titas <redneb8888@gmail.com> > Signed-off-by: Liu Bo <bo.li.liu@oracle.com> > --- > fs/btrfs/inode.c | 25 ------------------------- > 1 files changed, 0 insertions(+), 25 deletions(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index d9984fa..383a7d8 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -7325,29 +7325,6 @@ static int btrfs_getattr(struct vfsmount *mnt, > return 0; > } > > -/* > - * If a file is moved, it will inherit the cow and compression flags of the new > - * directory. > - */ > -static void fixup_inode_flags(struct inode *dir, struct inode *inode) > -{ > - struct btrfs_inode *b_dir = BTRFS_I(dir); > - struct btrfs_inode *b_inode = BTRFS_I(inode); > - > - if (b_dir->flags & BTRFS_INODE_NODATACOW) > - b_inode->flags |= BTRFS_INODE_NODATACOW; > - else > - b_inode->flags &= ~BTRFS_INODE_NODATACOW; > - > - if (b_dir->flags & BTRFS_INODE_COMPRESS) { > - b_inode->flags |= BTRFS_INODE_COMPRESS; > - b_inode->flags &= ~BTRFS_INODE_NOCOMPRESS; > - } else { > - b_inode->flags &= ~(BTRFS_INODE_COMPRESS | > - BTRFS_INODE_NOCOMPRESS); > - } > -} > - > static int btrfs_rename(struct inode *old_dir, struct dentry *old_dentry, > struct inode *new_dir, struct dentry *new_dentry) > { > @@ -7513,8 +7490,6 @@ static int btrfs_rename(struct inode *old_dir, struct dentry *old_dentry, > } > } > > - fixup_inode_flags(new_dir, old_inode); > - > ret = btrfs_add_link(trans, new_dir, old_inode, > new_dentry->d_name.name, > new_dentry->d_name.len, 0, index); > -- > 1.7.7.6 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Feb 24, 2013 at 09:04:42PM -0700, Liu Bo wrote: > Before we forced to change a file's NOCOW and COMPRESS flag due to > the parent directory's, but this ends up a bad idea, because it > confuses end users a lot about file's NOCOW status, eg. if someone > change a file to NOCOW via 'chattr' and then rename it in the current > directory which is without NOCOW attribute, the file will lose the > NOCOW flag silently. > > This diables 'change flags in rename', so from now on we'll only > inherit flags from the parent directory on creation stage while in > other places we can use 'chattr' to set NOCOW or COMPRESS flags. > I'm of the mind we definitely shouldn't drop flags we've set previously, but I think we should also inherit any flags we have set on the directory, so if we move a file into a NOCOW directory we should inherit the flag. I'm not married to the idea, but it seems to make the most sense to me. Thanks, Josef -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 25, 2013 at 01:56:47PM -0500, Josef Bacik wrote: > On Sun, Feb 24, 2013 at 09:04:42PM -0700, Liu Bo wrote: > > Before we forced to change a file's NOCOW and COMPRESS flag due to > > the parent directory's, but this ends up a bad idea, because it > > confuses end users a lot about file's NOCOW status, eg. if someone > > change a file to NOCOW via 'chattr' and then rename it in the current > > directory which is without NOCOW attribute, the file will lose the > > NOCOW flag silently. > > > > This diables 'change flags in rename', so from now on we'll only > > inherit flags from the parent directory on creation stage while in > > other places we can use 'chattr' to set NOCOW or COMPRESS flags. > > > > I'm of the mind we definitely shouldn't drop flags we've set previously, but I > think we should also inherit any flags we have set on the directory, so if we > move a file into a NOCOW directory we should inherit the flag. I'm not married > to the idea, but it seems to make the most sense to me. Thanks, > > Josef Hi Josef, (Said in another thread) I'm ok with either one, but... from some reports on the list, end users are more likely to control, use chattr files by themselves, inheriting flags via moving a file to a new directory is indeed not very welcomed. So for practical use, I assume that it's fairly enough to inherit flags only on creation? thanks, liubo -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Feb 26, 2013 at 08:11:17AM +0800, Liu Bo wrote: > On Mon, Feb 25, 2013 at 01:56:47PM -0500, Josef Bacik wrote: > > On Sun, Feb 24, 2013 at 09:04:42PM -0700, Liu Bo wrote: > > > Before we forced to change a file's NOCOW and COMPRESS flag due to > > > the parent directory's, but this ends up a bad idea, because it > > > confuses end users a lot about file's NOCOW status, eg. if someone > > > change a file to NOCOW via 'chattr' and then rename it in the current > > > directory which is without NOCOW attribute, the file will lose the > > > NOCOW flag silently. > > > > > > This diables 'change flags in rename', so from now on we'll only > > > inherit flags from the parent directory on creation stage while in > > > other places we can use 'chattr' to set NOCOW or COMPRESS flags. > > > > > > > I'm of the mind we definitely shouldn't drop flags we've set previously, but I > > think we should also inherit any flags we have set on the directory, so if we > > move a file into a NOCOW directory we should inherit the flag. I'm not married > > to the idea, but it seems to make the most sense to me. Thanks, > > > (Said in another thread) > I'm ok with either one, but... > from some reports on the list, end users are more likely to control, use chattr > files by themselves, inheriting flags via moving a file to a new directory is > indeed not very welcomed. > > So for practical use, I assume that it's fairly enough to inherit flags only on > creation? I still haven't figured out in what cases the silent flag inheritance (for a non-empty) file would help and the user would be happy that it works like this. david -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Feb 26, 2013 at 05:33:00AM -0700, David Sterba wrote: > On Tue, Feb 26, 2013 at 08:11:17AM +0800, Liu Bo wrote: > > On Mon, Feb 25, 2013 at 01:56:47PM -0500, Josef Bacik wrote: > > > On Sun, Feb 24, 2013 at 09:04:42PM -0700, Liu Bo wrote: > > > > Before we forced to change a file's NOCOW and COMPRESS flag due to > > > > the parent directory's, but this ends up a bad idea, because it > > > > confuses end users a lot about file's NOCOW status, eg. if someone > > > > change a file to NOCOW via 'chattr' and then rename it in the current > > > > directory which is without NOCOW attribute, the file will lose the > > > > NOCOW flag silently. > > > > > > > > This diables 'change flags in rename', so from now on we'll only > > > > inherit flags from the parent directory on creation stage while in > > > > other places we can use 'chattr' to set NOCOW or COMPRESS flags. > > > > > > > > > > I'm of the mind we definitely shouldn't drop flags we've set previously, but I > > > think we should also inherit any flags we have set on the directory, so if we > > > move a file into a NOCOW directory we should inherit the flag. I'm not married > > > to the idea, but it seems to make the most sense to me. Thanks, > > > > > (Said in another thread) > > I'm ok with either one, but... > > from some reports on the list, end users are more likely to control, use chattr > > files by themselves, inheriting flags via moving a file to a new directory is > > indeed not very welcomed. > > > > So for practical use, I assume that it's fairly enough to inherit flags only on > > creation? > > I still haven't figured out in what cases the silent flag inheritance > (for a non-empty) file would help and the user would be happy that it > works like this. > K, I'll take this as it is then and drop my previous patch. Thanks, Josef -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index d9984fa..383a7d8 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -7325,29 +7325,6 @@ static int btrfs_getattr(struct vfsmount *mnt, return 0; } -/* - * If a file is moved, it will inherit the cow and compression flags of the new - * directory. - */ -static void fixup_inode_flags(struct inode *dir, struct inode *inode) -{ - struct btrfs_inode *b_dir = BTRFS_I(dir); - struct btrfs_inode *b_inode = BTRFS_I(inode); - - if (b_dir->flags & BTRFS_INODE_NODATACOW) - b_inode->flags |= BTRFS_INODE_NODATACOW; - else - b_inode->flags &= ~BTRFS_INODE_NODATACOW; - - if (b_dir->flags & BTRFS_INODE_COMPRESS) { - b_inode->flags |= BTRFS_INODE_COMPRESS; - b_inode->flags &= ~BTRFS_INODE_NOCOMPRESS; - } else { - b_inode->flags &= ~(BTRFS_INODE_COMPRESS | - BTRFS_INODE_NOCOMPRESS); - } -} - static int btrfs_rename(struct inode *old_dir, struct dentry *old_dentry, struct inode *new_dir, struct dentry *new_dentry) { @@ -7513,8 +7490,6 @@ static int btrfs_rename(struct inode *old_dir, struct dentry *old_dentry, } } - fixup_inode_flags(new_dir, old_inode); - ret = btrfs_add_link(trans, new_dir, old_inode, new_dentry->d_name.name, new_dentry->d_name.len, 0, index);
Before we forced to change a file's NOCOW and COMPRESS flag due to the parent directory's, but this ends up a bad idea, because it confuses end users a lot about file's NOCOW status, eg. if someone change a file to NOCOW via 'chattr' and then rename it in the current directory which is without NOCOW attribute, the file will lose the NOCOW flag silently. This diables 'change flags in rename', so from now on we'll only inherit flags from the parent directory on creation stage while in other places we can use 'chattr' to set NOCOW or COMPRESS flags. Reported-by: Marios Titas <redneb8888@gmail.com> Signed-off-by: Liu Bo <bo.li.liu@oracle.com> --- fs/btrfs/inode.c | 25 ------------------------- 1 files changed, 0 insertions(+), 25 deletions(-)