On 7.11.18 г. 13:43 ч., Anand Jain wrote:
> In btrfs_dev_replace_cancel() we should check if the
> btrfs_scrub_cancel() is successful. If the btrfs_scrub_cancel() fails, return
> BTRFS_IOCTL_DEV_REPLACE_RESULT_NOT_STARTED so that user can try to
> cancel the replace again.
>
> Signed-off-by: Anand Jain <anand.jain@xxxxxxxxxx>
> ---
> fs/btrfs/dev-replace.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
> index 90c124edec76..c092ed559714 100644
> --- a/fs/btrfs/dev-replace.c
> +++ b/fs/btrfs/dev-replace.c
> @@ -799,18 +799,22 @@ int btrfs_dev_replace_cancel(struct btrfs_fs_info *fs_info)
> btrfs_dev_replace_write_unlock(dev_replace);
> break;
> case BTRFS_IOCTL_DEV_REPLACE_STATE_STARTED:
> - result = BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_ERROR;
> tgt_device = dev_replace->tgtdev;
> src_device = dev_replace->srcdev;
> btrfs_dev_replace_write_unlock(dev_replace);
> - btrfs_scrub_cancel(fs_info);
> - /*
> - * btrfs_dev_replace_finishing() will handle the cleanup part
> - */
> - btrfs_info_in_rcu(fs_info,
> - "dev_replace from %s (devid %llu) to %s canceled",
> - btrfs_dev_name(src_device), src_device->devid,
> - btrfs_dev_name(tgt_device));
> + ret = btrfs_scrub_cancel(fs_info);
> + if (ret) {
> + result = BTRFS_IOCTL_DEV_REPLACE_RESULT_NOT_STARTED;
> + } else {
> + result = BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_ERROR;
> + /*
> + * btrfs_dev_replace_finishing() will handle the cleanup part
> + */
> + btrfs_info_in_rcu(fs_info,
> + "dev_replace from %s (devid %llu) to %s canceled",
> + btrfs_dev_name(src_device), src_device->devid,
> + btrfs_dev_name(tgt_device));
This is identical to the btrfs_info_in_rcu several lines further down.
So if btrfs_scrub_cancel is successful you will print this messages
twice. Furthermore, there is already an unconditinal call to
btrfs_scrub_cancel. You are duplicating this function, this definitely
needs another look...
> + }
> break;
> case BTRFS_IOCTL_DEV_REPLACE_STATE_SUSPENDED:
> /*
>
