btrfs-progs: print-tree: Do correct offset output for ROOT_ITEM

PATCH Applied Qu Wenruo 4 participants 8 messages Mar 19, 2018 Btrfs file system

Share & tools

Qu Wenruo

author

2018-03-19 10:28 UTC #0 lore raw

PATCH 1/1 btrfs-progs: print-tree: Do correct offset output for ROOT_ITEM

Commit Fixes: 8c36786c8198 ("btrfs-progs: print-tree: Print offset as tree objectid for ROOT_ITEM") changes how we translate offset of ROOT_ITEM.

However the fact is, even for ROOT_ITEM, we have different meaning of offset.

For tree reloc tree, it's indeed subvolume id.
But for other trees, it's the transid of when it's created.

Fix it.

Reported-by Nikolay Borisov <[email protected]>

Signed-off-by Qu Wenruo <[email protected]>

---
 print-tree.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

print-tree.c +10 -1

index 45350fea0963..848e296c4e67 100644
@@ -834,7 +834,16 @@ void btrfs_print_key(struct btrfs_disk_key *disk_key)
 	 */
 	case BTRFS_ROOT_ITEM_KEY:
 		printf(" ");
-		print_objectid(stdout, offset, type);
+		/*
+		 * Normally offset of ROOT_ITEM should presents the generation
+		 * when this root is created.
+		 * However if this is tree reloc tree, offset is the subvolume
+		 * id of its source. Here we do extra check on this.
+		 */
+		if (objectid == BTRFS_TREE_RELOC_OBJECTID)
+			print_objectid(stdout, offset, type);
+		else
+			printf("%lld", offset);
 		printf(")");
 		break;
 	default:

Signature · 1 line

2.16.2

Nikolay Borisov

2018-03-19 10:46 UTC #1 lore raw

Quoted 24 lines · Qu Wenruo

Commit Fixes: 8c36786c8198 ("btrfs-progs: print-tree: Print offset as tree objectid for ROOT_ITEM") changes how we translate offset of ROOT_ITEM.
However the fact is, even for ROOT_ITEM, we have different meaning of offset.
For tree reloc tree, it's indeed subvolume id.
But for other trees, it's the transid of when it's created.
Fix it.
Reported-by Nikolay Borisov <[email protected]>
Signed-off-by Qu Wenruo <[email protected]>
---
 print-tree.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
print-tree.c +0 -0
index 45350fea0963..848e296c4e67 100644
@@ -834,7 +834,16 @@ void btrfs_print_key(struct btrfs_disk_key *disk_key)
 	 */
 	case BTRFS_ROOT_ITEM_KEY:

print-tree.c void btrfs_print_key(struct btrfs_disk_key *disk_key) · line 844

Quoted 10 lines

print-tree.c +8 -1

 		printf(" ");
-		print_objectid(stdout, offset, type);
+		/*
+		 * Normally offset of ROOT_ITEM should presents the generation
+		 * when this root is created.
+		 * However if this is tree reloc tree, offset is the subvolume
+		 * id of its source. Here we do extra check on this.
+		 */
+		if (objectid == BTRFS_TREE_RELOC_OBJECTID)
+			print_objectid(stdout, offset, type);

Do we even have to do this for the reloc tree. If the offset is just a numeric id tied to a subvolume then the only possible value different than a numerci value is FS_TREE (in case we have 5 as the source subvol) but even this is not very informative. I'd suggest just leaving it as a numeric value.

Quoted 6 lines

print-tree.c +2 -0

+		else
+			printf("%lld", offset);
 		printf(")");
 		break;
 	default:

Qu Wenruo

2018-03-19 10:51 UTC #2 lore raw

Quoted 35 lines · Nikolay Borisov

Quoted 32 lines · Qu Wenruo
Commit Fixes: 8c36786c8198 ("btrfs-progs: print-tree: Print offset as tree objectid for ROOT_ITEM") changes how we translate offset of ROOT_ITEM.
However the fact is, even for ROOT_ITEM, we have different meaning of offset.
For tree reloc tree, it's indeed subvolume id.
But for other trees, it's the transid of when it's created.
Fix it.
Reported-by Nikolay Borisov <[email protected]>
Signed-off-by Qu Wenruo <[email protected]>
---
 print-tree.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
print-tree.c +6 -1
index 45350fea0963..848e296c4e67 100644
@@ -834,7 +834,16 @@ void btrfs_print_key(struct btrfs_disk_key *disk_key)
 	 */
 	case BTRFS_ROOT_ITEM_KEY:
 		printf(" ");
-		print_objectid(stdout, offset, type);
+		/*
+		 * Normally offset of ROOT_ITEM should presents the generation
+		 * when this root is created.
+		 * However if this is tree reloc tree, offset is the subvolume
+		 * id of its source. Here we do extra check on this.
+		 */

Quoted 8 lines

print-tree.c void btrfs_print_key(struct btrfs_disk_key *disk_key) · line 844
Quoted 2 lines
print-tree.c +2 -0
+		if (objectid == BTRFS_TREE_RELOC_OBJECTID)
+			print_objectid(stdout, offset, type);
Do we even have to do this for the reloc tree. If the offset is just a numeric id tied to a subvolume then the only possible value different than a numerci value is FS_TREE (in case we have 5 as the source subvol) but even this is not very informative. I'd suggest just leaving it as a numeric value.

What about this value?
18446744073709551607

It's data reloc tree objectid.
And it's pretty long since it's -9ULL.

At least this makes 2 objectid more readable.

Thanks,
Qu

Quoted 12 lines

print-tree.c void btrfs_print_key(struct btrfs_disk_key *disk_key) · line 849
Quoted 7 lines
print-tree.c +2 -0
+		else
+			printf("%lld", offset);
 		printf(")");
 		break;
 	default:
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Nikolay Borisov

2018-03-19 10:54 UTC #3 lore raw

Quoted 7 lines · Qu Wenruo

What about this value?
18446744073709551607
It's data reloc tree objectid.
And it's pretty long since it's -9ULL.
At least this makes 2 objectid more readable.

Fair enough.

Nikolay Borisov

2018-03-19 11:15 UTC #4 lore raw

Quoted 6 lines · Nikolay Borisov

Quoted 3 lines · Qu Wenruo
What about this value?
18446744073709551607

Quoted 7 lines

Quoted 4 lines
It's data reloc tree objectid.
And it's pretty long since it's -9ULL.
At least this makes 2 objectid more readable.
Fair enough.

Looking a bit more I wonder if we should use the same print_objectid function for the "owner" field of a leaf block.

For the data reloc tree we currently get:

leaf 30490624 items 2 free space 16061 generation 4 owner 18446744073709551607

Or for the leaf belonging to the first fs tree (5):
leaf 30883840 items 11 free space 15354 generation 10 owner 5

Qu Wenruo

2018-03-19 11:18 UTC #5 lore raw

Quoted 20 lines · Nikolay Borisov

Quoted 6 lines · Nikolay Borisov
Quoted 3 lines · Qu Wenruo
What about this value?
18446744073709551607
Quoted 7 lines
Quoted 4 lines
It's data reloc tree objectid.
And it's pretty long since it's -9ULL.
At least this makes 2 objectid more readable.
Fair enough.
Looking a bit more I wonder if we should use the same print_objectid function for the "owner" field of a leaf block.

Quoted 8 lines

For the data reloc tree we currently get:
leaf 30490624 items 2 free space 16061 generation 4 owner 18446744073709551607
Or for the leaf belonging to the first fs tree (5):
leaf 30883840 items 11 free space 15354 generation 10 owner 5

Good idea!

Looks pretty nice to me.

Thanks,
Qu

Quoted 5 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Nikolay Borisov

Reviewed-by on v1 1/1

2018-03-19 12:07 UTC #6 lore raw

In reply to Qu Wenruo

Quoted 27 lines · Qu Wenruo

Commit Fixes: 8c36786c8198 ("btrfs-progs: print-tree: Print offset as tree objectid for ROOT_ITEM") changes how we translate offset of ROOT_ITEM.
However the fact is, even for ROOT_ITEM, we have different meaning of offset.
For tree reloc tree, it's indeed subvolume id.
But for other trees, it's the transid of when it's created.
Fix it.
Reported-by Nikolay Borisov <[email protected]>
Signed-off-by Qu Wenruo <[email protected]>
---
 print-tree.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
print-tree.c +1 -1
index 45350fea0963..848e296c4e67 100644
@@ -834,7 +834,16 @@ void btrfs_print_key(struct btrfs_disk_key *disk_key)
 	 */
 	case BTRFS_ROOT_ITEM_KEY:
 		printf(" ");
-		print_objectid(stdout, offset, type);
+		/*

print-tree.c void btrfs_print_key(struct btrfs_disk_key *disk_key) · line 847

Quoted 10 lines

print-tree.c +9 -0

+		 * Normally offset of ROOT_ITEM should presents the generation
+		 * when this root is created.
+		 * However if this is tree reloc tree, offset is the subvolume
+		 * id of its source. Here we do extra check on this.
+		 */
+		if (objectid == BTRFS_TREE_RELOC_OBJECTID)
+			print_objectid(stdout, offset, type);
+		else
+			printf("%lld", offset);
 		printf(")");

Reviewed-by Nikolay Borisov <[email protected]>

Quoted 3 lines

print-tree.c +0 -0

 		break;
 	default:

David Sterba

Applied

2018-03-19 18:17 UTC #7 lore raw

In reply to Qu Wenruo

Quoted 6 lines · Qu Wenruo

Commit Fixes: 8c36786c8198 ("btrfs-progs: print-tree: Print offset as tree objectid for ROOT_ITEM") changes how we translate offset of ROOT_ITEM.
However the fact is, even for ROOT_ITEM, we have different meaning of offset.

Quoted 7 lines

For tree reloc tree, it's indeed subvolume id.
But for other trees, it's the transid of when it's created.
Fix it.
Reported-by Nikolay Borisov <[email protected]>
Signed-off-by Qu Wenruo <[email protected]>

Applied, thanks.