Re: [PATCH] include/public: add command result definitions to vscsiif.h

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.03.2022 10:03, Juergen Gross wrote:
> On 14.03.22 10:55, Jan Beulich wrote:
>> On 28.02.2022 12:22, Juergen Gross wrote:
>>> +#define XEN_VSCSIIF_RSLT_HOST_OK         0
>>> +#define XEN_VSCSIIF_RSLT_HOST_NO_CONN    1 /* Couldn't connect before 
>>> timeout */
>>> +#define XEN_VSCSIIF_RSLT_HOST_BUS_BUSY   2 /* BUS busy through timeout */
>>> +#define XEN_VSCSIIF_RSLT_HOST_TIME_OUT   3 /* TIMED OUT for other reason */
>>> +#define XEN_VSCSIIF_RSLT_HOST_BAD_TARG   4 /* BAD target */
>>
>> Are the all-upper-case words really in need of mirroring this
>> aspect from Linux? To me it gives the impression of this being
>> acronyms of some sort at the first glance.
> 
> The backend can return all these values, so I think I need to define
> them here.

Oh, I realize I didn't say so explicitly and hence what I said
ended up being ambiguous: The remark was only about the all-
upper-case words in the comments. I would think they can be
spelled normally.

>>> +#define XEN_VSCSIIF_RSLT_HOST_ABORT      5 /* Abort for some other reason 
>>> */
>>> +#define XEN_VSCSIIF_RSLT_HOST_PARITY     6 /* Parity error */
>>> +#define XEN_VSCSIIF_RSLT_HOST_ERROR      7 /* Internal error */
>>> +#define XEN_VSCSIIF_RSLT_HOST_RESET      8 /* Reset by somebody */
>>> +#define XEN_VSCSIIF_RSLT_HOST_BAD_INTR   9 /* Unexpected interrupt */
>>> +#define XEN_VSCSIIF_RSLT_HOST_PASSTHR   10 /* Force command past mid-layer 
>>> */
>>> +#define XEN_VSCSIIF_RSLT_HOST_SOFT_ERR  11 /* Retry requested */
>>> +#define XEN_VSCSIIF_RSLT_HOST_IMM_RETR  12 /* Hidden retry requested */
>>> +#define XEN_VSCSIIF_RSLT_HOST_REQUEUE   13 /* Requeue command requested */
>>> +#define XEN_VSCSIIF_RSLT_HOST_DISRUPT   14 /* Transport error disrupted 
>>> I/O */
>>> +#define XEN_VSCSIIF_RSLT_HOST_FAILFAST  15 /* Transport class fastfailed */
>>> +#define XEN_VSCSIIF_RSLT_HOST_TARG_FAIL 16 /* Permanent target failure */
>>> +#define XEN_VSCSIIF_RSLT_HOST_NEX_FAIL  17 /* Permanent nexus failure on 
>>> path */
>>> +#define XEN_VSCSIIF_RSLT_HOST_NOMEM     18 /* Space allocation failed */
>>> +#define XEN_VSCSIIF_RSLT_HOST_MED_ERR   19 /* Medium error */
>>> +#define XEN_VSCSIIF_RSLT_HOST_MARGINAL  20 /* Transport marginal errors */
>>
>> Some of the name shortening that you did, comparing with the
>> Linux names, has gone a little too far for my taste. But you're
>> the maintainer ...
> 
> There are basically the following alternatives:
> 
> - use longer names (using the Linux names would end up in e.g.
>    XEN_VSCSIIF_RSLT_HOST_TRANSPORT_DISRUPTED, making it 10 chars longer
> - drop some part of the common prefix, e.g. the "RSLT_HOST_" part
> - keep it as is
> 
> I'm basically fine with any of those.

My personal preference would be in the order you named the
alternatives, perhaps with prepending them by "use longer names,
but in extreme cases not quite as long as Linux'es".

Jan