* [PATCH v1] btrfs-progs: chunk tree search solution for btrfs249
@ 2022-08-12 7:26 Flint.Wang
2022-08-12 7:57 ` Qu Wenruo
0 siblings, 1 reply; 2+ messages in thread
From: Flint.Wang @ 2022-08-12 7:26 UTC (permalink / raw)
To: quwenruo.btrfs; +Cc: linux-btrfs, dsterba, anand.jain, clm, hmsjwzb, strongbox8
Hi Qu,
Thanks for your proposal. I have implemented the first edition of chunk
tree search solution.
But it seems chunk tree search solution need root privilege.
Btrfs249 failed due to btrfs_ioctl_fs_info() return RW devices for
fi_args->num_devices. This patch search chunk tree to find no seed device.
Signed-off-by: Flint.Wang <hmsjwzb@zoho.com>
---
cmds/filesystem-usage.c | 89 +++++++++++++++++++++++++++++++++--------
cmds/filesystem-usage.h | 5 +++
common/utils.c | 11 +++--
common/utils.h | 2 +-
4 files changed, 85 insertions(+), 22 deletions(-)
diff --git a/cmds/filesystem-usage.c b/cmds/filesystem-usage.c
index 01729e18..ef022ae4 100644
--- a/cmds/filesystem-usage.c
+++ b/cmds/filesystem-usage.c
@@ -25,6 +25,7 @@
#include <getopt.h>
#include <fcntl.h>
#include <linux/limits.h>
+#include <uuid/uuid.h>
#include "common/utils.h"
#include "kerncompat.h"
@@ -689,6 +690,59 @@ out:
return ret;
}
+static int load_devid_uuid(int fd, struct devid_uuid *pair,
+ int ndev, u8 *noseed_fsid)
+{
+ struct btrfs_ioctl_search_args_v2 *args2;
+ struct btrfs_ioctl_search_key *sk;
+ struct btrfs_ioctl_search_header *sh;
+ struct btrfs_dev_item *dev_item;
+ int args2_size = 1024;
+ char args2_buf[args2_size];
+ int ret, i, num, noseed_dev = 0, pidx = 0;
+
+ args2 = (struct btrfs_ioctl_search_args_v2 *) args2_buf;
+ sk = &(args2->key);
+
+ sk->tree_id = BTRFS_CHUNK_TREE_OBJECTID;
+ sk->min_objectid = BTRFS_DEV_ITEMS_OBJECTID;
+ sk->max_objectid = BTRFS_DEV_ITEMS_OBJECTID;
+ sk->min_type = BTRFS_DEV_ITEM_KEY;
+ sk->max_type = BTRFS_DEV_ITEM_KEY;
+ sk->min_offset = 0;
+ sk->max_offset = (u64)-1;
+ sk->min_transid = 0;
+ sk->max_transid = (u64)-1;
+ sk->nr_items = -1;
+ args2->buf_size = args2_size - sizeof(struct btrfs_ioctl_search_args_v2);
+ ret = ioctl(fd, BTRFS_IOC_TREE_SEARCH_V2, args2);
+ if (ret != 0)
+ return -1;
+
+ sh = (struct btrfs_ioctl_search_header *) args2->buf;
+ num = sk->nr_items;
+
+ dev_item = (struct btrfs_dev_item *) (sh + 1);
+ for (i = 0; i < num; i++) {
+ if (!uuid_compare(dev_item->fsid, noseed_fsid)) {
+ noseed_dev += 1;
+ pair[pidx].devid = dev_item->devid;
+ memcpy(&pair[pidx++].uuid, dev_item->uuid, BTRFS_UUID_SIZE);
+ }
+ if (pidx > ndev) {
+ error("unexpected number of devices: %d >= %d", pidx, ndev);
+ return -1;
+ }
+ sh = (struct btrfs_ioctl_search_header *) dev_item + 1;
+ dev_item = (struct btrfs_dev_item *) sh + 1;
+ }
+
+ if (ndev != noseed_dev)
+ error("unexpected number of devices: %d != %d", ndev, noseed_dev);
+
+ return 0;
+}
+
/*
* This function loads the device_info structure and put them in an array
*/
@@ -699,6 +753,7 @@ static int load_device_info(int fd, struct device_info **device_info_ptr,
struct btrfs_ioctl_fs_info_args fi_args;
struct btrfs_ioctl_dev_info_args dev_info;
struct device_info *info;
+ struct devid_uuid *pair;
u8 fsid[BTRFS_UUID_SIZE];
*device_info_count = 0;
@@ -718,19 +773,23 @@ static int load_device_info(int fd, struct device_info **device_info_ptr,
return 1;
}
- for (i = 0, ndevs = 0 ; i <= fi_args.max_id ; i++) {
- if (ndevs >= fi_args.num_devices) {
- error("unexpected number of devices: %d >= %llu", ndevs,
- (unsigned long long)fi_args.num_devices);
- error(
- "if seed device is used, try running this command as root");
- goto out;
- }
+ pair = malloc(sizeof(struct devid_uuid) * fi_args.num_devices);
+ if (!pair) {
+ error("not enough memory");
+ return 1;
+ }
+
+ ret = load_devid_uuid(fd, pair, fi_args.num_devices, fi_args.fsid);
+ if (ret == -1)
+ goto out;
+
+ for (i = 0, ndevs = 0 ; i < fi_args.num_devices ; i++) {
memset(&dev_info, 0, sizeof(dev_info));
- ret = get_device_info(fd, i, &dev_info);
+ ret = get_device_info(fd, pair[i].devid, pair[i].uuid, &dev_info);
- if (ret == -ENODEV)
- continue;
+ if (ret == -ENODEV) {
+ error("device not found\n");
+ }
if (ret) {
error("cannot get info about device devid=%d", i);
goto out;
@@ -759,21 +818,17 @@ static int load_device_info(int fd, struct device_info **device_info_ptr,
++ndevs;
}
- if (ndevs != fi_args.num_devices) {
- error("unexpected number of devices: %d != %llu", ndevs,
- (unsigned long long)fi_args.num_devices);
- goto out;
- }
-
qsort(info, fi_args.num_devices,
sizeof(struct device_info), cmp_device_info);
*device_info_count = fi_args.num_devices;
*device_info_ptr = info;
+ free(pair);
return 0;
out:
+ free(pair);
free(info);
return ret;
}
diff --git a/cmds/filesystem-usage.h b/cmds/filesystem-usage.h
index cab38462..f78b2f2e 100644
--- a/cmds/filesystem-usage.h
+++ b/cmds/filesystem-usage.h
@@ -44,6 +44,11 @@ struct chunk_info {
u64 num_stripes;
};
+struct devid_uuid {
+ __le64 devid;
+ u8 uuid[BTRFS_UUID_SIZE];
+};
+
int load_chunk_and_device_info(int fd, struct chunk_info **chunkinfo,
int *chunkcount, struct device_info **devinfo, int *devcount);
void print_device_chunks(struct device_info *devinfo,
diff --git a/common/utils.c b/common/utils.c
index 1ed5571f..d09177ef 100644
--- a/common/utils.c
+++ b/common/utils.c
@@ -284,13 +284,16 @@ void btrfs_format_csum(u16 csum_type, const u8 *data, char *output)
}
}
-int get_device_info(int fd, u64 devid,
+int get_device_info(int fd, u64 devid, u8 *uuid,
struct btrfs_ioctl_dev_info_args *di_args)
{
int ret;
di_args->devid = devid;
- memset(&di_args->uuid, '\0', sizeof(di_args->uuid));
+ if (!uuid)
+ memset(&di_args->uuid, '\0', sizeof(di_args->uuid));
+ else
+ memcpy(&di_args->uuid, uuid, sizeof(di_args->uuid));
ret = ioctl(fd, BTRFS_IOC_DEV_INFO, di_args);
return ret < 0 ? -errno : 0;
@@ -498,7 +501,7 @@ int get_fs_info(const char *path, struct btrfs_ioctl_fs_info_args *fi_args,
* search_chunk_tree_for_fs_info() will lacks the devid 0
* so manual probe for it here.
*/
- ret = get_device_info(fd, 0, &tmp);
+ ret = get_device_info(fd, 0, NULL, &tmp);
if (!ret) {
fi_args->num_devices++;
ndevs++;
@@ -521,7 +524,7 @@ int get_fs_info(const char *path, struct btrfs_ioctl_fs_info_args *fi_args,
memcpy(di_args, &tmp, sizeof(tmp));
for (; last_devid <= fi_args->max_id && ndevs < fi_args->num_devices;
last_devid++) {
- ret = get_device_info(fd, last_devid, &di_args[ndevs]);
+ ret = get_device_info(fd, last_devid, NULL, &di_args[ndevs]);
if (ret == -ENODEV)
continue;
if (ret)
diff --git a/common/utils.h b/common/utils.h
index ea05fe5b..f1bbd807 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -67,7 +67,7 @@ int ask_user(const char *question);
int lookup_path_rootid(int fd, u64 *rootid);
int find_mount_fsroot(const char *subvol, const char *subvolid, char **mount);
int find_mount_root(const char *path, char **mount_root);
-int get_device_info(int fd, u64 devid,
+int get_device_info(int fd, u64 devid, u8 *uuid,
struct btrfs_ioctl_dev_info_args *di_args);
int get_df(int fd, struct btrfs_ioctl_space_args **sargs_ret);
--
2.37.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v1] btrfs-progs: chunk tree search solution for btrfs249
2022-08-12 7:26 [PATCH v1] btrfs-progs: chunk tree search solution for btrfs249 Flint.Wang
@ 2022-08-12 7:57 ` Qu Wenruo
0 siblings, 0 replies; 2+ messages in thread
From: Qu Wenruo @ 2022-08-12 7:57 UTC (permalink / raw)
To: Flint.Wang; +Cc: linux-btrfs, dsterba, anand.jain, clm, strongbox8
On 2022/8/12 15:26, Flint.Wang wrote:
> Hi Qu,
> Thanks for your proposal. I have implemented the first edition of chunk
> tree search solution.
> But it seems chunk tree search solution need root privilege.
Yes, and that's expected.
For a more elegant and less confusing solution, we either need to expand
the fs_info ioctl to return extra member indicating all devices, or go
sysfs to find out all devices.
Either solution needs certain kernel version, thus the tree search ioctl
based is needed as a fallback anyway.
>
> Btrfs249 failed due to btrfs_ioctl_fs_info() return RW devices for
> fi_args->num_devices. This patch search chunk tree to find no seed device.
I didn't understand the "to find no seed device" line.
Did you mean "to find any seed device" ?
>
> Signed-off-by: Flint.Wang <hmsjwzb@zoho.com>
> ---
> cmds/filesystem-usage.c | 89 +++++++++++++++++++++++++++++++++--------
> cmds/filesystem-usage.h | 5 +++
> common/utils.c | 11 +++--
> common/utils.h | 2 +-
> 4 files changed, 85 insertions(+), 22 deletions(-)
>
> diff --git a/cmds/filesystem-usage.c b/cmds/filesystem-usage.c
> index 01729e18..ef022ae4 100644
> --- a/cmds/filesystem-usage.c
> +++ b/cmds/filesystem-usage.c
> @@ -25,6 +25,7 @@
> #include <getopt.h>
> #include <fcntl.h>
> #include <linux/limits.h>
> +#include <uuid/uuid.h>
>
> #include "common/utils.h"
> #include "kerncompat.h"
> @@ -689,6 +690,59 @@ out:
> return ret;
> }
>
> +static int load_devid_uuid(int fd, struct devid_uuid *pair,
> + int ndev, u8 *noseed_fsid)
> +{
> + struct btrfs_ioctl_search_args_v2 *args2;
> + struct btrfs_ioctl_search_key *sk;
> + struct btrfs_ioctl_search_header *sh;
> + struct btrfs_dev_item *dev_item;
> + int args2_size = 1024;
> + char args2_buf[args2_size];
> + int ret, i, num, noseed_dev = 0, pidx = 0;
This doesn't look like the common style we go.
Normally we go one variable per line.
Another thing is, noseed_* related naming, can we just rename
@noseed_dev to @rw_devs, and @noseed_fsid to @fsid?
> +
> + args2 = (struct btrfs_ioctl_search_args_v2 *) args2_buf;
> + sk = &(args2->key);
> +
> + sk->tree_id = BTRFS_CHUNK_TREE_OBJECTID;
> + sk->min_objectid = BTRFS_DEV_ITEMS_OBJECTID;
> + sk->max_objectid = BTRFS_DEV_ITEMS_OBJECTID;
> + sk->min_type = BTRFS_DEV_ITEM_KEY;
> + sk->max_type = BTRFS_DEV_ITEM_KEY;
> + sk->min_offset = 0;
> + sk->max_offset = (u64)-1;
> + sk->min_transid = 0;
> + sk->max_transid = (u64)-1;
> + sk->nr_items = -1;
> + args2->buf_size = args2_size - sizeof(struct btrfs_ioctl_search_args_v2);
> + ret = ioctl(fd, BTRFS_IOC_TREE_SEARCH_V2, args2);
> + if (ret != 0)
> + return -1;
> +
> + sh = (struct btrfs_ioctl_search_header *) args2->buf;
> + num = sk->nr_items;
> +
> + dev_item = (struct btrfs_dev_item *) (sh + 1);
> + for (i = 0; i < num; i++) {
> + if (!uuid_compare(dev_item->fsid, noseed_fsid)) {
> + noseed_dev += 1;
> + pair[pidx].devid = dev_item->devid;
I don't think we need to bother a new struct devid_uuid, can't we just
reuse device_info structure?
Personally speaking, we can completely fill the device_info array inside
the function, we have total size, used bytes, devid, the only thing we
need from the dev_info ioctl is the path.
Which can be later handled by that for () loop inside load_device_info().
Thus to me, we can do the device_info_ptr allocation (and later
enlarge), fill devid/device_size/size members.
Then re-use the for loop just to grab the path.
> + memcpy(&pair[pidx++].uuid, dev_item->uuid, BTRFS_UUID_SIZE);
> + }
> + if (pidx > ndev) {
> + error("unexpected number of devices: %d >= %d", pidx, ndev);
> + return -1;
> + }
> + sh = (struct btrfs_ioctl_search_header *) dev_item + 1;
> + dev_item = (struct btrfs_dev_item *) sh + 1;
> + }
> +
> + if (ndev != noseed_dev)
> + error("unexpected number of devices: %d != %d", ndev, noseed_dev);
> +
> + return 0;
> +}
> +
> /*
> * This function loads the device_info structure and put them in an array
> */
> @@ -699,6 +753,7 @@ static int load_device_info(int fd, struct device_info **device_info_ptr,
> struct btrfs_ioctl_fs_info_args fi_args;
> struct btrfs_ioctl_dev_info_args dev_info;
> struct device_info *info;
> + struct devid_uuid *pair;
> u8 fsid[BTRFS_UUID_SIZE];
>
> *device_info_count = 0;
> @@ -718,19 +773,23 @@ static int load_device_info(int fd, struct device_info **device_info_ptr,
> return 1;
> }
>
> - for (i = 0, ndevs = 0 ; i <= fi_args.max_id ; i++) {
> - if (ndevs >= fi_args.num_devices) {
> - error("unexpected number of devices: %d >= %llu", ndevs,
> - (unsigned long long)fi_args.num_devices);
> - error(
> - "if seed device is used, try running this command as root");
> - goto out;
> - }
> + pair = malloc(sizeof(struct devid_uuid) * fi_args.num_devices);
> + if (!pair) {
> + error("not enough memory");
> + return 1;
> + }
> +
> + ret = load_devid_uuid(fd, pair, fi_args.num_devices, fi_args.fsid);
> + if (ret == -1)
> + goto out;
> +
> + for (i = 0, ndevs = 0 ; i < fi_args.num_devices ; i++) {
> memset(&dev_info, 0, sizeof(dev_info));
> - ret = get_device_info(fd, i, &dev_info);
> + ret = get_device_info(fd, pair[i].devid, pair[i].uuid, &dev_info);
>
> - if (ret == -ENODEV)
> - continue;
> + if (ret == -ENODEV) {
> + error("device not found\n");
> + }
> if (ret) {
> error("cannot get info about device devid=%d", i);
> goto out;
> @@ -759,21 +818,17 @@ static int load_device_info(int fd, struct device_info **device_info_ptr,
> ++ndevs;
> }
>
> - if (ndevs != fi_args.num_devices) {
> - error("unexpected number of devices: %d != %llu", ndevs,
> - (unsigned long long)fi_args.num_devices);
> - goto out;
> - }
> -
> qsort(info, fi_args.num_devices,
> sizeof(struct device_info), cmp_device_info);
>
> *device_info_count = fi_args.num_devices;
> *device_info_ptr = info;
> + free(pair);
>
> return 0;
>
> out:
> + free(pair);
> free(info);
> return ret;
> }
> diff --git a/cmds/filesystem-usage.h b/cmds/filesystem-usage.h
> index cab38462..f78b2f2e 100644
> --- a/cmds/filesystem-usage.h
> +++ b/cmds/filesystem-usage.h
> @@ -44,6 +44,11 @@ struct chunk_info {
> u64 num_stripes;
> };
>
> +struct devid_uuid {
> + __le64 devid;
> + u8 uuid[BTRFS_UUID_SIZE];
> +};
> +
> int load_chunk_and_device_info(int fd, struct chunk_info **chunkinfo,
> int *chunkcount, struct device_info **devinfo, int *devcount);
> void print_device_chunks(struct device_info *devinfo,
> diff --git a/common/utils.c b/common/utils.c
> index 1ed5571f..d09177ef 100644
> --- a/common/utils.c
> +++ b/common/utils.c
> @@ -284,13 +284,16 @@ void btrfs_format_csum(u16 csum_type, const u8 *data, char *output)
> }
> }
>
> -int get_device_info(int fd, u64 devid,
> +int get_device_info(int fd, u64 devid, u8 *uuid,
> struct btrfs_ioctl_dev_info_args *di_args)
> {
> int ret;
>
> di_args->devid = devid;
> - memset(&di_args->uuid, '\0', sizeof(di_args->uuid));
> + if (!uuid)
> + memset(&di_args->uuid, '\0', sizeof(di_args->uuid));
> + else
> + memcpy(&di_args->uuid, uuid, sizeof(di_args->uuid));
Any reason we need this new @uuid argument?
I know it's to make btrfs_ioctl_dev_info() to have the correct uuid to
do the search, but shouldn't the devid is enough to locate the seed device?
IIRC btrfs shouldn't have conflicting devid, even for seed devices.
Thus I doubt if we need the new @uuid arugment at all.
Thanks,
Qu
>
> ret = ioctl(fd, BTRFS_IOC_DEV_INFO, di_args);
> return ret < 0 ? -errno : 0;
> @@ -498,7 +501,7 @@ int get_fs_info(const char *path, struct btrfs_ioctl_fs_info_args *fi_args,
> * search_chunk_tree_for_fs_info() will lacks the devid 0
> * so manual probe for it here.
> */
> - ret = get_device_info(fd, 0, &tmp);
> + ret = get_device_info(fd, 0, NULL, &tmp);
> if (!ret) {
> fi_args->num_devices++;
> ndevs++;
> @@ -521,7 +524,7 @@ int get_fs_info(const char *path, struct btrfs_ioctl_fs_info_args *fi_args,
> memcpy(di_args, &tmp, sizeof(tmp));
> for (; last_devid <= fi_args->max_id && ndevs < fi_args->num_devices;
> last_devid++) {
> - ret = get_device_info(fd, last_devid, &di_args[ndevs]);
> + ret = get_device_info(fd, last_devid, NULL, &di_args[ndevs]);
> if (ret == -ENODEV)
> continue;
> if (ret)
> diff --git a/common/utils.h b/common/utils.h
> index ea05fe5b..f1bbd807 100644
> --- a/common/utils.h
> +++ b/common/utils.h
> @@ -67,7 +67,7 @@ int ask_user(const char *question);
> int lookup_path_rootid(int fd, u64 *rootid);
> int find_mount_fsroot(const char *subvol, const char *subvolid, char **mount);
> int find_mount_root(const char *path, char **mount_root);
> -int get_device_info(int fd, u64 devid,
> +int get_device_info(int fd, u64 devid, u8 *uuid,
> struct btrfs_ioctl_dev_info_args *di_args);
> int get_df(int fd, struct btrfs_ioctl_space_args **sargs_ret);
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-08-12 7:57 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-12 7:26 [PATCH v1] btrfs-progs: chunk tree search solution for btrfs249 Flint.Wang
2022-08-12 7:57 ` Qu Wenruo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox