Re: [dm-devel] [RFC PATCH v2 2/2] dm: add CONFIG_DM_MULTIPATH_SG_IO - failover for SG_IO on dm-multipath

[Bart Van Assche <bvanassche@acm.org>]

On 4/29/21 8:50 AM, mwilck@suse.com wrote:
> +	if (hdr.dxfer_len > (queue_max_hw_sectors(bdev->bd_disk->queue) << 9))
> +		return -EIO;

How about using SECTOR_SHIFT instead of the number 9?

> +		/*
> +		 * Errors resulting from invalid parameters shouldn't be retried
> +		 * on another path.
> +		 */
> +		switch (rc) {
> +		case -ENOIOCTLCMD:
> +		case -EFAULT:
> +		case -EINVAL:
> +		case -EPERM:
> +			goto out;
> +		default:
> +			break;
> +		}

Will -ENOMEM result in an immediate retry? Is that what's desired?

> +		if (rhdr.info & SG_INFO_CHECK) {
> +			int result;
> +			blk_status_t sts;
> +
> +			__set_status_byte(&result, rhdr.status);
> +			__set_msg_byte(&result, rhdr.msg_status);
> +			__set_host_byte(&result, rhdr.host_status);
> +			__set_driver_byte(&result, rhdr.driver_status);
> +
> +			sts = __scsi_result_to_blk_status(&result, result);
> +			rhdr.host_status = host_byte(result);
> +
> +			/* See if this is a target or path error. */
> +			if (sts == BLK_STS_OK)
> +				rc = 0;
> +			else if (blk_path_error(sts))
> +				rc = -EIO;
> +			else {
> +				rc = blk_status_to_errno(sts);
> +				goto out;
> +			}
> +		}

Will SAM_STAT_CHECK_CONDITION be treated as an I/O error? Is that what's
desired? If not, does that mean that scsi_result_to_blk_status()
shouldn't be used but instead that a custom SCSI result conversion is
needed?

If __scsi_result_to_blk_status() is the right function to call, how
about making that function accept the driver status, host status, msg
and SAM status as four separate arguments such that the __set_*_byte()
calls can be left out?

> +			char *argv[2] = { "fail_path", bdbuf };

Can the above array be declared static?

Thanks,

Bart.