Hi Filipe,
(2014/09/11 0:10), Filipe Manana wrote:
> The behaviour of a 'chattr -c' consists of getting the current flags,
> clearing the FS_COMPR_FL bit and then sending the result to the set
> flags ioctl - this means the bit FS_NOCOMP_FL isn't set in the flags
> passed to the ioctl. This results in the compression property not being
> cleared from the inode - it was cleared only if the bit FS_NOCOMP_FL
> was set in the received flags.
>
> Reproducer:
>
> $ mkfs.btrfs -f /dev/sdd
> $ mount /dev/sdd /mnt && cd /mnt
> $ mkdir a
> $ chattr +c a
> $ touch a/file
> $ lsattr a/file
> --------c------- a/file
> $ chattr -c a
> $ touch a/file2
> $ lsattr a/file2
> --------c------- a/file2
> $ lsattr -d a
> ---------------- a
>
> Reported-by: Andreas Schneider <asn@xxxxxxxxxxxxxx>
> Signed-off-by: Filipe Manana <fdmanana@xxxxxxxx>
> ---
> fs/btrfs/ioctl.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index a010c44..8e6950c 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -333,6 +333,9 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
>
> } else {
> ip->flags &= ~(BTRFS_INODE_COMPRESS | BTRFS_INODE_NOCOMPRESS);
> + ret = btrfs_set_prop(inode, "btrfs.compression", NULL, 0, 0);
IMHO, this patch is going on a wrong way since this patch
changes the meaning of chattr. Here is the correct behavior.
flag | BTRFS_INODE_COMPRESS | BTRFS_INODE_NOCOMPRESS |
=====+======================+========================+
+c | set | unset |
-c | unset | unset |
However, by your change, chattr -c results in unset
BTRFS_INODE_COMPRESS and *set* BTRFS_INODE_NOCOMPRESS.
It's because btrfs_set_prop() will finally lead to
prop_compression_apply() with setting its value to "".
It's the behavior of calling ioctl() with FS_NOCOMP_FL.
Please note that inode flag without both BTRFS_INODE_COMPRESS
nor BTRFS_INODE_NOCOMPRESS has its own meaning: whether file
is compress or not is decided by "compress" mount option.
My suggestion is as follows and I'll write a patch based
on it soon.
- Change the meaning of btrfs.compression == "" to mean
unset both BTRFS_INODE_COMPRESS and BTRFS_INODE_NOCOMPRESS,
for chattr -c.
- Add new value of "btrfs.compression" property, for example
"no" or "off", to mean unset BTRFS_INODE_COMPRESS and set
BTRFS_INODE_NOCOMPRESS. It's for ioctl() with FS_NOCOMP_FL.
- Unify duplicated code of changing inode flags to props.c.
Thanks,
Satoru
> + if (ret && ret != -ENODATA)
> + goto out_drop;
> }
>
> trans = btrfs_start_transaction(root, 1);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
