Skip to contentSkip to conversation

btrfs: bypass UUID API aliasing

PATCH In review Andy Shevchenko 3 participants 11 messages Feb 24, 2020

Subsystems (3)
Library code Btrfs file system Uuid helpers
lore
Share & tools
lore
author reviewer
#3 lore raw

PATCH v4 2/4 uuid: Provide a GUID generator for raw buffer

In reply to Andy Shevchenko

In some cases we would like to generate a GUID and export it. Though it would require either casting to internal kernel types or an intermediate buffer. Instead we may achieve this by supplying a pointer to raw buffer and make a complimentary API to existing one for UUIDs.

Signed-off-by Andy Shevchenko <[email protected]>
---
 include/linux/uuid.h |  1 +
 lib/uuid.c           | 10 ++++++++++
 2 files changed, 11 insertions(+)
include/linux/uuid.h +1 -0
index 8e4a5000da03..3780460a9a85 100644
@@ -84,6 +84,7 @@ static inline bool uuid_is_null(const uuid_t *uuid)
 }
 
 void generate_random_uuid(unsigned char uuid[16]);
+void generate_random_guid(unsigned char guid[16]);
 
 extern void guid_gen(guid_t *u);
 extern void uuid_gen(uuid_t *u);
lib/uuid.c +10 -0
index b6a1edb61d87..562d53977cab 100644
@@ -40,6 +40,16 @@ void generate_random_uuid(unsigned char uuid[16])
 }
 EXPORT_SYMBOL(generate_random_uuid);
 
+void generate_random_guid(unsigned char guid[16])
+{
+	get_random_bytes(guid, 16);
+	/* Set GUID version to 4 --- truly random generation */
+	guid[7] = (guid[7] & 0x0F) | 0x40;
+	/* Set the GUID variant to DCE */
+	guid[8] = (guid[8] & 0x3F) | 0x80;
+}
+EXPORT_SYMBOL(generate_random_guid);
+
 static void __uuid_gen_common(__u8 b[16])
 {
 	prandom_bytes(b, 16);
Signature · 1 line
2.25.0

Andy Shevchenko

author reviewer
#0 lore raw

v4 cover btrfs: bypass UUID API aliasing

UUID API has been converted to differentiate UUID and GUID. Btrfs is using aliases to the new API for some time.

Bypass that aliasing by converting btrfs to use API calls directly.

The series has been compiled only.

P.S. It may be applied either to btrfs tree or to UUID one.

Change v3:
- rebase to latest kernel base (Linux Next)
- explain in cover letter purpose of conversion (Johannes)

Andy Shevchenko (4):
uuid: Add inline helpers to import / export UUIDs
uuid: Provide a GUID generator for raw buffer
Btrfs: Switch to use new generic UUID API
uuid: Remove no more needed macro

 fs/btrfs/disk-io.c     |  6 +++---
 fs/btrfs/ioctl.c       |  4 +---
 fs/btrfs/root-tree.c   |  4 +---
 fs/btrfs/transaction.c |  7 +++----
 include/linux/uuid.h   | 22 +++++++++++++++++++++-
 lib/uuid.c             | 10 ++++++++++
 6 files changed, 39 insertions(+), 14 deletions(-)
Signature · 1 line
2.25.0
author reviewer
#7 lore raw

PATCH v4 4/4 uuid: Remove no more needed macro

uuid_le_gen() is no used anymore, remove it for good.

Signed-off-by Andy Shevchenko <[email protected]>
---
 include/linux/uuid.h | 1 -
 1 file changed, 1 deletion(-)
include/linux/uuid.h +0 -1
index 3780460a9a85..d41b0d3e9474 100644
@@ -98,7 +98,6 @@ int guid_parse(const char *uuid, guid_t *u);
 int uuid_parse(const char *uuid, uuid_t *u);
 
 /* backwards compatibility, don't use in new code */
-#define uuid_le_gen(u)		guid_gen(u)
 #define uuid_le_to_bin(guid, u)	guid_parse(guid, u)
 
 static inline int uuid_le_cmp(const guid_t u1, const guid_t u2)
Signature · 1 line
2.25.0
author reviewer
#1 lore raw

PATCH v4 1/4 uuid: Add inline helpers to import / export UUIDs

In reply to Andy Shevchenko

Sometimes we may need to import UUID from or export to the raw buffer, which is provided outside of kernel and can't be declared as UUID type. With current API this operation will require an explicit casting to one of UUID types and length, that is always a constant derived as sizeof the certain UUID type.

Provide a helpful set of inline helpers to minimize developer's effort in the cases when raw buffers are involved.

Suggested-by David Sterba <[email protected]>
Signed-off-by Andy Shevchenko <[email protected]>
---
 include/linux/uuid.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
include/linux/uuid.h +20 -0
index 0c631e2a73b6..8e4a5000da03 100644
@@ -43,6 +43,16 @@ static inline void guid_copy(guid_t *dst, const guid_t *src)
 	memcpy(dst, src, sizeof(guid_t));
 }
 
+static inline void import_guid(guid_t *dst, const __u8 *src)
+{
+	memcpy(dst, src, sizeof(guid_t));
+}
+
+static inline void export_guid(__u8 *dst, const guid_t *src)
+{
+	memcpy(dst, src, sizeof(guid_t));
+}
+
 static inline bool guid_is_null(const guid_t *guid)
 {
 	return guid_equal(guid, &guid_null);

@@ -58,6 +68,16 @@ static inline void uuid_copy(uuid_t *dst, const uuid_t *src)
 	memcpy(dst, src, sizeof(uuid_t));
 }
 
+static inline void import_uuid(uuid_t *dst, const __u8 *src)
+{
+	memcpy(dst, src, sizeof(uuid_t));
+}
+
+static inline void export_uuid(__u8 *dst, const uuid_t *src)
+{
+	memcpy(dst, src, sizeof(uuid_t));
+}
+
 static inline bool uuid_is_null(const uuid_t *uuid)
 {
 	return uuid_equal(uuid, &uuid_null);
Signature · 1 line
2.25.0
author reviewer
#5 lore raw

PATCH v4 3/4 Btrfs: Switch to use new generic UUID API

In reply to Andy Shevchenko

There are new types and helpers that are supposed to be used in new code.

As a preparation to get rid of legacy types and API functions do the conversion here.

Signed-off-by Andy Shevchenko <[email protected]>
---
 fs/btrfs/disk-io.c     | 6 +++---
 fs/btrfs/ioctl.c       | 4 +---
 fs/btrfs/root-tree.c   | 4 +---
 fs/btrfs/transaction.c | 7 +++----
 4 files changed, 8 insertions(+), 13 deletions(-)
fs/btrfs/disk-io.c +3 -3
index 1a552391344e..a1801a8923af 100644
@@ -1220,7 +1220,6 @@ struct btrfs_root *btrfs_create_tree(struct btrfs_trans_handle *trans,
 	struct btrfs_key key;
 	unsigned int nofs_flag;
 	int ret = 0;
-	uuid_le uuid = NULL_UUID_LE;
 
 	/*
 	 * We're holding a transaction handle, so use a NOFS memory allocation

@@ -1259,8 +1258,9 @@ struct btrfs_root *btrfs_create_tree(struct btrfs_trans_handle *trans,
 	btrfs_set_root_last_snapshot(&root->root_item, 0);
 	btrfs_set_root_dirid(&root->root_item, 0);
 	if (is_fstree(objectid))
-		uuid_le_gen(&uuid);
-	memcpy(root->root_item.uuid, uuid.b, BTRFS_UUID_SIZE);
+		generate_random_guid(root->root_item.uuid);
+	else
+		export_guid(root->root_item.uuid, &guid_null);
 	root->root_item.drop_level = 0;
 
 	key.objectid = objectid;
fs/btrfs/ioctl.c +1 -3
index 54168a7ede00..1d314b94fd93 100644
@@ -574,7 +574,6 @@ static noinline int create_subvol(struct inode *dir,
 	u64 objectid;
 	u64 new_dirid = BTRFS_FIRST_FREE_OBJECTID;
 	u64 index = 0;
-	uuid_le new_uuid;
 
 	root_item = kzalloc(sizeof(*root_item), GFP_KERNEL);
 	if (!root_item)

@@ -644,8 +643,7 @@ static noinline int create_subvol(struct inode *dir,
 
 	btrfs_set_root_generation_v2(root_item,
 			btrfs_root_generation(root_item));
-	uuid_le_gen(&new_uuid);
-	memcpy(root_item->uuid, new_uuid.b, BTRFS_UUID_SIZE);
+	generate_random_guid(root_item->uuid);
 	btrfs_set_stack_timespec_sec(&root_item->otime, cur_time.tv_sec);
 	btrfs_set_stack_timespec_nsec(&root_item->otime, cur_time.tv_nsec);
 	root_item->ctime = root_item->otime;
fs/btrfs/root-tree.c +1 -3
index b37904327c9e..98b6e0d980f9 100644
@@ -22,7 +22,6 @@
 static void btrfs_read_root_item(struct extent_buffer *eb, int slot,
 				struct btrfs_root_item *item)
 {
-	uuid_le uuid;
 	u32 len;
 	int need_reset = 0;
 

@@ -44,8 +43,7 @@ static void btrfs_read_root_item(struct extent_buffer *eb, int slot,
 			sizeof(*item) - offsetof(struct btrfs_root_item,
 					generation_v2));
 
-		uuid_le_gen(&uuid);
-		memcpy(item->uuid, uuid.b, BTRFS_UUID_SIZE);
+		generate_random_guid(item->uuid);
 	}
 }
fs/btrfs/transaction.c +3 -4
index d00af9c13c40..56db59ea9d93 100644
@@ -1478,7 +1478,6 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
 	u64 index = 0;
 	u64 objectid;
 	u64 root_flags;
-	uuid_le new_uuid;
 
 	ASSERT(pending->path);
 	path = pending->path;

@@ -1571,8 +1570,7 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
 
 	btrfs_set_root_generation_v2(new_root_item,
 			trans->transid);
-	uuid_le_gen(&new_uuid);
-	memcpy(new_root_item->uuid, new_uuid.b, BTRFS_UUID_SIZE);
+	generate_random_guid(new_root_item->uuid);
 	memcpy(new_root_item->parent_uuid, root->root_item.uuid,
 			BTRFS_UUID_SIZE);
 	if (!(root_flags & BTRFS_ROOT_SUBVOL_RDONLY)) {

@@ -1683,7 +1681,8 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
 		btrfs_abort_transaction(trans, ret);
 		goto fail;
 	}
-	ret = btrfs_uuid_tree_add(trans, new_uuid.b, BTRFS_UUID_KEY_SUBVOL,
+	ret = btrfs_uuid_tree_add(trans, new_root_item->uuid,
+				  BTRFS_UUID_KEY_SUBVOL,
 				  objectid);
 	if (ret) {
 		btrfs_abort_transaction(trans, ret);
Signature · 1 line
2.25.0
#9 lore raw
In reply to Andy Shevchenko
Quoted 6 lines · Andy Shevchenko

UUID API has been converted to differentiate UUID and GUID. Btrfs is using aliases to the new API for some time.

Bypass that aliasing by converting btrfs to use API calls directly.

The series has been compiled only.

Now it looks good to me, all the objections have been addressed, thanks.

Quoted 1 line

P.S. It may be applied either to btrfs tree or to UUID one.

Both works for me. There are no conflicts with current btrfs queue and I don't expect anything serious in the future.

Acked-by on v4 1/4
#2 lore raw

Re: [PATCH v4 1/4] uuid: Add inline helpers to import / export UUIDs

In reply to Andy Shevchenko
Quoted 11 lines · Andy Shevchenko

Sometimes we may need to import UUID from or export to the raw buffer, which is provided outside of kernel and can't be declared as UUID type. With current API this operation will require an explicit casting to one of UUID types and length, that is always a constant derived as sizeof the certain UUID type.

Provide a helpful set of inline helpers to minimize developer's effort in the cases when raw buffers are involved.

Suggested-by David Sterba <[email protected]>
Signed-off-by Andy Shevchenko <[email protected]>

No a huge fan of these trivial memcpy wrappers, but they also don't seem entirely horrible:

Acked-by Christoph Hellwig <[email protected]>
Reviewed-by on v4 2/4
#4 lore raw

Re: [PATCH v4 2/4] uuid: Provide a GUID generator for raw buffer

In reply to Andy Shevchenko
Quoted 7 lines · Andy Shevchenko

In some cases we would like to generate a GUID and export it. Though it would require either casting to internal kernel types or an intermediate buffer. Instead we may achieve this by supplying a pointer to raw buffer and make a complimentary API to existing one for UUIDs.

Signed-off-by Andy Shevchenko <[email protected]>

Looks good,

Reviewed-by Christoph Hellwig <[email protected]>
#10 lore raw
In reply to David Sterba
Quoted 6 lines · David Sterba
Quoted 5 lines · Andy Shevchenko

UUID API has been converted to differentiate UUID and GUID. Btrfs is using aliases to the new API for some time.

Bypass that aliasing by converting btrfs to use API calls directly.

Quoted 8 lines
Quoted 1 line

The series has been compiled only.

Now it looks good to me, all the objections have been addressed, thanks.

Quoted 1 line

P.S. It may be applied either to btrfs tree or to UUID one.

Both works for me. There are no conflicts with current btrfs queue and I don't expect anything serious in the future.

Please pick it up through the btrfs tree.