* [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size
@ 2020-06-24 11:55 Qu Wenruo
2020-06-24 11:55 ` [PATCH 2/2] btrfs-progs: tests/convert: Add test case to make sure we won't allocate dev extents beyond device boundary Qu Wenruo
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Qu Wenruo @ 2020-06-24 11:55 UTC (permalink / raw)
To: linux-btrfs; +Cc: Jiachen YANG
[BUG]
The following script could lead to corrupted btrfs fs after
btrfs-convert:
fallocate -l 1G test.img
mkfs.ext4 test.img
mount test.img $mnt
fallocate -l 200m $mnt/file1
fallocate -l 200m $mnt/file2
fallocate -l 200m $mnt/file3
fallocate -l 200m $mnt/file4
fallocate -l 205m $mnt/file1
fallocate -l 205m $mnt/file2
fallocate -l 205m $mnt/file3
fallocate -l 205m $mnt/file4
umount $mnt
btrfs-convert test.img
The result btrfs will have a device extent beyond its boundary:
pening filesystem to check...
Checking filesystem on test.img
UUID: bbcd7399-fd5b-41a7-81ae-d48bc6935e43
[1/7] checking root items
[2/7] checking extents
ERROR: dev extent devid 1 physical offset 993198080 len 85786624 is beyond device boundary 1073741824
ERROR: errors found in extent allocation tree or chunk allocation
[3/7] checking free space cache
[4/7] checking fs roots
[5/7] checking only csums items (without verifying data)
[6/7] checking root refs
[7/7] checking quota groups skipped (not enabled on this FS)
found 913960960 bytes used, error(s) found
total csum bytes: 891500
total tree bytes: 1064960
total fs tree bytes: 49152
total extent tree bytes: 16384
btree space waste bytes: 144885
file data blocks allocated: 2129063936
referenced 1772728320
[CAUSE]
Btrfs-convert first collect all used blocks in the original fs, then
slightly enlarge the used blocks range as new btrfs data chunks.
However the enlarge part has a problem, that it doesn't take the device
boundary into consideration.
Thus it caused device extents and data chunks to go beyond device
boundary.
[FIX]
Just to extra check before inserting data chunks into
btrfs_convert_context::data_chunk.
Reported-by: Jiachen YANG <farseerfc@gmail.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
convert/main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/convert/main.c b/convert/main.c
index c86ddd988c63..7709e9a6c085 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -669,6 +669,8 @@ static int calculate_available_space(struct btrfs_convert_context *cctx)
cur_off = cache->start;
cur_len = max(cache->start + cache->size - cur_off,
min_stripe_size);
+ /* data chunks should never exceed device boundary */
+ cur_len = min(cctx->total_bytes - cur_off, cur_len);
ret = add_merge_cache_extent(data_chunks, cur_off, cur_len);
if (ret < 0)
goto out;
--
2.27.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] btrfs-progs: tests/convert: Add test case to make sure we won't allocate dev extents beyond device boundary
2020-06-24 11:55 [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size Qu Wenruo
@ 2020-06-24 11:55 ` Qu Wenruo
2020-06-24 15:49 ` David Sterba
2020-06-24 15:51 ` David Sterba
2020-06-24 15:58 ` [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size David Sterba
` (2 subsequent siblings)
3 siblings, 2 replies; 14+ messages in thread
From: Qu Wenruo @ 2020-06-24 11:55 UTC (permalink / raw)
To: linux-btrfs; +Cc: Jiachen YANG
Add a test case to check if the converted fs has device extent beyond
boundary.
The disk layout of source ext4 fs needs some extents to make them
allocated at the very end of the fs.
The script is from the original reporter.
Also, since the existing convert tests always uses 512M as device size,
which is not suitable for this test case, make it to grab the existing
device size to co-operate with this test case.
Reported-by: Jiachen YANG <farseerfc@gmail.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
tests/common.convert | 14 ++++++++-
tests/convert-tests/017-fs-near-full/test.sh | 30 ++++++++++++++++++++
2 files changed, 43 insertions(+), 1 deletion(-)
create mode 100755 tests/convert-tests/017-fs-near-full/test.sh
diff --git a/tests/common.convert b/tests/common.convert
index f24ceb0d6a64..0c918387758d 100644
--- a/tests/common.convert
+++ b/tests/common.convert
@@ -53,6 +53,16 @@ convert_test_preamble() {
echo "creating test image with: $@" >> "$RESULTS"
}
+get_test_file_size() {
+ local path="$1"
+ local ret
+
+ ret=$(ls -l "$path" | cut -f5 -d\ )
+ if [ -z $ret ]; then
+ ret=512M
+ fi
+ echo $ret
+}
# prepare TEST_DEV before conversion, create filesystem and mount it, image
# size is 512MB
# $1: type of the filesystem
@@ -61,14 +71,16 @@ convert_test_prep_fs() {
local fstype
local force
local mountopts
+ local oldsize
fstype="$1"
shift
+ oldsize=$(get_test_file_size "$TEST_DEV")
# TEST_DEV not removed as the file might have special permissions, eg.
# when test image is on NFS and would not be writable for root
run_check truncate -s 0 "$TEST_DEV"
# 256MB is the smallest acceptable btrfs image.
- run_check truncate -s 512M "$TEST_DEV"
+ run_check truncate -s $oldsize "$TEST_DEV"
force=
mountopts=
case "$fstype" in
diff --git a/tests/convert-tests/017-fs-near-full/test.sh b/tests/convert-tests/017-fs-near-full/test.sh
new file mode 100755
index 000000000000..9459729ee87f
--- /dev/null
+++ b/tests/convert-tests/017-fs-near-full/test.sh
@@ -0,0 +1,30 @@
+#!/bin/bash
+# Check if btrfs-convert creates fs with dev extents boundary device boundary
+
+source "$TEST_TOP/common"
+source "$TEST_TOP/common.convert"
+
+setup_root_helper
+prepare_test_dev 1G
+check_prereq btrfs-convert
+check_global_prereq mke2fs
+check_global_prereq fallocate
+
+convert_test_prep_fs ext4 mke2fs -t ext4 -b 4096
+
+# Use up 800MiB first
+for i in $(seq 1 4); do
+ run_check $SUDO_HELPER fallocate -l 200M "$TEST_MNT/file$i"
+done
+
+# Then add 5MiB for above files. These 5 MiB will be allocated near the very
+# end of the fs, to confuse btrfs-convert
+for i in $(seq 1 4); do
+ run_check $SUDO_HELPER fallocate -l 205M "$TEST_MNT/file$i"
+done
+
+run_check_umount_test_dev
+
+# convert_test_do_convert() will call btrfs check, which should expose any
+# invalid inline extent with too large size
+convert_test_do_convert
--
2.27.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] btrfs-progs: tests/convert: Add test case to make sure we won't allocate dev extents beyond device boundary
2020-06-24 11:55 ` [PATCH 2/2] btrfs-progs: tests/convert: Add test case to make sure we won't allocate dev extents beyond device boundary Qu Wenruo
@ 2020-06-24 15:49 ` David Sterba
2020-06-24 15:51 ` David Sterba
1 sibling, 0 replies; 14+ messages in thread
From: David Sterba @ 2020-06-24 15:49 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, Jiachen YANG
On Wed, Jun 24, 2020 at 07:55:27PM +0800, Qu Wenruo wrote:
> Add a test case to check if the converted fs has device extent beyond
> boundary.
>
> The disk layout of source ext4 fs needs some extents to make them
> allocated at the very end of the fs.
> The script is from the original reporter.
>
> Also, since the existing convert tests always uses 512M as device size,
> which is not suitable for this test case, make it to grab the existing
> device size to co-operate with this test case.
>
> Reported-by: Jiachen YANG <farseerfc@gmail.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> tests/common.convert | 14 ++++++++-
> tests/convert-tests/017-fs-near-full/test.sh | 30 ++++++++++++++++++++
> 2 files changed, 43 insertions(+), 1 deletion(-)
> create mode 100755 tests/convert-tests/017-fs-near-full/test.sh
>
> diff --git a/tests/common.convert b/tests/common.convert
> index f24ceb0d6a64..0c918387758d 100644
> --- a/tests/common.convert
> +++ b/tests/common.convert
> @@ -53,6 +53,16 @@ convert_test_preamble() {
> echo "creating test image with: $@" >> "$RESULTS"
> }
>
> +get_test_file_size() {
> + local path="$1"
> + local ret
> +
> + ret=$(ls -l "$path" | cut -f5 -d\ )
Parsing ls output is quite fragile, and we have 'stat --format=%s'.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] btrfs-progs: tests/convert: Add test case to make sure we won't allocate dev extents beyond device boundary
2020-06-24 11:55 ` [PATCH 2/2] btrfs-progs: tests/convert: Add test case to make sure we won't allocate dev extents beyond device boundary Qu Wenruo
2020-06-24 15:49 ` David Sterba
@ 2020-06-24 15:51 ` David Sterba
1 sibling, 0 replies; 14+ messages in thread
From: David Sterba @ 2020-06-24 15:51 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, Jiachen YANG
On Wed, Jun 24, 2020 at 07:55:27PM +0800, Qu Wenruo wrote:
> Add a test case to check if the converted fs has device extent beyond
> boundary.
>
> The disk layout of source ext4 fs needs some extents to make them
> allocated at the very end of the fs.
> The script is from the original reporter.
>
> Also, since the existing convert tests always uses 512M as device size,
> which is not suitable for this test case, make it to grab the existing
> device size to co-operate with this test case.
>
> Reported-by: Jiachen YANG <farseerfc@gmail.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> tests/common.convert | 14 ++++++++-
> tests/convert-tests/017-fs-near-full/test.sh | 30 ++++++++++++++++++++
> 2 files changed, 43 insertions(+), 1 deletion(-)
> create mode 100755 tests/convert-tests/017-fs-near-full/test.sh
>
> diff --git a/tests/common.convert b/tests/common.convert
> index f24ceb0d6a64..0c918387758d 100644
> --- a/tests/common.convert
> +++ b/tests/common.convert
> @@ -53,6 +53,16 @@ convert_test_preamble() {
> echo "creating test image with: $@" >> "$RESULTS"
> }
>
> +get_test_file_size() {
> + local path="$1"
> + local ret
> +
> + ret=$(ls -l "$path" | cut -f5 -d\ )
> + if [ -z $ret ]; then
> + ret=512M
> + fi
> + echo $ret
> +}
> # prepare TEST_DEV before conversion, create filesystem and mount it, image
> # size is 512MB
> # $1: type of the filesystem
> @@ -61,14 +71,16 @@ convert_test_prep_fs() {
> local fstype
> local force
> local mountopts
> + local oldsize
>
> fstype="$1"
> shift
> + oldsize=$(get_test_file_size "$TEST_DEV")
The helper should be here, with a comment why the return size needs to
be 512M in some cases.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size
2020-06-24 11:55 [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size Qu Wenruo
2020-06-24 11:55 ` [PATCH 2/2] btrfs-progs: tests/convert: Add test case to make sure we won't allocate dev extents beyond device boundary Qu Wenruo
@ 2020-06-24 15:58 ` David Sterba
2020-08-15 13:38 ` Anand Jain
2021-02-16 14:45 ` Filipe Manana
3 siblings, 0 replies; 14+ messages in thread
From: David Sterba @ 2020-06-24 15:58 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, Jiachen YANG
On Wed, Jun 24, 2020 at 07:55:26PM +0800, Qu Wenruo wrote:
> [BUG]
> The following script could lead to corrupted btrfs fs after
> btrfs-convert:
>
> fallocate -l 1G test.img
> mkfs.ext4 test.img
> mount test.img $mnt
> fallocate -l 200m $mnt/file1
> fallocate -l 200m $mnt/file2
> fallocate -l 200m $mnt/file3
> fallocate -l 200m $mnt/file4
> fallocate -l 205m $mnt/file1
> fallocate -l 205m $mnt/file2
> fallocate -l 205m $mnt/file3
> fallocate -l 205m $mnt/file4
> umount $mnt
> btrfs-convert test.img
>
> The result btrfs will have a device extent beyond its boundary:
> pening filesystem to check...
> Checking filesystem on test.img
> UUID: bbcd7399-fd5b-41a7-81ae-d48bc6935e43
> [1/7] checking root items
> [2/7] checking extents
> ERROR: dev extent devid 1 physical offset 993198080 len 85786624 is beyond device boundary 1073741824
> ERROR: errors found in extent allocation tree or chunk allocation
> [3/7] checking free space cache
> [4/7] checking fs roots
> [5/7] checking only csums items (without verifying data)
> [6/7] checking root refs
> [7/7] checking quota groups skipped (not enabled on this FS)
> found 913960960 bytes used, error(s) found
> total csum bytes: 891500
> total tree bytes: 1064960
> total fs tree bytes: 49152
> total extent tree bytes: 16384
> btree space waste bytes: 144885
> file data blocks allocated: 2129063936
> referenced 1772728320
>
> [CAUSE]
> Btrfs-convert first collect all used blocks in the original fs, then
> slightly enlarge the used blocks range as new btrfs data chunks.
>
> However the enlarge part has a problem, that it doesn't take the device
> boundary into consideration.
>
> Thus it caused device extents and data chunks to go beyond device
> boundary.
>
> [FIX]
> Just to extra check before inserting data chunks into
> btrfs_convert_context::data_chunk.
>
> Reported-by: Jiachen YANG <farseerfc@gmail.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
With the minor tweaks, added to devel, thanks.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size
2020-06-24 11:55 [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size Qu Wenruo
2020-06-24 11:55 ` [PATCH 2/2] btrfs-progs: tests/convert: Add test case to make sure we won't allocate dev extents beyond device boundary Qu Wenruo
2020-06-24 15:58 ` [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size David Sterba
@ 2020-08-15 13:38 ` Anand Jain
2020-08-15 23:40 ` Qu Wenruo
2021-02-16 14:45 ` Filipe Manana
3 siblings, 1 reply; 14+ messages in thread
From: Anand Jain @ 2020-08-15 13:38 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, Jiachen YANG
Before this patch the converted ext4 was failing to mount due to 'beyond
device boundary' error.
After this patch
# ./btrfs-convert /dev/sda7
create btrfs filesystem:
blocksize: 4096
nodesize: 16384
features: extref, skinny-metadata (default)
checksum: crc32c
creating ext2 image file
ERROR: data bytenr 1644167168 is covered by non-data block group
1644167168 flags 0x4
ERROR: failed to create ext2_saved/image: -22
WARNING: an error occurred during conversion, filesystem is partially
created but not finalized and not mountable
Any idea?
On 24/6/20 7:55 pm, Qu Wenruo wrote:
> [BUG]
> The following script could lead to corrupted btrfs fs after
> btrfs-convert:
>
> fallocate -l 1G test.img
> mkfs.ext4 test.img
> mount test.img $mnt
> fallocate -l 200m $mnt/file1
> fallocate -l 200m $mnt/file2
> fallocate -l 200m $mnt/file3
> fallocate -l 200m $mnt/file4
> fallocate -l 205m $mnt/file1
> fallocate -l 205m $mnt/file2
> fallocate -l 205m $mnt/file3
> fallocate -l 205m $mnt/file4
> umount $mnt
> btrfs-convert test.img
>
> The result btrfs will have a device extent beyond its boundary:
> pening filesystem to check...
> Checking filesystem on test.img
> UUID: bbcd7399-fd5b-41a7-81ae-d48bc6935e43
> [1/7] checking root items
> [2/7] checking extents
> ERROR: dev extent devid 1 physical offset 993198080 len 85786624 is beyond device boundary 1073741824
> ERROR: errors found in extent allocation tree or chunk allocation
> [3/7] checking free space cache
> [4/7] checking fs roots
> [5/7] checking only csums items (without verifying data)
> [6/7] checking root refs
> [7/7] checking quota groups skipped (not enabled on this FS)
> found 913960960 bytes used, error(s) found
> total csum bytes: 891500
> total tree bytes: 1064960
> total fs tree bytes: 49152
> total extent tree bytes: 16384
> btree space waste bytes: 144885
> file data blocks allocated: 2129063936
> referenced 1772728320
>
> [CAUSE]
> Btrfs-convert first collect all used blocks in the original fs, then
> slightly enlarge the used blocks range as new btrfs data chunks.
>
> However the enlarge part has a problem, that it doesn't take the device
> boundary into consideration.
>
> Thus it caused device extents and data chunks to go beyond device
> boundary.
>
> [FIX]
> Just to extra check before inserting data chunks into
> btrfs_convert_context::data_chunk.
>
> Reported-by: Jiachen YANG <farseerfc@gmail.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> convert/main.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/convert/main.c b/convert/main.c
> index c86ddd988c63..7709e9a6c085 100644
> --- a/convert/main.c
> +++ b/convert/main.c
> @@ -669,6 +669,8 @@ static int calculate_available_space(struct btrfs_convert_context *cctx)
> cur_off = cache->start;
> cur_len = max(cache->start + cache->size - cur_off,
> min_stripe_size);
> + /* data chunks should never exceed device boundary */
> + cur_len = min(cctx->total_bytes - cur_off, cur_len);
> ret = add_merge_cache_extent(data_chunks, cur_off, cur_len);
> if (ret < 0)
> goto out;
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size
2020-08-15 13:38 ` Anand Jain
@ 2020-08-15 23:40 ` Qu Wenruo
0 siblings, 0 replies; 14+ messages in thread
From: Qu Wenruo @ 2020-08-15 23:40 UTC (permalink / raw)
To: Anand Jain, Qu Wenruo; +Cc: linux-btrfs, Jiachen YANG
[-- Attachment #1.1: Type: text/plain, Size: 3752 bytes --]
On 2020/8/15 下午9:38, Anand Jain wrote:
>
> Before this patch the converted ext4 was failing to mount due to 'beyond
> device boundary' error.
e2image -Q please.
Thanks,
Qu
>
> After this patch
>
> # ./btrfs-convert /dev/sda7
> create btrfs filesystem:
> blocksize: 4096
> nodesize: 16384
> features: extref, skinny-metadata (default)
> checksum: crc32c
> creating ext2 image file
> ERROR: data bytenr 1644167168 is covered by non-data block group
> 1644167168 flags 0x4
> ERROR: failed to create ext2_saved/image: -22
> WARNING: an error occurred during conversion, filesystem is partially
> created but not finalized and not mountable
>
>
> Any idea?
>
>
> On 24/6/20 7:55 pm, Qu Wenruo wrote:
>> [BUG]
>> The following script could lead to corrupted btrfs fs after
>> btrfs-convert:
>>
>> fallocate -l 1G test.img
>> mkfs.ext4 test.img
>> mount test.img $mnt
>> fallocate -l 200m $mnt/file1
>> fallocate -l 200m $mnt/file2
>> fallocate -l 200m $mnt/file3
>> fallocate -l 200m $mnt/file4
>> fallocate -l 205m $mnt/file1
>> fallocate -l 205m $mnt/file2
>> fallocate -l 205m $mnt/file3
>> fallocate -l 205m $mnt/file4
>> umount $mnt
>> btrfs-convert test.img
>>
>> The result btrfs will have a device extent beyond its boundary:
>> pening filesystem to check...
>> Checking filesystem on test.img
>> UUID: bbcd7399-fd5b-41a7-81ae-d48bc6935e43
>> [1/7] checking root items
>> [2/7] checking extents
>> ERROR: dev extent devid 1 physical offset 993198080 len 85786624 is
>> beyond device boundary 1073741824
>> ERROR: errors found in extent allocation tree or chunk allocation
>> [3/7] checking free space cache
>> [4/7] checking fs roots
>> [5/7] checking only csums items (without verifying data)
>> [6/7] checking root refs
>> [7/7] checking quota groups skipped (not enabled on this FS)
>> found 913960960 bytes used, error(s) found
>> total csum bytes: 891500
>> total tree bytes: 1064960
>> total fs tree bytes: 49152
>> total extent tree bytes: 16384
>> btree space waste bytes: 144885
>> file data blocks allocated: 2129063936
>> referenced 1772728320
>>
>> [CAUSE]
>> Btrfs-convert first collect all used blocks in the original fs, then
>> slightly enlarge the used blocks range as new btrfs data chunks.
>>
>> However the enlarge part has a problem, that it doesn't take the device
>> boundary into consideration.
>>
>> Thus it caused device extents and data chunks to go beyond device
>> boundary.
>>
>> [FIX]
>> Just to extra check before inserting data chunks into
>> btrfs_convert_context::data_chunk.
>>
>> Reported-by: Jiachen YANG <farseerfc@gmail.com>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> convert/main.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/convert/main.c b/convert/main.c
>> index c86ddd988c63..7709e9a6c085 100644
>> --- a/convert/main.c
>> +++ b/convert/main.c
>> @@ -669,6 +669,8 @@ static int calculate_available_space(struct
>> btrfs_convert_context *cctx)
>> cur_off = cache->start;
>> cur_len = max(cache->start + cache->size - cur_off,
>> min_stripe_size);
>> + /* data chunks should never exceed device boundary */
>> + cur_len = min(cctx->total_bytes - cur_off, cur_len);
>> ret = add_merge_cache_extent(data_chunks, cur_off, cur_len);
>> if (ret < 0)
>> goto out;
>>
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size
2020-06-24 11:55 [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size Qu Wenruo
` (2 preceding siblings ...)
2020-08-15 13:38 ` Anand Jain
@ 2021-02-16 14:45 ` Filipe Manana
2021-02-16 23:42 ` Qu Wenruo
3 siblings, 1 reply; 14+ messages in thread
From: Filipe Manana @ 2021-02-16 14:45 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, Jiachen YANG
On Wed, Jun 24, 2020 at 10:00 PM Qu Wenruo <wqu@suse.com> wrote:
>
> [BUG]
> The following script could lead to corrupted btrfs fs after
> btrfs-convert:
>
> fallocate -l 1G test.img
> mkfs.ext4 test.img
> mount test.img $mnt
> fallocate -l 200m $mnt/file1
> fallocate -l 200m $mnt/file2
> fallocate -l 200m $mnt/file3
> fallocate -l 200m $mnt/file4
> fallocate -l 205m $mnt/file1
> fallocate -l 205m $mnt/file2
> fallocate -l 205m $mnt/file3
> fallocate -l 205m $mnt/file4
> umount $mnt
> btrfs-convert test.img
>
> The result btrfs will have a device extent beyond its boundary:
> pening filesystem to check...
> Checking filesystem on test.img
> UUID: bbcd7399-fd5b-41a7-81ae-d48bc6935e43
> [1/7] checking root items
> [2/7] checking extents
> ERROR: dev extent devid 1 physical offset 993198080 len 85786624 is beyond device boundary 1073741824
> ERROR: errors found in extent allocation tree or chunk allocation
> [3/7] checking free space cache
> [4/7] checking fs roots
> [5/7] checking only csums items (without verifying data)
> [6/7] checking root refs
> [7/7] checking quota groups skipped (not enabled on this FS)
> found 913960960 bytes used, error(s) found
> total csum bytes: 891500
> total tree bytes: 1064960
> total fs tree bytes: 49152
> total extent tree bytes: 16384
> btree space waste bytes: 144885
> file data blocks allocated: 2129063936
> referenced 1772728320
>
> [CAUSE]
> Btrfs-convert first collect all used blocks in the original fs, then
> slightly enlarge the used blocks range as new btrfs data chunks.
>
> However the enlarge part has a problem, that it doesn't take the device
> boundary into consideration.
>
> Thus it caused device extents and data chunks to go beyond device
> boundary.
>
> [FIX]
> Just to extra check before inserting data chunks into
> btrfs_convert_context::data_chunk.
>
> Reported-by: Jiachen YANG <farseerfc@gmail.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
So, having upgraded a test box from btrfs-progs v5.6.1 to v5.10.1, I
now have btrfs/136 (fstests) failing:
$ ./check btrfs/136
FSTYP -- btrfs
PLATFORM -- Linux/x86_64 debian8 5.11.0-rc7-btrfs-next-81 #1 SMP
PREEMPT Tue Feb 16 12:29:07 WET 2021
MKFS_OPTIONS -- /dev/sdc
MOUNT_OPTIONS -- /dev/sdc /home/fdmanana/btrfs-tests/scratch_1
btrfs/136 7s ... [failed, exit status 1]- output mismatch (see
/home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad)
--- tests/btrfs/136.out 2020-06-10 19:29:03.818519162 +0100
+++ /home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad
2021-02-16 14:31:30.669559188 +0000
@@ -1,2 +1,3 @@
QA output created by 136
-Silence is golden
+btrfs-convert failed
+(see /home/fdmanana/git/hub/xfstests/results//btrfs/136.full for details)
...
(Run 'diff -u /home/fdmanana/git/hub/xfstests/tests/btrfs/136.out
/home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad' to see
the entire diff)
Ran: btrfs/136
Failures: btrfs/136
Failed 1 of 1 tests
A bisect pointed to this patch.
Did you get this failure on your test box as well?
Thanks.
> ---
> convert/main.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/convert/main.c b/convert/main.c
> index c86ddd988c63..7709e9a6c085 100644
> --- a/convert/main.c
> +++ b/convert/main.c
> @@ -669,6 +669,8 @@ static int calculate_available_space(struct btrfs_convert_context *cctx)
> cur_off = cache->start;
> cur_len = max(cache->start + cache->size - cur_off,
> min_stripe_size);
> + /* data chunks should never exceed device boundary */
> + cur_len = min(cctx->total_bytes - cur_off, cur_len);
> ret = add_merge_cache_extent(data_chunks, cur_off, cur_len);
> if (ret < 0)
> goto out;
> --
> 2.27.0
>
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size
2021-02-16 14:45 ` Filipe Manana
@ 2021-02-16 23:42 ` Qu Wenruo
2021-02-17 10:59 ` Filipe Manana
0 siblings, 1 reply; 14+ messages in thread
From: Qu Wenruo @ 2021-02-16 23:42 UTC (permalink / raw)
To: fdmanana, Qu Wenruo; +Cc: linux-btrfs, Jiachen YANG
On 2021/2/16 下午10:45, Filipe Manana wrote:
> On Wed, Jun 24, 2020 at 10:00 PM Qu Wenruo <wqu@suse.com> wrote:
>>
>> [BUG]
>> The following script could lead to corrupted btrfs fs after
>> btrfs-convert:
>>
>> fallocate -l 1G test.img
>> mkfs.ext4 test.img
>> mount test.img $mnt
>> fallocate -l 200m $mnt/file1
>> fallocate -l 200m $mnt/file2
>> fallocate -l 200m $mnt/file3
>> fallocate -l 200m $mnt/file4
>> fallocate -l 205m $mnt/file1
>> fallocate -l 205m $mnt/file2
>> fallocate -l 205m $mnt/file3
>> fallocate -l 205m $mnt/file4
>> umount $mnt
>> btrfs-convert test.img
>>
>> The result btrfs will have a device extent beyond its boundary:
>> pening filesystem to check...
>> Checking filesystem on test.img
>> UUID: bbcd7399-fd5b-41a7-81ae-d48bc6935e43
>> [1/7] checking root items
>> [2/7] checking extents
>> ERROR: dev extent devid 1 physical offset 993198080 len 85786624 is beyond device boundary 1073741824
>> ERROR: errors found in extent allocation tree or chunk allocation
>> [3/7] checking free space cache
>> [4/7] checking fs roots
>> [5/7] checking only csums items (without verifying data)
>> [6/7] checking root refs
>> [7/7] checking quota groups skipped (not enabled on this FS)
>> found 913960960 bytes used, error(s) found
>> total csum bytes: 891500
>> total tree bytes: 1064960
>> total fs tree bytes: 49152
>> total extent tree bytes: 16384
>> btree space waste bytes: 144885
>> file data blocks allocated: 2129063936
>> referenced 1772728320
>>
>> [CAUSE]
>> Btrfs-convert first collect all used blocks in the original fs, then
>> slightly enlarge the used blocks range as new btrfs data chunks.
>>
>> However the enlarge part has a problem, that it doesn't take the device
>> boundary into consideration.
>>
>> Thus it caused device extents and data chunks to go beyond device
>> boundary.
>>
>> [FIX]
>> Just to extra check before inserting data chunks into
>> btrfs_convert_context::data_chunk.
>>
>> Reported-by: Jiachen YANG <farseerfc@gmail.com>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>
> So, having upgraded a test box from btrfs-progs v5.6.1 to v5.10.1, I
> now have btrfs/136 (fstests) failing:
>
> $ ./check btrfs/136
> FSTYP -- btrfs
> PLATFORM -- Linux/x86_64 debian8 5.11.0-rc7-btrfs-next-81 #1 SMP
> PREEMPT Tue Feb 16 12:29:07 WET 2021
> MKFS_OPTIONS -- /dev/sdc
> MOUNT_OPTIONS -- /dev/sdc /home/fdmanana/btrfs-tests/scratch_1
>
> btrfs/136 7s ... [failed, exit status 1]- output mismatch (see
> /home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad)
> --- tests/btrfs/136.out 2020-06-10 19:29:03.818519162 +0100
> +++ /home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad
> 2021-02-16 14:31:30.669559188 +0000
> @@ -1,2 +1,3 @@
> QA output created by 136
> -Silence is golden
> +btrfs-convert failed
> +(see /home/fdmanana/git/hub/xfstests/results//btrfs/136.full for details)
> ...
> (Run 'diff -u /home/fdmanana/git/hub/xfstests/tests/btrfs/136.out
> /home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad' to see
> the entire diff)
> Ran: btrfs/136
> Failures: btrfs/136
> Failed 1 of 1 tests
>
> A bisect pointed to this patch.
> Did you get this failure on your test box as well?
Nope.
Just tested with btrfs-progs v5.10.1, it passes:
$ which btrfs
/usr/bin/btrfs
$ btrfs --version
btrfs-progs v5.10.1
$ sudo ./check btrfs/136
FSTYP -- btrfs
PLATFORM -- Linux/x86_64 btrfs-desktop-vm 5.11.0-rc4-custom+ #4
SMP PREEMPT Mon Jan 25 18:35:22 CST 2021
MKFS_OPTIONS -- /dev/mapper/test-scratch1
MOUNT_OPTIONS -- /dev/mapper/test-scratch1 /mnt/scratch
btrfs/136 6s ... 10s
Ran: btrfs/136
Passed all 1 tests
Would you mind to provide the 136.full to help debugging the failure?
Thanks,
Qu
>
> Thanks.
>
>> ---
>> convert/main.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/convert/main.c b/convert/main.c
>> index c86ddd988c63..7709e9a6c085 100644
>> --- a/convert/main.c
>> +++ b/convert/main.c
>> @@ -669,6 +669,8 @@ static int calculate_available_space(struct btrfs_convert_context *cctx)
>> cur_off = cache->start;
>> cur_len = max(cache->start + cache->size - cur_off,
>> min_stripe_size);
>> + /* data chunks should never exceed device boundary */
>> + cur_len = min(cctx->total_bytes - cur_off, cur_len);
>> ret = add_merge_cache_extent(data_chunks, cur_off, cur_len);
>> if (ret < 0)
>> goto out;
>> --
>> 2.27.0
>>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size
2021-02-16 23:42 ` Qu Wenruo
@ 2021-02-17 10:59 ` Filipe Manana
2021-02-17 11:28 ` Qu Wenruo
0 siblings, 1 reply; 14+ messages in thread
From: Filipe Manana @ 2021-02-17 10:59 UTC (permalink / raw)
To: Qu Wenruo; +Cc: Qu Wenruo, linux-btrfs, Jiachen YANG
On Tue, Feb 16, 2021 at 11:42 PM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>
>
>
> On 2021/2/16 下午10:45, Filipe Manana wrote:
> > On Wed, Jun 24, 2020 at 10:00 PM Qu Wenruo <wqu@suse.com> wrote:
> >>
> >> [BUG]
> >> The following script could lead to corrupted btrfs fs after
> >> btrfs-convert:
> >>
> >> fallocate -l 1G test.img
> >> mkfs.ext4 test.img
> >> mount test.img $mnt
> >> fallocate -l 200m $mnt/file1
> >> fallocate -l 200m $mnt/file2
> >> fallocate -l 200m $mnt/file3
> >> fallocate -l 200m $mnt/file4
> >> fallocate -l 205m $mnt/file1
> >> fallocate -l 205m $mnt/file2
> >> fallocate -l 205m $mnt/file3
> >> fallocate -l 205m $mnt/file4
> >> umount $mnt
> >> btrfs-convert test.img
> >>
> >> The result btrfs will have a device extent beyond its boundary:
> >> pening filesystem to check...
> >> Checking filesystem on test.img
> >> UUID: bbcd7399-fd5b-41a7-81ae-d48bc6935e43
> >> [1/7] checking root items
> >> [2/7] checking extents
> >> ERROR: dev extent devid 1 physical offset 993198080 len 85786624 is beyond device boundary 1073741824
> >> ERROR: errors found in extent allocation tree or chunk allocation
> >> [3/7] checking free space cache
> >> [4/7] checking fs roots
> >> [5/7] checking only csums items (without verifying data)
> >> [6/7] checking root refs
> >> [7/7] checking quota groups skipped (not enabled on this FS)
> >> found 913960960 bytes used, error(s) found
> >> total csum bytes: 891500
> >> total tree bytes: 1064960
> >> total fs tree bytes: 49152
> >> total extent tree bytes: 16384
> >> btree space waste bytes: 144885
> >> file data blocks allocated: 2129063936
> >> referenced 1772728320
> >>
> >> [CAUSE]
> >> Btrfs-convert first collect all used blocks in the original fs, then
> >> slightly enlarge the used blocks range as new btrfs data chunks.
> >>
> >> However the enlarge part has a problem, that it doesn't take the device
> >> boundary into consideration.
> >>
> >> Thus it caused device extents and data chunks to go beyond device
> >> boundary.
> >>
> >> [FIX]
> >> Just to extra check before inserting data chunks into
> >> btrfs_convert_context::data_chunk.
> >>
> >> Reported-by: Jiachen YANG <farseerfc@gmail.com>
> >> Signed-off-by: Qu Wenruo <wqu@suse.com>
> >
> > So, having upgraded a test box from btrfs-progs v5.6.1 to v5.10.1, I
> > now have btrfs/136 (fstests) failing:
> >
> > $ ./check btrfs/136
> > FSTYP -- btrfs
> > PLATFORM -- Linux/x86_64 debian8 5.11.0-rc7-btrfs-next-81 #1 SMP
> > PREEMPT Tue Feb 16 12:29:07 WET 2021
> > MKFS_OPTIONS -- /dev/sdc
> > MOUNT_OPTIONS -- /dev/sdc /home/fdmanana/btrfs-tests/scratch_1
> >
> > btrfs/136 7s ... [failed, exit status 1]- output mismatch (see
> > /home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad)
> > --- tests/btrfs/136.out 2020-06-10 19:29:03.818519162 +0100
> > +++ /home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad
> > 2021-02-16 14:31:30.669559188 +0000
> > @@ -1,2 +1,3 @@
> > QA output created by 136
> > -Silence is golden
> > +btrfs-convert failed
> > +(see /home/fdmanana/git/hub/xfstests/results//btrfs/136.full for details)
> > ...
> > (Run 'diff -u /home/fdmanana/git/hub/xfstests/tests/btrfs/136.out
> > /home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad' to see
> > the entire diff)
> > Ran: btrfs/136
> > Failures: btrfs/136
> > Failed 1 of 1 tests
> >
> > A bisect pointed to this patch.
> > Did you get this failure on your test box as well?
>
> Nope.
>
> Just tested with btrfs-progs v5.10.1, it passes:
> $ which btrfs
> /usr/bin/btrfs
> $ btrfs --version
> btrfs-progs v5.10.1
> $ sudo ./check btrfs/136
> FSTYP -- btrfs
> PLATFORM -- Linux/x86_64 btrfs-desktop-vm 5.11.0-rc4-custom+ #4
> SMP PREEMPT Mon Jan 25 18:35:22 CST 2021
> MKFS_OPTIONS -- /dev/mapper/test-scratch1
> MOUNT_OPTIONS -- /dev/mapper/test-scratch1 /mnt/scratch
>
> btrfs/136 6s ... 10s
> Ran: btrfs/136
> Passed all 1 tests
>
> Would you mind to provide the 136.full to help debugging the failure?
Sure, here it is (also at https://pastebin.com/XhEX2dju):
root 10:06:39 /home/fdmanana/git/hub/xfstests (master)> cat
/home/fdmanana/git/hub/xfstests/results//btrfs/136.full
mke2fs 1.45.6 (20-Mar-2020)
Discarding device blocks: done
Creating filesystem with 5242880 4k blocks and 1310720 inodes
Filesystem UUID: 9519afc8-8ea0-42da-8ac5-3b4c20469dd1
Superblock backups stored on blocks:
32768, 98304, 163840, 229376, 294912, 819200, 884736, 1605632, 2654208,
4096000
Allocating group tables: done
Writing inode tables: done
Creating journal (32768 blocks): done
Writing superblocks and filesystem accounting information: done
Run fsstress -p 20 -n 100 -d
/home/fdmanana/btrfs-tests/scratch_1/ext3_ext4_data/ext3
tune2fs 1.45.6 (20-Mar-2020)
e2fsck 1.45.6 (20-Mar-2020)
Pass 1: Checking inodes, blocks, and sizes
Pass 2: Checking directory structure
Pass 3: Checking directory connectivity
Pass 3A: Optimizing directories
Pass 4: Checking reference counts
Pass 5: Checking group summary information
/dev/sdc: ***** FILE SYSTEM WAS MODIFIED *****
/dev/sdc: 235/1310720 files (22.1% non-contiguous), 129757/5242880 blocks
Run fsstress -p 20 -n 100 -d
/home/fdmanana/btrfs-tests/scratch_1/ext3_ext4_data/ext4
ERROR: missing data block for bytenr 1048576
ERROR: failed to create ext2_saved/image: -2
WARNING: an error occurred during conversion, filesystem is partially
created but not finalized and not mountable
create btrfs filesystem:
blocksize: 4096
nodesize: 16384
features: extref, skinny-metadata (default)
checksum: crc32c
creating ext2 image file
btrfs-convert failed
root 10:14:18 /home/fdmanana/git/hub/xfstests (master)>
Thanks
>
> Thanks,
> Qu
> >
> > Thanks.
> >
> >> ---
> >> convert/main.c | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> diff --git a/convert/main.c b/convert/main.c
> >> index c86ddd988c63..7709e9a6c085 100644
> >> --- a/convert/main.c
> >> +++ b/convert/main.c
> >> @@ -669,6 +669,8 @@ static int calculate_available_space(struct btrfs_convert_context *cctx)
> >> cur_off = cache->start;
> >> cur_len = max(cache->start + cache->size - cur_off,
> >> min_stripe_size);
> >> + /* data chunks should never exceed device boundary */
> >> + cur_len = min(cctx->total_bytes - cur_off, cur_len);
> >> ret = add_merge_cache_extent(data_chunks, cur_off, cur_len);
> >> if (ret < 0)
> >> goto out;
> >> --
> >> 2.27.0
> >>
> >
> >
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size
2021-02-17 10:59 ` Filipe Manana
@ 2021-02-17 11:28 ` Qu Wenruo
2021-02-17 12:12 ` Filipe Manana
0 siblings, 1 reply; 14+ messages in thread
From: Qu Wenruo @ 2021-02-17 11:28 UTC (permalink / raw)
To: fdmanana; +Cc: Qu Wenruo, linux-btrfs, Jiachen YANG
On 2021/2/17 下午6:59, Filipe Manana wrote:
> On Tue, Feb 16, 2021 at 11:42 PM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>>
>>
>>
>> On 2021/2/16 下午10:45, Filipe Manana wrote:
>>> On Wed, Jun 24, 2020 at 10:00 PM Qu Wenruo <wqu@suse.com> wrote:
>>>>
>>>> [BUG]
>>>> The following script could lead to corrupted btrfs fs after
>>>> btrfs-convert:
>>>>
>>>> fallocate -l 1G test.img
>>>> mkfs.ext4 test.img
>>>> mount test.img $mnt
>>>> fallocate -l 200m $mnt/file1
>>>> fallocate -l 200m $mnt/file2
>>>> fallocate -l 200m $mnt/file3
>>>> fallocate -l 200m $mnt/file4
>>>> fallocate -l 205m $mnt/file1
>>>> fallocate -l 205m $mnt/file2
>>>> fallocate -l 205m $mnt/file3
>>>> fallocate -l 205m $mnt/file4
>>>> umount $mnt
>>>> btrfs-convert test.img
>>>>
>>>> The result btrfs will have a device extent beyond its boundary:
>>>> pening filesystem to check...
>>>> Checking filesystem on test.img
>>>> UUID: bbcd7399-fd5b-41a7-81ae-d48bc6935e43
>>>> [1/7] checking root items
>>>> [2/7] checking extents
>>>> ERROR: dev extent devid 1 physical offset 993198080 len 85786624 is beyond device boundary 1073741824
>>>> ERROR: errors found in extent allocation tree or chunk allocation
>>>> [3/7] checking free space cache
>>>> [4/7] checking fs roots
>>>> [5/7] checking only csums items (without verifying data)
>>>> [6/7] checking root refs
>>>> [7/7] checking quota groups skipped (not enabled on this FS)
>>>> found 913960960 bytes used, error(s) found
>>>> total csum bytes: 891500
>>>> total tree bytes: 1064960
>>>> total fs tree bytes: 49152
>>>> total extent tree bytes: 16384
>>>> btree space waste bytes: 144885
>>>> file data blocks allocated: 2129063936
>>>> referenced 1772728320
>>>>
>>>> [CAUSE]
>>>> Btrfs-convert first collect all used blocks in the original fs, then
>>>> slightly enlarge the used blocks range as new btrfs data chunks.
>>>>
>>>> However the enlarge part has a problem, that it doesn't take the device
>>>> boundary into consideration.
>>>>
>>>> Thus it caused device extents and data chunks to go beyond device
>>>> boundary.
>>>>
>>>> [FIX]
>>>> Just to extra check before inserting data chunks into
>>>> btrfs_convert_context::data_chunk.
>>>>
>>>> Reported-by: Jiachen YANG <farseerfc@gmail.com>
>>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>>
>>> So, having upgraded a test box from btrfs-progs v5.6.1 to v5.10.1, I
>>> now have btrfs/136 (fstests) failing:
>>>
>>> $ ./check btrfs/136
>>> FSTYP -- btrfs
>>> PLATFORM -- Linux/x86_64 debian8 5.11.0-rc7-btrfs-next-81 #1 SMP
>>> PREEMPT Tue Feb 16 12:29:07 WET 2021
>>> MKFS_OPTIONS -- /dev/sdc
>>> MOUNT_OPTIONS -- /dev/sdc /home/fdmanana/btrfs-tests/scratch_1
>>>
>>> btrfs/136 7s ... [failed, exit status 1]- output mismatch (see
>>> /home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad)
>>> --- tests/btrfs/136.out 2020-06-10 19:29:03.818519162 +0100
>>> +++ /home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad
>>> 2021-02-16 14:31:30.669559188 +0000
>>> @@ -1,2 +1,3 @@
>>> QA output created by 136
>>> -Silence is golden
>>> +btrfs-convert failed
>>> +(see /home/fdmanana/git/hub/xfstests/results//btrfs/136.full for details)
>>> ...
>>> (Run 'diff -u /home/fdmanana/git/hub/xfstests/tests/btrfs/136.out
>>> /home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad' to see
>>> the entire diff)
>>> Ran: btrfs/136
>>> Failures: btrfs/136
>>> Failed 1 of 1 tests
>>>
>>> A bisect pointed to this patch.
>>> Did you get this failure on your test box as well?
>>
>> Nope.
>>
>> Just tested with btrfs-progs v5.10.1, it passes:
>> $ which btrfs
>> /usr/bin/btrfs
>> $ btrfs --version
>> btrfs-progs v5.10.1
>> $ sudo ./check btrfs/136
>> FSTYP -- btrfs
>> PLATFORM -- Linux/x86_64 btrfs-desktop-vm 5.11.0-rc4-custom+ #4
>> SMP PREEMPT Mon Jan 25 18:35:22 CST 2021
>> MKFS_OPTIONS -- /dev/mapper/test-scratch1
>> MOUNT_OPTIONS -- /dev/mapper/test-scratch1 /mnt/scratch
>>
>> btrfs/136 6s ... 10s
>> Ran: btrfs/136
>> Passed all 1 tests
>>
>> Would you mind to provide the 136.full to help debugging the failure?
>
> Sure, here it is (also at https://pastebin.com/XhEX2dju):
>
> root 10:06:39 /home/fdmanana/git/hub/xfstests (master)> cat
> /home/fdmanana/git/hub/xfstests/results//btrfs/136.full
> mke2fs 1.45.6 (20-Mar-2020)
> Discarding device blocks: done
> Creating filesystem with 5242880 4k blocks and 1310720 inodes
> Filesystem UUID: 9519afc8-8ea0-42da-8ac5-3b4c20469dd1
> Superblock backups stored on blocks:
> 32768, 98304, 163840, 229376, 294912, 819200, 884736, 1605632, 2654208,
> 4096000
>
> Allocating group tables: done
> Writing inode tables: done
> Creating journal (32768 blocks): done
> Writing superblocks and filesystem accounting information: done
>
> Run fsstress -p 20 -n 100 -d
> /home/fdmanana/btrfs-tests/scratch_1/ext3_ext4_data/ext3
> tune2fs 1.45.6 (20-Mar-2020)
> e2fsck 1.45.6 (20-Mar-2020)
> Pass 1: Checking inodes, blocks, and sizes
> Pass 2: Checking directory structure
> Pass 3: Checking directory connectivity
> Pass 3A: Optimizing directories
> Pass 4: Checking reference counts
> Pass 5: Checking group summary information
>
> /dev/sdc: ***** FILE SYSTEM WAS MODIFIED *****
> /dev/sdc: 235/1310720 files (22.1% non-contiguous), 129757/5242880 blocks
> Run fsstress -p 20 -n 100 -d
> /home/fdmanana/btrfs-tests/scratch_1/ext3_ext4_data/ext4
> ERROR: missing data block for bytenr 1048576
> ERROR: failed to create ext2_saved/image: -2
> WARNING: an error occurred during conversion, filesystem is partially
The bytenr is 1M, which looks related to the super block relocation code.
Thus it maybe disk layout related.
Your disk used here is 20G, although I tried the same size disk, it
still passes.
If you could reproduce it reliably (as you can do the bisect), mind to
take a e2image -Q dump?
Thanks,
Qu
> created but not finalized and not mountable
> create btrfs filesystem:
> blocksize: 4096
> nodesize: 16384
> features: extref, skinny-metadata (default)
> checksum: crc32c
> creating ext2 image file
> btrfs-convert failed
> root 10:14:18 /home/fdmanana/git/hub/xfstests (master)>
>
>
> Thanks
>
>
>>
>> Thanks,
>> Qu
>>>
>>> Thanks.
>>>
>>>> ---
>>>> convert/main.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/convert/main.c b/convert/main.c
>>>> index c86ddd988c63..7709e9a6c085 100644
>>>> --- a/convert/main.c
>>>> +++ b/convert/main.c
>>>> @@ -669,6 +669,8 @@ static int calculate_available_space(struct btrfs_convert_context *cctx)
>>>> cur_off = cache->start;
>>>> cur_len = max(cache->start + cache->size - cur_off,
>>>> min_stripe_size);
>>>> + /* data chunks should never exceed device boundary */
>>>> + cur_len = min(cctx->total_bytes - cur_off, cur_len);
>>>> ret = add_merge_cache_extent(data_chunks, cur_off, cur_len);
>>>> if (ret < 0)
>>>> goto out;
>>>> --
>>>> 2.27.0
>>>>
>>>
>>>
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size
2021-02-17 11:28 ` Qu Wenruo
@ 2021-02-17 12:12 ` Filipe Manana
2021-02-17 12:29 ` Qu Wenruo
0 siblings, 1 reply; 14+ messages in thread
From: Filipe Manana @ 2021-02-17 12:12 UTC (permalink / raw)
To: Qu Wenruo; +Cc: Qu Wenruo, linux-btrfs, Jiachen YANG
[-- Attachment #1: Type: text/plain, Size: 8239 bytes --]
On Wed, Feb 17, 2021 at 11:28 AM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>
>
>
> On 2021/2/17 下午6:59, Filipe Manana wrote:
> > On Tue, Feb 16, 2021 at 11:42 PM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
> >>
> >>
> >>
> >> On 2021/2/16 下午10:45, Filipe Manana wrote:
> >>> On Wed, Jun 24, 2020 at 10:00 PM Qu Wenruo <wqu@suse.com> wrote:
> >>>>
> >>>> [BUG]
> >>>> The following script could lead to corrupted btrfs fs after
> >>>> btrfs-convert:
> >>>>
> >>>> fallocate -l 1G test.img
> >>>> mkfs.ext4 test.img
> >>>> mount test.img $mnt
> >>>> fallocate -l 200m $mnt/file1
> >>>> fallocate -l 200m $mnt/file2
> >>>> fallocate -l 200m $mnt/file3
> >>>> fallocate -l 200m $mnt/file4
> >>>> fallocate -l 205m $mnt/file1
> >>>> fallocate -l 205m $mnt/file2
> >>>> fallocate -l 205m $mnt/file3
> >>>> fallocate -l 205m $mnt/file4
> >>>> umount $mnt
> >>>> btrfs-convert test.img
> >>>>
> >>>> The result btrfs will have a device extent beyond its boundary:
> >>>> pening filesystem to check...
> >>>> Checking filesystem on test.img
> >>>> UUID: bbcd7399-fd5b-41a7-81ae-d48bc6935e43
> >>>> [1/7] checking root items
> >>>> [2/7] checking extents
> >>>> ERROR: dev extent devid 1 physical offset 993198080 len 85786624 is beyond device boundary 1073741824
> >>>> ERROR: errors found in extent allocation tree or chunk allocation
> >>>> [3/7] checking free space cache
> >>>> [4/7] checking fs roots
> >>>> [5/7] checking only csums items (without verifying data)
> >>>> [6/7] checking root refs
> >>>> [7/7] checking quota groups skipped (not enabled on this FS)
> >>>> found 913960960 bytes used, error(s) found
> >>>> total csum bytes: 891500
> >>>> total tree bytes: 1064960
> >>>> total fs tree bytes: 49152
> >>>> total extent tree bytes: 16384
> >>>> btree space waste bytes: 144885
> >>>> file data blocks allocated: 2129063936
> >>>> referenced 1772728320
> >>>>
> >>>> [CAUSE]
> >>>> Btrfs-convert first collect all used blocks in the original fs, then
> >>>> slightly enlarge the used blocks range as new btrfs data chunks.
> >>>>
> >>>> However the enlarge part has a problem, that it doesn't take the device
> >>>> boundary into consideration.
> >>>>
> >>>> Thus it caused device extents and data chunks to go beyond device
> >>>> boundary.
> >>>>
> >>>> [FIX]
> >>>> Just to extra check before inserting data chunks into
> >>>> btrfs_convert_context::data_chunk.
> >>>>
> >>>> Reported-by: Jiachen YANG <farseerfc@gmail.com>
> >>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
> >>>
> >>> So, having upgraded a test box from btrfs-progs v5.6.1 to v5.10.1, I
> >>> now have btrfs/136 (fstests) failing:
> >>>
> >>> $ ./check btrfs/136
> >>> FSTYP -- btrfs
> >>> PLATFORM -- Linux/x86_64 debian8 5.11.0-rc7-btrfs-next-81 #1 SMP
> >>> PREEMPT Tue Feb 16 12:29:07 WET 2021
> >>> MKFS_OPTIONS -- /dev/sdc
> >>> MOUNT_OPTIONS -- /dev/sdc /home/fdmanana/btrfs-tests/scratch_1
> >>>
> >>> btrfs/136 7s ... [failed, exit status 1]- output mismatch (see
> >>> /home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad)
> >>> --- tests/btrfs/136.out 2020-06-10 19:29:03.818519162 +0100
> >>> +++ /home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad
> >>> 2021-02-16 14:31:30.669559188 +0000
> >>> @@ -1,2 +1,3 @@
> >>> QA output created by 136
> >>> -Silence is golden
> >>> +btrfs-convert failed
> >>> +(see /home/fdmanana/git/hub/xfstests/results//btrfs/136.full for details)
> >>> ...
> >>> (Run 'diff -u /home/fdmanana/git/hub/xfstests/tests/btrfs/136.out
> >>> /home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad' to see
> >>> the entire diff)
> >>> Ran: btrfs/136
> >>> Failures: btrfs/136
> >>> Failed 1 of 1 tests
> >>>
> >>> A bisect pointed to this patch.
> >>> Did you get this failure on your test box as well?
> >>
> >> Nope.
> >>
> >> Just tested with btrfs-progs v5.10.1, it passes:
> >> $ which btrfs
> >> /usr/bin/btrfs
> >> $ btrfs --version
> >> btrfs-progs v5.10.1
> >> $ sudo ./check btrfs/136
> >> FSTYP -- btrfs
> >> PLATFORM -- Linux/x86_64 btrfs-desktop-vm 5.11.0-rc4-custom+ #4
> >> SMP PREEMPT Mon Jan 25 18:35:22 CST 2021
> >> MKFS_OPTIONS -- /dev/mapper/test-scratch1
> >> MOUNT_OPTIONS -- /dev/mapper/test-scratch1 /mnt/scratch
> >>
> >> btrfs/136 6s ... 10s
> >> Ran: btrfs/136
> >> Passed all 1 tests
> >>
> >> Would you mind to provide the 136.full to help debugging the failure?
> >
> > Sure, here it is (also at https://pastebin.com/XhEX2dju):
> >
> > root 10:06:39 /home/fdmanana/git/hub/xfstests (master)> cat
> > /home/fdmanana/git/hub/xfstests/results//btrfs/136.full
> > mke2fs 1.45.6 (20-Mar-2020)
> > Discarding device blocks: done
> > Creating filesystem with 5242880 4k blocks and 1310720 inodes
> > Filesystem UUID: 9519afc8-8ea0-42da-8ac5-3b4c20469dd1
> > Superblock backups stored on blocks:
> > 32768, 98304, 163840, 229376, 294912, 819200, 884736, 1605632, 2654208,
> > 4096000
> >
> > Allocating group tables: done
> > Writing inode tables: done
> > Creating journal (32768 blocks): done
> > Writing superblocks and filesystem accounting information: done
> >
> > Run fsstress -p 20 -n 100 -d
> > /home/fdmanana/btrfs-tests/scratch_1/ext3_ext4_data/ext3
> > tune2fs 1.45.6 (20-Mar-2020)
> > e2fsck 1.45.6 (20-Mar-2020)
> > Pass 1: Checking inodes, blocks, and sizes
> > Pass 2: Checking directory structure
> > Pass 3: Checking directory connectivity
> > Pass 3A: Optimizing directories
> > Pass 4: Checking reference counts
> > Pass 5: Checking group summary information
> >
> > /dev/sdc: ***** FILE SYSTEM WAS MODIFIED *****
> > /dev/sdc: 235/1310720 files (22.1% non-contiguous), 129757/5242880 blocks
> > Run fsstress -p 20 -n 100 -d
> > /home/fdmanana/btrfs-tests/scratch_1/ext3_ext4_data/ext4
> > ERROR: missing data block for bytenr 1048576
> > ERROR: failed to create ext2_saved/image: -2
> > WARNING: an error occurred during conversion, filesystem is partially
>
> The bytenr is 1M, which looks related to the super block relocation code.
>
> Thus it maybe disk layout related.
>
> Your disk used here is 20G, although I tried the same size disk, it
> still passes.
>
> If you could reproduce it reliably (as you can do the bisect), mind to
> take a e2image -Q dump?
Yep, I can reproduce it reliably, always fails with btrfs-progs v5.7+.
Image attached. I took it after the test converts the ext3 fs to ext4
and right before the test calls btrfs-convert.
Thanks.
>
> Thanks,
> Qu
>
> > created but not finalized and not mountable
> > create btrfs filesystem:
> > blocksize: 4096
> > nodesize: 16384
> > features: extref, skinny-metadata (default)
> > checksum: crc32c
> > creating ext2 image file
> > btrfs-convert failed
> > root 10:14:18 /home/fdmanana/git/hub/xfstests (master)>
> >
> >
> > Thanks
> >
> >
> >>
> >> Thanks,
> >> Qu
> >>>
> >>> Thanks.
> >>>
> >>>> ---
> >>>> convert/main.c | 2 ++
> >>>> 1 file changed, 2 insertions(+)
> >>>>
> >>>> diff --git a/convert/main.c b/convert/main.c
> >>>> index c86ddd988c63..7709e9a6c085 100644
> >>>> --- a/convert/main.c
> >>>> +++ b/convert/main.c
> >>>> @@ -669,6 +669,8 @@ static int calculate_available_space(struct btrfs_convert_context *cctx)
> >>>> cur_off = cache->start;
> >>>> cur_len = max(cache->start + cache->size - cur_off,
> >>>> min_stripe_size);
> >>>> + /* data chunks should never exceed device boundary */
> >>>> + cur_len = min(cctx->total_bytes - cur_off, cur_len);
> >>>> ret = add_merge_cache_extent(data_chunks, cur_off, cur_len);
> >>>> if (ret < 0)
> >>>> goto out;
> >>>> --
> >>>> 2.27.0
> >>>>
> >>>
> >>>
> >
> >
> >
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
[-- Attachment #2: scratch_dev.qcow2.tar.xz --]
[-- Type: application/x-xz, Size: 69036 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size
2021-02-17 12:12 ` Filipe Manana
@ 2021-02-17 12:29 ` Qu Wenruo
2021-02-17 12:47 ` Filipe Manana
0 siblings, 1 reply; 14+ messages in thread
From: Qu Wenruo @ 2021-02-17 12:29 UTC (permalink / raw)
To: fdmanana; +Cc: Qu Wenruo, linux-btrfs, Jiachen YANG
On 2021/2/17 下午8:12, Filipe Manana wrote:
> On Wed, Feb 17, 2021 at 11:28 AM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>>
>>
>>
>> On 2021/2/17 下午6:59, Filipe Manana wrote:
>>> On Tue, Feb 16, 2021 at 11:42 PM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>>>>
>>>>
>>>>
>>>> On 2021/2/16 下午10:45, Filipe Manana wrote:
>>>>> On Wed, Jun 24, 2020 at 10:00 PM Qu Wenruo <wqu@suse.com> wrote:
>>>>>>
>>>>>> [BUG]
>>>>>> The following script could lead to corrupted btrfs fs after
>>>>>> btrfs-convert:
>>>>>>
>>>>>> fallocate -l 1G test.img
>>>>>> mkfs.ext4 test.img
>>>>>> mount test.img $mnt
>>>>>> fallocate -l 200m $mnt/file1
>>>>>> fallocate -l 200m $mnt/file2
>>>>>> fallocate -l 200m $mnt/file3
>>>>>> fallocate -l 200m $mnt/file4
>>>>>> fallocate -l 205m $mnt/file1
>>>>>> fallocate -l 205m $mnt/file2
>>>>>> fallocate -l 205m $mnt/file3
>>>>>> fallocate -l 205m $mnt/file4
>>>>>> umount $mnt
>>>>>> btrfs-convert test.img
>>>>>>
>>>>>> The result btrfs will have a device extent beyond its boundary:
>>>>>> pening filesystem to check...
>>>>>> Checking filesystem on test.img
>>>>>> UUID: bbcd7399-fd5b-41a7-81ae-d48bc6935e43
>>>>>> [1/7] checking root items
>>>>>> [2/7] checking extents
>>>>>> ERROR: dev extent devid 1 physical offset 993198080 len 85786624 is beyond device boundary 1073741824
>>>>>> ERROR: errors found in extent allocation tree or chunk allocation
>>>>>> [3/7] checking free space cache
>>>>>> [4/7] checking fs roots
>>>>>> [5/7] checking only csums items (without verifying data)
>>>>>> [6/7] checking root refs
>>>>>> [7/7] checking quota groups skipped (not enabled on this FS)
>>>>>> found 913960960 bytes used, error(s) found
>>>>>> total csum bytes: 891500
>>>>>> total tree bytes: 1064960
>>>>>> total fs tree bytes: 49152
>>>>>> total extent tree bytes: 16384
>>>>>> btree space waste bytes: 144885
>>>>>> file data blocks allocated: 2129063936
>>>>>> referenced 1772728320
>>>>>>
>>>>>> [CAUSE]
>>>>>> Btrfs-convert first collect all used blocks in the original fs, then
>>>>>> slightly enlarge the used blocks range as new btrfs data chunks.
>>>>>>
>>>>>> However the enlarge part has a problem, that it doesn't take the device
>>>>>> boundary into consideration.
>>>>>>
>>>>>> Thus it caused device extents and data chunks to go beyond device
>>>>>> boundary.
>>>>>>
>>>>>> [FIX]
>>>>>> Just to extra check before inserting data chunks into
>>>>>> btrfs_convert_context::data_chunk.
>>>>>>
>>>>>> Reported-by: Jiachen YANG <farseerfc@gmail.com>
>>>>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>>>>
>>>>> So, having upgraded a test box from btrfs-progs v5.6.1 to v5.10.1, I
>>>>> now have btrfs/136 (fstests) failing:
>>>>>
>>>>> $ ./check btrfs/136
>>>>> FSTYP -- btrfs
>>>>> PLATFORM -- Linux/x86_64 debian8 5.11.0-rc7-btrfs-next-81 #1 SMP
>>>>> PREEMPT Tue Feb 16 12:29:07 WET 2021
>>>>> MKFS_OPTIONS -- /dev/sdc
>>>>> MOUNT_OPTIONS -- /dev/sdc /home/fdmanana/btrfs-tests/scratch_1
>>>>>
>>>>> btrfs/136 7s ... [failed, exit status 1]- output mismatch (see
>>>>> /home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad)
>>>>> --- tests/btrfs/136.out 2020-06-10 19:29:03.818519162 +0100
>>>>> +++ /home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad
>>>>> 2021-02-16 14:31:30.669559188 +0000
>>>>> @@ -1,2 +1,3 @@
>>>>> QA output created by 136
>>>>> -Silence is golden
>>>>> +btrfs-convert failed
>>>>> +(see /home/fdmanana/git/hub/xfstests/results//btrfs/136.full for details)
>>>>> ...
>>>>> (Run 'diff -u /home/fdmanana/git/hub/xfstests/tests/btrfs/136.out
>>>>> /home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad' to see
>>>>> the entire diff)
>>>>> Ran: btrfs/136
>>>>> Failures: btrfs/136
>>>>> Failed 1 of 1 tests
>>>>>
>>>>> A bisect pointed to this patch.
>>>>> Did you get this failure on your test box as well?
>>>>
>>>> Nope.
>>>>
>>>> Just tested with btrfs-progs v5.10.1, it passes:
>>>> $ which btrfs
>>>> /usr/bin/btrfs
>>>> $ btrfs --version
>>>> btrfs-progs v5.10.1
>>>> $ sudo ./check btrfs/136
>>>> FSTYP -- btrfs
>>>> PLATFORM -- Linux/x86_64 btrfs-desktop-vm 5.11.0-rc4-custom+ #4
>>>> SMP PREEMPT Mon Jan 25 18:35:22 CST 2021
>>>> MKFS_OPTIONS -- /dev/mapper/test-scratch1
>>>> MOUNT_OPTIONS -- /dev/mapper/test-scratch1 /mnt/scratch
>>>>
>>>> btrfs/136 6s ... 10s
>>>> Ran: btrfs/136
>>>> Passed all 1 tests
>>>>
>>>> Would you mind to provide the 136.full to help debugging the failure?
>>>
>>> Sure, here it is (also at https://pastebin.com/XhEX2dju):
>>>
>>> root 10:06:39 /home/fdmanana/git/hub/xfstests (master)> cat
>>> /home/fdmanana/git/hub/xfstests/results//btrfs/136.full
>>> mke2fs 1.45.6 (20-Mar-2020)
>>> Discarding device blocks: done
>>> Creating filesystem with 5242880 4k blocks and 1310720 inodes
>>> Filesystem UUID: 9519afc8-8ea0-42da-8ac5-3b4c20469dd1
>>> Superblock backups stored on blocks:
>>> 32768, 98304, 163840, 229376, 294912, 819200, 884736, 1605632, 2654208,
>>> 4096000
>>>
>>> Allocating group tables: done
>>> Writing inode tables: done
>>> Creating journal (32768 blocks): done
>>> Writing superblocks and filesystem accounting information: done
>>>
>>> Run fsstress -p 20 -n 100 -d
>>> /home/fdmanana/btrfs-tests/scratch_1/ext3_ext4_data/ext3
>>> tune2fs 1.45.6 (20-Mar-2020)
>>> e2fsck 1.45.6 (20-Mar-2020)
>>> Pass 1: Checking inodes, blocks, and sizes
>>> Pass 2: Checking directory structure
>>> Pass 3: Checking directory connectivity
>>> Pass 3A: Optimizing directories
>>> Pass 4: Checking reference counts
>>> Pass 5: Checking group summary information
>>>
>>> /dev/sdc: ***** FILE SYSTEM WAS MODIFIED *****
>>> /dev/sdc: 235/1310720 files (22.1% non-contiguous), 129757/5242880 blocks
>>> Run fsstress -p 20 -n 100 -d
>>> /home/fdmanana/btrfs-tests/scratch_1/ext3_ext4_data/ext4
>>> ERROR: missing data block for bytenr 1048576
>>> ERROR: failed to create ext2_saved/image: -2
>>> WARNING: an error occurred during conversion, filesystem is partially
>>
>> The bytenr is 1M, which looks related to the super block relocation code.
>>
>> Thus it maybe disk layout related.
>>
>> Your disk used here is 20G, although I tried the same size disk, it
>> still passes.
>>
>> If you could reproduce it reliably (as you can do the bisect), mind to
>> take a e2image -Q dump?
>
> Yep, I can reproduce it reliably, always fails with btrfs-progs v5.7+.
>
> Image attached. I took it after the test converts the ext3 fs to ext4
> and right before the test calls btrfs-convert.
Something doesn't go correct.
v5.10.1 btrfs-convert still passes on the image in my test env.
$ ./btrfs-convert /dev/vdc
create btrfs filesystem:
blocksize: 4096
nodesize: 16384
features: extref, skinny-metadata (default)
checksum: crc32c
free space report:
total: 21474836480
free: 15690694656 (73.07%)
creating ext2 image file
creating btrfs metadata
copy inodes [o] [ 4/ 527]
conversion complete
$ ./btrfs --version
btrfs-progs v5.10.1
I have no idea what's going on now...
I'm afraid you have to dig the bug by yourself, as I really can't
reproduce the bug, even with the image dump...
Thanks,
Qu
>
> Thanks.
>
>>
>> Thanks,
>> Qu
>>
>>> created but not finalized and not mountable
>>> create btrfs filesystem:
>>> blocksize: 4096
>>> nodesize: 16384
>>> features: extref, skinny-metadata (default)
>>> checksum: crc32c
>>> creating ext2 image file
>>> btrfs-convert failed
>>> root 10:14:18 /home/fdmanana/git/hub/xfstests (master)>
>>>
>>>
>>> Thanks
>>>
>>>
>>>>
>>>> Thanks,
>>>> Qu
>>>>>
>>>>> Thanks.
>>>>>
>>>>>> ---
>>>>>> convert/main.c | 2 ++
>>>>>> 1 file changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/convert/main.c b/convert/main.c
>>>>>> index c86ddd988c63..7709e9a6c085 100644
>>>>>> --- a/convert/main.c
>>>>>> +++ b/convert/main.c
>>>>>> @@ -669,6 +669,8 @@ static int calculate_available_space(struct btrfs_convert_context *cctx)
>>>>>> cur_off = cache->start;
>>>>>> cur_len = max(cache->start + cache->size - cur_off,
>>>>>> min_stripe_size);
>>>>>> + /* data chunks should never exceed device boundary */
>>>>>> + cur_len = min(cctx->total_bytes - cur_off, cur_len);
>>>>>> ret = add_merge_cache_extent(data_chunks, cur_off, cur_len);
>>>>>> if (ret < 0)
>>>>>> goto out;
>>>>>> --
>>>>>> 2.27.0
>>>>>>
>>>>>
>>>>>
>>>
>>>
>>>
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size
2021-02-17 12:29 ` Qu Wenruo
@ 2021-02-17 12:47 ` Filipe Manana
0 siblings, 0 replies; 14+ messages in thread
From: Filipe Manana @ 2021-02-17 12:47 UTC (permalink / raw)
To: Qu Wenruo; +Cc: Qu Wenruo, linux-btrfs, Jiachen YANG
On Wed, Feb 17, 2021 at 12:29 PM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>
>
>
> On 2021/2/17 下午8:12, Filipe Manana wrote:
> > On Wed, Feb 17, 2021 at 11:28 AM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
> >>
> >>
> >>
> >> On 2021/2/17 下午6:59, Filipe Manana wrote:
> >>> On Tue, Feb 16, 2021 at 11:42 PM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 2021/2/16 下午10:45, Filipe Manana wrote:
> >>>>> On Wed, Jun 24, 2020 at 10:00 PM Qu Wenruo <wqu@suse.com> wrote:
> >>>>>>
> >>>>>> [BUG]
> >>>>>> The following script could lead to corrupted btrfs fs after
> >>>>>> btrfs-convert:
> >>>>>>
> >>>>>> fallocate -l 1G test.img
> >>>>>> mkfs.ext4 test.img
> >>>>>> mount test.img $mnt
> >>>>>> fallocate -l 200m $mnt/file1
> >>>>>> fallocate -l 200m $mnt/file2
> >>>>>> fallocate -l 200m $mnt/file3
> >>>>>> fallocate -l 200m $mnt/file4
> >>>>>> fallocate -l 205m $mnt/file1
> >>>>>> fallocate -l 205m $mnt/file2
> >>>>>> fallocate -l 205m $mnt/file3
> >>>>>> fallocate -l 205m $mnt/file4
> >>>>>> umount $mnt
> >>>>>> btrfs-convert test.img
> >>>>>>
> >>>>>> The result btrfs will have a device extent beyond its boundary:
> >>>>>> pening filesystem to check...
> >>>>>> Checking filesystem on test.img
> >>>>>> UUID: bbcd7399-fd5b-41a7-81ae-d48bc6935e43
> >>>>>> [1/7] checking root items
> >>>>>> [2/7] checking extents
> >>>>>> ERROR: dev extent devid 1 physical offset 993198080 len 85786624 is beyond device boundary 1073741824
> >>>>>> ERROR: errors found in extent allocation tree or chunk allocation
> >>>>>> [3/7] checking free space cache
> >>>>>> [4/7] checking fs roots
> >>>>>> [5/7] checking only csums items (without verifying data)
> >>>>>> [6/7] checking root refs
> >>>>>> [7/7] checking quota groups skipped (not enabled on this FS)
> >>>>>> found 913960960 bytes used, error(s) found
> >>>>>> total csum bytes: 891500
> >>>>>> total tree bytes: 1064960
> >>>>>> total fs tree bytes: 49152
> >>>>>> total extent tree bytes: 16384
> >>>>>> btree space waste bytes: 144885
> >>>>>> file data blocks allocated: 2129063936
> >>>>>> referenced 1772728320
> >>>>>>
> >>>>>> [CAUSE]
> >>>>>> Btrfs-convert first collect all used blocks in the original fs, then
> >>>>>> slightly enlarge the used blocks range as new btrfs data chunks.
> >>>>>>
> >>>>>> However the enlarge part has a problem, that it doesn't take the device
> >>>>>> boundary into consideration.
> >>>>>>
> >>>>>> Thus it caused device extents and data chunks to go beyond device
> >>>>>> boundary.
> >>>>>>
> >>>>>> [FIX]
> >>>>>> Just to extra check before inserting data chunks into
> >>>>>> btrfs_convert_context::data_chunk.
> >>>>>>
> >>>>>> Reported-by: Jiachen YANG <farseerfc@gmail.com>
> >>>>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
> >>>>>
> >>>>> So, having upgraded a test box from btrfs-progs v5.6.1 to v5.10.1, I
> >>>>> now have btrfs/136 (fstests) failing:
> >>>>>
> >>>>> $ ./check btrfs/136
> >>>>> FSTYP -- btrfs
> >>>>> PLATFORM -- Linux/x86_64 debian8 5.11.0-rc7-btrfs-next-81 #1 SMP
> >>>>> PREEMPT Tue Feb 16 12:29:07 WET 2021
> >>>>> MKFS_OPTIONS -- /dev/sdc
> >>>>> MOUNT_OPTIONS -- /dev/sdc /home/fdmanana/btrfs-tests/scratch_1
> >>>>>
> >>>>> btrfs/136 7s ... [failed, exit status 1]- output mismatch (see
> >>>>> /home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad)
> >>>>> --- tests/btrfs/136.out 2020-06-10 19:29:03.818519162 +0100
> >>>>> +++ /home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad
> >>>>> 2021-02-16 14:31:30.669559188 +0000
> >>>>> @@ -1,2 +1,3 @@
> >>>>> QA output created by 136
> >>>>> -Silence is golden
> >>>>> +btrfs-convert failed
> >>>>> +(see /home/fdmanana/git/hub/xfstests/results//btrfs/136.full for details)
> >>>>> ...
> >>>>> (Run 'diff -u /home/fdmanana/git/hub/xfstests/tests/btrfs/136.out
> >>>>> /home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad' to see
> >>>>> the entire diff)
> >>>>> Ran: btrfs/136
> >>>>> Failures: btrfs/136
> >>>>> Failed 1 of 1 tests
> >>>>>
> >>>>> A bisect pointed to this patch.
> >>>>> Did you get this failure on your test box as well?
> >>>>
> >>>> Nope.
> >>>>
> >>>> Just tested with btrfs-progs v5.10.1, it passes:
> >>>> $ which btrfs
> >>>> /usr/bin/btrfs
> >>>> $ btrfs --version
> >>>> btrfs-progs v5.10.1
> >>>> $ sudo ./check btrfs/136
> >>>> FSTYP -- btrfs
> >>>> PLATFORM -- Linux/x86_64 btrfs-desktop-vm 5.11.0-rc4-custom+ #4
> >>>> SMP PREEMPT Mon Jan 25 18:35:22 CST 2021
> >>>> MKFS_OPTIONS -- /dev/mapper/test-scratch1
> >>>> MOUNT_OPTIONS -- /dev/mapper/test-scratch1 /mnt/scratch
> >>>>
> >>>> btrfs/136 6s ... 10s
> >>>> Ran: btrfs/136
> >>>> Passed all 1 tests
> >>>>
> >>>> Would you mind to provide the 136.full to help debugging the failure?
> >>>
> >>> Sure, here it is (also at https://pastebin.com/XhEX2dju):
> >>>
> >>> root 10:06:39 /home/fdmanana/git/hub/xfstests (master)> cat
> >>> /home/fdmanana/git/hub/xfstests/results//btrfs/136.full
> >>> mke2fs 1.45.6 (20-Mar-2020)
> >>> Discarding device blocks: done
> >>> Creating filesystem with 5242880 4k blocks and 1310720 inodes
> >>> Filesystem UUID: 9519afc8-8ea0-42da-8ac5-3b4c20469dd1
> >>> Superblock backups stored on blocks:
> >>> 32768, 98304, 163840, 229376, 294912, 819200, 884736, 1605632, 2654208,
> >>> 4096000
> >>>
> >>> Allocating group tables: done
> >>> Writing inode tables: done
> >>> Creating journal (32768 blocks): done
> >>> Writing superblocks and filesystem accounting information: done
> >>>
> >>> Run fsstress -p 20 -n 100 -d
> >>> /home/fdmanana/btrfs-tests/scratch_1/ext3_ext4_data/ext3
> >>> tune2fs 1.45.6 (20-Mar-2020)
> >>> e2fsck 1.45.6 (20-Mar-2020)
> >>> Pass 1: Checking inodes, blocks, and sizes
> >>> Pass 2: Checking directory structure
> >>> Pass 3: Checking directory connectivity
> >>> Pass 3A: Optimizing directories
> >>> Pass 4: Checking reference counts
> >>> Pass 5: Checking group summary information
> >>>
> >>> /dev/sdc: ***** FILE SYSTEM WAS MODIFIED *****
> >>> /dev/sdc: 235/1310720 files (22.1% non-contiguous), 129757/5242880 blocks
> >>> Run fsstress -p 20 -n 100 -d
> >>> /home/fdmanana/btrfs-tests/scratch_1/ext3_ext4_data/ext4
> >>> ERROR: missing data block for bytenr 1048576
> >>> ERROR: failed to create ext2_saved/image: -2
> >>> WARNING: an error occurred during conversion, filesystem is partially
> >>
> >> The bytenr is 1M, which looks related to the super block relocation code.
> >>
> >> Thus it maybe disk layout related.
> >>
> >> Your disk used here is 20G, although I tried the same size disk, it
> >> still passes.
> >>
> >> If you could reproduce it reliably (as you can do the bisect), mind to
> >> take a e2image -Q dump?
> >
> > Yep, I can reproduce it reliably, always fails with btrfs-progs v5.7+.
> >
> > Image attached. I took it after the test converts the ext3 fs to ext4
> > and right before the test calls btrfs-convert.
>
> Something doesn't go correct.
>
> v5.10.1 btrfs-convert still passes on the image in my test env.
>
> $ ./btrfs-convert /dev/vdc
> create btrfs filesystem:
> blocksize: 4096
> nodesize: 16384
> features: extref, skinny-metadata (default)
> checksum: crc32c
> free space report:
> total: 21474836480
> free: 15690694656 (73.07%)
> creating ext2 image file
> creating btrfs metadata
> copy inodes [o] [ 4/ 527]
> conversion complete
> $ ./btrfs --version
> btrfs-progs v5.10.1
>
>
> I have no idea what's going on now...
>
> I'm afraid you have to dig the bug by yourself, as I really can't
> reproduce the bug, even with the image dump...
I'll add it to my list of things to check.
Thanks for looking into it!
>
> Thanks,
> Qu
>
> >
> > Thanks.
> >
> >>
> >> Thanks,
> >> Qu
> >>
> >>> created but not finalized and not mountable
> >>> create btrfs filesystem:
> >>> blocksize: 4096
> >>> nodesize: 16384
> >>> features: extref, skinny-metadata (default)
> >>> checksum: crc32c
> >>> creating ext2 image file
> >>> btrfs-convert failed
> >>> root 10:14:18 /home/fdmanana/git/hub/xfstests (master)>
> >>>
> >>>
> >>> Thanks
> >>>
> >>>
> >>>>
> >>>> Thanks,
> >>>> Qu
> >>>>>
> >>>>> Thanks.
> >>>>>
> >>>>>> ---
> >>>>>> convert/main.c | 2 ++
> >>>>>> 1 file changed, 2 insertions(+)
> >>>>>>
> >>>>>> diff --git a/convert/main.c b/convert/main.c
> >>>>>> index c86ddd988c63..7709e9a6c085 100644
> >>>>>> --- a/convert/main.c
> >>>>>> +++ b/convert/main.c
> >>>>>> @@ -669,6 +669,8 @@ static int calculate_available_space(struct btrfs_convert_context *cctx)
> >>>>>> cur_off = cache->start;
> >>>>>> cur_len = max(cache->start + cache->size - cur_off,
> >>>>>> min_stripe_size);
> >>>>>> + /* data chunks should never exceed device boundary */
> >>>>>> + cur_len = min(cctx->total_bytes - cur_off, cur_len);
> >>>>>> ret = add_merge_cache_extent(data_chunks, cur_off, cur_len);
> >>>>>> if (ret < 0)
> >>>>>> goto out;
> >>>>>> --
> >>>>>> 2.27.0
> >>>>>>
> >>>>>
> >>>>>
> >>>
> >>>
> >>>
> >
> >
> >
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-02-17 12:48 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-24 11:55 [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size Qu Wenruo
2020-06-24 11:55 ` [PATCH 2/2] btrfs-progs: tests/convert: Add test case to make sure we won't allocate dev extents beyond device boundary Qu Wenruo
2020-06-24 15:49 ` David Sterba
2020-06-24 15:51 ` David Sterba
2020-06-24 15:58 ` [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size David Sterba
2020-08-15 13:38 ` Anand Jain
2020-08-15 23:40 ` Qu Wenruo
2021-02-16 14:45 ` Filipe Manana
2021-02-16 23:42 ` Qu Wenruo
2021-02-17 10:59 ` Filipe Manana
2021-02-17 11:28 ` Qu Wenruo
2021-02-17 12:12 ` Filipe Manana
2021-02-17 12:29 ` Qu Wenruo
2021-02-17 12:47 ` Filipe Manana
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.