Re: [PATCH] Btrfs: fix removal of raid[56|1c34} incompat flags after removing block group

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2749 bytes --]



On 2020/3/24 上午3:28, David Sterba wrote:
> On Sun, Mar 22, 2020 at 08:42:20AM +0800, Qu Wenruo wrote:
>>
>>
>> On 2020/3/22 上午1:45, David Sterba wrote:
>>> On Sat, Mar 21, 2020 at 09:43:21AM +0800, Qu Wenruo wrote:
>>>>
>>>>
>>>> On 2020/3/21 上午2:43, fdmanana@kernel.org wrote:
>>>>> From: Filipe Manana <fdmanana@suse.com>
>>>>>
>>>>> We are incorrectly dropping the raid56 and raid1c34 incompat flags when
>>>>> there are still raid56 and raid1c34 block groups, not when we do not any
>>>>> of those anymore. The logic just got unintentionally broken after adding
>>>>> the support for the raid1c34 modes.
>>>>>
>>>>> Fix this by clear the flags only if we do not have block groups with the
>>>>> respective profiles.
>>>>>
>>>>> Fixes: 9c907446dce3 ("btrfs: drop incompat bit for raid1c34 after last block group is gone")
>>>>> Signed-off-by: Filipe Manana <fdmanana@suse.com>
>>>>
>>>> The fix is OK.
>>>>
>>>> Reviewed-by: Qu Wenruo <wqu@suse.com>
>>>>
>>>> Just interesting do we really need to remove such flags?
>>>> To me, keep the flag is completely sane.
>>>
>>> So you'd suggest to keep a flag for a feature that's not used on the
>>> filesystem so it's not possible to mount the filesystem on an older
>>> kernel?
>>>
>> If user is using this feature, they aren't expecting mounting it on
>> older kernel either.
> 
> Before we go in a loop throwing single statements, let me take a broader
> look.
> 
> First thing, the removal of incompat bit was asked for by users, Hugo is
> as reporter in the commit 6d58a55a894e863.
> 
> https://lore.kernel.org/linux-btrfs/20190610144806.GB21016@carfax.org.uk/
> 
>   "   We've had a couple of cases in the past where people have tried out
>   a new feature on a new kernel, then turned it off again and not been
>   able to go back to an earlier kernel. Particularly in this case, I can
>   see people being surprised at the trapdoor. "I don't have any RAID13C
>   on this filesystem: why can't I go back to 5.2?"
> 
> That itself is a strong sign to me that there's a need or usecase or a
> good idea. Though we have a lot of them, this one was simple to
> implement and made sense to me. For the raid56 it's a simple check,
> unlike for other features that would need to go through significant
> portion of metadata.
> 
> Booting older kernel might sound like, why would anybody want to do
> that, but if the bit is there preventing boot/mount, then it's an
> unnecessary complication. In rescue environmnents it's gold.
> 
> Usability improvements tend to be boring from code POV but it is
> something that users can observe and make use of.
> 
Thanks for the full picture.

That makes sense.

Thanks,
Qu


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]