On 12/03/2017 05:43 PM, Nikolay Borisov wrote:
On 2.12.2017 01:23, Anand Jain wrote:
On 12/01/2017 05:19 PM, Nikolay Borisov wrote:
Currently this function executes the inner loop at most 1 due to the i
= 0;
i < 1 condition. Furthermore, the btrfs_super_generation(super) >
transid code
in the if condition is never executed due to latest always set to NULL
hence the
first part of the condition always triggering. The gist of
btrfs_read_dev_super
is really to read the first superblock.
Signed-off-by: Nikolay Borisov <nbori...@suse.com>
---
fs/btrfs/disk-io.c | 27 ++++-----------------------
1 file changed, 4 insertions(+), 23 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 82c96607fc46..6d5f632fd1e7 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3170,37 +3170,18 @@ int btrfs_read_dev_one_super(struct
block_device *bdev, int copy_num,
struct buffer_head *btrfs_read_dev_super(struct block_device *bdev)
{
struct buffer_head *bh;
- struct buffer_head *latest = NULL;
- struct btrfs_super_block *super;
- int i;
- u64 transid = 0;
- int ret = -EINVAL;
+ int ret;
/* we would like to check all the supers, but that would make
* a btrfs mount succeed after a mkfs from a different FS.
* So, we need to add a special mount option to scan for
* later supers, using BTRFS_SUPER_MIRROR_MAX instead
*/
We need below loop to support the above comment at some point,
And when is that, since I don't see anyone working on it.
Furthermore
what is it that we are losing in terms of functionality by not
supporting the comment?
As of now if the primary SB is corrupted we don't recover from it
automatically and external tools are broken.
It seems this code was just slapt here without
any vision how/when to implement it?
I have in my todo list. I am ok if you want to fix it as needed.
Furthermore, you seem to be aware of what the comment is talking about,
I have to admit I'm not.
Is the idea that if another filesystem does
mkfs and doesn't overwrite ALL superblock copies that btrfs writes (at
64k, 64mb, 256gb and 1 PiB) then it's possible for this code to
erroneously detect btrfs when in fact there is a different fs?
Right.
IMO the above comment is wrong as well for which it made the
for-loop to read only primary SB.
If we have a feature to maintain backup SB, then that feature
is only complete when we would automatically recover from the
backup SB.
If a user overwrites btrfs primary SB and still mounts btrfs
with -t btrfs options, then its use-end problem we should be
able to recover from backup SB. So IMO looping through other
SB is fine.
Thanks, Anand
I don't understand what problem *should* be solved here...
instead of removing I would prefer to fix as per above comments.
Thanks, Anand
- for (i = 0; i < 1; i++) {
- ret = btrfs_read_dev_one_super(bdev, i, &bh);
- if (ret)
- continue;
-
- super = (struct btrfs_super_block *)bh->b_data;
-
- if (!latest || btrfs_super_generation(super) > transid) {
- brelse(latest);
- latest = bh;
- transid = btrfs_super_generation(super);
- } else {
- brelse(bh);
- }
- }
-
- if (!latest)
+ ret = btrfs_read_dev_one_super(bdev, 0, &bh);
+ if (ret)
return ERR_PTR(ret);
- return latest;
+ return bh;
}
/*
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
