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
>
> (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@xxxxxxxxx>
>>>> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
>>>> ---
>>>> 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;
>>>> }
>>>>
>>>
>>
Attachment:
signature.asc
Description: OpenPGP digital signature
