Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Message-ID: <89365bd3-b4b0-de2d-f863-afbaad118649@huawei.com>
Date:   Mon, 9 Oct 2023 15:19:24 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH v5 10/10] scsi: scsi_debug: Add param to control sdev's
 allow_restart
Content-Language: en-US
To:     <dgilbert@interlog.com>,
        "James E . J . Bottomley" <jejb@linux.ibm.com>,
        "Martin K . Petersen" <martin.petersen@oracle.com>,
        <open-iscsi@googlegroups.com>, <linux-scsi@vger.kernel.org>
CC:     <linux-kernel@vger.kernel.org>, <louhongxiang@huawei.com>
References: <20230922092906.2645265-1-haowenchao2@huawei.com>
 <20230922092906.2645265-11-haowenchao2@huawei.com>
 <d61e88d3-e1b7-44e0-ba9b-f633be0b5b30@interlog.com>
From:   Wenchao Hao <haowenchao2@huawei.com>
In-Reply-To: <d61e88d3-e1b7-44e0-ba9b-f633be0b5b30@interlog.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

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>
> 
> 
> 
> 
>