[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-btrfs
Subject:    Re: [PATCH v2] btrfs: free device without BTRFS_MAGIC
From:       Josef Bacik <josef () toxicpanda ! com>
Date:       2020-09-30 12:41:39
Message-ID: 65f23ad0-6c83-27dd-d1d8-2862f6d6fd34 () toxicpanda ! com
[Download RAW message or body]

On 9/30/20 3:21 AM, Anand Jain wrote:
> 
> 
> On 29/9/20 2:14 am, Josef Bacik wrote:
>> On 9/21/20 11:13 PM, Anand Jain wrote:
>>> Many things can happen after the device is scanned and before the device
>>> is mounted.
>>>
>>> One such thing is losing the BTRFS_MAGIC on the device.
>>>
>>> If it happens we still won't free that device from the memory and causes
>>> the userland to confuse.
>>>
>>> For example: As the BTRFS_IOC_DEV_INFO still carries the device path which
>>> does not have the BTRFS_MAGIC, the btrfs fi show still shows device
>>> which does not belong. As shown below.
>>>
>>> mkfs.btrfs -fq -draid1 -mraid1 /dev/sda /dev/sdb
>>>
>>> wipefs -a /dev/sdb
>>> mount -o degraded /dev/sda /btrfs
>>> btrfs fi show -m
>>>
>>> /dev/sdb does not contain BTRFS_MAGIC and we still show it as part of
>>> btrfs.
>>> Label: none  uuid: 470ec6fb-646b-4464-b3cb-df1b26c527bd
>>>          Total devices 2 FS bytes used 128.00KiB
>>>          devid    1 size 3.00GiB used 571.19MiB path /dev/sda
>>>          devid    2 size 3.00GiB used 571.19MiB path /dev/sdb
>>>
>>
>> Wouldn't this also happen if the bytenrs didn't match?  In that case you 
>> aren't freeing anything, and we'd still show the improper device correct?  So 
>> why not deal with that case in a similar way?  Thanks,
> 
> Freeing the device without the BTRFS_MAGIC is mandatory because the
> device does not belong to btrfs even though we could notice from the
> sysfs that there is missing flag on this devid.
> 
> I think I should check for the BTRFS_MAGIC first before bytenr check,
> I shall swap them in v2 if there are no other comments. We need this
> patch as a fix for the test case btrfs/198.
> 
> However bytenrs mismatch indicates corruption. If the degraded mount
> option is not provided we would fail the mount. The user shall have the
> opportunity to fix the corrupted superblock. We don't automatically
> recover the corrupted superblock from the backup superblock copies. If
> the degraded mount option is provided the corrupted device still be in
> the device_list but with the missing flag set. Just by looking at btrfs
> fi show the user won't know that one of the devices is not part of the
> volume however when he looks into the /sys/fs/btrfs/fsid/devinfo/<devid>
> /missing it shall show 1. Our serviceability part of the degraded volume
> has some unfinished business when we evaluate it against the standard
> RAS features, but we are slowly getting there.
> 

Alright that's fair.  You can add

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef
[prev in list] [next in list] [prev in thread] [next in thread]