Re: [PATCH] btrfs: do not allow -o compress-force to override per-inode settings

[Josef Bacik <josef@toxicpanda.com>]

On 11/30/20 10:04 AM, Hugo Mills wrote:
> On Mon, Nov 30, 2020 at 09:50:13AM -0500, Josef Bacik wrote:
>> On 11/30/20 9:08 AM, Roman Mamedov wrote:
>>> On Mon, 30 Nov 2020 08:46:21 -0500
>>> Josef Bacik <josef@toxicpanda.com> wrote:
>>>
>>>> However some time later we got chattr -c, which is a user way of
>>>> indicating that the file shouldn't be compressed.  This is at odds with
>>>> -o compress-force.  We should be honoring what the user wants, which is
>>>> to disable compression.
>>>
>>> But chattr -c only removes the previously set chattr +c. There's no
>>> "negative-c" to be forced by user in attributes. And +c is already unset on all
>>> files by default. Unless I'm missing something? Thanks
>>>
>>
>> The thing you're missing is that when we do chattr -c we're setting
>> NOCOMPRESS on the file.  The thing that I'm missing is what exactly we're
>> trying to allow.  If chattr -c is supposed to just be the removal of +c,
>> then btrfs is doing the wrong thing by setting NOCOMPRESS.  We also do the
>> same thing when we clear a btrfs.compression property.
> 
>     If I'm understanding this right, there's more than two states
> here. There's a "default", and there's two "force" options -- forcing
> nocompress, and forcing/allowing compression. If there's no c flag
> set, the file could be in one of two states: default behaviour
> (presumably defined by the value of the mount options), and
> NOCOMPRESS. How do I tell which it is?

Yeah you can't really.  If you're mounted with -o compress-force then you won't 
get NOCOMPRESS set automatically, so you can sort of (emphasis on sort of) 
assume that if you have NOCOMPRESS set then it was done by some user specific 
action.  But it seems like we don't even really clearly define that this is an 
option, so we're probably better off just killing that usage everywhere.  Thanks,

Josef