Re: [PATCH 06/15] scsi: core: Convert to scsi_execute_cmd

Mike Christie
2022-11-22T20:07:08Z

On 11/22/22 12:46 PM, Bart Van Assche wrote:
> On 11/22/22 08:13, Mike Christie wrote:
>> On 11/22/22 12:38 AM, Christoph Hellwig wrote:
>>> On Mon, Nov 21, 2022 at 09:39:25PM -0600, Mike Christie wrote:
>>>> +    result = scsi_execute_cmd(sdev, cmd, REQ_OP_DRV_IN, buffer,
>>>> +                  request_len, 30 * HZ, 3,
>>>> +                  ((struct scsi_exec_args) { .sshdr = &sshdr }));
>>>
>>> Maybe it's me, but I hate the syntax to declare structs inside argument
>>
>> I'll change it to setup the scsi_exec_args struct like normal. I've got this
>> comment several times now.
>>
>> With the current design we know all the args when we declare the variables so
>> I can do it then like normal.
>
> Will struct scsi_exec_args be retained? I'm asking this because I'd like to add another argument to the (already too large) argument list of scsi_execute().

Yes. I just meant for the above chunk I would move the struct setup like this:

@@ -509,6 +509,9 @@ int scsi_report_opcode(struct scsi_device *sdev, unsigned char *buffer,
{
unsigned char cmd[16];
struct scsi_sense_hdr sshdr;
+ struct scsi_exec_args exec_args = {
+ .sshdr = &sshdr,
+ };
int result, request_len;

if (sdev->no_report_opcodes || sdev->scsi_level < SCSI_SPC_3)
@@ -532,8 +535,7 @@ int scsi_report_opcode(struct scsi_device *sdev, unsigned char *buffer,
memset(buffer, 0, len);

result = scsi_execute_cmd(sdev, cmd, REQ_OP_DRV_IN, buffer,
- request_len, 30 * HZ, 3,
- ((struct scsi_exec_args) { .sshdr = &sshdr }));
+ request_len, 30 * HZ, 3, exec_args);

if (result < 0)
return result;