[PATCH] btrfs: resize: Allow user to shrink missing device

[PATCH] btrfs: resize: Allow user to shrink missing device

[Qu Wenruo]

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.

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;
 	}
-- 
2.24.0

Re: [PATCH] btrfs: resize: Allow user to shrink missing device

[Anand Jain]

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.

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;
>   	}
> 

Re: [PATCH] btrfs: resize: Allow user to shrink missing device

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 4228 bytes --]



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.

That's the root problem the patch is going to solve.

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;
>>       }
>>
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] btrfs: resize: Allow user to shrink missing device

[Anand Jain]

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?

  (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;
>>>        }
>>>
>>
> 

Re: [PATCH] btrfs: resize: Allow user to shrink missing device

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 5614 bytes --]



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@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;
>>>>        }
>>>>
>>>
>>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]

Re: [PATCH] btrfs: resize: Allow user to shrink missing device

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 6060 bytes --]



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;
>>>>>        }
>>>>>
>>>>
>>>
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]

Re: [PATCH] btrfs: resize: Allow user to shrink missing device

[David Sterba]

On Tue, Nov 19, 2019 at 03:40:42PM +0800, Anand Jain wrote:
>   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.

The reappearing device is always tricky. If the device is missing from
the beginning, it'll exist in the fs_devices structure with MISSING bit
set. If it reappears, device_list_add will find it, update the path and
drop the missing bit.

From that point the device is regular but might miss some data updates.
There's a check for last generation of the device to pick the latest
one, but this applies only to mount.

Now when there's a replace in progress, and on a redundant profile, like
in the reported case, the device can be used as source device as long as
the data are not stale. This is detected by generation mismatch.

The resize of missing device happens on the in-memory structure as it is
represented in the fs_devices list, before the device reappears. And
after it's scanned again, the device item in memory is not changed, so
the size stays and is used until replace finishes.

Which is IMO all ok for the usecase, but the device states are tricky so
I could have missed something.

Re: [PATCH] btrfs: resize: Allow user to shrink missing device

[Anand Jain]

On 11/19/19 10:34 PM, David Sterba wrote:
> On Tue, Nov 19, 2019 at 03:40:42PM +0800, Anand Jain wrote:
>>    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.
> 
> The reappearing device is always tricky. If the device is missing from
> the beginning, it'll exist in the fs_devices structure with MISSING bit
> set. If it reappears, device_list_add will find it, update the path and
> drop the missing bit.

  Right.

>  From that point the device is regular but might miss some data updates.

  Um no its not regular yet. If the device has reappeared after mount it
  won't be part of the dev_alloc_list, so no new chunks are allocated
  on it. It needs patch [1] to be regular but as I said in other
  thread its better not to do that unless writehole is fixed.
   [1] [PATCH] btrfs: handle dynamically reappearing missing device

> There's a check for last generation of the device to pick the latest
> one, but this applies only to mount.

  This will work as long as there is only one umount mount sequence.
  A umount mount sequence of two times will make generation on all disks
  equal including device with missing some data. But that's fine..

> Now when there's a replace in progress, and on a redundant profile, like
> in the reported case, the device can be used as source device as long as
> the data are not stale. This is detected by generation mismatch.

  Right. Reading dev_items are safe as they are read from the tree.
  (Just off topic - but non-inline file data are not so lucky in this
  scenario as they are read off-tree and there is no header, ML patch:
  "[PATCH] fstests: btrfs test read from disks of different generation"
  reproduced it).

> The resize of missing device happens on the in-memory structure as it is
> represented in the fs_devices list, before the device reappears. And
> after it's scanned again, the device item in memory is not changed, so
> the size stays and is used until replace finishes.

  Right, I checked it, it looks safe now. Thanks for verifying it.

> Which is IMO all ok for the usecase, but the device states are tricky so
> I could have missed something.

  Missing device state was only relevant here.. I can't think of any
  other.

Thanks, Anand