[PATCH] Revert "Btrfs: allow superblock mismatch from older mkfs"

[PATCH] Revert "Btrfs: allow superblock mismatch from older mkfs"

[Liu Bo]

This reverts commit 667e7d94a1683661cff5fe9a0fa0d7f8fdd2c007.

This was used to make sure that a fresh btrfs from an older mkfs.btrfs,
and it also allows us to mount a buggy btrfs if this btrfs has the right
superblock head part but has something wrong with chunk tree part[1], and
after that we can hit BUG_ON()s set in the code to prevent something
impossible.

Since David has gave us "Btrfs progs v3.19-rc2", just remove the check,
if anyone who wants to make a fresh btrfs, please use the latest one.

[1]: http://www.spinics.net/lists/linux-btrfs/msg42358.html

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>

Conflicts:
	fs/btrfs/disk-io.c
---
 fs/btrfs/disk-io.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 41b320e..f63aacf 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -418,12 +418,6 @@ static int btrfs_check_super_csum(char *raw_disk_sb)
 
 		if (memcmp(raw_disk_sb, result, csum_size))
 			ret = 1;
-
-		if (ret && btrfs_super_generation(disk_sb) < 10) {
-			printk(KERN_WARNING
-				"BTRFS: super block crcs don't match, older mkfs detected\n");
-			ret = 0;
-		}
 	}
 
 	if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes)) {
-- 
1.8.1.4

Re: [PATCH] Revert "Btrfs: allow superblock mismatch from older mkfs"

[Qu Wenruo]

I sent such patch some time ago but was rejected by Chris,
hopes this time it can be accepted.

This one can make btrfs much more robust.

Thanks,
Qu
-------- Original Message  --------
Subject: [PATCH] Revert "Btrfs: allow superblock mismatch from older mkfs"
From: Liu Bo <bo.li.liu@oracle.com>
To: <linux-btrfs@vger.kernel.org>
Date: 2015年03月12日 13:40

> This reverts commit 667e7d94a1683661cff5fe9a0fa0d7f8fdd2c007.
>
> This was used to make sure that a fresh btrfs from an older mkfs.btrfs,
> and it also allows us to mount a buggy btrfs if this btrfs has the right
> superblock head part but has something wrong with chunk tree part[1], and
> after that we can hit BUG_ON()s set in the code to prevent something
> impossible.
>
> Since David has gave us "Btrfs progs v3.19-rc2", just remove the check,
> if anyone who wants to make a fresh btrfs, please use the latest one.
>
> [1]: http://www.spinics.net/lists/linux-btrfs/msg42358.html
>
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
>
> Conflicts:
> 	fs/btrfs/disk-io.c
> ---
>   fs/btrfs/disk-io.c | 6 ------
>   1 file changed, 6 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 41b320e..f63aacf 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -418,12 +418,6 @@ static int btrfs_check_super_csum(char *raw_disk_sb)
>
>   		if (memcmp(raw_disk_sb, result, csum_size))
>   			ret = 1;
> -
> -		if (ret && btrfs_super_generation(disk_sb) < 10) {
> -			printk(KERN_WARNING
> -				"BTRFS: super block crcs don't match, older mkfs detected\n");
> -			ret = 0;
> -		}
>   	}
>
>   	if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes)) {
>

Re: [PATCH] Revert "Btrfs: allow superblock mismatch from older mkfs"

[David Sterba]

On Thu, Mar 12, 2015 at 01:40:47PM +0800, Liu Bo wrote:
> This reverts commit 667e7d94a1683661cff5fe9a0fa0d7f8fdd2c007.
> 
> This was used to make sure that a fresh btrfs from an older mkfs.btrfs,
> and it also allows us to mount a buggy btrfs if this btrfs has the right
> superblock head part but has something wrong with chunk tree part[1], and
> after that we can hit BUG_ON()s set in the code to prevent something
> impossible.
> 
> Since David has gave us "Btrfs progs v3.19-rc2", just remove the check,
> if anyone who wants to make a fresh btrfs, please use the latest one.

I think the fixed progs are out long enough and the combination of
really old progs + really new kernel is very uncommon and we don't need
to care about that anymore.

However, please do not do it as a revert, it's a normal patch although
it effectively reverts some other patch.

Re: [PATCH] Revert "Btrfs: allow superblock mismatch from older mkfs"

[David Sterba]

On Thu, Mar 12, 2015 at 01:45:05PM +0800, Qu Wenruo wrote:
> I sent such patch some time ago but was rejected by Chris,
> hopes this time it can be accepted.

http://www.mail-archive.com/linux-btrfs%40vger.kernel.org/msg34937.html

I'm OK with removing the special case now because there are additional
supreblock checks merged (3.19) that were motivated by your previous
attempt to remove it.

o btrfs: add more checks to btrfs_read_sys_array
o btrfs: cleanup, rename a few variables in btrfs_read_sys_array
o btrfs: add checks for sys_chunk_array sizes
o btrfs: more superblock checks, lower bounds on devices and sectorsize/nodesize

Re: [PATCH] Revert "Btrfs: allow superblock mismatch from older mkfs"

[Liu Bo]

On Thu, Mar 12, 2015 at 06:25:13PM +0100, David Sterba wrote:
> On Thu, Mar 12, 2015 at 01:40:47PM +0800, Liu Bo wrote:
> > This reverts commit 667e7d94a1683661cff5fe9a0fa0d7f8fdd2c007.
> > 
> > This was used to make sure that a fresh btrfs from an older mkfs.btrfs,
> > and it also allows us to mount a buggy btrfs if this btrfs has the right
> > superblock head part but has something wrong with chunk tree part[1], and
> > after that we can hit BUG_ON()s set in the code to prevent something
> > impossible.
> > 
> > Since David has gave us "Btrfs progs v3.19-rc2", just remove the check,
> > if anyone who wants to make a fresh btrfs, please use the latest one.
> 
> I think the fixed progs are out long enough and the combination of
> really old progs + really new kernel is very uncommon and we don't need
> to care about that anymore.
> 
> However, please do not do it as a revert, it's a normal patch although
> it effectively reverts some other patch.

Okay, I'll update this ASAP, thanks for your review.

Thanks,

-liubo

Re: [PATCH] Revert "Btrfs: allow superblock mismatch from older mkfs"

[Liu Bo]

On Thu, Mar 12, 2015 at 06:34:15PM +0100, David Sterba wrote:
> On Thu, Mar 12, 2015 at 01:45:05PM +0800, Qu Wenruo wrote:
> > I sent such patch some time ago but was rejected by Chris,
> > hopes this time it can be accepted.
> 
> http://www.mail-archive.com/linux-btrfs%40vger.kernel.org/msg34937.html
> 
> I'm OK with removing the special case now because there are additional
> supreblock checks merged (3.19) that were motivated by your previous
> attempt to remove it.

So David, are you going to take the above patch instead?

I'm okay with that since they're exactly same, but it'd be good that Qu has its commit log updated.

Thanks,

-liubo

> 
> o btrfs: add more checks to btrfs_read_sys_array
> o btrfs: cleanup, rename a few variables in btrfs_read_sys_array
> o btrfs: add checks for sys_chunk_array sizes
> o btrfs: more superblock checks, lower bounds on devices and sectorsize/nodesize
> 
> 

Re: [PATCH] Revert "Btrfs: allow superblock mismatch from older mkfs"

[David Sterba]

On Fri, Mar 13, 2015 at 04:15:08AM +0800, Liu Bo wrote:
> On Thu, Mar 12, 2015 at 06:34:15PM +0100, David Sterba wrote:
> > On Thu, Mar 12, 2015 at 01:45:05PM +0800, Qu Wenruo wrote:
> > > I sent such patch some time ago but was rejected by Chris,
> > > hopes this time it can be accepted.
> > 
> > http://www.mail-archive.com/linux-btrfs%40vger.kernel.org/msg34937.html
> > 
> > I'm OK with removing the special case now because there are additional
> > supreblock checks merged (3.19) that were motivated by your previous
> > attempt to remove it.
> 
> So David, are you going to take the above patch instead?
> 
> I'm okay with that since they're exactly same, but it'd be good that
> Qu has its commit log updated.

Although Qu's patch was first and should be normally applied in case
more patches doing the same thing, the changelog reasoning is IMHO not
all fine, so I'm inclined to take your patch if you refine the subject,
and drop the "Conflicts:". Thanks.

Re: [PATCH] Revert "Btrfs: allow superblock mismatch from older mkfs"

[Qu Wenruo]

-------- Original Message  --------
Subject: Re: [PATCH] Revert "Btrfs: allow superblock mismatch from older 
mkfs"
From: David Sterba <dsterba@suse.cz>
To: Liu Bo <bo.li.liu@oracle.com>
Date: 2015年03月17日 00:25

> On Fri, Mar 13, 2015 at 04:15:08AM +0800, Liu Bo wrote:
>> On Thu, Mar 12, 2015 at 06:34:15PM +0100, David Sterba wrote:
>>> On Thu, Mar 12, 2015 at 01:45:05PM +0800, Qu Wenruo wrote:
>>>> I sent such patch some time ago but was rejected by Chris,
>>>> hopes this time it can be accepted.
>>>
>>> http://www.mail-archive.com/linux-btrfs%40vger.kernel.org/msg34937.html
>>>
>>> I'm OK with removing the special case now because there are additional
>>> supreblock checks merged (3.19) that were motivated by your previous
>>> attempt to remove it.
>>
>> So David, are you going to take the above patch instead?
>>
>> I'm okay with that since they're exactly same, but it'd be good that
>> Qu has its commit log updated.
>
> Although Qu's patch was first and should be normally applied in case
> more patches doing the same thing, the changelog reasoning is IMHO not
> all fine, so I'm inclined to take your patch if you refine the subject,
> and drop the "Conflicts:". Thanks.
>
I'm completely OK with picking Liu's patch, since his patch is submitted 
at a much more appropriate time than mine.
My patch is somewhat too early.

Thanks,
Qu

[PATCH v2] Btrfs: Remove the check for old-style mkfs

[Liu Bo]

This was used to make sure that a fresh btrfs from an older mkfs.btrfs,
but it also allows us to mount a buggy btrfs if this btrfs has the right
superblock head part but has something wrong with chunk tree part[1], and
after that we can hit BUG_ON()s set in the code to prevent something
impossible.

Since David has released "Btrfs progs v3.19-rc2", just remove the check,
if anyone who wants to make a fresh btrfs, please use the latest one.

[1]: http://www.spinics.net/lists/linux-btrfs/msg42358.html

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
v2: Refine the patch subject.

 fs/btrfs/disk-io.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 41b320e..f63aacf 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -418,12 +418,6 @@ static int btrfs_check_super_csum(char *raw_disk_sb)
 
 		if (memcmp(raw_disk_sb, result, csum_size))
 			ret = 1;
-
-		if (ret && btrfs_super_generation(disk_sb) < 10) {
-			printk(KERN_WARNING
-				"BTRFS: super block crcs don't match, older mkfs detected\n");
-			ret = 0;
-		}
 	}
 
 	if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes)) {
-- 
1.8.1.4

Re: [PATCH v2] Btrfs: Remove the check for old-style mkfs

[Omar Sandoval]

On Tue, Mar 17, 2015 at 02:34:16PM +0800, Liu Bo wrote:
> This was used to make sure that a fresh btrfs from an older mkfs.btrfs,
> but it also allows us to mount a buggy btrfs if this btrfs has the right
> superblock head part but has something wrong with chunk tree part[1], and
> after that we can hit BUG_ON()s set in the code to prevent something
> impossible.
> 
> Since David has released "Btrfs progs v3.19-rc2", just remove the check,
> if anyone who wants to make a fresh btrfs, please use the latest one.
> 
> [1]: http://www.spinics.net/lists/linux-btrfs/msg42358.html
> 
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>

# mount -o loop btrfs.img /mnt/test
# dmesg | tail
[   28.582698] BTRFS: device fsid 1c0ed5d6-550d-4010-b1b4-ce1828270713 devid 1 transid 4 /dev/loop0
[   28.598365] BTRFS: superblock checksum mismatch
[   28.617205] BTRFS: open_ctree failed

Looks good to me.

Reviewed-by: Omar Sandoval <osandov@osandov.com>

> ---
> v2: Refine the patch subject.
> 
>  fs/btrfs/disk-io.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 41b320e..f63aacf 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -418,12 +418,6 @@ static int btrfs_check_super_csum(char *raw_disk_sb)
>  
>  		if (memcmp(raw_disk_sb, result, csum_size))
>  			ret = 1;
> -
> -		if (ret && btrfs_super_generation(disk_sb) < 10) {
> -			printk(KERN_WARNING
> -				"BTRFS: super block crcs don't match, older mkfs detected\n");
> -			ret = 0;
> -		}
>  	}
>  
>  	if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes)) {
> -- 
> 1.8.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Omar

Re: [PATCH v2] Btrfs: Remove the check for old-style mkfs

[David Sterba]

On Tue, Mar 17, 2015 at 02:34:16PM +0800, Liu Bo wrote:
> This was used to make sure that a fresh btrfs from an older mkfs.btrfs,
> but it also allows us to mount a buggy btrfs if this btrfs has the right
> superblock head part but has something wrong with chunk tree part[1], and
> after that we can hit BUG_ON()s set in the code to prevent something
> impossible.
> 
> Since David has released "Btrfs progs v3.19-rc2", just remove the check,
> if anyone who wants to make a fresh btrfs, please use the latest one.
> 
> [1]: http://www.spinics.net/lists/linux-btrfs/msg42358.html
> 
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>

Reviewed-by: David Sterba <dsterba@suse.cz>

Thanks.