Re: [PATCH] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

From: Bart Van Assche
Date: Fri Jun 10 2016 - 15:18:55 EST


On 05/24/2016 01:00 PM, Bryant G Ly wrote:
Quoting Bart Van Assche <bart.vanassche@xxxxxxxxxxx>:
On 05/24/2016 06:52 AM, Bryant G. Ly wrote:
+static uint64_t ibmvscsis_unpack_lun(const uint8_t *lun, int len)
+{
+ uint64_t res = NO_SUCH_LUN;
+ int addressing_method;
+
+ if (unlikely(len < 2)) {
+ pr_err("Illegal LUN length %d, expected 2 bytes or more\n",
+ len);
+ goto out;
+ }
+
+ switch (len) {
+ case 8:
+ if ((*((__be64 *)lun) & cpu_to_be64(0x0000FFFFFFFFFFFFLL))
!= 0)
+ goto out_err;
+ break;
+ case 4:
+ if (*((__be16 *)&lun[2]) != 0)
+ goto out_err;
+ break;
+ case 6:
+ if (*((__be32 *)&lun[2]) != 0)
+ goto out_err;
+ break;
+ case 2:
+ break;
+ default:
+ goto out_err;
+ }
+
+ addressing_method = (*lun) >> 6; /* highest two bits of byte 0 */
+ switch (addressing_method) {
+ case SCSI_LUN_ADDR_METHOD_PERIPHERAL:
+ case SCSI_LUN_ADDR_METHOD_FLAT:
+ case SCSI_LUN_ADDR_METHOD_LUN:
+ res = *(lun + 1) | (((*lun) & 0x3f) << 8);
+ break;
+
+ case SCSI_LUN_ADDR_METHOD_EXTENDED_LUN:
+ default:
+ pr_err("Unimplemented LUN addressing method %u\n",
+ addressing_method);
+ break;
+ }
+
+out:
+ return res;
+out_err:
+ pr_err("Support for multi-level LUNs has not yet been
implemented\n");
+ goto out;
+}

In the above function there is nothing that is specific to the VIO
mechanism. Please consider to merge this function with
scsilun_to_int(), e.g. by introducing a new function that accepts a
SCSI LUN and the addressing method as arguments and by making
scsilun_to_int() call that function with
SCSI_LUN_ADDR_METHOD_PERIPHERAL as one of its arguments.


We are going to keep this here atm, until we can make sure canonical can
pick up
these changes, since we have a deliverable for a 3rd party vendor who is
going to
fully utilize this driver. Once they can pick it up in 4.4 kernel I will
make the
change you recommended / move the scsi_lun_addr_method addressed below.
I will also
address the changes that ib_srpt needs to utilize this new function too.

A much better approach would be to move this function into the SCSI core and to add a copy of that function to the backported driver that will be provided to Canonical.

Thanks,

Bart.