[PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size

[PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size

[Qu Wenruo]

[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

[PATCH 2/2] btrfs-progs: tests/convert: Add test case to make sure we won't allocate dev extents beyond device boundary

[Qu Wenruo]

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

Re: [PATCH 2/2] btrfs-progs: tests/convert: Add test case to make sure we won't allocate dev extents beyond device boundary

[David Sterba]

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

Re: [PATCH 2/2] btrfs-progs: tests/convert: Add test case to make sure we won't allocate dev extents beyond device boundary

[David Sterba]

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.

Re: [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size

[David Sterba]

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.

Re: [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size

[Anand Jain]

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

Re: [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size

[Qu Wenruo]

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

Re: [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size

[Filipe Manana]

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

Re: [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size

[Qu Wenruo]

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

Re: [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size

[Filipe Manana]

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

Re: [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size

[Qu Wenruo]

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

Re: [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size

[Filipe Manana]

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

Re: [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size

[Qu Wenruo]

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

Re: [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size

[Filipe Manana]

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