Re: [PATCH 7/8] btrfs: verify checksum for all devices in mount context

2018-03-27 Thread Anand Jain 



On 03/27/2018 08:21 PM, Nikolay Borisov wrote:



On 26.03.2018 11:27, Anand Jain wrote:

During mount context, we aren't verifying the superblock checksum
for all the devices, instead, we verify it only for the
struct btrfs_fs_device::latest_bdev. This patch fixes it
by moving the checksum verification code from the function open_ctree()
into the function btrfs_read_dev_one_super().

By doing this now we are verifying the superblock checksum
in the mount-context, device-replace and, device-delete context.


Which call chain provides the device-replace and device-delete contexts?
I think it is worth it documenting them in the changelog.


 Will copy it here from the cover-letter.



Signed-off-by: Anand Jain 


Reviewed-by: Nikolay Borisov 





---
  fs/btrfs/disk-io.c | 35 +--
  1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 6299ab18da5f..3cc50041c0b9 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2565,24 +2565,6 @@ int open_ctree(struct super_block *sb,
}
  
  	/*

-* We want to check superblock checksum, the type is stored inside.
-* Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
-*/
-   err = btrfs_check_super_csum(bh->b_data);
-   if (err) {
-   if (err == -EINVAL)
-   pr_err("BTRFS error (device %pg): "\
-   "unsupported checksum algorithm",
-   fs_devices->latest_bdev);
-   else
-   pr_err("BTRFS error (device %pg): "\
-   "superblock checksum mismatch",
-   fs_devices->latest_bdev);
-   brelse(bh);
-   goto fail_alloc;
-   }
-
-   /*
 * super_copy is zeroed at allocation time and we never touch the
 * following bytes up to INFO_SIZE, the checksum is calculated from
 * the whole block of INFO_SIZE
@@ -3128,6 +3110,7 @@ int btrfs_read_dev_one_super(struct block_device *bdev, 
int copy_num,
struct buffer_head *bh;
struct btrfs_super_block *super;
u64 bytenr;
+   int err;
  
  	bytenr = btrfs_sb_offset(copy_num);

if (bytenr + BTRFS_SUPER_INFO_SIZE >= i_size_read(bdev->bd_inode))
@@ -3148,6 +3131,22 @@ int btrfs_read_dev_one_super(struct block_device *bdev, 
int copy_num,
return -EINVAL;
}
  
+	/*

+* Check the superblock checksum, the type is stored inside.
+* Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
+*/
+   err = btrfs_check_super_csum(bh->b_data);
+   if (err) {


I am fixing this to if (err < 0) {


+   if (err == -EINVAL)
+   pr_err("BTRFS error (device %pg): unsupported checksum 
algorithm",
+   bdev);
+   else if (err == -EUCLEAN)
+   pr_err("BTRFS error (device %pg): superblock checksum 
mismatch",
+   bdev);


 I am also dropping else if to else in v2.

Thanks, Anand



+   brelse(bh);
+   return err;
+   }
+
*bh_ret = bh;
return 0;
  }


--
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

Re: [PATCH 7/8] btrfs: verify checksum for all devices in mount context

2018-03-27 Thread Nikolay Borisov 


On 26.03.2018 11:27, Anand Jain wrote:
> During mount context, we aren't verifying the superblock checksum
> for all the devices, instead, we verify it only for the
> struct btrfs_fs_device::latest_bdev. This patch fixes it
> by moving the checksum verification code from the function open_ctree()
> into the function btrfs_read_dev_one_super().
> 
> By doing this now we are verifying the superblock checksum
> in the mount-context, device-replace and, device-delete context.

Which call chain provides the device-replace and device-delete contexts?
I think it is worth it documenting them in the changelog.

> 
> Signed-off-by: Anand Jain 

Reviewed-by: Nikolay Borisov 

> ---
>  fs/btrfs/disk-io.c | 35 +--
>  1 file changed, 17 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 6299ab18da5f..3cc50041c0b9 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -2565,24 +2565,6 @@ int open_ctree(struct super_block *sb,
>   }
>  
>   /*
> -  * We want to check superblock checksum, the type is stored inside.
> -  * Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
> -  */
> - err = btrfs_check_super_csum(bh->b_data);
> - if (err) {
> - if (err == -EINVAL)
> - pr_err("BTRFS error (device %pg): "\
> - "unsupported checksum algorithm",
> - fs_devices->latest_bdev);
> - else
> - pr_err("BTRFS error (device %pg): "\
> - "superblock checksum mismatch",
> - fs_devices->latest_bdev);
> - brelse(bh);
> - goto fail_alloc;
> - }
> -
> - /*
>* super_copy is zeroed at allocation time and we never touch the
>* following bytes up to INFO_SIZE, the checksum is calculated from
>* the whole block of INFO_SIZE
> @@ -3128,6 +3110,7 @@ int btrfs_read_dev_one_super(struct block_device *bdev, 
> int copy_num,
>   struct buffer_head *bh;
>   struct btrfs_super_block *super;
>   u64 bytenr;
> + int err;
>  
>   bytenr = btrfs_sb_offset(copy_num);
>   if (bytenr + BTRFS_SUPER_INFO_SIZE >= i_size_read(bdev->bd_inode))
> @@ -3148,6 +3131,22 @@ int btrfs_read_dev_one_super(struct block_device 
> *bdev, int copy_num,
>   return -EINVAL;
>   }
>  
> + /*
> +  * Check the superblock checksum, the type is stored inside.
> +  * Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
> +  */
> + err = btrfs_check_super_csum(bh->b_data);
> + if (err) {
> + if (err == -EINVAL)
> + pr_err("BTRFS error (device %pg): unsupported checksum 
> algorithm",
> + bdev);
> + else if (err == -EUCLEAN)
> + pr_err("BTRFS error (device %pg): superblock checksum 
> mismatch",
> + bdev);
> + brelse(bh);
> + return err;
> + }
> +
>   *bh_ret = bh;
>   return 0;
>  }
> 
--
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

[PATCH 7/8] btrfs: verify checksum for all devices in mount context

2018-03-26 Thread Anand Jain 
During mount context, we aren't verifying the superblock checksum
for all the devices, instead, we verify it only for the
struct btrfs_fs_device::latest_bdev. This patch fixes it
by moving the checksum verification code from the function open_ctree()
into the function btrfs_read_dev_one_super().

By doing this now we are verifying the superblock checksum
in the mount-context, device-replace and, device-delete context.

Signed-off-by: Anand Jain 
---
 fs/btrfs/disk-io.c | 35 +--
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 6299ab18da5f..3cc50041c0b9 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2565,24 +2565,6 @@ int open_ctree(struct super_block *sb,
}
 
/*
-* We want to check superblock checksum, the type is stored inside.
-* Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
-*/
-   err = btrfs_check_super_csum(bh->b_data);
-   if (err) {
-   if (err == -EINVAL)
-   pr_err("BTRFS error (device %pg): "\
-   "unsupported checksum algorithm",
-   fs_devices->latest_bdev);
-   else
-   pr_err("BTRFS error (device %pg): "\
-   "superblock checksum mismatch",
-   fs_devices->latest_bdev);
-   brelse(bh);
-   goto fail_alloc;
-   }
-
-   /*
 * super_copy is zeroed at allocation time and we never touch the
 * following bytes up to INFO_SIZE, the checksum is calculated from
 * the whole block of INFO_SIZE
@@ -3128,6 +3110,7 @@ int btrfs_read_dev_one_super(struct block_device *bdev, 
int copy_num,
struct buffer_head *bh;
struct btrfs_super_block *super;
u64 bytenr;
+   int err;
 
bytenr = btrfs_sb_offset(copy_num);
if (bytenr + BTRFS_SUPER_INFO_SIZE >= i_size_read(bdev->bd_inode))
@@ -3148,6 +3131,22 @@ int btrfs_read_dev_one_super(struct block_device *bdev, 
int copy_num,
return -EINVAL;
}
 
+   /*
+* Check the superblock checksum, the type is stored inside.
+* Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
+*/
+   err = btrfs_check_super_csum(bh->b_data);
+   if (err) {
+   if (err == -EINVAL)
+   pr_err("BTRFS error (device %pg): unsupported checksum 
algorithm",
+   bdev);
+   else if (err == -EUCLEAN)
+   pr_err("BTRFS error (device %pg): superblock checksum 
mismatch",
+   bdev);
+   brelse(bh);
+   return err;
+   }
+
*bh_ret = bh;
return 0;
 }
-- 
2.7.0

--
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