Re: [PATCH 7/7] btrfs: add helper function check device delete able
On 07/20/2018 09:34 AM, Anand Jain wrote: On 07/19/2018 07:45 PM, David Sterba wrote: On Mon, Jul 16, 2018 at 10:58:12PM +0800, Anand Jain wrote: Move the section of the code which performs the check if the device is indelible, move that into a helper function. Signed-off-by: Anand Jain --- v1->v2: Rename function to btrfs_get_device_for_delete(), thanks Nikolay. fs/btrfs/volumes.c | 49 ++--- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 1c0b56374992..0cefc24b028c 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -1859,6 +1859,33 @@ static inline u64 btrfs_num_devices(struct btrfs_fs_info *fs_info) return num_devices; } +static struct btrfs_device *btrfs_get_device_for_delete( + struct btrfs_fs_info *fs_info, + const char *device_path, u64 devid) +{ + int ret; + struct btrfs_device *device; + + ret = btrfs_check_raid_min_devices(fs_info, + btrfs_num_devices(fs_info) - 1); + if (ret) + return ERR_PTR(ret); + + ret = btrfs_find_device_by_devspec(fs_info, devid, device_path, + &device); + if (ret) + return ERR_PTR(ret); + + if (test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state)) + return ERR_PTR(BTRFS_ERROR_DEV_TGT_REPLACE); This is wrong, the BTRFS_ERROR valueas are >= 1, but the IS_ERR, ERR_PTR work for errno values -4095..0 . Thouth ERR_PTR would cast the integer into pointer, the callers of btrfs_get_device_for_delete will not detect the error and continue. Argh. Will fix. Pls ignore this patch. Thanks, Anand + + if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) && + fs_info->fs_devices->rw_devices == 1) + return ERR_PTR(BTRFS_ERROR_DEV_ONLY_WRITABLE); + + return device; +} + int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path, u64 devid) { @@ -1872,25 +1899,9 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path, mutex_lock(&uuid_mutex); - num_devices = btrfs_num_devices(fs_info); - - ret = btrfs_check_raid_min_devices(fs_info, num_devices - 1); - if (ret) - goto out; - - ret = btrfs_find_device_by_devspec(fs_info, devid, device_path, - &device); - if (ret) - goto out; - - if (test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state)) { - ret = BTRFS_ERROR_DEV_TGT_REPLACE; - goto out; - } - - if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) && - fs_info->fs_devices->rw_devices == 1) { - ret = BTRFS_ERROR_DEV_ONLY_WRITABLE; + device = btrfs_get_device_for_delete(fs_info, device_path, devid); + if (IS_ERR(device)) { BTRFS_ERROR_DEV_ONLY_WRITABLE won't work here. + ret = PTR_ERR(device); goto out; } -- 2.7.0 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] btrfs: add helper function check device delete able
On 07/19/2018 07:45 PM, David Sterba wrote: On Mon, Jul 16, 2018 at 10:58:12PM +0800, Anand Jain wrote: Move the section of the code which performs the check if the device is indelible, move that into a helper function. Signed-off-by: Anand Jain --- v1->v2: Rename function to btrfs_get_device_for_delete(), thanks Nikolay. fs/btrfs/volumes.c | 49 ++--- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 1c0b56374992..0cefc24b028c 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -1859,6 +1859,33 @@ static inline u64 btrfs_num_devices(struct btrfs_fs_info *fs_info) return num_devices; } +static struct btrfs_device *btrfs_get_device_for_delete( + struct btrfs_fs_info *fs_info, + const char *device_path, u64 devid) +{ + int ret; + struct btrfs_device *device; + + ret = btrfs_check_raid_min_devices(fs_info, + btrfs_num_devices(fs_info) - 1); + if (ret) + return ERR_PTR(ret); + + ret = btrfs_find_device_by_devspec(fs_info, devid, device_path, + &device); + if (ret) + return ERR_PTR(ret); + + if (test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state)) + return ERR_PTR(BTRFS_ERROR_DEV_TGT_REPLACE); This is wrong, the BTRFS_ERROR valueas are >= 1, but the IS_ERR, ERR_PTR work for errno values -4095..0 . Thouth ERR_PTR would cast the integer into pointer, the callers of btrfs_get_device_for_delete will not detect the error and continue. Argh. Will fix. Thanks, Anand + + if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) && + fs_info->fs_devices->rw_devices == 1) + return ERR_PTR(BTRFS_ERROR_DEV_ONLY_WRITABLE); + + return device; +} + int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path, u64 devid) { @@ -1872,25 +1899,9 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path, mutex_lock(&uuid_mutex); - num_devices = btrfs_num_devices(fs_info); - - ret = btrfs_check_raid_min_devices(fs_info, num_devices - 1); - if (ret) - goto out; - - ret = btrfs_find_device_by_devspec(fs_info, devid, device_path, - &device); - if (ret) - goto out; - - if (test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state)) { - ret = BTRFS_ERROR_DEV_TGT_REPLACE; - goto out; - } - - if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) && - fs_info->fs_devices->rw_devices == 1) { - ret = BTRFS_ERROR_DEV_ONLY_WRITABLE; + device = btrfs_get_device_for_delete(fs_info, device_path, devid); + if (IS_ERR(device)) { BTRFS_ERROR_DEV_ONLY_WRITABLE won't work here. + ret = PTR_ERR(device); goto out; } -- 2.7.0 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] btrfs: add helper function check device delete able
On Mon, Jul 16, 2018 at 10:58:12PM +0800, Anand Jain wrote: > Move the section of the code which performs the check if the device is > indelible, move that into a helper function. > > Signed-off-by: Anand Jain > --- > v1->v2: Rename function to btrfs_get_device_for_delete(), thanks > Nikolay. > > fs/btrfs/volumes.c | 49 ++--- > 1 file changed, 30 insertions(+), 19 deletions(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 1c0b56374992..0cefc24b028c 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -1859,6 +1859,33 @@ static inline u64 btrfs_num_devices(struct > btrfs_fs_info *fs_info) > return num_devices; > } > > +static struct btrfs_device *btrfs_get_device_for_delete( > + struct btrfs_fs_info *fs_info, > + const char *device_path, u64 devid) > +{ > + int ret; > + struct btrfs_device *device; > + > + ret = btrfs_check_raid_min_devices(fs_info, > +btrfs_num_devices(fs_info) - 1); > + if (ret) > + return ERR_PTR(ret); > + > + ret = btrfs_find_device_by_devspec(fs_info, devid, device_path, > +&device); > + if (ret) > + return ERR_PTR(ret); > + > + if (test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state)) > + return ERR_PTR(BTRFS_ERROR_DEV_TGT_REPLACE); This is wrong, the BTRFS_ERROR valueas are >= 1, but the IS_ERR, ERR_PTR work for errno values -4095..0 . Thouth ERR_PTR would cast the integer into pointer, the callers of btrfs_get_device_for_delete will not detect the error and continue. > + > + if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) && > + fs_info->fs_devices->rw_devices == 1) > + return ERR_PTR(BTRFS_ERROR_DEV_ONLY_WRITABLE); > + > + return device; > +} > + > int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path, > u64 devid) > { > @@ -1872,25 +1899,9 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, > const char *device_path, > > mutex_lock(&uuid_mutex); > > - num_devices = btrfs_num_devices(fs_info); > - > - ret = btrfs_check_raid_min_devices(fs_info, num_devices - 1); > - if (ret) > - goto out; > - > - ret = btrfs_find_device_by_devspec(fs_info, devid, device_path, > -&device); > - if (ret) > - goto out; > - > - if (test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state)) { > - ret = BTRFS_ERROR_DEV_TGT_REPLACE; > - goto out; > - } > - > - if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) && > - fs_info->fs_devices->rw_devices == 1) { > - ret = BTRFS_ERROR_DEV_ONLY_WRITABLE; > + device = btrfs_get_device_for_delete(fs_info, device_path, devid); > + if (IS_ERR(device)) { BTRFS_ERROR_DEV_ONLY_WRITABLE won't work here. > + ret = PTR_ERR(device); > goto out; > } > > -- > 2.7.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/7] btrfs: add helper function check device delete able
Move the section of the code which performs the check if the device is indelible, move that into a helper function. Signed-off-by: Anand Jain --- v1->v2: Rename function to btrfs_get_device_for_delete(), thanks Nikolay. fs/btrfs/volumes.c | 49 ++--- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 1c0b56374992..0cefc24b028c 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -1859,6 +1859,33 @@ static inline u64 btrfs_num_devices(struct btrfs_fs_info *fs_info) return num_devices; } +static struct btrfs_device *btrfs_get_device_for_delete( + struct btrfs_fs_info *fs_info, + const char *device_path, u64 devid) +{ + int ret; + struct btrfs_device *device; + + ret = btrfs_check_raid_min_devices(fs_info, + btrfs_num_devices(fs_info) - 1); + if (ret) + return ERR_PTR(ret); + + ret = btrfs_find_device_by_devspec(fs_info, devid, device_path, + &device); + if (ret) + return ERR_PTR(ret); + + if (test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state)) + return ERR_PTR(BTRFS_ERROR_DEV_TGT_REPLACE); + + if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) && + fs_info->fs_devices->rw_devices == 1) + return ERR_PTR(BTRFS_ERROR_DEV_ONLY_WRITABLE); + + return device; +} + int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path, u64 devid) { @@ -1872,25 +1899,9 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path, mutex_lock(&uuid_mutex); - num_devices = btrfs_num_devices(fs_info); - - ret = btrfs_check_raid_min_devices(fs_info, num_devices - 1); - if (ret) - goto out; - - ret = btrfs_find_device_by_devspec(fs_info, devid, device_path, - &device); - if (ret) - goto out; - - if (test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state)) { - ret = BTRFS_ERROR_DEV_TGT_REPLACE; - goto out; - } - - if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) && - fs_info->fs_devices->rw_devices == 1) { - ret = BTRFS_ERROR_DEV_ONLY_WRITABLE; + device = btrfs_get_device_for_delete(fs_info, device_path, devid); + if (IS_ERR(device)) { + ret = PTR_ERR(device); goto out; } -- 2.7.0 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html