[PATCH V4 1/1] nvme: Add verbose error logging

[Alan Adamson]

> On Jan 25, 2022, at 2:54 PM, Sagi Grimberg <sagi at grimberg.me> wrote:
> 
> 
>> +#endif
>> + if (err_str == NULL)
>> + err_str =  "I/O Error";
>> +
>> + if (ns) {
>> +#ifdef CONFIG_NVME_VERBOSE_ERRORS
>> + for (i = 0, entry = nvme_ops ; i < ARRAY_SIZE(nvme_ops) ; i++)
>> + if (entry[i].code == nr->cmd->common.opcode)
>> + op_str = entry[i].string;
>> Same here, the loop looks redundant to me:
>> if (nr->cmd->common.opcode <= ARRAY_SIZE(nvme_ops) &&
>>    nvme_ops[nr->cmd->common.opcode] != NULL)
>> op_str = nvme_ops[nr->cmd->common.opcode].string;
>> else
>> op_str = "Unknown"
>> Currently the nvme_ops and nvme_admin_ops arrays are not built by index so I need to walk the array to find the
>> appropriate opcode/name.  Are you asking to change these arrays to be build by index?
>> static const char * const nvme_admin_ops[] = {
>>         [nvme_admin_delete_sq] = "Delete SQ",
>>         [nvme_admin_create_sq] = "Create SQ",
>>         [nvme_admin_get_log_page] = "Get Log Page”,
>> Rather than:
>> static const struct nvme_string_table nvme_admin_ops[] = {
>>         { nvme_admin_delete_sq,         "Delete SQ" },
>>         { nvme_admin_create_sq,         "Create SQ" },
>>         { nvme_admin_get_log_page,      "Get Log Page" },
> 
> Yes, is that a problem?

No, not a problem.

Alan