From: Naohiro Aota <na...@elisp.net>

In the current implementation, compression property == "" has
the two different meanings: one is with BTRFS_INODE_NOCOMPRESS,
and the other is without this flag.

So, even if the two files a and b have the same compression
property, "", and the same contents, one file seems to be
compressed and the other is not. It's difficult to understand
for users and also confuses them.

Here is the real example. Let assume the following two cases.

  a) A file created freshly (under a directory without both
     COMPRESS and NOCOMPRESS flag.)

  b) A existing file which is explicitly set ""
     to compression property.

In addition, here is the command log (I attached the source of
"getflags" program in this patch.)

=======
$ rm -f a b; touch a b
$ btrfs prop set b compression ""
 # both a and b have the same compression property: ""
$ btrfs prop get a compression
$ btrfs prop get b compression
 # but ... let's take a look at inode flags
$ ./getflags a
0x0
$ ./getflags b
0x400             # 0x400 (FS_NOCOMP_FL) corresponds to BTRFS_INODE_NOCOMPRESS
=======

So both these two files have their compression property == "",
but have different NOCOMPRESS flag state leading to different
behavior.

case | BTRFS_INODE_NOCOMPRESS | behavior
=====+========================+=========================
   a | unset                  | might be compressed
   b | set                    | never be compressed

I consider that we should not expect users to remember
whether their files are case a or b and should introduce
another value for compress property anyway.

getflags.c:
===================
#include <sys/ioctl.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <stdio.h>
#include <linux/fs.h>

int main(int argc, char const* argv[])
{
  const char *name = argv[1];
  int fd = open(name, O_RDONLY);
  long x;
  ioctl(fd, FS_IOC_GETFLAGS, &x);
  printf("0x%lx\n", x);
  return 0;
}
===================

Signed-off-by: Naohiro Aota <na...@elisp.net>
Signed-off-by: Satoru Takeuchi <takeuchi_sat...@jp.fujitsu.com>
---
 fs/btrfs/props.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
index 129b1dd..bf005f4 100644
--- a/fs/btrfs/props.c
+++ b/fs/btrfs/props.c
@@ -393,8 +393,8 @@ static int prop_compression_apply(struct inode *inode,
        int type;
 
        if (len == 0) {
-               BTRFS_I(inode)->flags |= BTRFS_INODE_NOCOMPRESS;
-               BTRFS_I(inode)->flags &= ~BTRFS_INODE_COMPRESS;
+               BTRFS_I(inode)->flags &=
+                       ~(BTRFS_INODE_COMPRESS | BTRFS_INODE_NOCOMPRESS);
                BTRFS_I(inode)->force_compress = BTRFS_COMPRESS_NONE;
 
                return 0;
-- 1.8.3.1 

--
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

Reply via email to