Re: [PATCH 2/3 RFC] btrfs: total_devices should count replacing devices

[Anand Jain <Anand.Jain@oracle.com>]

Hello Wang,

  sure will do. Thanks for the comments.

Anand

On 13/05/14 17:17, Wang Shilong wrote:
> Hello Anand,
>
> I agree we can export @total_devices to fix 'btrfs file show' problem.
> This patch addressed two problem, it is better to split it into two
> patches.
>
> Could you please resend the patch, at least we should fix 'btrfs file show'
> problem firstly.
>
> Thanks,
> Wang
>
> On 03/07/2014 11:48 PM, Anand Jain wrote:
>> ioctl(BTRFS_IOC_FS_INFO) returns num_devices which does not
>> count seed device. num_devices is used to calculate
>> the number of slots during the ioctl(BTRFS_IOC_DEV_INFO)
>> but ioctl(BTRFS_IOC_DEV_INFO) would count seed devices as well.
>> Due to this miss match btrfs_progs get_fs_info() hits the bug..
>>              get_fs_info()
>>              ::
>>                      BUG_ON(ndevs >= fi_args->num_devices);
>>
>> what I notice is total_devices should be returned by the
>> ioctl(BTRFS_IOC_FS_INFO)), however total_devices does not
>> count (transient) replacing device but num_devices does.
>>
>> first, num_devices which appears to be a subset of
>> total_devices helps to count the number of devices
>> under a FSID which excludes the seed devices but
>> includes the transient replacing device.
>>
>> total_devices which includes seed devices is as of
>> now does not count the tansient replacing device,
>> which IMO is a bug or the implementation details
>> are not clear enough to state the role of num_devices
>> and total_devices.
>>
>> The user land on the otherhand would want to know
>> all the devices that would come out of probe
>> against a FSID
>>
>> As of now in this patch I am making the ioctl.c local
>> changes (instead of asking replace thread to update
>> total_devices) to  include replacing device.
>>
>> Signed-off-by: Anand Jain <Anand.Jain@oracle.com>
>> ---
>>   fs/btrfs/ioctl.c | 13 ++++++++++++-
>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index 5036f9d..ec7d5c6 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -2510,6 +2510,7 @@ static long btrfs_ioctl_fs_info(struct
>> btrfs_root *root, void __user *arg)
>>       struct btrfs_device *next;
>>       struct btrfs_fs_devices *fs_devices = root->fs_info->fs_devices;
>>       int ret = 0;
>> +    int dev_replace_running = 0;
>>       if (!capable(CAP_SYS_ADMIN))
>>           return -EPERM;
>> @@ -2518,8 +2519,18 @@ static long btrfs_ioctl_fs_info(struct
>> btrfs_root *root, void __user *arg)
>>       if (!fi_args)
>>           return -ENOMEM;
>> +    btrfs_dev_replace_lock(&root->fs_info->dev_replace);
>> +    if (btrfs_dev_replace_is_ongoing(&root->fs_info->dev_replace))
>> +        dev_replace_running = 1;
>> +
>> +    btrfs_dev_replace_unlock(&root->fs_info->dev_replace);
>> +
>>       mutex_lock(&fs_devices->device_list_mutex);
>> -    fi_args->num_devices = fs_devices->num_devices;
>> +    if (dev_replace_running)
>> +        fi_args->num_devices = fs_devices->total_devices + 1;
>> +    else
>> +        fi_args->num_devices = fs_devices->total_devices;
>> +
>>       memcpy(&fi_args->fsid, root->fs_info->fsid, sizeof(fi_args->fsid));
>>       list_for_each_entry_safe(device, next, &fs_devices->devices,
>> dev_list) {
>
> --
> 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