btrfs-progs: allow "no" to disable compression for convenience

PATCH Superseded Satoru Takeuchi 3 participants 6 messages Sep 17, 2017 – Oct 14, 2017

Subsystems (2)

Documentation Btrfs file system

lore

Share & tools

lore

Satoru Takeuchi

author

2017-09-17 00:36 UTC #0 lore raw

PATCH 1/1 btrfs-progs: allow "no" to disable compression for convenience

It's messy to use "" to disable compression. Introduce the new value "no" which can also be used for this purpose.

Signed-off-by Satoru Takeuchi <[email protected]>

---
 Documentation/btrfs-property.asciidoc | 2 +-
 props.c                               | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

Documentation/btrfs-property.asciidoc +1 -1

index 7ed6a7d..97b90d6 100644
@@ -43,7 +43,7 @@ read-only flag of subvolume: true or false
 label::::
 label of device
 compression::::
-compression setting for an inode: lzo, zlib, zstd, or "" (empty string)
+compression setting for an inode: lzo, zlib, zstd, no, or "" (empty string). Both no and "" are for disabling compression.
 
 *list* [-t <type>] <object>::
 Lists available properties with their descriptions for the given object.

props.c +4 -2

index a7e3e96..a2df868 100644
@@ -142,9 +142,11 @@ static int prop_compression(enum prop_object_type type,
 	memcpy(xattr_name + XATTR_BTRFS_PREFIX_LEN, name, strlen(name));
 	xattr_name[XATTR_BTRFS_PREFIX_LEN + strlen(name)] = '\0';
 
-	if (value)
+	if (value) {
+		if (!strcmp(value, "no"))
+			value = "";
 		sret = fsetxattr(fd, xattr_name, value, strlen(value), 0);
-	else
+	} else
 		sret = fgetxattr(fd, xattr_name, NULL, 0);
 	if (sret < 0) {
 		ret = -errno;

Signature · 1 line

2.7.4

Mike Fleetwood

2017-09-17 13:08 UTC #1 lore raw

Quoted 4 lines · Satoru Takeuchi

It's messy to use "" to disable compression. Introduce the new value "no" which can also be used for this purpose.
From an English language point of view, "none" would be better. None

says the absence of, where as no is more general negative.

Mike

Satoru Takeuchi

author

2017-09-18 00:41 UTC #2 lore raw

PATCH v2 1/1 btrfs-progs: allow "none" to disable compression for convenience

At Sun, 17 Sep 2017 14:08:40 +0100,

Quoted 8 lines · Mike Fleetwood

Quoted 2 lines · Satoru Takeuchi
It's messy to use "" to disable compression. Introduce the new value "no" which can also be used for this purpose.
From an English language point of view, "none" would be better. None says the absence of, where as no is more general negative.

Thank you for your comment. How about is it?

---
It's messy to use "" to disable compression. Introduce the new value "none" which can also be used for this purpose.

Signed-off-by Satoru Takeuchi <[email protected]>

---
 Documentation/btrfs-property.asciidoc | 2 +-
 props.c                               | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

Documentation/btrfs-property.asciidoc +1 -1

index 7ed6a7d..786af9b 100644
@@ -43,7 +43,7 @@ read-only flag of subvolume: true or false
 label::::
 label of device
 compression::::
-compression setting for an inode: lzo, zlib, zstd, or "" (empty string)
+compression setting for an inode: lzo, zlib, zstd, none, or "" (empty string). Both none and "" are for disabling compression.
 
 *list* [-t <type>] <object>::
 Lists available properties with their descriptions for the given object.

props.c +4 -2

index a7e3e96..8d85181 100644
@@ -142,9 +142,11 @@ static int prop_compression(enum prop_object_type type,
 	memcpy(xattr_name + XATTR_BTRFS_PREFIX_LEN, name, strlen(name));
 	xattr_name[XATTR_BTRFS_PREFIX_LEN + strlen(name)] = '\0';
 
-	if (value)
+	if (value) {
+		if (!strcmp(value, "none"))
+			value = "";
 		sret = fsetxattr(fd, xattr_name, value, strlen(value), 0);
-	else
+	} else
 		sret = fgetxattr(fd, xattr_name, NULL, 0);
 	if (sret < 0) {
 		ret = -errno;

Signature · 1 line

2.7.4

David Sterba

2017-09-19 15:15 UTC #3 lore raw

Re: [PATCH v2] btrfs-progs: allow "none" to disable compression for convenience

Quoted 8 lines · Satoru Takeuchi

At Sun, 17 Sep 2017 14:08:40 +0100,
Quoted 6 lines · Mike Fleetwood
Quoted 2 lines · Satoru Takeuchi
It's messy to use "" to disable compression. Introduce the new value "no" which can also be used for this purpose.

Quoted 8 lines

Quoted 2 lines
From an English language point of view, "none" would be better. None says the absence of, where as no is more general negative.
Thank you for your comment. How about is it?
---
It's messy to use "" to disable compression. Introduce the new value "none" which can also be used for this purpose.

I'd allow both values, 'no' and 'none', similar to the mount options, that also accept both (technically, the 'no' + anything is accepted for disabling compression).

Satoru Takeuchi

author

2017-09-20 02:03 UTC #4 lore raw

Re: [PATCH v2] btrfs-progs: allow "none" to disable compression for convenience

At Tue, 19 Sep 2017 17:14:27 +0200,

Quoted 15 lines · David Sterba

Quoted 13 lines · Satoru Takeuchi
At Sun, 17 Sep 2017 14:08:40 +0100,
Quoted 8 lines · Mike Fleetwood
Quoted 2 lines · Satoru Takeuchi
It's messy to use "" to disable compression. Introduce the new value "no" which can also be used for this purpose.
From an English language point of view, "none" would be better. None says the absence of, where as no is more general negative.
Thank you for your comment. How about is it?

Quoted 7 lines

Quoted 3 lines
---
It's messy to use "" to disable compression. Introduce the new value "none" which can also be used for this purpose.
I'd allow both values, 'no' and 'none', similar to the mount options, that also accept both (technically, the 'no' + anything is accepted for disabling compression).

As a result of reading "man 5 btrfs", now I prefer "no". It's used to mean disabling compression there. On the other hand, "none" is not used at all.

Quoted 1 line

From man 5 btrfs:

===
...
FILE ATTRIBUTES
...
compress, compress=type, compress-force, compress-force=type
(default: off)

Control BTRFS file data compression. Type may be specified as zlib, lzo or no (for no compression, used for remounting). If no type is specified, zlib is used. If compress-force is specified, all files will be compressed, whether or not they compress well.
...
X
no compression, permanently turn off compression on the given file, other compression mount options will not affect that
...
===

So David, please apply my v1 patcth if it looks good for you.

Thanks,
Satoru

Satoru Takeuchi

author

2017-10-14 23:35 UTC #5 lore raw

Re: [PATCH v2] btrfs-progs: allow "none" to disable compression for convenience

In reply to David Sterba

At Tue, 19 Sep 2017 17:14:27 +0200,

Quoted 15 lines · David Sterba

Quoted 13 lines · Satoru Takeuchi
At Sun, 17 Sep 2017 14:08:40 +0100,
Quoted 8 lines · Mike Fleetwood
Quoted 2 lines · Satoru Takeuchi
It's messy to use "" to disable compression. Introduce the new value "no" which can also be used for this purpose.
From an English language point of view, "none" would be better. None says the absence of, where as no is more general negative.
Thank you for your comment. How about is it?

Quoted 7 lines

Quoted 3 lines
---
It's messy to use "" to disable compression. Introduce the new value "none" which can also be used for this purpose.
I'd allow both values, 'no' and 'none', similar to the mount options, that also accept both (technically, the 'no' + anything is accepted for disabling compression).

But only "no" is writtein in man 5 btrfs.