[PATCH] btrfs-progs: allow user to insert property compression=none to file.

[PATCH] btrfs-progs: allow user to insert property compression=none to file.

[Li Zhang]

1. If the user adds the property "compression=none" to the file,
remove the code that converts the None string to an empty string.

Signed-off-by: Li Zhang <zhanglikernel@gmail.com>
---
 cmds/property.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/cmds/property.c b/cmds/property.c
index b3ccc0f..ec1b408 100644
--- a/cmds/property.c
+++ b/cmds/property.c
@@ -190,8 +190,6 @@ static int prop_compression(enum prop_object_type type,
 	xattr_name[XATTR_BTRFS_PREFIX_LEN + strlen(name)] = '\0';
 
 	if (value) {
-		if (strcmp(value, "no") == 0 || strcmp(value, "none") == 0)
-			value = "";
 		sret = fsetxattr(fd, xattr_name, value, strlen(value), 0);
 	} else {
 		sret = fgetxattr(fd, xattr_name, NULL, 0);
-- 
1.8.3.1

Re: [PATCH] btrfs-progs: allow user to insert property compression=none to file.

[David Sterba]

On Sun, Jan 16, 2022 at 04:52:43PM +0800, Li Zhang wrote:
> 1. If the user adds the property "compression=none" to the file,
> remove the code that converts the None string to an empty string.

This is related to 5548c8c6f55b ("btrfs: props: change how empty value
is interpreted") and needs some explanation that what it does on old and
new kernel. Maybe some backward compatibility code in progs is needed to
take the version into account.

> 
> Signed-off-by: Li Zhang <zhanglikernel@gmail.com>
> ---
>  cmds/property.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/cmds/property.c b/cmds/property.c
> index b3ccc0f..ec1b408 100644
> --- a/cmds/property.c
> +++ b/cmds/property.c
> @@ -190,8 +190,6 @@ static int prop_compression(enum prop_object_type type,
>  	xattr_name[XATTR_BTRFS_PREFIX_LEN + strlen(name)] = '\0';
>  
>  	if (value) {
> -		if (strcmp(value, "no") == 0 || strcmp(value, "none") == 0)
> -			value = "";
>  		sret = fsetxattr(fd, xattr_name, value, strlen(value), 0);
>  	} else {
>  		sret = fgetxattr(fd, xattr_name, NULL, 0);
> -- 
> 1.8.3.1

Re: [PATCH] btrfs-progs: allow user to insert property compression=none to file.

[li zhang]

Old behavior:
# Insert compressed none in the file
$ btrfs property set file compression none
$ btrfs property gets file
# no output, (none value converted to empty string)

New behavior:
# Insert compressed none in the file
$ btrfs property set file compression none
$ btrfs property gets file
compression=none
(with compression=none inserted in the file's xattr)


Convert the attribute compression=none to an empty string "", which was
introduced in commit df11e2787b5b57ecdb313f2725dc5c9a5e549576(btrfs-progs),
according to the comments, in the past it seemed that the empty string
"" represented
no compression, but after commit 5548c8c6f55b(btrfs-devel), the
character The string
"none" means no compression. so the command
"btrfs property set <file> compression none" is not working.

David Sterba <dsterba@suse.cz> 于2022年1月17日周一 21:45写道：
>
> On Sun, Jan 16, 2022 at 04:52:43PM +0800, Li Zhang wrote:
> > 1. If the user adds the property "compression=none" to the file,
> > remove the code that converts the None string to an empty string.
>
> This is related to 5548c8c6f55b ("btrfs: props: change how empty value
> is interpreted") and needs some explanation that what it does on old and
> new kernel. Maybe some backward compatibility code in progs is needed to
> take the version into account.
>
> >
> > Signed-off-by: Li Zhang <zhanglikernel@gmail.com>
> > ---
> >  cmds/property.c | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/cmds/property.c b/cmds/property.c
> > index b3ccc0f..ec1b408 100644
> > --- a/cmds/property.c
> > +++ b/cmds/property.c
> > @@ -190,8 +190,6 @@ static int prop_compression(enum prop_object_type type,
> >       xattr_name[XATTR_BTRFS_PREFIX_LEN + strlen(name)] = '\0';
> >
> >       if (value) {
> > -             if (strcmp(value, "no") == 0 || strcmp(value, "none") == 0)
> > -                     value = "";
> >               sret = fsetxattr(fd, xattr_name, value, strlen(value), 0);
> >       } else {
> >               sret = fgetxattr(fd, xattr_name, NULL, 0);
> > --
> > 1.8.3.1

Re: [PATCH] btrfs-progs: allow user to insert property compression=none to file.

[Qu Wenruo]

Furthermore, that kernel commit has a big problem for a lot of
compression users:

  No way to disable compression on a per-file basis, if using
compression= mount option.

As progs always convert "none"/"no" to "", this leads means older progs
on newer kernel will use default value, and no way to really set
"none"/"no".

This is already a big behavior breakage.

Thanks,
Qu

On 2022/1/19 18:49, li zhang wrote:
> Old behavior:
> # Insert compressed none in the file
> $ btrfs property set file compression none
> $ btrfs property gets file
> # no output, (none value converted to empty string)
>
> New behavior:
> # Insert compressed none in the file
> $ btrfs property set file compression none
> $ btrfs property gets file
> compression=none
> (with compression=none inserted in the file's xattr)
>
>
> Convert the attribute compression=none to an empty string "", which was
> introduced in commit df11e2787b5b57ecdb313f2725dc5c9a5e549576(btrfs-progs),
> according to the comments, in the past it seemed that the empty string
> "" represented
> no compression, but after commit 5548c8c6f55b(btrfs-devel), the
> character The string
> "none" means no compression. so the command
> "btrfs property set <file> compression none" is not working.
>
> David Sterba <dsterba@suse.cz> 于2022年1月17日周一 21:45写道：
>>
>> On Sun, Jan 16, 2022 at 04:52:43PM +0800, Li Zhang wrote:
>>> 1. If the user adds the property "compression=none" to the file,
>>> remove the code that converts the None string to an empty string.
>>
>> This is related to 5548c8c6f55b ("btrfs: props: change how empty value
>> is interpreted") and needs some explanation that what it does on old and
>> new kernel. Maybe some backward compatibility code in progs is needed to
>> take the version into account.
>>
>>>
>>> Signed-off-by: Li Zhang <zhanglikernel@gmail.com>
>>> ---
>>>   cmds/property.c | 2 --
>>>   1 file changed, 2 deletions(-)
>>>
>>> diff --git a/cmds/property.c b/cmds/property.c
>>> index b3ccc0f..ec1b408 100644
>>> --- a/cmds/property.c
>>> +++ b/cmds/property.c
>>> @@ -190,8 +190,6 @@ static int prop_compression(enum prop_object_type type,
>>>        xattr_name[XATTR_BTRFS_PREFIX_LEN + strlen(name)] = '\0';
>>>
>>>        if (value) {
>>> -             if (strcmp(value, "no") == 0 || strcmp(value, "none") == 0)
>>> -                     value = "";
>>>                sret = fsetxattr(fd, xattr_name, value, strlen(value), 0);
>>>        } else {
>>>                sret = fgetxattr(fd, xattr_name, NULL, 0);
>>> --
>>> 1.8.3.1