Re: [PATCH] mptfusion - fc transport attributes

[James Smart <James.Smart@Emulex.Com>]

As we are all trying to avoid a value of zero, we should just make the
transport set function validate the range so that
1 <= dev_loss_tmo <= whatever_the_max_define_is

-- james s

Michael Reed wrote:
> lpfc and qla2x00 have something similar.  I chose the driver default instead of
> 1.  Is setting it to one correct for the api?
> 
> static void
> lpfc_set_rport_loss_tmo(struct fc_rport *rport, uint32_t timeout)
> {
>         /*
>          * The driver doesn't have a per-target timeout setting.  Set
>          * this value globally. lpfc_nodev_tmo should be greater then 0.
>          */
>         if (timeout)
>                 lpfc_nodev_tmo = timeout;
>         else
>                 lpfc_nodev_tmo = 1;
>         rport->dev_loss_tmo = lpfc_nodev_tmo + 5;
> }
> 
> Mike
> 
> James Smart wrote:
>>> +static void
>>> +mptfc_set_rport_loss_tmo(struct fc_rport *rport, uint32_t timeout)
>>> +{
>>> +    if (timeout > 0)
>>> +        rport->dev_loss_tmo = timeout;
>>> +    else
>>> +        rport->dev_loss_tmo = mptfc_dev_loss_tmo;
>>> +}
>> The only function of the if-test here is checking for 0 or >0, as
>> the fc transport ensures it is nothing else. What bothers me is
>> if the value is 0, then you are overriding it with the mptfc default
>> value without any warning or error report to the user.
>>
>> Perhaps we should be dealing with a zero value differently in the
>> transport.
>>
>> The rest looks ok...
>>
>> -- james s
>> -
>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>