On 24.10.18 г. 15:30 ч., David Sterba wrote:
> On Fri, Oct 12, 2018 at 05:57:29PM +0800, Qu Wenruo wrote:
>>>>>> + printf("log_root_transid (deprecated)\t%llu\n",
>>>>>> + le64_to_cpu(sb->__unused_log_root_transid));
>>>>>
>>>>> This should be entirely removed.
>>>>
>>>> It looks OK to me.
>>>> Just like the old leafsize.
>>>>
>>>> And if we try to use this member again, even old progs could show it
>>>> without problem.
>>>
>>> But what is the point of having something which always shows 0 and is
>>> essentially unused?
>>
>> Personally speaking, even it's in fact never used, it is still part of
>> the specification of btrfs super blocks.
>>
>> Even it's a bad decision we made a long time ago, we still need to mark
>> it as unused, other than converting it back to "reserved".
>>
>> To make it short and clear, if some member is specified in early
>> specification, even it's never used, we still need to mark it unused and
>> keep part of naming.
>
> Agreed, follows the same patern as the leafsize. The superblock dump is
> a debugging and bug report tool, the full output is desirable.
I really don't want to get into the argument but I think we should
really name unused fields unusedX where X is some number. Looking at the
code in struct btrfs_root_backup we have 2 unused fields. In the
superblock we have unused space which is called 'reserved'. In struct
btrfs_disk_balance_args there is a field called unused, same thing for
struct btrfs_balance_item.
In btrfs_inode_item we have 'reserved' field, same thing for
btrfs_root_item.
So the code base is yet again highly inconsistent and looking at it
there is only a single field following the __unused_original_name, so
it's really the exception rather than the rule.
Again, I would argue that once a field is deprecated or in this case it
has _never_ actually been used then we might as well rename it to
something cryptic so as not to attach (false) meaning to it. Just my 50
cents
