Re: [PATCH 3/3 v4] btrfs-progs: disable using backup superblock by default

[Anand Jain <Anand.Jain@oracle.com>]

>>>> <maybe a little more commit log would be good?>
>>>>
>>>> So here is what confuses me now. :)
>>>>
>>>> *every* caller of btrfs_read_dev_super() is now called with
>>>> 0 for the flags variable, so it never reads the backup
>>>> under any circumstance.
>>>>
>>>> If it's always called w/ 0, what is the point of the argument?
>>>> Is there another patch you have planned that would use this argument
>>>> later?
>>>
>>>   Thanks for the review. yes true. as of now it (BTRFS_SCAN_BACKUP_SB)
>>>   only serves the purpose if in future should we need it.
>>>   purpose is something like a user initiated thread which
>>>   should to go to the backup-SB if primary-SB is not found ?.
>>>   Or I can drop BTRFS_SCAN_BACKUP_SB idea depending on how
>>>   it is convenient as a whole.
>>
>> See what others think, perhaps, but if nobody is using it, I think
>> it should just go away.  I'd call it "dead code." :)
>>
>> But I am surprised that none of the commands which accept alternate
>> superblock locations find their way into btrfs_read_dev_super() -
>> that seems odd to me.  Is it re-implemented or open-coded in other
>> places?
>
> So to be clearer, rather than removing the code right away, maybe
> it's worth a look to see if the other commands which *want* backup
> superblocks should be using this same code.  Then you'd have a reason
> for your new flag.  :)


  when non primary SB (sb_bytenr) is specified in btrfs_read_dev_super()
  (that is when user is involved)  it would directly fetch it. so its
  not a problem when we know which SB to read other than the primary SB.

  However when primary SB is specified it would look for only primary SB
  unless BTRFS_SCAN_BACKUP_SB flag is set (with the patch). Now, do we
  need this flag ? looks like Yes ! (sorry to change my opinion here
  though) and as below..

  In some cases when user is _not_ involved. Like in

  check_mounted().

  In a multi dev btrfs mounted fs. If by any chance the primary SB
  is corrupted then we would say the device is NOT mounted even
  if it is mounted.

eg:
# mkfs.btrfs /dev/sdb /dev/sdc -f && mount /dev/sdb /btrfs
# ./check-mounted /dev/sdc
its btrfs
/dev/sdc is currently mounted. Aborting.
# dd if=/dev/zero of=/dev/sdc count=8 seek=$(((64 * 1024)/512))
# ./check-mounted /dev/sdc
Not mounted
# cat /proc/mounts | egrep btrfs
/dev/sdb /btrfs btrfs rw,seclabel,relatime,noacl,space_cache 0 0


  So we have to set BTRFS_SCAN_BACKUP_SB for check_mounted()
  But the above scenario is not simple enough to be practical though.


-Anand