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

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

[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 <nborisov@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 print-tree.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/print-tree.c b/print-tree.c
index 45350fea0963..848e296c4e67 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -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:
-- 
2.16.2

Re: [PATCH] btrfs-progs: print-tree: Do correct offset output for ROOT_ITEM

[Nikolay Borisov]

On 19.03.2018 12:28, Qu Wenruo wrote:
> 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 <nborisov@suse.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  print-tree.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/print-tree.c b/print-tree.c
> index 45350fea0963..848e296c4e67 100644
> --- a/print-tree.c
> +++ b/print-tree.c
> @@ -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);

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.

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

Re: [PATCH] btrfs-progs: print-tree: Do correct offset output for ROOT_ITEM

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 2182 bytes --]



On 2018年03月19日 18:46, Nikolay Borisov wrote:
> 
> 
> On 19.03.2018 12:28, Qu Wenruo wrote:
>> 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 <nborisov@suse.com>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  print-tree.c | 11 ++++++++++-
>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/print-tree.c b/print-tree.c
>> index 45350fea0963..848e296c4e67 100644
>> --- a/print-tree.c
>> +++ b/print-tree.c
>> @@ -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);
> 
> 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

> 
>> +		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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]

Re: [PATCH] btrfs-progs: print-tree: Do correct offset output for ROOT_ITEM

[Nikolay Borisov]

On 19.03.2018 12:50, Qu Wenruo wrote:
> 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.

Re: [PATCH] btrfs-progs: print-tree: Do correct offset output for ROOT_ITEM

[Nikolay Borisov]

On 19.03.2018 12:54, Nikolay Borisov wrote:
> 
> 
> On 19.03.2018 12:50, Qu Wenruo wrote:
>> 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.
> 

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

Re: [PATCH] btrfs-progs: print-tree: Do correct offset output for ROOT_ITEM

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1046 bytes --]



On 2018年03月19日 19:15, Nikolay Borisov wrote:
> 
> 
> On 19.03.2018 12:54, Nikolay Borisov wrote:
>>
>>
>> On 19.03.2018 12:50, Qu Wenruo wrote:
>>> 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.
>>
> 
> 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

Good idea!

Looks pretty nice to me.

Thanks,
Qu

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]

Re: [PATCH] btrfs-progs: print-tree: Do correct offset output for ROOT_ITEM

[Nikolay Borisov]

On 19.03.2018 12:28, Qu Wenruo wrote:
> 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 <nborisov@suse.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  print-tree.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/print-tree.c b/print-tree.c
> index 45350fea0963..848e296c4e67 100644
> --- a/print-tree.c
> +++ b/print-tree.c
> @@ -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(")");

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

>  		break;
>  	default:
> 

Re: [PATCH] btrfs-progs: print-tree: Do correct offset output for ROOT_ITEM

[David Sterba]

On Mon, Mar 19, 2018 at 06:28:10PM +0800, Qu Wenruo wrote:
> 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 <nborisov@suse.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Applied, thanks.