[PATCH v2 0/7] btrfs-progs: fix -Wmissing-prototypes warnings and enable that warning option

[PATCH v2 0/7] btrfs-progs: fix -Wmissing-prototypes warnings and enable that warning option

[Qu Wenruo]

We have at least one case that some function is exported but never got
utilized in the first place.

Let's prevent this problem from happening again by enable
-Wmissing-prototypes to debug builds at least.

Fixes for  the existing warnings are split into several patches:

- Remove unused functions
  Two patches, the first is to remove a function that never got
  utilized from the introduction.

  The second is to remove a very old feature (only for <3.12 kernels)
  in libbtrfs.
  In fact this functionality for fs without an UUID tree is already
  broken during previous cleanups.
  (Need to export subvol_uuid_search_add() and
   subvol_uuid_search_finit(), as it's callers' responsibility to
   search for the target subvolume by themselves)

  And since no one is complaining ever since, there is really no need
  to maintain such an old and deprecated feature in libbtrfs.

- Fixes for crypto related function
  Two patches, one for each csum algo (blake2 and sha256).
  Involves extra declarations in the headers.

- Trivial fixes
  Mostly unexport and add needed headers.

Qu Wenruo (7):
  btrfs-progs: remove function btrfs_check_allocatable_zones()
  btrfs-progs: libbtrfs: remove the support for fs without uuid tree
  btrfs-progs: crypto/blake2: remove blake2 simple API
  btrfs-progs: crypto/blake2: move optimized declarations to blake2b.h
  btrfs-progs: crypto/sha: declare the x86 optimized implementation
  btrfs-progs: fix -Wmissing-prototypes warnings
  btrfs-progs: Makefile: enable -Wmissing-prototypes

 Makefile              |   3 +-
 cmds/qgroup.c         |   2 +-
 cmds/reflink.c        |   2 +-
 cmds/subvolume-list.c |   2 +-
 common/device-utils.c |   2 +-
 common/utils.c        |   2 +-
 crypto/blake2.h       |   5 +
 crypto/blake2b-ref.c  |   8 -
 crypto/sha.h          |   3 +
 crypto/sha256-x86.c   |   2 +
 kernel-shared/ulist.c |   2 +-
 kernel-shared/zoned.c |  60 +------
 libbtrfs/send-utils.c | 396 ------------------------------------------
 libbtrfs/send-utils.h |  20 ---
 tune/change-csum.c    |   1 +
 tune/change-uuid.c    |   1 +
 tune/convert-bgt.c    |   1 +
 tune/seeding.c        |   1 +
 tune/tune.h           |   2 +
 19 files changed, 25 insertions(+), 490 deletions(-)

-- 
2.39.2

[PATCH v2 1/7] btrfs-progs: remove function btrfs_check_allocatable_zones()

[Qu Wenruo]

This function is introduced by commit b031fe84fda8 ("btrfs-progs: zoned:
implement zoned chunk allocator") but it never got called since then.

Furthermore in the kernel zoned code, there is no such function from the
very beginning, and everything is handled by
btrfs_find_allocatable_zones().

Thus we can safely remove the function.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 kernel-shared/zoned.c | 58 -------------------------------------------
 1 file changed, 58 deletions(-)

diff --git a/kernel-shared/zoned.c b/kernel-shared/zoned.c
index 7d2e68d08bcc..16abb042f5b0 100644
--- a/kernel-shared/zoned.c
+++ b/kernel-shared/zoned.c
@@ -596,64 +596,6 @@ size_t btrfs_sb_io(int fd, void *buf, off_t offset, int rw)
 	return ret_sz;
 }
 
-/*
- * Check if spcecifeid region is suitable for allocation
- *
- * @device:	the device to allocate a region
- * @pos:	the position of the region
- * @num_bytes:	the size of the region
- *
- * In non-ZONED device, anywhere is suitable for allocation. In ZONED
- * device, check if:
- * 1) the region is not on non-empty sequential zones,
- * 2) all zones in the region have the same zone type,
- * 3) it does not contain super block location
- */
-bool btrfs_check_allocatable_zones(struct btrfs_device *device, u64 pos,
-				   u64 num_bytes)
-{
-	struct btrfs_zoned_device_info *zinfo = device->zone_info;
-	u64 nzones, begin, end;
-	u64 sb_pos;
-	bool is_sequential;
-	int shift;
-	int i;
-
-	if (!zinfo || zinfo->model == ZONED_NONE)
-		return true;
-
-	nzones = num_bytes / zinfo->zone_size;
-	begin = pos / zinfo->zone_size;
-	end = begin + nzones;
-
-	ASSERT(IS_ALIGNED(pos, zinfo->zone_size));
-	ASSERT(IS_ALIGNED(num_bytes, zinfo->zone_size));
-
-	if (end > zinfo->nr_zones)
-		return false;
-
-	shift = ilog2(zinfo->zone_size);
-	for (i = 0; i < BTRFS_SUPER_MIRROR_MAX; i++) {
-		sb_pos = sb_zone_number(shift, i);
-		if (!(end < sb_pos || sb_pos + 1 < begin))
-			return false;
-	}
-
-	is_sequential = btrfs_dev_is_sequential(device, pos);
-
-	while (num_bytes) {
-		if (is_sequential && !btrfs_dev_is_empty_zone(device, pos))
-			return false;
-		if (is_sequential != btrfs_dev_is_sequential(device, pos))
-			return false;
-
-		pos += zinfo->zone_size;
-		num_bytes -= zinfo->zone_size;
-	}
-
-	return true;
-}
-
 /**
  * btrfs_find_allocatable_zones - find allocatable zones within a given region
  *
-- 
2.39.2

Re: [PATCH v2 1/7] btrfs-progs: remove function btrfs_check_allocatable_zones()

[Anand Jain]

On 03/05/2023 14:03, Qu Wenruo wrote:
> This function is introduced by commit b031fe84fda8 ("btrfs-progs: zoned:
> implement zoned chunk allocator") but it never got called since then.
> 
> Furthermore in the kernel zoned code, there is no such function from the
> very beginning, and everything is handled by
> btrfs_find_allocatable_zones().
> 
> Thus we can safely remove the function.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Anand Jain <anand.jain@oracle.com>

[PATCH v2 2/7] btrfs-progs: libbtrfs: remove the support for fs without uuid tree

[Qu Wenruo]

Since kernel 3.12, any btrfs mounted by a kernel would have an UUID
tree created, to record all the UUID of its subvolumes.

Without UUID tree, libbtrfs send functionality has to go through all the
subvolumes seen so far, and record those subvolumes' UUID internally so
that libbtrfs send can locate a desired subvolume.

Since commit 194b90aa2c5a ("btrfs-progs: libbtrfs: remove declarations
without exports in send-utils") we're deprecating this old interface
already, meaning deprecated users won't be able to build its own
subvolume list already.

And we received no error report on this so far.

So let's finish the cleanup by removing the support for fs without an UUID
tree.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 libbtrfs/send-utils.c | 396 ------------------------------------------
 libbtrfs/send-utils.h |  20 ---
 2 files changed, 416 deletions(-)

diff --git a/libbtrfs/send-utils.c b/libbtrfs/send-utils.c
index 831ec0dc1222..74bf86be7673 100644
--- a/libbtrfs/send-utils.c
+++ b/libbtrfs/send-utils.c
@@ -199,73 +199,6 @@ static int btrfs_read_root_item(int mnt_fd, u64 root_id,
 	return 0;
 }
 
-#ifdef BTRFS_COMPAT_SEND_NO_UUID_TREE
-static struct rb_node *tree_insert(struct rb_root *root,
-				   struct subvol_info *si,
-				   enum subvol_search_type type)
-{
-	struct rb_node **p = &root->rb_node;
-	struct rb_node *parent = NULL;
-	struct subvol_info *entry;
-	__s64 comp;
-
-	while (*p) {
-		parent = *p;
-		if (type == subvol_search_by_received_uuid) {
-			entry = rb_entry(parent, struct subvol_info,
-					rb_received_node);
-
-			comp = memcmp(entry->received_uuid, si->received_uuid,
-					BTRFS_UUID_SIZE);
-			if (!comp) {
-				if (entry->stransid < si->stransid)
-					comp = -1;
-				else if (entry->stransid > si->stransid)
-					comp = 1;
-				else
-					comp = 0;
-			}
-		} else if (type == subvol_search_by_uuid) {
-			entry = rb_entry(parent, struct subvol_info,
-					rb_local_node);
-			comp = memcmp(entry->uuid, si->uuid, BTRFS_UUID_SIZE);
-		} else if (type == subvol_search_by_root_id) {
-			entry = rb_entry(parent, struct subvol_info,
-					rb_root_id_node);
-			comp = entry->root_id - si->root_id;
-		} else if (type == subvol_search_by_path) {
-			entry = rb_entry(parent, struct subvol_info,
-					rb_path_node);
-			comp = strcmp(entry->path, si->path);
-		} else {
-			BUG();
-		}
-
-		if (comp < 0)
-			p = &(*p)->rb_left;
-		else if (comp > 0)
-			p = &(*p)->rb_right;
-		else
-			return parent;
-	}
-
-	if (type == subvol_search_by_received_uuid) {
-		rb_link_node(&si->rb_received_node, parent, p);
-		rb_insert_color(&si->rb_received_node, root);
-	} else if (type == subvol_search_by_uuid) {
-		rb_link_node(&si->rb_local_node, parent, p);
-		rb_insert_color(&si->rb_local_node, root);
-	} else if (type == subvol_search_by_root_id) {
-		rb_link_node(&si->rb_root_id_node, parent, p);
-		rb_insert_color(&si->rb_root_id_node, root);
-	} else if (type == subvol_search_by_path) {
-		rb_link_node(&si->rb_path_node, parent, p);
-		rb_insert_color(&si->rb_path_node, root);
-	}
-	return NULL;
-}
-#endif
-
 int btrfs_subvolid_resolve(int fd, char *path, size_t path_len, u64 subvol_id)
 {
 	if (path_len < 1)
@@ -364,114 +297,6 @@ static int btrfs_subvolid_resolve_sub(int fd, char *path, size_t *path_len,
 	return 0;
 }
 
-#ifdef BTRFS_COMPAT_SEND_NO_UUID_TREE
-static int count_bytes(void *buf, int len, char b)
-{
-	int cnt = 0;
-	int i;
-
-	for (i = 0; i < len; i++) {
-		if (((char *)buf)[i] == b)
-			cnt++;
-	}
-	return cnt;
-}
-
-void subvol_uuid_search_add(struct subvol_uuid_search *s,
-			    struct subvol_info *si)
-{
-	int cnt;
-
-	tree_insert(&s->root_id_subvols, si, subvol_search_by_root_id);
-	tree_insert(&s->path_subvols, si, subvol_search_by_path);
-
-	cnt = count_bytes(si->uuid, BTRFS_UUID_SIZE, 0);
-	if (cnt != BTRFS_UUID_SIZE)
-		tree_insert(&s->local_subvols, si, subvol_search_by_uuid);
-	cnt = count_bytes(si->received_uuid, BTRFS_UUID_SIZE, 0);
-	if (cnt != BTRFS_UUID_SIZE)
-		tree_insert(&s->received_subvols, si,
-				subvol_search_by_received_uuid);
-}
-
-static struct subvol_info *tree_search(struct rb_root *root,
-				       u64 root_id, const u8 *uuid,
-				       u64 stransid, const char *path,
-				       enum subvol_search_type type)
-{
-	struct rb_node *n = root->rb_node;
-	struct subvol_info *entry;
-	__s64 comp;
-
-	while (n) {
-		if (type == subvol_search_by_received_uuid) {
-			entry = rb_entry(n, struct subvol_info,
-					rb_received_node);
-			comp = memcmp(entry->received_uuid, uuid,
-					BTRFS_UUID_SIZE);
-			if (!comp) {
-				if (entry->stransid < stransid)
-					comp = -1;
-				else if (entry->stransid > stransid)
-					comp = 1;
-				else
-					comp = 0;
-			}
-		} else if (type == subvol_search_by_uuid) {
-			entry = rb_entry(n, struct subvol_info, rb_local_node);
-			comp = memcmp(entry->uuid, uuid, BTRFS_UUID_SIZE);
-		} else if (type == subvol_search_by_root_id) {
-			entry = rb_entry(n, struct subvol_info,
-					 rb_root_id_node);
-			comp = entry->root_id - root_id;
-		} else if (type == subvol_search_by_path) {
-			entry = rb_entry(n, struct subvol_info, rb_path_node);
-			comp = strcmp(entry->path, path);
-		} else {
-			BUG();
-		}
-		if (comp < 0)
-			n = n->rb_left;
-		else if (comp > 0)
-			n = n->rb_right;
-		else
-			return entry;
-	}
-	return NULL;
-}
-
-/*
- * this function will be only called if kernel doesn't support uuid tree.
- */
-static struct subvol_info *subvol_uuid_search_old(struct subvol_uuid_search *s,
-				       u64 root_id, const u8 *uuid, u64 transid,
-				       const char *path,
-				       enum subvol_search_type type)
-{
-	struct rb_root *root;
-	if (type == subvol_search_by_received_uuid)
-		root = &s->received_subvols;
-	else if (type == subvol_search_by_uuid)
-		root = &s->local_subvols;
-	else if (type == subvol_search_by_root_id)
-		root = &s->root_id_subvols;
-	else if (type == subvol_search_by_path)
-		root = &s->path_subvols;
-	else
-		return NULL;
-	return tree_search(root, root_id, uuid, transid, path, type);
-}
-#else
-void subvol_uuid_search_add(struct subvol_uuid_search *s,
-			    struct subvol_info *si)
-{
-	if (si) {
-		free(si->path);
-		free(si);
-	}
-}
-#endif
-
 static struct subvol_info *subvol_uuid_search2(struct subvol_uuid_search *s,
 				       u64 root_id, const u8 *uuid, u64 transid,
 				       const char *path,
@@ -569,11 +394,6 @@ static struct subvol_info *subvol_uuid_search2(struct subvol_uuid_search *s,
 	struct btrfs_root_item root_item;
 	struct subvol_info *info = NULL;
 
-#ifdef BTRFS_COMPAT_SEND_NO_UUID_TREE
-	if (!s->uuid_tree_existed)
-		return subvol_uuid_search_old(s, root_id, uuid, transid,
-					     path, type);
-#endif
 	switch (type) {
 	case subvol_search_by_received_uuid:
 		ret = btrfs_uuid_tree_lookup_any(s->mnt_fd, uuid,
@@ -640,219 +460,3 @@ out:
 
 	return info;
 }
-
-#ifdef BTRFS_COMPAT_SEND_NO_UUID_TREE
-static int is_uuid_tree_supported(int fd)
-{
-	int ret;
-	struct btrfs_ioctl_search_args args;
-	struct btrfs_ioctl_search_key *sk = &args.key;
-
-	memset(&args, 0, sizeof(args));
-
-	sk->tree_id = BTRFS_ROOT_TREE_OBJECTID;
-
-	sk->min_objectid = BTRFS_UUID_TREE_OBJECTID;
-	sk->max_objectid = BTRFS_UUID_TREE_OBJECTID;
-	sk->max_type = BTRFS_ROOT_ITEM_KEY;
-	sk->min_type = BTRFS_ROOT_ITEM_KEY;
-	sk->max_offset = (u64)-1;
-	sk->max_transid = (u64)-1;
-	sk->nr_items = 1;
-
-	ret = ioctl(fd, BTRFS_IOC_TREE_SEARCH, &args);
-	if (ret < 0)
-		return ret;
-
-	/* the ioctl returns the number of item it found in nr_items */
-	if (sk->nr_items == 0)
-		return 0;
-
-	return 1;
-}
-
-/*
- * this function is mainly used to read all root items
- * it will be only used when we use older kernel which uuid
- * tree is not supported yet
- */
-int subvol_uuid_search_init(int mnt_fd, struct subvol_uuid_search *s)
-{
-	int ret;
-	struct btrfs_ioctl_search_args args;
-	struct btrfs_ioctl_search_key *sk = &args.key;
-	struct btrfs_ioctl_search_header *sh;
-	struct btrfs_root_item *root_item_ptr;
-	struct btrfs_root_item root_item = {};
-	struct subvol_info *si = NULL;
-	int root_item_valid = 0;
-	unsigned long off = 0;
-	int i;
-
-	s->mnt_fd = mnt_fd;
-
-	s->root_id_subvols = RB_ROOT;
-	s->local_subvols = RB_ROOT;
-	s->received_subvols = RB_ROOT;
-	s->path_subvols = RB_ROOT;
-
-	ret = is_uuid_tree_supported(mnt_fd);
-	if (ret < 0) {
-		fprintf(stderr,
-			"ERROR: check if we support uuid tree fails - %m\n");
-		return ret;
-	} else if (ret) {
-		/* uuid tree is supported */
-		s->uuid_tree_existed = 1;
-		return 0;
-	}
-	memset(&args, 0, sizeof(args));
-
-	sk->tree_id = BTRFS_ROOT_TREE_OBJECTID;
-
-	sk->max_objectid = (u64)-1;
-	sk->max_offset = (u64)-1;
-	sk->max_transid = (u64)-1;
-	sk->min_type = BTRFS_ROOT_ITEM_KEY;
-	sk->max_type = BTRFS_ROOT_BACKREF_KEY;
-	sk->nr_items = 4096;
-
-	while (1) {
-		ret = ioctl(mnt_fd, BTRFS_IOC_TREE_SEARCH, &args);
-		if (ret < 0) {
-			fprintf(stderr, "ERROR: can't perform the search - %m\n");
-			return ret;
-		}
-		if (sk->nr_items == 0)
-			break;
-
-		off = 0;
-
-		for (i = 0; i < sk->nr_items; i++) {
-			sh = (struct btrfs_ioctl_search_header *)(args.buf +
-								  off);
-			off += sizeof(*sh);
-
-			if ((btrfs_search_header_objectid(sh) != 5 &&
-			     btrfs_search_header_objectid(sh)
-			     < BTRFS_FIRST_FREE_OBJECTID) ||
-			    btrfs_search_header_objectid(sh)
-			    > BTRFS_LAST_FREE_OBJECTID) {
-				goto skip;
-			}
-
-			if (btrfs_search_header_type(sh)
-			    == BTRFS_ROOT_ITEM_KEY) {
-				/* older kernels don't have uuids+times */
-				if (btrfs_search_header_len(sh)
-				    < sizeof(root_item)) {
-					root_item_valid = 0;
-					goto skip;
-				}
-				root_item_ptr = (struct btrfs_root_item *)
-						(args.buf + off);
-				memcpy(&root_item, root_item_ptr,
-						sizeof(root_item));
-				root_item_valid = 1;
-			} else if (btrfs_search_header_type(sh)
-				   == BTRFS_ROOT_BACKREF_KEY ||
-				   root_item_valid) {
-				char path_buf[PATH_MAX];
-				char *path;
-
-				if (!root_item_valid)
-					goto skip;
-
-				ret = btrfs_subvolid_resolve(mnt_fd, path_buf,
-						sizeof(path_buf),
-						btrfs_search_header_objectid(sh));
-				if (ret < 0) {
-					errno = -ret;
-					fprintf(stderr, "ERROR: unable to "
-							"resolve path "
-							"for root %llu: %m\n",
-						btrfs_search_header_objectid(sh));
-					goto out;
-				}
-				path = strdup(path_buf);
-
-				si = calloc(1, sizeof(*si));
-				si->root_id = btrfs_search_header_objectid(sh);
-				memcpy(si->uuid, root_item.uuid,
-						BTRFS_UUID_SIZE);
-				memcpy(si->parent_uuid, root_item.parent_uuid,
-						BTRFS_UUID_SIZE);
-				memcpy(si->received_uuid,
-						root_item.received_uuid,
-						BTRFS_UUID_SIZE);
-				si->ctransid = btrfs_root_ctransid(&root_item);
-				si->otransid = btrfs_root_otransid(&root_item);
-				si->stransid = btrfs_root_stransid(&root_item);
-				si->rtransid = btrfs_root_rtransid(&root_item);
-				si->path = path;
-				subvol_uuid_search_add(s, si);
-				root_item_valid = 0;
-			} else {
-				goto skip;
-			}
-
-skip:
-			off += btrfs_search_header_len(sh);
-
-			/*
-			 * record the mins in sk so we can make sure the
-			 * next search doesn't repeat this root
-			 */
-			sk->min_objectid = btrfs_search_header_objectid(sh);
-			sk->min_offset = btrfs_search_header_offset(sh);
-			sk->min_type = btrfs_search_header_type(sh);
-		}
-		sk->nr_items = 4096;
-		if (sk->min_offset < (u64)-1)
-			sk->min_offset++;
-		else if (sk->min_objectid < (u64)-1) {
-			sk->min_objectid++;
-			sk->min_offset = 0;
-			sk->min_type = 0;
-		} else
-			break;
-	}
-
-out:
-	return ret;
-}
-
-void subvol_uuid_search_finit(struct subvol_uuid_search *s)
-{
-	struct rb_root *root = &s->root_id_subvols;
-	struct rb_node *node;
-
-	if (!s->uuid_tree_existed)
-		return;
-
-	while ((node = rb_first(root))) {
-		struct subvol_info *entry =
-			rb_entry(node, struct subvol_info, rb_root_id_node);
-
-		free(entry->path);
-		rb_erase(node, root);
-		free(entry);
-	}
-
-	s->root_id_subvols = RB_ROOT;
-	s->local_subvols = RB_ROOT;
-	s->received_subvols = RB_ROOT;
-	s->path_subvols = RB_ROOT;
-}
-#else
-int subvol_uuid_search_init(int mnt_fd, struct subvol_uuid_search *s)
-{
-	s->mnt_fd = mnt_fd;
-
-	return 0;
-}
-
-void subvol_uuid_search_finit(struct subvol_uuid_search *s)
-{
-}
-#endif
diff --git a/libbtrfs/send-utils.h b/libbtrfs/send-utils.h
index b39ee9255598..be6f91b1d7bb 100644
--- a/libbtrfs/send-utils.h
+++ b/libbtrfs/send-utils.h
@@ -35,12 +35,6 @@
 extern "C" {
 #endif
 
-/*
- * Compatibility code for kernels < 3.12; the UUID tree is not available there
- * and we have to do the slow search. This should be deprecated someday.
- */
-#define BTRFS_COMPAT_SEND_NO_UUID_TREE 1
-
 enum subvol_search_type {
 	subvol_search_by_root_id,
 	subvol_search_by_uuid,
@@ -49,12 +43,6 @@ enum subvol_search_type {
 };
 
 struct subvol_info {
-#ifdef BTRFS_COMPAT_SEND_NO_UUID_TREE
-	struct rb_node rb_root_id_node;
-	struct rb_node rb_local_node;
-	struct rb_node rb_received_node;
-	struct rb_node rb_path_node;
-#endif
 
 	u64 root_id;
 	u8 uuid[BTRFS_UUID_SIZE];
@@ -70,14 +58,6 @@ struct subvol_info {
 
 struct subvol_uuid_search {
 	int mnt_fd;
-#ifdef BTRFS_COMPAT_SEND_NO_UUID_TREE
-	int uuid_tree_existed;
-
-	struct rb_root root_id_subvols;
-	struct rb_root local_subvols;
-	struct rb_root received_subvols;
-	struct rb_root path_subvols;
-#endif
 };
 
 int subvol_uuid_search_init(int mnt_fd, struct subvol_uuid_search *s);
-- 
2.39.2

Re: [PATCH v2 2/7] btrfs-progs: libbtrfs: remove the support for fs without uuid tree

[David Sterba]

On Wed, May 03, 2023 at 02:03:38PM +0800, Qu Wenruo wrote:
> Since kernel 3.12, any btrfs mounted by a kernel would have an UUID
> tree created, to record all the UUID of its subvolumes.
> 
> Without UUID tree, libbtrfs send functionality has to go through all the
> subvolumes seen so far, and record those subvolumes' UUID internally so
> that libbtrfs send can locate a desired subvolume.
> 
> Since commit 194b90aa2c5a ("btrfs-progs: libbtrfs: remove declarations
> without exports in send-utils") we're deprecating this old interface
> already, meaning deprecated users won't be able to build its own
> subvolume list already.
> 
> And we received no error report on this so far.
> 
> So let's finish the cleanup by removing the support for fs without an UUID
> tree.

This changes is the only one that worries me, I saw the potential to
remove the code in the past but was hesitant to do so to avoid further
breakage caused by libbtrfs changes.

However, I'd like to get rid of the code too so let's do it. With the
past experience of breaking some 3rd party tools I now at least know
what to test in advance. Debian code search did not find anything
relevant for the removed struct members .h, nor
BTRFS_COMPAT_SEND_NO_UUID_TREE so this is good.

Re: [PATCH v2 2/7] btrfs-progs: libbtrfs: remove the support for fs without uuid tree

[Qu Wenruo]

On 2023/5/4 02:35, David Sterba wrote:
> On Wed, May 03, 2023 at 02:03:38PM +0800, Qu Wenruo wrote:
>> Since kernel 3.12, any btrfs mounted by a kernel would have an UUID
>> tree created, to record all the UUID of its subvolumes.
>>
>> Without UUID tree, libbtrfs send functionality has to go through all the
>> subvolumes seen so far, and record those subvolumes' UUID internally so
>> that libbtrfs send can locate a desired subvolume.
>>
>> Since commit 194b90aa2c5a ("btrfs-progs: libbtrfs: remove declarations
>> without exports in send-utils") we're deprecating this old interface
>> already, meaning deprecated users won't be able to build its own
>> subvolume list already.
>>
>> And we received no error report on this so far.
>>
>> So let's finish the cleanup by removing the support for fs without an UUID
>> tree.
>
> This changes is the only one that worries me, I saw the potential to
> remove the code in the past but was hesitant to do so to avoid further
> breakage caused by libbtrfs changes.
>
> However, I'd like to get rid of the code too so let's do it. With the
> past experience of breaking some 3rd party tools I now at least know
> what to test in advance. Debian code search did not find anything
> relevant for the removed struct members .h, nor
> BTRFS_COMPAT_SEND_NO_UUID_TREE so this is good.


Even if we drop this patch, the damage is already done in the recent
releases.

Without the proper search/add function exported, the libbtrfs users can
not handle the fs without UUID tree already.

Considering no such report so far, and those functions are only for UUID
search (to implement a receive-like functionality, which is already
super rare), I believe it's the time now.

Thanks,
Qu

Re: [PATCH v2 2/7] btrfs-progs: libbtrfs: remove the support for fs without uuid tree

[David Sterba]

On Wed, May 03, 2023 at 02:03:38PM +0800, Qu Wenruo wrote:
> -#else
> -int subvol_uuid_search_init(int mnt_fd, struct subvol_uuid_search *s)
> -{
> -	s->mnt_fd = mnt_fd;
> -
> -	return 0;
> -}
> -
> -void subvol_uuid_search_finit(struct subvol_uuid_search *s)
> -{
> -}
> -#endif

This patch breaks 'make library-test', I haven't noticed until now. The
functions are exported in libbtrfs so can't be delted, only the first
part of the #ifdef.

[PATCH v2 3/7] btrfs-progs: crypto/blake2: remove blake2 simple API

[Qu Wenruo]

We never utilize such simple API, just remove it.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 crypto/blake2b-ref.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/crypto/blake2b-ref.c b/crypto/blake2b-ref.c
index eac4cf0c48da..f28dce3ae2a8 100644
--- a/crypto/blake2b-ref.c
+++ b/crypto/blake2b-ref.c
@@ -326,10 +326,6 @@ int blake2b( void *out, size_t outlen, const void *in, size_t inlen, const void
   return 0;
 }
 
-int blake2( void *out, size_t outlen, const void *in, size_t inlen, const void *key, size_t keylen ) {
-  return blake2b(out, outlen, in, inlen, key, keylen);
-}
-
 #if defined(SUPERCOP)
 int crypto_hash( unsigned char *out, unsigned char *in, unsigned long long inlen )
 {
-- 
2.39.2

Re: [PATCH v2 3/7] btrfs-progs: crypto/blake2: remove blake2 simple API

[Anand Jain]

On 03/05/2023 14:03, Qu Wenruo wrote:
> We never utilize such simple API, just remove it.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>




> ---
>   crypto/blake2b-ref.c | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/crypto/blake2b-ref.c b/crypto/blake2b-ref.c
> index eac4cf0c48da..f28dce3ae2a8 100644
> --- a/crypto/blake2b-ref.c
> +++ b/crypto/blake2b-ref.c
> @@ -326,10 +326,6 @@ int blake2b( void *out, size_t outlen, const void *in, size_t inlen, const void
>     return 0;
>   }
>   
> -int blake2( void *out, size_t outlen, const void *in, size_t inlen, const void *key, size_t keylen ) {
> -  return blake2b(out, outlen, in, inlen, key, keylen);
> -}
> -

It came from the ref implementation. With minimum changes. Maybe needed 
it for future sync? No?

-----
commit 3778ece7ff4114dc071667cf13a60c4ef1936576

     btrfs-progs: add blake2b reference implementation

     Upstream commit 997fa5ba1e14b52c554fb03ce39e579e6f27b90c,
     git repository: git://github.com/BLAKE2/BLAKE2
----

Thanks, Anand

>   #if defined(SUPERCOP)
>   int crypto_hash( unsigned char *out, unsigned char *in, unsigned long long inlen )
>   {

Re: [PATCH v2 3/7] btrfs-progs: crypto/blake2: remove blake2 simple API

[David Sterba]

On Thu, May 04, 2023 at 05:08:44PM +0800, Anand Jain wrote:
> On 03/05/2023 14:03, Qu Wenruo wrote:
> > We never utilize such simple API, just remove it.
> > 
> > Signed-off-by: Qu Wenruo <wqu@suse.com>
> 
> 
> 
> 
> > ---
> >   crypto/blake2b-ref.c | 4 ----
> >   1 file changed, 4 deletions(-)
> > 
> > diff --git a/crypto/blake2b-ref.c b/crypto/blake2b-ref.c
> > index eac4cf0c48da..f28dce3ae2a8 100644
> > --- a/crypto/blake2b-ref.c
> > +++ b/crypto/blake2b-ref.c
> > @@ -326,10 +326,6 @@ int blake2b( void *out, size_t outlen, const void *in, size_t inlen, const void
> >     return 0;
> >   }
> >   
> > -int blake2( void *out, size_t outlen, const void *in, size_t inlen, const void *key, size_t keylen ) {
> > -  return blake2b(out, outlen, in, inlen, key, keylen);
> > -}
> > -
> 
> It came from the ref implementation. With minimum changes. Maybe needed 
> it for future sync? No?

It's not needed anymore, the code sync can be done from any incremental git
commits, blake2 upstream isn't changing much.

Re: [PATCH v2 3/7] btrfs-progs: crypto/blake2: remove blake2 simple API

[Qu Wenruo]

On 2023/5/4 17:08, Anand Jain wrote:
> On 03/05/2023 14:03, Qu Wenruo wrote:
>> We never utilize such simple API, just remove it.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>
>
>
>
>> ---
>>   crypto/blake2b-ref.c | 4 ----
>>   1 file changed, 4 deletions(-)
>>
>> diff --git a/crypto/blake2b-ref.c b/crypto/blake2b-ref.c
>> index eac4cf0c48da..f28dce3ae2a8 100644
>> --- a/crypto/blake2b-ref.c
>> +++ b/crypto/blake2b-ref.c
>> @@ -326,10 +326,6 @@ int blake2b( void *out, size_t outlen, const void
>> *in, size_t inlen, const void
>>     return 0;
>>   }
>> -int blake2( void *out, size_t outlen, const void *in, size_t inlen,
>> const void *key, size_t keylen ) {
>> -  return blake2b(out, outlen, in, inlen, key, keylen);
>> -}
>> -
>
> It came from the ref implementation. With minimum changes. Maybe needed
> it for future sync? No?

No need, that function is only defining an easy to use interface for
callers who doesn't care the details but only want a fast hash algo.

In our case, we explicitly want to use blake2b, thus we don't need that.

Thanks,
Qu
>
> -----
> commit 3778ece7ff4114dc071667cf13a60c4ef1936576
>
>      btrfs-progs: add blake2b reference implementation
>
>      Upstream commit 997fa5ba1e14b52c554fb03ce39e579e6f27b90c,
>      git repository: git://github.com/BLAKE2/BLAKE2
> ----
>
> Thanks, Anand
>
>>   #if defined(SUPERCOP)
>>   int crypto_hash( unsigned char *out, unsigned char *in, unsigned
>> long long inlen )
>>   {
>

[PATCH v2 4/7] btrfs-progs: crypto/blake2: move optimized declarations to blake2b.h

[Qu Wenruo]

This is to avoid -Wmissing-prototypes warnings.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 crypto/blake2.h      | 5 +++++
 crypto/blake2b-ref.c | 4 ----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/crypto/blake2.h b/crypto/blake2.h
index cd89adb65a9b..052afe34a651 100644
--- a/crypto/blake2.h
+++ b/crypto/blake2.h
@@ -92,6 +92,11 @@ extern "C" {
 
   void blake2_init_accel(void);
 
+  /* Export optimized versions to silent -Wmissing-prototypes warnings. */
+  void blake2b_compress_avx2( blake2b_state *S, const uint8_t block[BLAKE2B_BLOCKBYTES] );
+  void blake2b_compress_sse2( blake2b_state *S, const uint8_t block[BLAKE2B_BLOCKBYTES] );
+  void blake2b_compress_sse41( blake2b_state *S, const uint8_t block[BLAKE2B_BLOCKBYTES] );
+
 #if defined(__cplusplus)
 }
 #endif
diff --git a/crypto/blake2b-ref.c b/crypto/blake2b-ref.c
index f28dce3ae2a8..489af399f945 100644
--- a/crypto/blake2b-ref.c
+++ b/crypto/blake2b-ref.c
@@ -220,10 +220,6 @@ static void blake2b_compress_ref( blake2b_state *S, const uint8_t block[BLAKE2B_
 #undef G
 #undef ROUND
 
-void blake2b_compress_sse2( blake2b_state *S, const uint8_t block[BLAKE2B_BLOCKBYTES] );
-void blake2b_compress_sse41( blake2b_state *S, const uint8_t block[BLAKE2B_BLOCKBYTES] );
-void blake2b_compress_avx2( blake2b_state *S, const uint8_t block[BLAKE2B_BLOCKBYTES] );
-
 static void (*blake2b_compress)( blake2b_state *S, const uint8_t block[BLAKE2B_BLOCKBYTES] ) = blake2b_compress_ref;
 
 void blake2_init_accel(void)
-- 
2.39.2

[PATCH v2 5/7] btrfs-progs: crypto/sha: declare the x86 optimized implementation

[Qu Wenruo]

The optimized implementation sha256_process_x86() is not declared
anywhere, this can be caught by -Wmissing-prototypes option.

Just declare it properly in sha.h.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 crypto/sha.h        | 3 +++
 crypto/sha256-x86.c | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/crypto/sha.h b/crypto/sha.h
index e65418ccd0d3..ea387c212dc6 100644
--- a/crypto/sha.h
+++ b/crypto/sha.h
@@ -211,4 +211,7 @@ extern int hmacResult(HMACContext *context,
 
 void sha256_init_accel(void);
 
+/* Export for optimized version to silent -Wmissing-prototypes. */
+void sha256_process_x86(uint32_t state[8], const uint8_t data[], uint32_t length);
+
 #endif /* _SHA_H_ */
diff --git a/crypto/sha256-x86.c b/crypto/sha256-x86.c
index 602c53cf4f60..57be3f0db38f 100644
--- a/crypto/sha256-x86.c
+++ b/crypto/sha256-x86.c
@@ -13,6 +13,8 @@
 # include <x86intrin.h>
 #endif
 
+#include "sha.h"
+
 /* Process multiple blocks. The caller is responsible for setting the initial */
 /*  state, and the caller is responsible for padding the final block.        */
 void sha256_process_x86(uint32_t state[8], const uint8_t data[], uint32_t length)
-- 
2.39.2

[PATCH v2 6/7] btrfs-progs: fix -Wmissing-prototypes warnings

[Qu Wenruo]

The fixes involve the following changes:

- Unexport functions which are not utilized out of the file
  * print_path_column()
  * parse_reflink_range()
  * btrfs_list_setup_print_column()
  * device_get_partition_size_sysfs()
  * max_zone_append_size()

- Include related headers before implementing the function
  * change-uuid.c
  * convert-bgt.c
  * seed.h

- Add missing headers caused by the above header changes
  * include <uuid/uuid.h> for tune/tune.h.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 cmds/qgroup.c         | 2 +-
 cmds/reflink.c        | 2 +-
 cmds/subvolume-list.c | 2 +-
 common/device-utils.c | 2 +-
 common/utils.c        | 2 +-
 kernel-shared/ulist.c | 2 +-
 kernel-shared/zoned.c | 2 +-
 tune/change-csum.c    | 1 +
 tune/change-uuid.c    | 1 +
 tune/convert-bgt.c    | 1 +
 tune/seeding.c        | 1 +
 tune/tune.h           | 2 ++
 12 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/cmds/qgroup.c b/cmds/qgroup.c
index 49014d5702ec..70a749aa4062 100644
--- a/cmds/qgroup.c
+++ b/cmds/qgroup.c
@@ -289,7 +289,7 @@ static void print_qgroup_column_add_blank(enum btrfs_qgroup_column_enum column,
 		printf(" ");
 }
 
-void print_path_column(struct btrfs_qgroup *qgroup)
+static void print_path_column(struct btrfs_qgroup *qgroup)
 {
 	struct btrfs_qgroup_list *list = NULL;
 
diff --git a/cmds/reflink.c b/cmds/reflink.c
index 24e562a744ff..14201349fc9f 100644
--- a/cmds/reflink.c
+++ b/cmds/reflink.c
@@ -58,7 +58,7 @@ struct reflink_range {
 	bool same_file;
 };
 
-void parse_reflink_range(const char *str, u64 *from, u64 *length, u64 *to)
+static void parse_reflink_range(const char *str, u64 *from, u64 *length, u64 *to)
 {
 	char tmp[512];
 	int i;
diff --git a/cmds/subvolume-list.c b/cmds/subvolume-list.c
index 750fc4e6354d..a667290c1585 100644
--- a/cmds/subvolume-list.c
+++ b/cmds/subvolume-list.c
@@ -280,7 +280,7 @@ static struct {
 static btrfs_list_filter_func all_filter_funcs[];
 static btrfs_list_comp_func all_comp_funcs[];
 
-void btrfs_list_setup_print_column(enum btrfs_list_column_enum column)
+static void btrfs_list_setup_print_column(enum btrfs_list_column_enum column)
 {
 	int i;
 
diff --git a/common/device-utils.c b/common/device-utils.c
index cb1a7a9d328a..70fbee49df1a 100644
--- a/common/device-utils.c
+++ b/common/device-utils.c
@@ -332,7 +332,7 @@ u64 device_get_partition_size_fd(int fd)
 	return result;
 }
 
-u64 device_get_partition_size_sysfs(const char *dev)
+static u64 device_get_partition_size_sysfs(const char *dev)
 {
 	int ret;
 	char path[PATH_MAX] = {};
diff --git a/common/utils.c b/common/utils.c
index 2c359dcf220f..3c67e1eb453c 100644
--- a/common/utils.c
+++ b/common/utils.c
@@ -498,7 +498,7 @@ static bool valid_escape(const char *str)
  * - line is advanced to the final separator or nul character
  * - returned path is a valid string terminated by zero or whitespace separator
  */
-char *read_path(char **line)
+static char *read_path(char **line)
 {
 	char *ret = *line;
 	char *out = *line;
diff --git a/kernel-shared/ulist.c b/kernel-shared/ulist.c
index e193b02d5f33..6f231a3d9eab 100644
--- a/kernel-shared/ulist.c
+++ b/kernel-shared/ulist.c
@@ -72,7 +72,7 @@ void ulist_init(struct ulist *ulist)
  * This is useful in cases where the base 'struct ulist' has been statically
  * allocated.
  */
-void ulist_release(struct ulist *ulist)
+static void ulist_release(struct ulist *ulist)
 {
 	struct ulist_node *node;
 	struct ulist_node *next;
diff --git a/kernel-shared/zoned.c b/kernel-shared/zoned.c
index 16abb042f5b0..d187c5763406 100644
--- a/kernel-shared/zoned.c
+++ b/kernel-shared/zoned.c
@@ -92,7 +92,7 @@ u64 zone_size(const char *file)
 	return strtoull((const char *)chunk, NULL, 10) << SECTOR_SHIFT;
 }
 
-u64 max_zone_append_size(const char *file)
+static u64 max_zone_append_size(const char *file)
 {
 	char chunk[32];
 	int ret;
diff --git a/tune/change-csum.c b/tune/change-csum.c
index a11316860e3c..4531f2190f06 100644
--- a/tune/change-csum.c
+++ b/tune/change-csum.c
@@ -24,6 +24,7 @@
 #include "kernel-shared/transaction.h"
 #include "common/messages.h"
 #include "common/internal.h"
+#include "tune/tune.h"
 
 static int change_tree_csum(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 			    int csum_type)
diff --git a/tune/change-uuid.c b/tune/change-uuid.c
index f148b9e54915..99ce0b15b2b9 100644
--- a/tune/change-uuid.c
+++ b/tune/change-uuid.c
@@ -25,6 +25,7 @@
 #include "kernel-shared/volumes.h"
 #include "common/defs.h"
 #include "common/messages.h"
+#include "tune/tune.h"
 #include "ioctl.h"
 
 static int change_fsid_prepare(struct btrfs_fs_info *fs_info, uuid_t new_fsid)
diff --git a/tune/convert-bgt.c b/tune/convert-bgt.c
index 27953cb26ada..c6bd4361f8ac 100644
--- a/tune/convert-bgt.c
+++ b/tune/convert-bgt.c
@@ -22,6 +22,7 @@
 #include "kernel-shared/transaction.h"
 #include "common/messages.h"
 #include "common/extent-cache.h"
+#include "tune/tune.h"
 
 /* After this many block groups we need to commit transaction. */
 #define BLOCK_GROUP_BATCH	64
diff --git a/tune/seeding.c b/tune/seeding.c
index 80b075e53baa..99ad455e9468 100644
--- a/tune/seeding.c
+++ b/tune/seeding.c
@@ -18,6 +18,7 @@
 #include "kernel-shared/ctree.h"
 #include "kernel-shared/transaction.h"
 #include "common/messages.h"
+#include "tune/tune.h"
 
 int update_seeding_flag(struct btrfs_root *root, const char *device, int set_flag, int force)
 {
diff --git a/tune/tune.h b/tune/tune.h
index 6beae9fc9ef7..753dc95eb138 100644
--- a/tune/tune.h
+++ b/tune/tune.h
@@ -17,6 +17,8 @@
 #ifndef __BTRFS_TUNE_H__
 #define __BTRFS_TUNE_H__
 
+#include <uuid/uuid.h>
+
 struct btrfs_root;
 struct btrfs_fs_info;
 
-- 
2.39.2

[PATCH v2 7/7] btrfs-progs: Makefile: enable -Wmissing-prototypes

[Qu Wenruo]

With all the known warnings fixes, we can enable -Wmissing-prototypes
and prevent such warnings from happening.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index b03367f26158..cdb3dee3ca4a 100644
--- a/Makefile
+++ b/Makefile
@@ -84,7 +84,8 @@ DISABLE_WARNING_FLAGS := $(call cc-disable-warning, format-truncation) \
 	$(call cc-disable-warning, address-of-packed-member)
 
 # Warnings that we want by default
-ENABLE_WARNING_FLAGS := $(call cc-option, -Wimplicit-fallthrough)
+ENABLE_WARNING_FLAGS := $(call cc-option, -Wimplicit-fallthrough) \
+			$(call cc-option, -Wmissing-prototypes)
 
 # Common build flags
 CFLAGS = $(SUBST_CFLAGS) \
-- 
2.39.2

Re: [PATCH v2 0/7] btrfs-progs: fix -Wmissing-prototypes warnings and enable that warning option

[David Sterba]

On Wed, May 03, 2023 at 02:03:36PM +0800, Qu Wenruo wrote:
> We have at least one case that some function is exported but never got
> utilized in the first place.
> 
> Let's prevent this problem from happening again by enable
> -Wmissing-prototypes to debug builds at least.
> 
> Fixes for  the existing warnings are split into several patches:
> 
> - Remove unused functions
>   Two patches, the first is to remove a function that never got
>   utilized from the introduction.
> 
>   The second is to remove a very old feature (only for <3.12 kernels)
>   in libbtrfs.
>   In fact this functionality for fs without an UUID tree is already
>   broken during previous cleanups.
>   (Need to export subvol_uuid_search_add() and
>    subvol_uuid_search_finit(), as it's callers' responsibility to
>    search for the target subvolume by themselves)
> 
>   And since no one is complaining ever since, there is really no need
>   to maintain such an old and deprecated feature in libbtrfs.
> 
> - Fixes for crypto related function
>   Two patches, one for each csum algo (blake2 and sha256).
>   Involves extra declarations in the headers.
> 
> - Trivial fixes
>   Mostly unexport and add needed headers.
> 
> Qu Wenruo (7):
>   btrfs-progs: remove function btrfs_check_allocatable_zones()
>   btrfs-progs: libbtrfs: remove the support for fs without uuid tree
>   btrfs-progs: crypto/blake2: remove blake2 simple API
>   btrfs-progs: crypto/blake2: move optimized declarations to blake2b.h
>   btrfs-progs: crypto/sha: declare the x86 optimized implementation
>   btrfs-progs: fix -Wmissing-prototypes warnings
>   btrfs-progs: Makefile: enable -Wmissing-prototypes

Added to devel, thanks.