On 11/07/2018 08:15 PM, Nikolay Borisov wrote:


On 7.11.18 г. 13:43 ч., Anand Jain wrote:
We recast the replace return status
BTRFS_IOCTL_DEV_REPLACE_RESULT_SCRUB_INPROGRESS to 0, to indicate no
error.
And since BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_ERROR should also return 0,
which is also declared as 0, so we just return. Instead add it to the if
statement so that there is enough clarity while reading the code.

Signed-off-by: Anand Jain <anand.j...@oracle.com>
---
  fs/btrfs/dev-replace.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index cf3554554616..ca44998189c7 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -533,8 +533,9 @@ int btrfs_dev_replace_by_ioctl(struct btrfs_fs_info 
*fs_info,
                                        
args->start.cont_reading_from_srcdev_mode);
        args->result = ret;
        /* don't warn if EINPROGRESS, someone else might be running scrub */
-       if (ret == BTRFS_IOCTL_DEV_REPLACE_RESULT_SCRUB_INPROGRESS)
-               ret = 0;
+       if (ret == BTRFS_IOCTL_DEV_REPLACE_RESULT_SCRUB_INPROGRESS ||
+           ret == BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_ERROR)

BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_ERROR can only be returned from
btrfs_dev_replace_cancel,

 It can return 0 and which is also
 BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_ERROR which this patch makes it
 explicit.

 Looking at this again tells me that, as btrfs_dev_replace_start()
 is internal helper function, its better if it free from all usage
 of BTRFS_IOCTL_DEV_REPLACE_RESULT*.

Thanks, Anand

so what you are doing with checking ret ==
BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_ERROR is redundant. So using
BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_ERROR is at the very least misleading
here.

I suggest you drop this patch.

+               return 0;
return ret;
  }

Reply via email to