* [PATCH] btrfs: resize: Allow user to shrink missing device @ 2019-11-18 7:05 Qu Wenruo 2019-11-18 11:38 ` Anand Jain 0 siblings, 1 reply; 8+ messages in thread From: Qu Wenruo @ 2019-11-18 7:05 UTC (permalink / raw) To: linux-btrfs; +Cc: Nathan Dehnel 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: resize: Allow user to shrink missing device 2019-11-18 7:05 [PATCH] btrfs: resize: Allow user to shrink missing device Qu Wenruo @ 2019-11-18 11:38 ` Anand Jain 2019-11-18 12:02 ` Qu Wenruo 0 siblings, 1 reply; 8+ messages in thread From: Anand Jain @ 2019-11-18 11:38 UTC (permalink / raw) To: Qu Wenruo, linux-btrfs; +Cc: Nathan Dehnel 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; > } > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: resize: Allow user to shrink missing device 2019-11-18 11:38 ` Anand Jain @ 2019-11-18 12:02 ` Qu Wenruo 2019-11-19 7:40 ` Anand Jain 0 siblings, 1 reply; 8+ messages in thread From: Qu Wenruo @ 2019-11-18 12:02 UTC (permalink / raw) To: Anand Jain, Qu Wenruo, linux-btrfs; +Cc: Nathan Dehnel [-- 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 --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: resize: Allow user to shrink missing device 2019-11-18 12:02 ` Qu Wenruo @ 2019-11-19 7:40 ` Anand Jain 2019-11-19 7:54 ` Qu Wenruo 2019-11-19 14:34 ` David Sterba 0 siblings, 2 replies; 8+ messages in thread From: Anand Jain @ 2019-11-19 7:40 UTC (permalink / raw) To: Qu Wenruo, Qu Wenruo, linux-btrfs; +Cc: Nathan Dehnel 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; >>> } >>> >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: resize: Allow user to shrink missing device 2019-11-19 7:40 ` Anand Jain @ 2019-11-19 7:54 ` Qu Wenruo 2019-11-19 8:03 ` Qu Wenruo 2019-11-19 14:34 ` David Sterba 1 sibling, 1 reply; 8+ messages in thread From: Qu Wenruo @ 2019-11-19 7:54 UTC (permalink / raw) To: Anand Jain, Qu Wenruo, linux-btrfs; +Cc: Nathan Dehnel [-- 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 --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: resize: Allow user to shrink missing device 2019-11-19 7:54 ` Qu Wenruo @ 2019-11-19 8:03 ` Qu Wenruo 0 siblings, 0 replies; 8+ messages in thread From: Qu Wenruo @ 2019-11-19 8:03 UTC (permalink / raw) To: Anand Jain, Qu Wenruo, linux-btrfs; +Cc: Nathan Dehnel [-- 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 --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: resize: Allow user to shrink missing device 2019-11-19 7:40 ` Anand Jain 2019-11-19 7:54 ` Qu Wenruo @ 2019-11-19 14:34 ` David Sterba 2019-11-20 5:52 ` Anand Jain 1 sibling, 1 reply; 8+ messages in thread From: David Sterba @ 2019-11-19 14:34 UTC (permalink / raw) To: Anand Jain; +Cc: Qu Wenruo, Qu Wenruo, linux-btrfs, Nathan Dehnel 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: resize: Allow user to shrink missing device 2019-11-19 14:34 ` David Sterba @ 2019-11-20 5:52 ` Anand Jain 0 siblings, 0 replies; 8+ messages in thread From: Anand Jain @ 2019-11-20 5:52 UTC (permalink / raw) To: dsterba, Qu Wenruo, Qu Wenruo, linux-btrfs, Nathan Dehnel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-11-20 5:55 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-11-18 7:05 [PATCH] btrfs: resize: Allow user to shrink missing device Qu Wenruo 2019-11-18 11:38 ` Anand Jain 2019-11-18 12:02 ` Qu Wenruo 2019-11-19 7:40 ` Anand Jain 2019-11-19 7:54 ` Qu Wenruo 2019-11-19 8:03 ` Qu Wenruo 2019-11-19 14:34 ` David Sterba 2019-11-20 5:52 ` Anand Jain
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.