[v5,10/10] scsi: scsi_debug: Add param to control sdev's allow_restart

Message ID 20230922092906.2645265-11-haowenchao2@huawei.com
State New
Headers
Series scsi:scsi_debug: Add error injection for single device |

Commit Message

Wenchao Hao Sept. 22, 2023, 9:29 a.m. UTC
  Add new module param "allow_restart" to control if setup
scsi_device's allow_restart flag. This is used to test scsi
command finished with sense_key 0x6, asc 0x4 and ascq 0x2

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

Comments

Douglas Gilbert Oct. 8, 2023, 11:17 p.m. UTC | #1
On 2023-09-22 05:29, Wenchao Hao wrote:
> Add new module param "allow_restart" to control if setup
> scsi_device's allow_restart flag. This is used to test scsi
> command finished with sense_key 0x6, asc 0x4 and ascq 0x2
> 
> Signed-off-by: Wenchao Hao <haowenchao2@huawei.com>

Hi,
Looked at this and verified that the allow_restart flag of scsi_debug
devices (disks ?) is usually 0 and when the scsi_debug module is
started with allow_restart=1 then the allow_restart flag does indeed
change to 1. For example:
    # cat /sys/class/scsi_disk/1\:0\:0\:0/allow_restart
    1

That ASC/ASCQ code means: "Logical unit not ready, initializing command
required" according to my library. Played around with sg_start but didn't
see any change in how it reacts. According to scsi_device.h that flag's
description is: "issue START_UNIT in error handler" which implies it
changes how the EH handler reacts.

Perhaps the 3 line patch description could say a little more about how
to use this new parameter...

Tested-by: Douglas Gilbert <dgilbert@interlog.com>

<snip>
  
Wenchao Hao Oct. 9, 2023, 7:19 a.m. UTC | #2
On 2023/10/9 7:17, Douglas Gilbert wrote:
> On 2023-09-22 05:29, Wenchao Hao wrote:
>> Add new module param "allow_restart" to control if setup
>> scsi_device's allow_restart flag. This is used to test scsi
>> command finished with sense_key 0x6, asc 0x4 and ascq 0x2
>>
>> Signed-off-by: Wenchao Hao <haowenchao2@huawei.com>
> 
> Hi,
> Looked at this and verified that the allow_restart flag of scsi_debug
> devices (disks ?) is usually 0 and when the scsi_debug module is
> started with allow_restart=1 then the allow_restart flag does indeed
> change to 1. For example:
>     # cat /sys/class/scsi_disk/1\:0\:0\:0/allow_restart
>     1
> 
> That ASC/ASCQ code means: "Logical unit not ready, initializing command
> required" according to my library. Played around with sg_start but didn't
> see any change in how it reacts. According to scsi_device.h that flag's
> description is: "issue START_UNIT in error handler" which implies it
> changes how the EH handler reacts.
> 
> Perhaps the 3 line patch description could say a little more about how
> to use this new parameter...

Sorry I did not write in detail. As you mentioned above, this is to
determine if to trigger error. I would update the commit message to
following lines:

Add new module param "allow_restart" to control if setup scsi_device's
allow_restart flag, this flag determines if trigger EH after command
finished with sense_key 0x6, asc 0x4 and ascq 0x2, EH would be triggered
if allow_restart=1 in this condition.

The new param can be used with error inject added in patch6 to test how
commands finished with sense_key 0x6, asc 0x4 and ascq 0x2 are handled.

> 
> Tested-by: Douglas Gilbert <dgilbert@interlog.com>
> 
> <snip>
> 
> 
> 
> 
>
  

Patch

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index ab4a6f7de1ef..52a9ddea57d3 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -843,6 +843,7 @@  static bool have_dif_prot;
 static bool write_since_sync;
 static bool sdebug_statistics = DEF_STATISTICS;
 static bool sdebug_wp;
+static bool sdebug_allow_restart;
 /* Following enum: 0: no zbc, def; 1: host aware; 2: host managed */
 static enum blk_zoned_model sdeb_zbc_model = BLK_ZONED_NONE;
 static char *sdeb_zbc_model_s;
@@ -5469,6 +5470,9 @@  static int scsi_debug_slave_configure(struct scsi_device *sdp)
 		sdp->no_uld_attach = 1;
 	config_cdb_len(sdp);
 
+	if (sdebug_allow_restart)
+		sdp->allow_restart = 1;
+
 	devip->debugfs_entry = debugfs_create_dir(dev_name(&sdp->sdev_dev),
 				sdebug_debugfs_root);
 	debugfs_create_file("error", 0600, devip->debugfs_entry, sdp,
@@ -6186,6 +6190,7 @@  module_param_named(zone_cap_mb, sdeb_zbc_zone_cap_mb, int, S_IRUGO);
 module_param_named(zone_max_open, sdeb_zbc_max_open, int, S_IRUGO);
 module_param_named(zone_nr_conv, sdeb_zbc_nr_conv, int, S_IRUGO);
 module_param_named(zone_size_mb, sdeb_zbc_zone_size_mb, int, S_IRUGO);
+module_param_named(allow_restart, sdebug_allow_restart, bool, S_IRUGO | S_IWUSR);
 
 MODULE_AUTHOR("Eric Youngdale + Douglas Gilbert");
 MODULE_DESCRIPTION("SCSI debug adapter driver");
@@ -6258,6 +6263,7 @@  MODULE_PARM_DESC(zone_cap_mb, "Zone capacity in MiB (def=zone size)");
 MODULE_PARM_DESC(zone_max_open, "Maximum number of open zones; [0] for no limit (def=auto)");
 MODULE_PARM_DESC(zone_nr_conv, "Number of conventional zones (def=1)");
 MODULE_PARM_DESC(zone_size_mb, "Zone size in MiB (def=auto)");
+MODULE_PARM_DESC(allow_restart, "Set scsi_device's allow_restart flag(def=0)");
 
 #define SDEBUG_INFO_LEN 256
 static char sdebug_info[SDEBUG_INFO_LEN];