From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from userp2130.oracle.com ([156.151.31.86]:38222 "EHLO userp2130.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730384AbeGTCRY (ORCPT ); Thu, 19 Jul 2018 22:17:24 -0400 Subject: Re: [PATCH 7/7] btrfs: add helper function check device delete able To: dsterba@suse.cz, linux-btrfs@vger.kernel.org References: <20180716145812.20836-1-anand.jain@oracle.com> <20180716145812.20836-8-anand.jain@oracle.com> <20180719114546.GH26141@twin.jikos.cz> From: Anand Jain Message-ID: <01235d29-80ba-25fa-2df3-f06585b88bc0@oracle.com> Date: Fri, 20 Jul 2018 09:34:56 +0800 MIME-Version: 1.0 In-Reply-To: <20180719114546.GH26141@twin.jikos.cz> Content-Type: text/plain; charset=utf-8; format=flowed Sender: linux-btrfs-owner@vger.kernel.org List-ID: 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 majordomo@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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >