btrfs: Don't generate UUID for non-fs tree
diff mbox

Message ID 20171031060816.683-1-wqu@suse.com
State New
Headers show

Commit Message

Qu WenRuo Oct. 31, 2017, 6:08 a.m. UTC
btrfs_create_tree() will unconditionally generate UUID for any root.
So for quota tree and data reloc tree created by kernel, they will have
unique UUIDs.

However UUID in root item is only referred by UUID tree, which only
records UUID for fs trees.
This makes unique UUIDs for quota/data reloc tree meaningless.

Leave the UUID as zero for non-fs tree, making btrfs-debug-tree output
less confusing.

Reported-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/disk-io.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

David Sterba Nov. 6, 2017, 3:28 p.m. UTC | #1
On Tue, Oct 31, 2017 at 02:08:16PM +0800, Qu Wenruo wrote:
> btrfs_create_tree() will unconditionally generate UUID for any root.
> So for quota tree and data reloc tree created by kernel, they will have
> unique UUIDs.
> 
> However UUID in root item is only referred by UUID tree, which only
> records UUID for fs trees.
> This makes unique UUIDs for quota/data reloc tree meaningless.
> 
> Leave the UUID as zero for non-fs tree, making btrfs-debug-tree output
> less confusing.
> 
> Reported-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>

Can you please point me to the report?

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

Reviewed-by: David Sterba <dsterba@suse.com>
--
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
Qu Wenruo Nov. 7, 2017, 12:43 a.m. UTC | #2
On 2017年11月06日 23:28, David Sterba wrote:
> On Tue, Oct 31, 2017 at 02:08:16PM +0800, Qu Wenruo wrote:
>> btrfs_create_tree() will unconditionally generate UUID for any root.
>> So for quota tree and data reloc tree created by kernel, they will have
>> unique UUIDs.
>>
>> However UUID in root item is only referred by UUID tree, which only
>> records UUID for fs trees.
>> This makes unique UUIDs for quota/data reloc tree meaningless.
>>
>> Leave the UUID as zero for non-fs tree, making btrfs-debug-tree output
>> less confusing.
>>
>> Reported-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
> 
> Can you please point me to the report?

Here it is:

https://marc.info/?l=linux-btrfs&m=150941856508074&w=2

Thanks,
Qu
> 
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
> 
> Reviewed-by: David Sterba <dsterba@suse.com>
> --
> 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
>
David Sterba Jan. 3, 2018, 2:37 p.m. UTC | #3
On Tue, Oct 31, 2017 at 02:08:16PM +0800, Qu Wenruo wrote:
> btrfs_create_tree() will unconditionally generate UUID for any root.
> So for quota tree and data reloc tree created by kernel, they will have
> unique UUIDs.
> 
> However UUID in root item is only referred by UUID tree, which only
> records UUID for fs trees.
> This makes unique UUIDs for quota/data reloc tree meaningless.
> 
> Leave the UUID as zero for non-fs tree, making btrfs-debug-tree output
> less confusing.
> 
> Reported-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/disk-io.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index dfdab849037b..d85e04a675fe 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1403,7 +1403,7 @@ struct btrfs_root *btrfs_create_tree(struct btrfs_trans_handle *trans,
>  	struct btrfs_root *root;
>  	struct btrfs_key key;
>  	int ret = 0;
> -	uuid_le uuid;
> +	uuid_le uuid = { 0 };

I get a warning with gcc 4.8.5

fs/btrfs/disk-io.c:1236:2: warning: missing braces around initializer [-Wmissing-braces]

but no warning with gcc 7.2.1 (built as 'make ccflags-y=-Wmissing-braces
and checking that the option is really there). I think we should use
NULL_UUID_LE.
--
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
Qu Wenruo Jan. 4, 2018, 1:38 a.m. UTC | #4
On 2018年01月03日 22:37, David Sterba wrote:
> On Tue, Oct 31, 2017 at 02:08:16PM +0800, Qu Wenruo wrote:
>> btrfs_create_tree() will unconditionally generate UUID for any root.
>> So for quota tree and data reloc tree created by kernel, they will have
>> unique UUIDs.
>>
>> However UUID in root item is only referred by UUID tree, which only
>> records UUID for fs trees.
>> This makes unique UUIDs for quota/data reloc tree meaningless.
>>
>> Leave the UUID as zero for non-fs tree, making btrfs-debug-tree output
>> less confusing.
>>
>> Reported-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  fs/btrfs/disk-io.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index dfdab849037b..d85e04a675fe 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -1403,7 +1403,7 @@ struct btrfs_root *btrfs_create_tree(struct btrfs_trans_handle *trans,
>>  	struct btrfs_root *root;
>>  	struct btrfs_key key;
>>  	int ret = 0;
>> -	uuid_le uuid;
>> +	uuid_le uuid = { 0 };
> 
> I get a warning with gcc 4.8.5
> 
> fs/btrfs/disk-io.c:1236:2: warning: missing braces around initializer [-Wmissing-braces]
> 
> but no warning with gcc 7.2.1 (built as 'make ccflags-y=-Wmissing-braces
> and checking that the option is really there). I think we should use
> NULL_UUID_LE.

Do I need to resend the whole patch or a new delta patch?

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
>
David Sterba Jan. 5, 2018, 12:48 p.m. UTC | #5
On Thu, Jan 04, 2018 at 09:38:46AM +0800, Qu Wenruo wrote:
> >> -	uuid_le uuid;
> >> +	uuid_le uuid = { 0 };
> > 
> > I get a warning with gcc 4.8.5
> > 
> > fs/btrfs/disk-io.c:1236:2: warning: missing braces around initializer [-Wmissing-braces]
> > 
> > but no warning with gcc 7.2.1 (built as 'make ccflags-y=-Wmissing-braces
> > and checking that the option is really there). I think we should use
> > NULL_UUID_LE.
> 
> Do I need to resend the whole patch or a new delta patch?

Not needed, I've fixed in the patch,

uuid_le = NULL_UUID_LE;
--
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

Patch
diff mbox

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index dfdab849037b..d85e04a675fe 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1403,7 +1403,7 @@  struct btrfs_root *btrfs_create_tree(struct btrfs_trans_handle *trans,
 	struct btrfs_root *root;
 	struct btrfs_key key;
 	int ret = 0;
-	uuid_le uuid;
+	uuid_le uuid = { 0 };
 
 	root = btrfs_alloc_root(fs_info, GFP_KERNEL);
 	if (!root)
@@ -1444,7 +1444,8 @@  struct btrfs_root *btrfs_create_tree(struct btrfs_trans_handle *trans,
 	btrfs_set_root_used(&root->root_item, leaf->len);
 	btrfs_set_root_last_snapshot(&root->root_item, 0);
 	btrfs_set_root_dirid(&root->root_item, 0);
-	uuid_le_gen(&uuid);
+	if (is_fstree(objectid))
+		uuid_le_gen(&uuid);
 	memcpy(root->root_item.uuid, uuid.b, BTRFS_UUID_SIZE);
 	root->root_item.drop_level = 0;