On 2019/11/19 下午3:54, Qu Wenruo wrote:
> 
> 
> On 2019/11/19 下午3:40, Anand Jain wrote:
>>
>>
>> On 11/18/19 8:02 PM, Qu Wenruo wrote:
>>>
>>>
>>> On 2019/11/18 下午7:38, Anand Jain wrote:
>>>> On 18/11/19 3:05 PM, Qu Wenruo wrote:
>>>>> One user reported an use case where one device can't be replaced due to
>>>>> tiny device size difference.
>>>>>
>>>>> Since it's a RAID10 fs, if we go regular "remove missing" it can take a
>>>>> long time and even not be possible due to lack of space.
>>>>>
>>>>> So here we work around this situation by allowing user to shrink
>>>>> missing
>>>>> device.
>>>>> Then user can go shrink the device first, then replace it.
>>>>
>>>>
>>>>   Why not introduce --resize option in the replace command.
>>>>   Which shall allow replace command to resize the source-device
>>>>   to the size of the replace target-device.
>>>
>>> Nope, it won't work for degraded mount.
>>
>>  Umm.. Why?
> 
> Try it.
> 
> Mount a raid1 fs with missing devices, degraded,
> And then, try to resize the missing device, without this patch.
> 
> I should have made this point pretty clear in both the title and the
> commit message.
> 
> Thanks,
> Qu
> 

And just in case you didn't get the point again why this is important,
search the reporter's name in the mail list and check the thread.

And just in case you can't find it:
https://www.spinics.net/lists/linux-btrfs/msg94533.html

>>
>>  (IMHO reasoning adds clarity to the technical discussions. my 1c).
>>
>>> That's the root problem the patch is going to solve.
>>
>>  IMO this patch does not the solve the root of the problem and its
>>  approach is wrong.
>>
>>  The core problem as I see - currently we are determining the required
>>  size for the replace-target by means of source-disk size, instead it
>>  should be calculated by the source-disk space consumption.
>>  Also warn if target is smaller than source and fail if target is
>>  smaller than the consumption by the source-disk.
>>
>>  IMO changing the size of the missing device is point less. What if
>>  in between the resize and replace the missing device reappears
>>  in the following unmount and mount.
>>
>> Thanks, Anand
>>
>>> Thanks,
>>> Qu
>>>
>>>>
>>>> Thanks, Anand
>>>>
>>>>> Reported-by: Nathan Dehnel <ncdehnel@gmail.com>
>>>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>>>> ---
>>>>>    fs/btrfs/ioctl.c | 29 +++++++++++++++++++++++++----
>>>>>    1 file changed, 25 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>>>>> index de730e56d3f5..ebd2f40aca6f 100644
>>>>> --- a/fs/btrfs/ioctl.c
>>>>> +++ b/fs/btrfs/ioctl.c
>>>>> @@ -1604,6 +1604,7 @@ static noinline int btrfs_ioctl_resize(struct
>>>>> file *file,
>>>>>        char *sizestr;
>>>>>        char *retptr;
>>>>>        char *devstr = NULL;
>>>>> +    bool missing;
>>>>>        int ret = 0;
>>>>>        int mod = 0;
>>>>>    @@ -1651,7 +1652,10 @@ static noinline int btrfs_ioctl_resize(struct
>>>>> file *file,
>>>>>            goto out_free;
>>>>>        }
>>>>>    -    if (!test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state)) {
>>>>> +
>>>>> +    missing = test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state);
>>>>> +    if (!test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) &&
>>>>> +        !missing) {
>>>>>            btrfs_info(fs_info,
>>>>>                   "resizer unable to apply on readonly device %llu",
>>>>>                   devid);
>>>>> @@ -1659,13 +1663,24 @@ static noinline int btrfs_ioctl_resize(struct
>>>>> file *file,
>>>>>            goto out_free;
>>>>>        }
>>>>>    -    if (!strcmp(sizestr, "max"))
>>>>> +    if (!strcmp(sizestr, "max")) {
>>>>> +        if (missing) {
>>>>> +            btrfs_info(fs_info,
>>>>> +                "'max' can't be used for missing device %llu",
>>>>> +                   devid);
>>>>> +            ret = -EPERM;
>>>>> +            goto out_free;
>>>>> +        }
>>>>>            new_size = device->bdev->bd_inode->i_size;
>>>>> -    else {
>>>>> +    } else {
>>>>>            if (sizestr[0] == '-') {
>>>>>                mod = -1;
>>>>>                sizestr++;
>>>>>            } else if (sizestr[0] == '+') {
>>>>> +            if (missing)
>>>>> +                btrfs_info(fs_info,
>>>>> +                "'+size' can't be used for missing device %llu",
>>>>> +                       devid);
>>>>>                mod = 1;
>>>>>                sizestr++;
>>>>>            }
>>>>> @@ -1694,6 +1709,12 @@ static noinline int btrfs_ioctl_resize(struct
>>>>> file *file,
>>>>>                ret = -ERANGE;
>>>>>                goto out_free;
>>>>>            }
>>>>> +        if (missing) {
>>>>> +            ret = -EINVAL;
>>>>> +            btrfs_info(fs_info,
>>>>> +            "can not increase device size for missing device %llu",
>>>>> +                   devid);
>>>>> +        }
>>>>>            new_size = old_size + new_size;
>>>>>        }
>>>>>    @@ -1701,7 +1722,7 @@ static noinline int btrfs_ioctl_resize(struct
>>>>> file *file,
>>>>>            ret = -EINVAL;
>>>>>            goto out_free;
>>>>>        }
>>>>> -    if (new_size > device->bdev->bd_inode->i_size) {
>>>>> +    if (!missing && new_size > device->bdev->bd_inode->i_size) {
>>>>>            ret = -EFBIG;
>>>>>            goto out_free;
>>>>>        }
>>>>>
>>>>
>>>
>