'btrfs property set <file> compression no' does not enable the "NOCOMPRESS" attribute

[Forza-tng]

According to https://btrfs.wiki.kernel.org/index.php/Changelog I believe should be possible to set the NOCOMRESS attribute using btrfs property set <file> compression no.

From the changelog:

property value reset | 5.14 | Change how empty value is interpreted. New behaviour will delete the value and reset it to default. This affects btrfs.compression where value no sets NOCOMPRESS bit while empty value resets all compression settings (either compression or NOCOMPRESS bit).

However in my testing I found it does not work. We can see that compression is still applied even if btrfs property set <file> compression no was used, while compression is not applied when chattr +m is used.

# touch foobar
# btrfs property set foobar compression zstd
# lsattr
--------c------------- ./foobar

# getfattr -n btrfs.compression foobar
file: foobar
btrfs.compression="zstd"

# btrfs property set foobar compression no
# lsattr
---------------------- ./foobar

# getfattr -n btrfs.compression foobar
foobar: btrfs.compression: No such attribute

# dd if=/dev/zero of=foobar count=100
100+0 records in
100+0 records out
51200 bytes (51 kB, 50 KiB) copied, 0.000799556 s, 64.0 MB/s

# compsize foobar
Processed 1 file, 1 regular extents (1 refs), 0 inline.
Type       Perc     Disk Usage   Uncompressed Referenced
TOTAL        7%      4.0K          52K          52K
zstd         7%      4.0K          52K          52K

# truncate -s 0 foobar
# chattr +m foobar
---------------------m ./foobar

# dd if=/dev/zero of=foobar count=100
100+0 records in
100+0 records out
51200 bytes (51 kB, 50 KiB) copied, 0.000883995 s, 57.9 MB/s

# compsize foobar
Processed 1 file, 1 regular extents (1 refs), 0 inline.
Type       Perc     Disk Usage   Uncompressed Referenced
TOTAL      100%       52K          52K          52K
none       100%       52K          52K          52K

I'm using kernel-5.15.19 and btrfs-progs v5.16

[zhanglikernel]

Hi, I noticed this phenomenon before, I propose a patch, details are here https://www.spinics.net/lists/linux-btrfs/msg120306.html, But based on David's comment, this patch doesn't seem to be backwards compatible with older versions, I'll dig into it more later.

[kdave]

It's my fault, I forgot to update the progs when I did the kernel change. The correct behaviour is to pass to kernel the value exactly as specified, ie. no translation from no or none to empty value. Zhang's patch is correct.

Backward compatibility:

unpatched kernel, patched progs:

• empty value -> set NOCOMPRESS, reset property compression to NONE
• no/none -> EINVAL

patched kernel, unpatched progs:

• empty value in progs -> empty value in kernel, set defaults
• no/none in progs (translated to empty value) -> same, set defaults

patched kernel, patched progs:

• no/none -> NOCOMPRESS
• empty value -> set defaults

[kdave]

There are various combinations of progs and kernel out there, people don't update when not necessary, so as least surprise we can draw the line at 5.14 for kernel and 5.16.x for progs where both work in the way above (which was requested by users). I'm open to other options.

[Forza-tng]

This sounds reasonable to me. What happens if booting an older unpatched kernel with "none" set? Would that be causing an error?

[zhanglikernel]

Hi, Forza-tng, you mean if a file is already set to compression none and the current kernel doesn't support the compress none attribute? for example, you add compression none to the file in the patched kernel and reboot the system with the unpatched kernel. In this case files already set to compression none will not be compressed because those files are set to NOCOMPRESS in the inode flags, but in an unpatched kernel if you want to insert compression none into the file it will return EINVAL.

[zhanglikernel]

Hi David, I have a question, it seems that the logic to convert from compression description to btrfs_compression_type enum is not only in the code, in btrfs-devel super.c btrfs_parse_options function parses compression description as btrfs_compression_type(mount time option) , btrfs-devel props.c prop_compression_apply function(set file compression property), in btrfs-prop common/parse-utils.c parse_compress_type function(set file defrag compression), is there any plan to combine these three logics into one? It seems that if we are going to support another compression algorithm, we should write the conversion logic in these three places.

[Forza-tng]

Hi, Forza-tng, you mean if a file is already set to compression none and the current kernel doesn't support the compress none attribute? for example, you add compression none to the file in the patched kernel and reboot the system with the unpatched kernel. In this case files already set to compression none will not be compressed because those files are set to NOCOMPRESS in the inode flags, but in an unpatched kernel if you want to insert compression none into the file it will return EINVAL.

The compression attribute is stored as both an xattr and an fsattr. getfattr -n btrfs.compression myfile.db and lsattr myfile.db. It is the xattr I was wondering about. If it is set as btrfs.compression="none", would this cause issues with old kernels?

[kdave]

Fix applied to 5.17, I'll keep the issue open as there are some questions

[Forza-tng]

Just tested again and everything seems to work as intended. Perhaps improve documentation that empty values are specified as two double or single quotes with no space between them?

# btrfs property set loop.sh  compression " "
ERROR: failed to set compression for loop.sh: Invalid argument

# btrfs property set loop.sh  compression ""

[kdave]

Thanks for testing, yeah the manual page was not readable. I've restructured it so it spells the options as a list, the empty value is double quotes and is hopefully understandable.

[rabhatumaragar]

@Forza-tng Thanks a bunch for this man!

[kdave]

If there are any remaining questions or bugs to fix, please open a new issue. Closing.