Re: [PATCH] mptfusion - fc transport attributes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
-
: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



-
: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux