Re: [PATCH 1/2] btrfs: Populate btrfs_super_block::log_root_transid

[Nikolay Borisov <nborisov@suse.com>]

On 11.10.2018 15:45, Qu Wenruo wrote:
> 
> 
> On 2018/10/11 下午8:31, David Sterba wrote:
>> On Thu, Oct 04, 2018 at 01:49:49PM +0800, Qu Wenruo wrote:
>>> We have btrfs_super_block::log_root_transid to record the transid of log
>>> tree root.
>>>
>>> However it's never populated and it's always 0, just as the following
>>> dump-super:
>>>  log_root                30572544
>>>  log_root_transid        0
>>>  log_root_level          0
>>>
>>> This patch will populate it with log tree root correctly, so the result
>>> will be:
>>>  log_root                30572544
>>>  log_root_transid        6
>>>  log_root_level          0
>>>
>>> This won't affect current kernel behavior or btrfs check result as we
>>> already expect log tree root generation always to be super block
>>> genration + 1.
>>>
>>> But it could be later used to detect log tree corruption early.
>>
>> The backward compatibility seems to be ok in general, I found one
>> scenario where the check will fail:
>>
>> * mount with unpatched kernel, log_root_transid = 0
>> * mount with patched kernel, log_root_transid 100, generation 101
>> * mount with unpatched kernel, log_root_transid 100 (unchanged), generation 201
>> * mount with patched kernel -> check fails
> 
> Indeed, this is a problem I missed.
> 
> It provides the old principle, if we're going to change how kernel
> use/interprete a on-disk memeber, we have to introduce a new
> incompatible flag.
> 
> Please just drop the series of patch.

Since this member really doesn't have any purpose now I think the
prudent thing to do is to rename it to "reserved1" or "padding" or
whatever but basically signal that it's no longer in used. In the future
when a new incompaat bit will be required for some feature it can be
re-used.

> 
> Thanks,
> Qu
>>
>> So the problem is when log_root_transid is not 0 and changes out of sync
>> with the generation. The above sequence of kernels can simply happen
>> when switching between old stable and current releases.
>>
>