[PATCH 0/3] btrfs-progs: make check handle invalid bg items

[PATCH 0/3] btrfs-progs: make check handle invalid bg items

[Josef Bacik]

Hello,

While writing code for extent tree v2 I noticed that I was generating a fs with
an invalid block group ->used value.  However fsck wasn't catching this, because
we don't actuall check the used value of the block group items in normal mode.
lowmem mode does this properly thankfully, so this only needs to be added to the
normal fsck mode.

I've added code to btrfs-corrupt-block to generate the corrupt image I need for
the test case.  Then of course the actual patch to detect and fix the problem.
Thanks,

Josef

Josef Bacik (3):
  btrfs-progs: add the ability to corrupt block group items
  btrfs-progs: make check detect and fix invalid used for block groups
  btrfs-progs: add a test image with a corrupt block group item

 btrfs-corrupt-block.c                         | 108 +++++++++++++++++-
 check/common.h                                |   5 +
 check/main.c                                  |  89 ++++++++++++++-
 .../default.img.xz                            | Bin 0 -> 1036 bytes
 4 files changed, 197 insertions(+), 5 deletions(-)
 create mode 100644 tests/fsck-tests/050-invalid-block-group-used/default.img.xz

-- 
2.26.3

[PATCH 1/3] btrfs-progs: add the ability to corrupt block group items

[Josef Bacik]

While doing the extent tree v2 stuff I noticed that fsck doesn't detect
an invalid ->used value on the block group item in the normal mode.  To
build a test case for this I need the ability to corrupt block group
items.  This allows us to corrupt the various fields of a block group.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 btrfs-corrupt-block.c | 108 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 107 insertions(+), 1 deletion(-)

diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c
index 77bdc810..80622f29 100644
--- a/btrfs-corrupt-block.c
+++ b/btrfs-corrupt-block.c
@@ -348,6 +348,24 @@ enum btrfs_key_field {
 	BTRFS_KEY_BAD,
 };
 
+enum btrfs_block_group_field {
+	BTRFS_BLOCK_GROUP_ITEM_USED,
+	BTRFS_BLOCK_GROUP_ITEM_FLAGS,
+	BTRFS_BLOCK_GROUP_ITEM_CHUNK_OBJECTID,
+	BTRFS_BLOCK_GROUP_ITEM_BAD,
+};
+
+static enum btrfs_block_group_field convert_block_group_field(char *field)
+{
+	if (!strncmp(field, "used", FIELD_BUF_LEN))
+		return BTRFS_BLOCK_GROUP_ITEM_USED;
+	if (!strncmp(field, "flags", FIELD_BUF_LEN))
+		return BTRFS_BLOCK_GROUP_ITEM_FLAGS;
+	if (!strncmp(field, "chunk_objectid", FIELD_BUF_LEN))
+		return BTRFS_BLOCK_GROUP_ITEM_CHUNK_OBJECTID;
+	return BTRFS_BLOCK_GROUP_ITEM_BAD;
+}
+
 static enum btrfs_inode_field convert_inode_field(char *field)
 {
 	if (!strncmp(field, "isize", FIELD_BUF_LEN))
@@ -442,6 +460,83 @@ static u8 generate_u8(u8 orig)
 	return ret;
 }
 
+static int corrupt_block_group(struct btrfs_root *root, u64 bg, char *field)
+{
+	struct btrfs_trans_handle *trans;
+	struct btrfs_path *path;
+	struct btrfs_block_group_item *bgi;
+	struct btrfs_key key;
+	enum btrfs_block_group_field corrupt_field;
+	u64 orig, bogus;
+	int ret = 0;
+
+	root = root->fs_info->extent_root;
+
+	corrupt_field = convert_block_group_field(field);
+	if (corrupt_field == BTRFS_BLOCK_GROUP_ITEM_BAD) {
+		fprintf(stderr, "Invalid field %s\n", field);
+		return -EINVAL;
+	}
+
+	path = btrfs_alloc_path();
+	if (!path)
+		return -ENOMEM;
+
+	trans = btrfs_start_transaction(root, 1);
+	if (IS_ERR(trans)) {
+		btrfs_free_path(path);
+		fprintf(stderr, "Couldn't start transaction %ld\n",
+			PTR_ERR(trans));
+		return PTR_ERR(trans);
+	}
+
+	key.objectid = bg;
+	key.type = BTRFS_BLOCK_GROUP_ITEM_KEY;
+	key.offset = 0;
+
+	ret = btrfs_search_slot(trans, root, &key, path, 0, 1);
+	if (ret < 0) {
+		fprintf(stderr, "Error searching for bg %llu %d\n", bg, ret);
+		goto out;
+	}
+
+	ret = 0;
+	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
+	if (key.type != BTRFS_BLOCK_GROUP_ITEM_KEY) {
+		fprintf(stderr, "Couldn't find the bg %llu\n", bg);
+		goto out;
+	}
+
+	bgi = btrfs_item_ptr(path->nodes[0], path->slots[0],
+			     struct btrfs_block_group_item);
+	switch (corrupt_field) {
+	case BTRFS_BLOCK_GROUP_ITEM_USED:
+		orig = btrfs_block_group_used(path->nodes[0], bgi);
+		bogus = generate_u64(orig);
+		btrfs_set_block_group_used(path->nodes[0], bgi, bogus);
+		break;
+	case BTRFS_BLOCK_GROUP_ITEM_CHUNK_OBJECTID:
+		orig = btrfs_block_group_chunk_objectid(path->nodes[0], bgi);
+		bogus = generate_u64(orig);
+		btrfs_set_block_group_chunk_objectid(path->nodes[0], bgi,
+						     bogus);
+		break;
+	case BTRFS_BLOCK_GROUP_ITEM_FLAGS:
+		orig = btrfs_block_group_flags(path->nodes[0], bgi);
+		bogus = generate_u64(orig);
+		btrfs_set_block_group_flags(path->nodes[0], bgi, bogus);
+		break;
+	default:
+		ret = -EINVAL;
+		goto out;
+	}
+	btrfs_mark_buffer_dirty(path->nodes[0]);
+out:
+	btrfs_commit_transaction(trans, root);
+	btrfs_free_path(path);
+	return ret;
+}
+
 static int corrupt_key(struct btrfs_root *root, struct btrfs_key *key,
 		       char *field)
 {
@@ -1150,6 +1245,7 @@ int main(int argc, char **argv)
 	u64 file_extent = (u64)-1;
 	u64 root_objectid = 0;
 	u64 csum_bytenr = 0;
+	u64 block_group = 0;
 	char field[FIELD_BUF_LEN];
 
 	field[0] = '\0';
@@ -1177,11 +1273,12 @@ int main(int argc, char **argv)
 			{ "delete", no_argument, NULL, 'd'},
 			{ "root", no_argument, NULL, 'r'},
 			{ "csum", required_argument, NULL, 'C'},
+			{ "block-group", required_argument, NULL, 'B'},
 			{ "help", no_argument, NULL, GETOPT_VAL_HELP},
 			{ NULL, 0, NULL, 0 }
 		};
 
-		c = getopt_long(argc, argv, "l:c:b:eEkuUi:f:x:m:K:I:D:d:r:C:",
+		c = getopt_long(argc, argv, "l:c:b:eEkuUi:f:x:m:K:I:D:d:r:C:B:",
 				long_options, NULL);
 		if (c < 0)
 			break;
@@ -1244,6 +1341,9 @@ int main(int argc, char **argv)
 			case 'C':
 				csum_bytenr = arg_strtou64(optarg);
 				break;
+			case 'B':
+				block_group = arg_strtou64(optarg);
+				break;
 			case GETOPT_VAL_HELP:
 			default:
 				print_usage(c != GETOPT_VAL_HELP);
@@ -1385,6 +1485,12 @@ int main(int argc, char **argv)
 		ret = corrupt_key(target_root, &key, field);
 		goto out_close;
 	}
+	if (block_group) {
+		if (*field == 0)
+			print_usage(1);
+		ret = corrupt_block_group(root, block_group, field);
+		goto out_close;
+	}
 	/*
 	 * If we made it here and we have extent set then we didn't specify
 	 * inode and we're screwed.
-- 
2.26.3

Re: [PATCH 1/3] btrfs-progs: add the ability to corrupt block group items

[David Sterba]

On Wed, Aug 18, 2021 at 12:39:20AM -0400, Josef Bacik wrote:
> While doing the extent tree v2 stuff I noticed that fsck doesn't detect
> an invalid ->used value on the block group item in the normal mode.  To
> build a test case for this I need the ability to corrupt block group
> items.  This allows us to corrupt the various fields of a block group.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  btrfs-corrupt-block.c | 108 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 107 insertions(+), 1 deletion(-)
> 
> diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c
> index 77bdc810..80622f29 100644
> --- a/btrfs-corrupt-block.c
> +++ b/btrfs-corrupt-block.c
> @@ -348,6 +348,24 @@ enum btrfs_key_field {
>  	BTRFS_KEY_BAD,
>  };
>  
> +enum btrfs_block_group_field {
> +	BTRFS_BLOCK_GROUP_ITEM_USED,
> +	BTRFS_BLOCK_GROUP_ITEM_FLAGS,
> +	BTRFS_BLOCK_GROUP_ITEM_CHUNK_OBJECTID,
> +	BTRFS_BLOCK_GROUP_ITEM_BAD,
> +};
> +
> +static enum btrfs_block_group_field convert_block_group_field(char *field)
> +{
> +	if (!strncmp(field, "used", FIELD_BUF_LEN))
> +		return BTRFS_BLOCK_GROUP_ITEM_USED;
> +	if (!strncmp(field, "flags", FIELD_BUF_LEN))
> +		return BTRFS_BLOCK_GROUP_ITEM_FLAGS;
> +	if (!strncmp(field, "chunk_objectid", FIELD_BUF_LEN))
> +		return BTRFS_BLOCK_GROUP_ITEM_CHUNK_OBJECTID;
> +	return BTRFS_BLOCK_GROUP_ITEM_BAD;
> +}
> +
>  static enum btrfs_inode_field convert_inode_field(char *field)
>  {
>  	if (!strncmp(field, "isize", FIELD_BUF_LEN))
> @@ -442,6 +460,83 @@ static u8 generate_u8(u8 orig)
>  	return ret;
>  }
>  
> +static int corrupt_block_group(struct btrfs_root *root, u64 bg, char *field)
> +{
> +	struct btrfs_trans_handle *trans;
> +	struct btrfs_path *path;
> +	struct btrfs_block_group_item *bgi;
> +	struct btrfs_key key;
> +	enum btrfs_block_group_field corrupt_field;
> +	u64 orig, bogus;
> +	int ret = 0;
> +
> +	root = root->fs_info->extent_root;
> +
> +	corrupt_field = convert_block_group_field(field);
> +	if (corrupt_field == BTRFS_BLOCK_GROUP_ITEM_BAD) {
> +		fprintf(stderr, "Invalid field %s\n", field);
> +		return -EINVAL;
> +	}
> +
> +	path = btrfs_alloc_path();
> +	if (!path)
> +		return -ENOMEM;
> +
> +	trans = btrfs_start_transaction(root, 1);
> +	if (IS_ERR(trans)) {
> +		btrfs_free_path(path);
> +		fprintf(stderr, "Couldn't start transaction %ld\n",
> +			PTR_ERR(trans));
> +		return PTR_ERR(trans);
> +	}
> +
> +	key.objectid = bg;
> +	key.type = BTRFS_BLOCK_GROUP_ITEM_KEY;
> +	key.offset = 0;
> +
> +	ret = btrfs_search_slot(trans, root, &key, path, 0, 1);
> +	if (ret < 0) {
> +		fprintf(stderr, "Error searching for bg %llu %d\n", bg, ret);
> +		goto out;
> +	}
> +
> +	ret = 0;
> +	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
> +	if (key.type != BTRFS_BLOCK_GROUP_ITEM_KEY) {
> +		fprintf(stderr, "Couldn't find the bg %llu\n", bg);
> +		goto out;
> +	}
> +
> +	bgi = btrfs_item_ptr(path->nodes[0], path->slots[0],
> +			     struct btrfs_block_group_item);
> +	switch (corrupt_field) {
> +	case BTRFS_BLOCK_GROUP_ITEM_USED:
> +		orig = btrfs_block_group_used(path->nodes[0], bgi);
> +		bogus = generate_u64(orig);
> +		btrfs_set_block_group_used(path->nodes[0], bgi, bogus);
> +		break;
> +	case BTRFS_BLOCK_GROUP_ITEM_CHUNK_OBJECTID:
> +		orig = btrfs_block_group_chunk_objectid(path->nodes[0], bgi);
> +		bogus = generate_u64(orig);
> +		btrfs_set_block_group_chunk_objectid(path->nodes[0], bgi,
> +						     bogus);
> +		break;
> +	case BTRFS_BLOCK_GROUP_ITEM_FLAGS:
> +		orig = btrfs_block_group_flags(path->nodes[0], bgi);
> +		bogus = generate_u64(orig);
> +		btrfs_set_block_group_flags(path->nodes[0], bgi, bogus);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +	btrfs_mark_buffer_dirty(path->nodes[0]);
> +out:
> +	btrfs_commit_transaction(trans, root);
> +	btrfs_free_path(path);
> +	return ret;
> +}
> +
>  static int corrupt_key(struct btrfs_root *root, struct btrfs_key *key,
>  		       char *field)
>  {
> @@ -1150,6 +1245,7 @@ int main(int argc, char **argv)
>  	u64 file_extent = (u64)-1;
>  	u64 root_objectid = 0;
>  	u64 csum_bytenr = 0;
> +	u64 block_group = 0;
>  	char field[FIELD_BUF_LEN];
>  
>  	field[0] = '\0';
> @@ -1177,11 +1273,12 @@ int main(int argc, char **argv)
>  			{ "delete", no_argument, NULL, 'd'},
>  			{ "root", no_argument, NULL, 'r'},
>  			{ "csum", required_argument, NULL, 'C'},
> +			{ "block-group", required_argument, NULL, 'B'},

The command line interface of corrupt-block is absolute mess because of
the incremental additions like so this so at least please don't use
single letter options. Updated in devel in a separate patch.

[PATCH 2/3] btrfs-progs: make check detect and fix invalid used for block groups

[Josef Bacik]

The lowmem mode validates the used field of the block group item, but
the normal mode does not.  Fix this by keeping a running tally of what
we think the used value for the block group should be, and then if it
mismatches report an error and fix the problem if we have repair set.
We have to keep track of pending extents because we process leaves as we
see them, so it could be much later in the process that we find the
block group item to associate the extents with.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 check/common.h |  5 +++
 check/main.c   | 89 +++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 90 insertions(+), 4 deletions(-)

diff --git a/check/common.h b/check/common.h
index e72379a0..ba4e291e 100644
--- a/check/common.h
+++ b/check/common.h
@@ -37,10 +37,14 @@ struct block_group_record {
 	u64 offset;
 
 	u64 flags;
+
+	u64 disk_used;
+	u64 actual_used;
 };
 
 struct block_group_tree {
 	struct cache_tree tree;
+	struct extent_io_tree pending_extents;
 	struct list_head block_groups;
 };
 
@@ -141,6 +145,7 @@ u64 calc_stripe_length(u64 type, u64 length, int num_stripes);
 static inline void block_group_tree_init(struct block_group_tree *tree)
 {
 	cache_tree_init(&tree->tree);
+	extent_io_tree_init(&tree->pending_extents);
 	INIT_LIST_HEAD(&tree->block_groups);
 }
 
diff --git a/check/main.c b/check/main.c
index a8851815..f7285865 100644
--- a/check/main.c
+++ b/check/main.c
@@ -5083,9 +5083,27 @@ static void free_block_group_record(struct cache_extent *cache)
 
 void free_block_group_tree(struct block_group_tree *tree)
 {
+	extent_io_tree_cleanup(&tree->pending_extents);
 	cache_tree_free_extents(&tree->tree, free_block_group_record);
 }
 
+static void update_block_group_used(struct block_group_tree *tree,
+				    u64 bytenr, u64 num_bytes)
+{
+	struct cache_extent *bg_item;
+	struct block_group_record *bg_rec;
+
+	bg_item = lookup_cache_extent(&tree->tree, bytenr, num_bytes);
+	if (!bg_item) {
+		set_extent_dirty(&tree->pending_extents, bytenr,
+				 bytenr + num_bytes - 1);
+		return;
+	}
+	bg_rec = container_of(bg_item, struct block_group_record,
+			      cache);
+	bg_rec->actual_used += num_bytes;
+}
+
 int insert_device_extent_record(struct device_extent_tree *tree,
 				struct device_extent_record *de_rec)
 {
@@ -5270,6 +5288,7 @@ btrfs_new_block_group_record(struct extent_buffer *leaf, struct btrfs_key *key,
 
 	ptr = btrfs_item_ptr(leaf, slot, struct btrfs_block_group_item);
 	rec->flags = btrfs_block_group_flags(leaf, ptr);
+	rec->disk_used = btrfs_block_group_used(leaf, ptr);
 
 	INIT_LIST_HEAD(&rec->list);
 
@@ -5281,6 +5300,7 @@ static int process_block_group_item(struct block_group_tree *block_group_cache,
 				    struct extent_buffer *eb, int slot)
 {
 	struct block_group_record *rec;
+	u64 start, end;
 	int ret = 0;
 
 	rec = btrfs_new_block_group_record(eb, key, slot);
@@ -5289,6 +5309,22 @@ static int process_block_group_item(struct block_group_tree *block_group_cache,
 		fprintf(stderr, "Block Group[%llu, %llu] existed.\n",
 			rec->objectid, rec->offset);
 		free(rec);
+		return ret;
+	}
+
+	while (!find_first_extent_bit(&block_group_cache->pending_extents,
+				      rec->objectid, &start, &end,
+				      EXTENT_DIRTY)) {
+		u64 len;
+
+		if (start >= rec->objectid + rec->offset)
+			break;
+		start = max(start, rec->objectid);
+		len = min(end - start + 1,
+			  rec->objectid + rec->offset - start);
+		rec->actual_used += len;
+		clear_extent_dirty(&block_group_cache->pending_extents, start,
+				   start + len - 1);
 	}
 
 	return ret;
@@ -5352,6 +5388,7 @@ process_device_extent_item(struct device_extent_tree *dev_extent_cache,
 
 static int process_extent_item(struct btrfs_root *root,
 			       struct cache_tree *extent_cache,
+			       struct block_group_tree *block_group_cache,
 			       struct extent_buffer *eb, int slot)
 {
 	struct btrfs_extent_item *ei;
@@ -5380,6 +5417,8 @@ static int process_extent_item(struct btrfs_root *root,
 		num_bytes = key.offset;
 	}
 
+	update_block_group_used(block_group_cache, key.objectid, num_bytes);
+
 	if (!IS_ALIGNED(key.objectid, gfs_info->sectorsize)) {
 		error("ignoring invalid extent, bytenr %llu is not aligned to %u",
 		      key.objectid, gfs_info->sectorsize);
@@ -6348,13 +6387,13 @@ static int run_next_block(struct btrfs_root *root,
 				continue;
 			}
 			if (key.type == BTRFS_EXTENT_ITEM_KEY) {
-				process_extent_item(root, extent_cache, buf,
-						    i);
+				process_extent_item(root, extent_cache,
+						    block_group_cache, buf, i);
 				continue;
 			}
 			if (key.type == BTRFS_METADATA_ITEM_KEY) {
-				process_extent_item(root, extent_cache, buf,
-						    i);
+				process_extent_item(root, extent_cache,
+						    block_group_cache, buf, i);
 				continue;
 			}
 			if (key.type == BTRFS_EXTENT_CSUM_KEY) {
@@ -8599,6 +8638,41 @@ static int deal_root_from_list(struct list_head *list,
 	return ret;
 }
 
+static int check_block_groups(struct block_group_tree *bg_cache)
+{
+	struct btrfs_trans_handle *trans;
+	struct cache_extent *item;
+	struct block_group_record *bg_rec;
+	int ret = 0;
+
+	for (item = first_cache_extent(&bg_cache->tree); item;
+	     item = next_cache_extent(item)) {
+		bg_rec = container_of(item, struct block_group_record,
+				      cache);
+		if (bg_rec->disk_used == bg_rec->actual_used)
+			continue;
+		fprintf(stderr,
+			"block group[%llu %llu] used %llu but extent items used %llu\n",
+			bg_rec->objectid, bg_rec->offset, bg_rec->disk_used,
+			bg_rec->actual_used);
+		ret = -1;
+	}
+
+	if (!repair || !ret)
+		return ret;
+
+	trans = btrfs_start_transaction(gfs_info->extent_root, 1);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		fprintf(stderr, "Failed to start a transaction\n");
+		return ret;
+	}
+
+	ret = btrfs_fix_block_accounting(trans);
+	btrfs_commit_transaction(trans, gfs_info->extent_root);
+	return ret ? ret : -EAGAIN;
+}
+
 /**
  * parse_tree_roots - Go over all roots in the tree root and add each one to
  *		      a list.
@@ -8890,6 +8964,13 @@ again:
 		goto out;
 	}
 
+	ret = check_block_groups(&block_group_cache);
+	if (ret) {
+		if (ret == -EAGAIN)
+			goto loop;
+		goto out;
+	}
+
 	ret = check_devices(&dev_cache, &dev_extent_cache);
 	if (ret && err)
 		ret = err;
-- 
2.26.3

[PATCH 3/3] btrfs-progs: add a test image with a corrupt block group item

[Josef Bacik]

This image has a broken used field of a block group item to validate
fsck does the correct thing.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 .../050-invalid-block-group-used/default.img.xz  | Bin 0 -> 1036 bytes
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 tests/fsck-tests/050-invalid-block-group-used/default.img.xz

diff --git a/tests/fsck-tests/050-invalid-block-group-used/default.img.xz b/tests/fsck-tests/050-invalid-block-group-used/default.img.xz
new file mode 100644
index 0000000000000000000000000000000000000000..6425ba16349395416fc1918ce4c386059d618409
GIT binary patch
literal 1036
zcmV+n1oQj-H+ooF000E$*0e?f03iVu0001VFXf}+6aNFrT>wRyj;C3^v%$$4d1rE0
zjjaF1$8Jv*pMMbE@{&83eAipquzP5y*Z^dMFaaah{5${uN^u50JUj3Gv&TKFetZNb
zeGi-h_Sc-xdQ?TDr}!X-75pO<Qv3dfP)udQ#2BMlhKV!<)h0wx#0gBFB3oujWhIL&
zL%Q_)XZGuT<s*G=df*h(28)|6jezRgn^fw3f_X9cr>3s7;Q#Cs@eB}(-tDiTrOkuL
zr-`Q{!4@d7$vYR${O40b$KD!sxV##0^*`8ePjSki5__Mw0>a{sam+nk%_Taf1QUIl
zCPC+p!>ol%J~2Y3anuU4qT<_;R2!C0z*;OS_X<C(1PY_piT;4cC;rb`fiOWnUh;Y5
z+VpHb7TtwJG9h(H?eIvX{dw_9_d^y!4q2*1)lacoD~cQb2zYFYf$yU6F73JuP-a_~
z^nRPN>lH|4cwp*YYCcEjpgm>gLEL94KJ)NIDw(fAxBE)HBeuH?SFG)P2n*qCcX@dj
zl6!cB_HK~tM6u@FF~?28<QXd$N2j((D}P0X*4u&8MljFY;=CO4qv}A|Yfk0*J1L<0
zvtlIvVt#RxD#E>(XL*Vme;}!4%~G!$l6uxm(%@F-`~K9;P=VVx6iDWsw2&F)MC!ne
z{yU2uSdn?OJQ<Lt*^Y0*dZ}@9Mbh>k99D$H$r|?eQ%u-&&dW^Q%ogbbG<!ht<u}AO
zPK!nm?T;rPEW*}JR&_-PG_1)0l`D=?gQ%=PFA7tIi*Hc#g_0U1=2f8&+0+M2vL0@Y
zRepZ8%3sj#@uRLS03@G+a$5t}CT2~i>`T74qDMIaXhLRLP+<sRw5KZtAG@&9m`Ls0
zS5b66v3%FLAinwOLU5f}@`Y!nIlBhSV}9k)|B|7ydoH$5f8x-VX7s|n=;9N;KXH?H
z@kBA19X?ijYFlm44FX3(6!04T9(Yd!8hL5JWd}`cJ{=UOnZVNi<^S&%@5!isxf&15
z3`Qf*v}A6h?C9<3y1jv7oiCsGu3lhGk{+-4VN_WAkphoN6}JCC>j+OmK|vlc6+Q+8
z>i9;tP|5yL$Hg>IZhAK@v(3YzO&IxH_W<yblu5-O+ic3l3LzTL=eJ!H;1FmLNMh9z
zP}b#vH2+}dMT<T6wQ}~GA0mWw5XZl*-GXoYp{R!3<B4{}7<uJK<0J_L%nX=dW8jjl
zJo6(aM0F6xZK|Og`Pbn0U-IJOVOIUa)MDdsCv%q+fGEK_y4SF$G)B_i)~Dk);s!z~
zd&otC2e}ecZOkpbFV;}uV=M0f0000E#S;ff2MZnm0p$mPs0aWTvKPg%#Ao{g00000
G1X)_aZ2)fo

literal 0
HcmV?d00001

-- 
2.26.3

Re: [PATCH 0/3] btrfs-progs: make check handle invalid bg items

[Qu Wenruo]

On 2021/8/18 下午12:39, Josef Bacik wrote:
> Hello,
>
> While writing code for extent tree v2

Any doc on the extent tree v2?

Maybe it's a good chance for me to prompt my previous skinny block group
tree?

As that would greatly reduce mount time, and since it will introduce
incompat flags anyway, it may be a good time to pack those two features
together?

> I noticed that I was generating a fs with
> an invalid block group ->used value.  However fsck wasn't catching this, because
> we don't actuall check the used value of the block group items in normal mode.
> lowmem mode does this properly thankfully, so this only needs to be added to the
> normal fsck mode.

Lowmem mode has a lot of hidden extra checks waiting to be found. :)

>
> I've added code to btrfs-corrupt-block to generate the corrupt image I need for
> the test case.  Then of course the actual patch to detect and fix the problem.
> Thanks,
>
> Josef
>

For the whole series:

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> Josef Bacik (3):
>    btrfs-progs: add the ability to corrupt block group items
>    btrfs-progs: make check detect and fix invalid used for block groups
>    btrfs-progs: add a test image with a corrupt block group item
>
>   btrfs-corrupt-block.c                         | 108 +++++++++++++++++-
>   check/common.h                                |   5 +
>   check/main.c                                  |  89 ++++++++++++++-
>   .../default.img.xz                            | Bin 0 -> 1036 bytes
>   4 files changed, 197 insertions(+), 5 deletions(-)
>   create mode 100644 tests/fsck-tests/050-invalid-block-group-used/default.img.xz
>

Re: [PATCH 0/3] btrfs-progs: make check handle invalid bg items

[David Sterba]

On Wed, Aug 18, 2021 at 12:39:19AM -0400, Josef Bacik wrote:
> Hello,
> 
> While writing code for extent tree v2 I noticed that I was generating a fs with
> an invalid block group ->used value.  However fsck wasn't catching this, because
> we don't actuall check the used value of the block group items in normal mode.
> lowmem mode does this properly thankfully, so this only needs to be added to the
> normal fsck mode.
> 
> I've added code to btrfs-corrupt-block to generate the corrupt image I need for
> the test case.  Then of course the actual patch to detect and fix the problem.
> Thanks,
> 
> Josef
> 
> Josef Bacik (3):
>   btrfs-progs: add the ability to corrupt block group items
>   btrfs-progs: make check detect and fix invalid used for block groups
>   btrfs-progs: add a test image with a corrupt block group item

Please use prefixes for the "subsystems" that get changed like

btrfs-progs: corrupt-block: add ability to corrupt block group items
btrfs-progs: check: detect and fix invalid used for block groups
btrfs-progs: tests: add image with a corrupt block group item

Series added to devel, thanks.