Re: [PATCH v2] btrfs-progs: chunk tree search solution for btrfs249

[hmsjwzb <hmsjwzb@zoho.com>]

Hi Qu,

Thanks for the notification.
I have updated the commit message.

Thanks,
Flint

On 12/9/22 01:24, Qu Wenruo wrote:
> 
> 
> On 2022/8/15 10:43, Flint.Wang wrote:
>> Hi Qu,
>>
>> Thanks for your comment. I fix the issue you suggest.
>> It is much clean now.
>>
>> Btrfs249 failed due to btrfs_ioctl_fs_info() return RW devices for fi_args->num_devices.
>> This patch search chunk tree to find rw devices.
>>
>> v2 change:
>> 1. code style fix.
>> 2. noseed_dev => rw_devs, noseed_fsid => fsid.
>> 3. remove redundant structure devid_uuid.
>> 4. reuse the dev_info structure.
>> 5. remove redundant uuid argument.
>>
>> Signed-off-by: Flint.Wang <hmsjwzb@zoho.com>
> 
> Any update on this patch? IIRC btrfs/249 still fails without this patch.
> 
> Although Anand Jain has a similar patchset, but his solution requires a new kernel sysfs, which can still cause compatibility problems.
> 
> Thus your patch is still the better one, but your commit message still needs some update.
> 
> Thanks,
> Qu
>> ---
>>   cmds/filesystem-usage.c | 83 ++++++++++++++++++++++++++++++++---------
>>   1 file changed, 66 insertions(+), 17 deletions(-)
>>
>> diff --git a/cmds/filesystem-usage.c b/cmds/filesystem-usage.c
>> index 01729e18..71f0e14c 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,62 @@ out:
>>       return ret;
>>   }
>>   +static int load_devid(int fd, struct device_info *info,
>> +                int ndev, u8 *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 = 0;
>> +    int i = 0;
>> +    int num = 0;
>> +    int rw_devs = 0;
>> +    int idx = 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, fsid)) {
>> +            rw_devs += 1;
>> +            info[idx++].devid = dev_item->devid;
>> +        }
>> +        if (idx > ndev) {
>> +            error("unexpected number of devices: %d >= %d", idx, ndev);
>> +            return -1;
>> +        }
>> +        sh = (struct btrfs_ioctl_search_header *) dev_item + 1;
>> +        dev_item = (struct btrfs_dev_item *) sh + 1;
>> +    }
>> +
>> +    if (ndev != rw_devs)
>> +        error("unexpected number of devices: %d != %d", ndev, rw_devs);
>> +
>> +    return 0;
>> +}
>> +
>>   /*
>>    *  This function loads the device_info structure and put them in an array
>>    */
>> @@ -718,19 +775,17 @@ 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;
>> -        }
>> +    ret = load_devid(fd, info, 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, info[i].devid, &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,12 +814,6 @@ 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);
>>