[RFC PATCH 0/5] scsi:scsi_debug:Add error injection for single lun

[RFC PATCH 0/5] scsi:scsi_debug:Add error injection for single lun

[Wenchao Hao]

The original error injection mechanism was based on scsi_host which
could not inject fault for a single SCSI device.

This patchset provides the ability to inject errors for a single
SCSI device. Now we supports inject timeout errors, queuecommand
errors, and hostbyte, driverbyte, statusbyte, and sense data for
specific SCSI Command

The first patch add an sysfs interface to add and inquiry single
device's error injection info; the second patch defined how to remove
an injection which has been added. The following 3 patches use the
injection info and generate the related error type.

Wenchao Hao (5):
  scsi:scsi_debug: Add sysfs interface to manager single devices' error inject
  scsi:scsi_debug: Add interface to remove injection which has been added
  scsi:scsi_debug: make command timeout if timeout error is injected
  scsi:scsi_debug: Return failed value for specific command's queuecommand
  scsi:scsi_debug: fail specific scsi command with result and sense data

 drivers/scsi/scsi_debug.c | 295 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 295 insertions(+)

-- 
2.35.3

[RFC PATCH 1/5] scsi:scsi_debug: Add sysfs interface to manager single devices' error inject

[Wenchao Hao]

Add interface "/sys/block/sdX/device/error_inject/error" which is
readable and writable. Write this file to add new error injection,
read it to get the error injection info which has been added.

You can add error inject to specific scsi device's specific scsi
command. And you can inject different error for one scsi command.
When multiple error injects are added to one scsi command, the one
with smaller error code would take effect.

The format is line-by-line integer separated by spaces. The first two
integer has a fixed meaning which are "error code" and "error count".

Each error code is an unsigned integer which is 1 bytes length. It
stands for which type of error to inject. Now I defined 3 error types:
  - 0: timeout specific SCSI command
  - 1: make queuecommand return specific value
  - 2: set scsi_cmd's status or sense data to specific value

Error cnt stands for how many times this inject rules would take effect.
a positive value means this rule would always take effect;
a negative value means this rule would take effect for the absolute
value of this error count;
0 means this rule would not take effect

Timeout specific SCSI command format

  +--------+------+-------------------------------------------------------+
  | Column | Type | Description                                           |
  +--------+------+-------------------------------------------------------+
  |   1    |  s32 | Error type, fixed to 0x0                              |
  +--------+------+-------------------------------------------------------+
  |   2    |  s32 | Error Count                                           |
  |        |      |  0: this rule would not take effect                   |
  |        |      |  positive: this rule would always take effect         |
  |        |      |  negative: take effect for the absolute of this value |
  +--------+------+-------------------------------------------------------+
  |   3    |  u8  | SCSI command this rule is for, 0xff for all command   |
  +--------+------+-------------------------------------------------------+

  the following command make sdb's inquiry timeout for 10 time:
    echo "0 -10 0x12" > /sys/block/sdb/device/error_inject/error
  the following command make sdb's inquiry timeout forever:
    echo "0 1 0x12" > /sys/block/sdb/device/error_inject/error

Make queuecommand return specific value

  +--------+------+-------------------------------------------------------+
  | Column | Type | Description                                           |
  +--------+------+-------------------------------------------------------+
  |   1    |  s32 | Error type, fixed to 0x1                              |
  +--------+------+-------------------------------------------------------+
  |   2    |  s32 | Error Count                                           |
  |        |      |  0: this rule would not take effect                   |
  |        |      |  positive: this rule would always take effect         |
  |        |      |  negative: take effect for the absolute of this value |
  +--------+------+-------------------------------------------------------+
  |   3    |  u8  | SCSI command this rule is for, 0xff for all command   |
  +--------+------+-------------------------------------------------------+
  |   4    |  s32 | The return value of queuecommand we desired           |
  +--------+------+-------------------------------------------------------+

  for example the following command make sdb's inquiry command return
  0x1055(SCSI_MLQUEUE_HOST_BUSY) forever:
    echo "1 1 0x12 0x1055" >/sys/block/sdb/device/error_inject/error

Set scsi_cmd's status and sense data to specific value

  +--------+------+-------------------------------------------------------+
  | Column | Type | Description                                           |
  +--------+------+-------------------------------------------------------+
  |   1    |  s32 | Error type, fixed to 0x2                              |
  +--------+------+-------------------------------------------------------+
  |   2    |  s32 | Error Count                                           |
  |        |      |  0: this rule would not take effect                   |
  |        |      |  positive: this rule would always take effect         |
  |        |      |  negative: take effect for the absolute of this value |
  +--------+------+-------------------------------------------------------+
  |   3    |  u8  | SCSI command this rule is for, 0xff for all command   |
  +--------+------+-------------------------------------------------------+
  |   4    |  u8  | Host byte in scsi_cmd's status                        |
  +--------+------+-------------------------------------------------------+
  |   5    |  u8  | Driver byte in scsi_cmd's status                      |
  +--------+------+-------------------------------------------------------+
  |   6    |  u8  | Status byte in scsi_cmd's status                      |
  +--------+------+-------------------------------------------------------+
  |   7    |  u8  | Sense Key of scsi_cmnd                                |
  +--------+------+-------------------------------------------------------+
  |   8    |  u8  | ASC of scsi_cmnd                                      |
  +--------+------+-------------------------------------------------------+
  |   9    |  u8  | ASQ of scsi_cmnd                                      |
  +--------+------+-------------------------------------------------------+

  for example the following means make sdb's read command return with
  media error UNC:
    error=/sys/block/sdb/device/error_inject/error
    echo "2 -10 0x88 0 0 0x2 0x3 0x11 0x0" >$error

Signed-off-by: Wenchao Hao <haowenchao@huawei.com>
---
 drivers/scsi/scsi_debug.c | 161 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 161 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 697fc57bc711..0cdc9599b628 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -280,6 +280,41 @@ struct sdeb_zone_state {	/* ZBC: per zone state */
 	sector_t z_wp;
 };
 
+enum sdebug_err_type {
+	ERR_TMOUT_CMD		= 0,	/* make specific scsi command timeout */
+	ERR_FAIL_QUEUE_CMD	= 1,	/* make specific scsi command's */
+					/* queuecmd return failed */
+	ERR_FAIL_CMD		= 2,	/* make specific scsi command's */
+					/* queuecmd return succeed but */
+					/* with errors set in scsi_cmnd */
+};
+
+struct sdebug_err_inject {
+	int type;
+	struct list_head list;
+	int cnt;
+	unsigned char cmd;
+
+	union {
+		/*
+		 * For ERR_FAIL_QUEUE_CMD
+		 */
+		int queuecmd_ret;
+
+		/*
+		 * For ERR_FAIL_CMD
+		 */
+		struct {
+			unsigned char host_byte;
+			unsigned char driver_byte;
+			unsigned char status_byte;
+			unsigned char sense_key;
+			unsigned char asc;
+			unsigned char asq;
+		};
+	};
+};
+
 struct sdebug_dev_info {
 	struct list_head dev_list;
 	unsigned int channel;
@@ -306,6 +341,8 @@ struct sdebug_dev_info {
 	unsigned int max_open;
 	ktime_t create_ts;	/* time since bootup that this device was created */
 	struct sdeb_zone_state *zstate;
+
+	struct list_head inject_err_list;
 };
 
 struct sdebug_host_info {
@@ -5150,6 +5187,7 @@ static struct sdebug_dev_info *sdebug_device_create(
 		devip->sdbg_host = sdbg_host;
 		devip->create_ts = ktime_get_boottime();
 		atomic_set(&devip->stopped, (sdeb_tur_ms_to_ready > 0 ? 2 : 0));
+		INIT_LIST_HEAD(&devip->inject_err_list);
 		list_add_tail(&devip->dev_list, &sdbg_host->dev_info_list);
 	}
 	return devip;
@@ -7597,6 +7635,128 @@ static int sdebug_blk_mq_poll(struct Scsi_Host *shost, unsigned int queue_num)
 	return num_entries;
 }
 
+static void sdebug_err_add(struct device *dev, struct sdebug_err_inject *new)
+{
+	struct scsi_device *sdev = to_scsi_device(dev);
+	struct sdebug_dev_info *devip = (struct sdebug_dev_info *)sdev->hostdata;
+	struct sdebug_err_inject *tmp, *err;
+
+	list_for_each_entry_safe(err, tmp, &devip->inject_err_list, list) {
+		if (err->type == new->type && err->cmd == new->cmd) {
+			sdev_printk(KERN_INFO, sdev, "Substituted %d 0x%x\n",
+				err->type, err->cmd);
+			list_del(&err->list);
+			kfree(err);
+		}
+	}
+
+	list_add_tail(&new->list, &devip->inject_err_list);
+}
+
+static ssize_t error_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	int tmp, ret = 0;
+	struct scsi_device *sdev = to_scsi_device(dev);
+	struct sdebug_dev_info *devip = (struct sdebug_dev_info *)sdev->hostdata;
+	struct sdebug_err_inject *err;
+
+	tmp = sysfs_emit_at(buf, ret, "Type\tCount\tCommand\n");
+	ret += tmp;
+
+	list_for_each_entry(err, &devip->inject_err_list, list) {
+		switch (err->type) {
+		case ERR_TMOUT_CMD:
+			tmp = sysfs_emit_at(buf, ret, "%d\t%d\t0x%x\n",
+				err->type, err->cnt, err->cmd);
+			ret += tmp;
+		break;
+
+		case ERR_FAIL_QUEUE_CMD:
+			tmp = sysfs_emit_at(buf, ret, "%d\t%d\t0x%x\t0x%x\n",
+				err->type, err->cnt, err->cmd, err->queuecmd_ret);
+			ret += tmp;
+		break;
+
+		case ERR_FAIL_CMD:
+			tmp = sysfs_emit_at(buf, ret,
+				"%d\t%d\t0x%x\t0x%x 0x%x 0x%x 0x%x 0x%x 0x%x\n",
+				err->type, err->cnt, err->cmd,
+				err->host_byte, err->driver_byte,
+				err->status_byte, err->sense_key,
+				err->asc, err->asq);
+			ret += tmp;
+		break;
+		}
+	}
+
+	return ret;
+}
+static ssize_t error_store(struct device *dev, struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	unsigned int inject_type;
+	struct sdebug_err_inject *inject;
+
+	if (sscanf(buf, "%d", &inject_type) != 1)
+		return -EINVAL;
+
+	inject = kzalloc(sizeof(struct sdebug_err_inject), GFP_KERNEL);
+	if (!inject)
+		return -ENOMEM;
+
+	switch (inject_type) {
+	case ERR_TMOUT_CMD:
+		if (sscanf(buf, "%d %d %hhx", &inject->type, &inject->cnt,
+			   &inject->cmd) != 3)
+			goto out_error;
+	break;
+
+	case ERR_FAIL_QUEUE_CMD:
+		if (sscanf(buf, "%d %d %hhx %x", &inject->type, &inject->cnt,
+			   &inject->cmd, &inject->queuecmd_ret) != 4)
+			goto out_error;
+	break;
+
+	case ERR_FAIL_CMD:
+		if (sscanf(buf, "%d %d %hhx %hhx %hhx %hhx %hhx %hhx %hhx",
+			   &inject->type, &inject->cnt, &inject->cmd,
+			   &inject->host_byte, &inject->driver_byte,
+			   &inject->status_byte, &inject->sense_key,
+			   &inject->asc, &inject->asq) != 9)
+			goto out_error;
+	break;
+
+	default:
+		goto out_error;
+	break;
+	}
+
+	sdebug_err_add(dev, inject);
+
+	return count;
+
+out_error:
+	kfree(inject);
+	return -EINVAL;
+}
+static DEVICE_ATTR_RW(error);
+
+static struct attribute *sdebug_sdev_attrs[] = {
+	&dev_attr_error.attr,
+	NULL
+};
+
+static const struct attribute_group sdebug_sdev_attr_group = {
+	.name = "error_inject",
+	.attrs = sdebug_sdev_attrs,
+};
+
+const struct attribute_group *sdebug_sdev_groups[] = {
+	&sdebug_sdev_attr_group,
+	NULL
+};
+
 static int scsi_debug_queuecommand(struct Scsi_Host *shost,
 				   struct scsi_cmnd *scp)
 {
@@ -7776,6 +7936,7 @@ static struct scsi_host_template sdebug_driver_template = {
 	.ioctl =		scsi_debug_ioctl,
 	.queuecommand =		scsi_debug_queuecommand,
 	.change_queue_depth =	sdebug_change_qdepth,
+	.sdev_groups =		sdebug_sdev_groups,
 	.map_queues =		sdebug_map_queues,
 	.mq_poll =		sdebug_blk_mq_poll,
 	.eh_abort_handler =	scsi_debug_abort,
-- 
2.35.3

[RFC PATCH 2/5] scsi:scsi_debug: Add interface to remove injection which has been added

[Wenchao Hao]

The removal interface is still "/sys/block/sdX/device/error_inect/error".
The format is still line-by-line integer separated by spaces with fixed 3
column.
  first column is "-", which tells this is a removal operation;
  second column is error code;
  third column is the scsi command.

For example the following command would remove timeout injection of
inquiry command.

  echo "- 0 0x12" > /sys/block/sdb/device/error_inect/error

Signed-off-by: Wenchao Hao <haowenchao@huawei.com>
---
 drivers/scsi/scsi_debug.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 0cdc9599b628..06e3150812fa 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -7653,6 +7653,30 @@ static void sdebug_err_add(struct device *dev, struct sdebug_err_inject *new)
 	list_add_tail(&new->list, &devip->inject_err_list);
 }
 
+static int sdebug_err_remove(struct device *dev, const char *buf, size_t count)
+{
+	struct scsi_device *sdev = to_scsi_device(dev);
+	struct sdebug_dev_info *devip = (struct sdebug_dev_info *)sdev->hostdata;
+	struct sdebug_err_inject *tmp, *err;
+	int type;
+	unsigned char cmd;
+
+	if (sscanf(buf, "- %d %hhx", &type, &cmd) != 2)
+		return -EINVAL;
+
+	list_for_each_entry_safe(err, tmp, &devip->inject_err_list, list) {
+		if (err->type == type && err->cmd == cmd) {
+			sdev_printk(KERN_INFO, sdev, "Remove %d 0x%x\n",
+				err->type, err->cmd);
+			list_del(&err->list);
+			kfree(err);
+			return count;
+		}
+	}
+
+	return -EINVAL;
+}
+
 static ssize_t error_show(struct device *dev, struct device_attribute *attr,
 		char *buf)
 {
@@ -7698,6 +7722,9 @@ static ssize_t error_store(struct device *dev, struct device_attribute *attr,
 	unsigned int inject_type;
 	struct sdebug_err_inject *inject;
 
+	if (buf[0] == '-')
+		return sdebug_err_remove(dev, buf, count);
+
 	if (sscanf(buf, "%d", &inject_type) != 1)
 		return -EINVAL;
 
-- 
2.35.3

[RFC PATCH 3/5] scsi:scsi_debug: make command timeout if timeout error is injected

[Wenchao Hao]

If a timeout error is injected for specific scsi command, just return
0 in queuecommand to make it timeout.

For example, the following command would inject timeout error for
inquiry(0x12) command for 10 times:
  echo "0 -10 0x12" >/sys/block/sdb/device/error_inject/error

Signed-off-by: Wenchao Hao <haowenchao@huawei.com>
---
 drivers/scsi/scsi_debug.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 06e3150812fa..761e1e3bcb9a 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -7784,6 +7784,30 @@ const struct attribute_group *sdebug_sdev_groups[] = {
 	NULL
 };
 
+static int sdebug_timeout_cmd(struct scsi_cmnd *cmnd)
+{
+	struct scsi_device *sdp = cmnd->device;
+	struct sdebug_dev_info *devip = (struct sdebug_dev_info *)sdp->hostdata;
+	struct sdebug_err_inject *err;
+	unsigned char *cmd = cmnd->cmnd;
+	int ret = 0;
+
+	if (devip == NULL)
+		return 0;
+
+	list_for_each_entry(err, &devip->inject_err_list, list) {
+		if (err->type == ERR_TMOUT_CMD &&
+		    (err->cmd == cmd[0] || err->cmd == 0xff)) {
+			ret = !!err->cnt;
+			if (err->cnt < 0)
+				err->cnt++;
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
 static int scsi_debug_queuecommand(struct Scsi_Host *shost,
 				   struct scsi_cmnd *scp)
 {
@@ -7842,6 +7866,12 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
 		if (NULL == devip)
 			goto err_out;
 	}
+
+	if (sdebug_timeout_cmd(scp)) {
+		scmd_printk(KERN_INFO, scp, "timeout command 0x%x\n", opcode);
+		return 0;
+	}
+
 	if (unlikely(inject_now && !atomic_read(&sdeb_inject_pending)))
 		atomic_set(&sdeb_inject_pending, 1);
 
-- 
2.35.3

[RFC PATCH 4/5] scsi:scsi_debug: Return failed value for specific command's queuecommand

[Wenchao Hao]

If a fail queuecommand error is injected for specific scsi command, just
return the failed value rejected in queuecommand.

We can make any scsi command's queuecommand return the value we desired,
for example make it return SCSI_MLQUEUE_HOST_BUSY.

For example the following command would make all inquiry(0x12) command's
queuecommand return 0x1055:

  echo "1 1 0x12 0x1055" >/sys/block/sdb/device/error_inject/error

Signed-off-by: Wenchao Hao <haowenchao@huawei.com>
---
 drivers/scsi/scsi_debug.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 761e1e3bcb9a..217a9e892391 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -7808,6 +7808,30 @@ static int sdebug_timeout_cmd(struct scsi_cmnd *cmnd)
 	return 0;
 }
 
+static int sdebug_fail_queue_cmd(struct scsi_cmnd *cmnd)
+{
+	struct scsi_device *sdp = cmnd->device;
+	struct sdebug_dev_info *devip = (struct sdebug_dev_info *)sdp->hostdata;
+	struct sdebug_err_inject *err;
+	unsigned char *cmd = cmnd->cmnd;
+	int ret = 0;
+
+	if (devip == NULL)
+		return 0;
+
+	list_for_each_entry(err, &devip->inject_err_list, list) {
+		if (err->type == ERR_FAIL_QUEUE_CMD &&
+		    (err->cmd == cmd[0] || err->cmd == 0xff)) {
+			ret = err->cnt ? err->queuecmd_ret : 0;
+			if (err->cnt < 0)
+				err->cnt++;
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
 static int scsi_debug_queuecommand(struct Scsi_Host *shost,
 				   struct scsi_cmnd *scp)
 {
@@ -7827,6 +7851,7 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
 	u8 opcode = cmd[0];
 	bool has_wlun_rl;
 	bool inject_now;
+	int ret = 0;
 
 	scsi_set_resid(scp, 0);
 	if (sdebug_statistics) {
@@ -7872,6 +7897,13 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
 		return 0;
 	}
 
+	ret = sdebug_fail_queue_cmd(scp);
+	if (ret) {
+		scmd_printk(KERN_INFO, scp, "fail queue command 0x%x with 0x%x\n",
+				opcode, ret);
+		return ret;
+	}
+
 	if (unlikely(inject_now && !atomic_read(&sdeb_inject_pending)))
 		atomic_set(&sdeb_inject_pending, 1);
 
-- 
2.35.3

[RFC PATCH 5/5] scsi:scsi_debug: fail specific scsi command with result and sense data

[Wenchao Hao]

If a fail commnd error is injected for specific scsi command, set the
scsi command's status and sense data, then finish this scsi command.

For example, the following command would make read(0x88) command finished
with UNC for 8 times:

  error=/sys/block/sdb/device/error_inject/error
  echo "2 -8 0x88 0 0 0x2 0x3 0x11 0x0" >$error

Signed-off-by: Wenchao Hao <haowenchao@huawei.com>
---
 drivers/scsi/scsi_debug.c | 46 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 217a9e892391..06fe2914117f 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -7832,6 +7832,41 @@ static int sdebug_fail_queue_cmd(struct scsi_cmnd *cmnd)
 	return 0;
 }
 
+static int sdebug_fail_cmd(struct scsi_cmnd *cmnd, int *retval,
+			   struct sdebug_err_inject *info)
+{
+	struct scsi_device *sdp = cmnd->device;
+	struct sdebug_dev_info *devip = (struct sdebug_dev_info *)sdp->hostdata;
+	struct sdebug_err_inject *err;
+	unsigned char *cmd = cmnd->cmnd;
+	int ret = 0;
+	int result;
+
+	if (devip == NULL)
+		return 0;
+
+	list_for_each_entry(err, &devip->inject_err_list, list) {
+		if (err->type == ERR_FAIL_CMD &&
+		    (err->cmd == cmd[0] || err->cmd == 0xff)) {
+			if (!err->cnt)
+				return 0;
+			ret = !!err->cnt;
+			goto out_handle;
+		}
+	}
+	return 0;
+
+out_handle:
+	if (err->cnt < 0)
+		err->cnt++;
+	mk_sense_buffer(cmnd, err->sense_key, err->asc, err->asq);
+	result = err->status_byte | err->host_byte << 16 | err->driver_byte << 24;
+	*info = *err;
+	*retval = schedule_resp(cmnd, devip, result, NULL, 0, 0);
+
+	return ret;
+}
+
 static int scsi_debug_queuecommand(struct Scsi_Host *shost,
 				   struct scsi_cmnd *scp)
 {
@@ -7852,6 +7887,7 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
 	bool has_wlun_rl;
 	bool inject_now;
 	int ret = 0;
+	struct sdebug_err_inject err;
 
 	scsi_set_resid(scp, 0);
 	if (sdebug_statistics) {
@@ -7904,6 +7940,16 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
 		return ret;
 	}
 
+	if (sdebug_fail_cmd(scp, &ret, &err)) {
+		scmd_printk(KERN_INFO, scp,
+			"fail command 0x%x with hostbyte=0x%x, "
+			"driverbyte=0x%x, statusbyte=0x%x, "
+			"sense_key=0x%x, asc=0x%x, asq=0x%x\n",
+			opcode, err.host_byte, err.driver_byte,
+			err.status_byte, err.sense_key, err.asc, err.asq);
+		return ret;
+	}
+
 	if (unlikely(inject_now && !atomic_read(&sdeb_inject_pending)))
 		atomic_set(&sdeb_inject_pending, 1);
 
-- 
2.35.3

Re: [RFC PATCH 0/5] scsi:scsi_debug:Add error injection for single lun

[Douglas Gilbert]

On 2022-11-09 10:59, Wenchao Hao wrote:
> The original error injection mechanism was based on scsi_host which
> could not inject fault for a single SCSI device.
> 
> This patchset provides the ability to inject errors for a single
> SCSI device. Now we supports inject timeout errors, queuecommand
> errors, and hostbyte, driverbyte, statusbyte, and sense data for
> specific SCSI Command
> 
> The first patch add an sysfs interface to add and inquiry single
> device's error injection info; the second patch defined how to remove
> an injection which has been added. The following 3 patches use the
> injection info and generate the related error type.
> 
> Wenchao Hao (5):
>    scsi:scsi_debug: Add sysfs interface to manager single devices' error inject
>    scsi:scsi_debug: Add interface to remove injection which has been added
>    scsi:scsi_debug: make command timeout if timeout error is injected
>    scsi:scsi_debug: Return failed value for specific command's queuecommand
>    scsi:scsi_debug: fail specific scsi command with result and sense data
> 
>   drivers/scsi/scsi_debug.c | 295 ++++++++++++++++++++++++++++++++++++++
>   1 file changed, 295 insertions(+)

Hi,
This patchset seems to assume all scsi_debug devices will be disk (-like) SCSI
devices. That leaves out other device types: tapes, enclosures, WLUNs, etc.

Have you considered putting these device specific additions under:
    /sys/class/scsi_device/<hctl>/device/error_inject/
instead of
    /sys/block/sdb/device/error_inject/

?

Doug Gilbert

Re: [RFC PATCH 0/5] scsi:scsi_debug:Add error injection for single lun

[Damien Le Moal]

On 11/14/22 06:48, Douglas Gilbert wrote:
> On 2022-11-09 10:59, Wenchao Hao wrote:
>> The original error injection mechanism was based on scsi_host which
>> could not inject fault for a single SCSI device.
>>
>> This patchset provides the ability to inject errors for a single
>> SCSI device. Now we supports inject timeout errors, queuecommand
>> errors, and hostbyte, driverbyte, statusbyte, and sense data for
>> specific SCSI Command
>>
>> The first patch add an sysfs interface to add and inquiry single
>> device's error injection info; the second patch defined how to remove
>> an injection which has been added. The following 3 patches use the
>> injection info and generate the related error type.
>>
>> Wenchao Hao (5):
>>    scsi:scsi_debug: Add sysfs interface to manager single devices' error inject
>>    scsi:scsi_debug: Add interface to remove injection which has been added
>>    scsi:scsi_debug: make command timeout if timeout error is injected
>>    scsi:scsi_debug: Return failed value for specific command's queuecommand
>>    scsi:scsi_debug: fail specific scsi command with result and sense data
>>
>>   drivers/scsi/scsi_debug.c | 295 ++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 295 insertions(+)
> 
> Hi,
> This patchset seems to assume all scsi_debug devices will be disk (-like) SCSI
> devices. That leaves out other device types: tapes, enclosures, WLUNs, etc.
> 
> Have you considered putting these device specific additions under:
>     /sys/class/scsi_device/<hctl>/device/error_inject/
> instead of
>     /sys/block/sdb/device/error_inject/

But these are the same, no ?
At least on my Fedora box, I see:

/sys/block/sdp/device/ being
/sys/devices/pseudo_0/adapter0/host19/target19:0:0/19:0:0:0

and /sys/class/scsi_device/19:0:0:0/device being
/sys/devices/pseudo_0/adapter0/host19/target19:0:0/19:0:0:0

> 
> ?
> 
> Doug Gilbert
> 
> 
> 

-- 
Damien Le Moal
Western Digital Research

Re: [RFC PATCH 0/5] scsi:scsi_debug:Add error injection for single lun

[Douglas Gilbert]

On 2022-11-13 19:30, Damien Le Moal wrote:
> On 11/14/22 06:48, Douglas Gilbert wrote:
>> On 2022-11-09 10:59, Wenchao Hao wrote:
>>> The original error injection mechanism was based on scsi_host which
>>> could not inject fault for a single SCSI device.
>>>
>>> This patchset provides the ability to inject errors for a single
>>> SCSI device. Now we supports inject timeout errors, queuecommand
>>> errors, and hostbyte, driverbyte, statusbyte, and sense data for
>>> specific SCSI Command
>>>
>>> The first patch add an sysfs interface to add and inquiry single
>>> device's error injection info; the second patch defined how to remove
>>> an injection which has been added. The following 3 patches use the
>>> injection info and generate the related error type.
>>>
>>> Wenchao Hao (5):
>>>     scsi:scsi_debug: Add sysfs interface to manager single devices' error inject
>>>     scsi:scsi_debug: Add interface to remove injection which has been added
>>>     scsi:scsi_debug: make command timeout if timeout error is injected
>>>     scsi:scsi_debug: Return failed value for specific command's queuecommand
>>>     scsi:scsi_debug: fail specific scsi command with result and sense data
>>>
>>>    drivers/scsi/scsi_debug.c | 295 ++++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 295 insertions(+)
>>
>> Hi,
>> This patchset seems to assume all scsi_debug devices will be disk (-like) SCSI
>> devices. That leaves out other device types: tapes, enclosures, WLUNs, etc.
>>
>> Have you considered putting these device specific additions under:
>>      /sys/class/scsi_device/<hctl>/device/error_inject/
>> instead of
>>      /sys/block/sdb/device/error_inject/
> 
> But these are the same, no ?
> At least on my Fedora box, I see:
> 
> /sys/block/sdp/device/ being
> /sys/devices/pseudo_0/adapter0/host19/target19:0:0/19:0:0:0
> 
> and /sys/class/scsi_device/19:0:0:0/device being
> /sys/devices/pseudo_0/adapter0/host19/target19:0:0/19:0:0:0

Well the patch descriptions are all in terms of /sys/block/sd<letter>/
which will not exist if scsi_debug is called like this:
   $ modprobe scsi_debug ptype=1

That creates a pseudo host with an attached (virtual) tape drive. If
the patchset works for other peripheral device types (i.e. that don't
use the sd driver) then the description should at least mention the
more general case (i.e. /sys/class/scsi_device/<hctl>/device ) IMO.

Doug Gilbert

Re: [RFC PATCH 0/5] scsi:scsi_debug:Add error injection for single lun

[Damien Le Moal]

On 11/14/22 11:54, Douglas Gilbert wrote:
> On 2022-11-13 19:30, Damien Le Moal wrote:
>> On 11/14/22 06:48, Douglas Gilbert wrote:
>>> On 2022-11-09 10:59, Wenchao Hao wrote:
>>>> The original error injection mechanism was based on scsi_host which
>>>> could not inject fault for a single SCSI device.
>>>>
>>>> This patchset provides the ability to inject errors for a single
>>>> SCSI device. Now we supports inject timeout errors, queuecommand
>>>> errors, and hostbyte, driverbyte, statusbyte, and sense data for
>>>> specific SCSI Command
>>>>
>>>> The first patch add an sysfs interface to add and inquiry single
>>>> device's error injection info; the second patch defined how to remove
>>>> an injection which has been added. The following 3 patches use the
>>>> injection info and generate the related error type.
>>>>
>>>> Wenchao Hao (5):
>>>>     scsi:scsi_debug: Add sysfs interface to manager single devices' error inject
>>>>     scsi:scsi_debug: Add interface to remove injection which has been added
>>>>     scsi:scsi_debug: make command timeout if timeout error is injected
>>>>     scsi:scsi_debug: Return failed value for specific command's queuecommand
>>>>     scsi:scsi_debug: fail specific scsi command with result and sense data
>>>>
>>>>    drivers/scsi/scsi_debug.c | 295 ++++++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 295 insertions(+)
>>>
>>> Hi,
>>> This patchset seems to assume all scsi_debug devices will be disk (-like) SCSI
>>> devices. That leaves out other device types: tapes, enclosures, WLUNs, etc.
>>>
>>> Have you considered putting these device specific additions under:
>>>      /sys/class/scsi_device/<hctl>/device/error_inject/
>>> instead of
>>>      /sys/block/sdb/device/error_inject/
>>
>> But these are the same, no ?
>> At least on my Fedora box, I see:
>>
>> /sys/block/sdp/device/ being
>> /sys/devices/pseudo_0/adapter0/host19/target19:0:0/19:0:0:0
>>
>> and /sys/class/scsi_device/19:0:0:0/device being
>> /sys/devices/pseudo_0/adapter0/host19/target19:0:0/19:0:0:0
> 
> Well the patch descriptions are all in terms of /sys/block/sd<letter>/
> which will not exist if scsi_debug is called like this:
>    $ modprobe scsi_debug ptype=1
> 
> That creates a pseudo host with an attached (virtual) tape drive. If
> the patchset works for other peripheral device types (i.e. that don't
> use the sd driver) then the description should at least mention the
> more general case (i.e. /sys/class/scsi_device/<hctl>/device ) IMO.

Got it.

> 
> Doug Gilbert
> 

-- 
Damien Le Moal
Western Digital Research

Re: [RFC PATCH 0/5] scsi:scsi_debug:Add error injection for single lun

[Wenchao Hao]

On 2022/11/14 5:48, Douglas Gilbert wrote:
> On 2022-11-09 10:59, Wenchao Hao wrote:
>> The original error injection mechanism was based on scsi_host which
>> could not inject fault for a single SCSI device.
>>
>> This patchset provides the ability to inject errors for a single
>> SCSI device. Now we supports inject timeout errors, queuecommand
>> errors, and hostbyte, driverbyte, statusbyte, and sense data for
>> specific SCSI Command
>>
>> The first patch add an sysfs interface to add and inquiry single
>> device's error injection info; the second patch defined how to remove
>> an injection which has been added. The following 3 patches use the
>> injection info and generate the related error type.
>>
>> Wenchao Hao (5):
>>    scsi:scsi_debug: Add sysfs interface to manager single devices' error inject
>>    scsi:scsi_debug: Add interface to remove injection which has been added
>>    scsi:scsi_debug: make command timeout if timeout error is injected
>>    scsi:scsi_debug: Return failed value for specific command's queuecommand
>>    scsi:scsi_debug: fail specific scsi command with result and sense data
>>
>>   drivers/scsi/scsi_debug.c | 295 ++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 295 insertions(+)
> 
> Hi,
> This patchset seems to assume all scsi_debug devices will be disk (-like) SCSI
> devices. That leaves out other device types: tapes, enclosures, WLUNs, etc.
> 
> Have you considered putting these device specific additions under:
>    /sys/class/scsi_device/<hctl>/device/error_inject/
> instead of
>    /sys/block/sdb/device/error_inject/
> 
> ?
> 
> Doug Gilbert
> 

Thanks for your reply. I did not notice other device types, but the error_inject interface is
added for scsi_device, so it is available for all types. I would update the path in patch description.

Do you have other suggestions?