Re: [PATCH 2/4] btrfs: detect uninitialized btrfs_root::anon_dev for user visible subvolumes

Qu Wenruo
2020-06-17T23:39:39Z

On 2020/6/17 下午9:37, Josef Bacik wrote:
> On 6/17/20 7:31 AM, David Sterba wrote:
>> On Wed, Jun 17, 2020 at 07:49:33AM +0800, Qu Wenruo wrote:
>>>>>> Can we handle stat->dev not having a device set?  Or will this
>>>>>> blow up
>>>>>> in other ways?  Thanks,
>>>>>
>>>>> We can handle it without any problem, just users get confused.
>>>>>
>>>>> As a common practice, we use different bdev as a namespace for
>>>>> different
>>>>> subvolumes.
>>>>> Without a valid bdev, some user space tools may not be able to
>>>>> distinguish inodes in different subvolumes.
>>>>>
>>>>
>>>> Alright that's fine then.  But I feel like stat is one of those things
>>>> that'll flood the console, can we put this somewhere else that's going
>>>> to be hit less? Thanks,
>>>
>>> Unfortunately, stat() is the only user of btrfs_root::anon_dev.
>>>
>>> While fortunately, the logical is pretty simple, even without the safe
>>> net we can understand the lifespan pretty well.
>>>
>>> I'm fine to drop this patch if you're concerned about the possible
>>> warning flood, as the benefit is really not that much.
>>
>> It could be a developer-only warning but if there's a root with a bad
>> anon_dev, a simple 'ls -l' would flood the log for sure.
>>
>
> We'll know in btrfs_init_fs_root() when get_anon_bdev() fails right?
> Can't we just complain then?  That seems less spammy.  Thanks,

That's exactly where we do the allocation, that would be something like
warning about the allocation result.

That's why I'm fine to drop this patch if it's too spammy.

Thanks,
Qu
>
> Josef