2012/1/27 Ross S. W. Walker <RWalker@medallion.com>:
> Arne Redlich [mailto:arne.redlich@googlemail.com] wrote:
>>
>> I fully agree with those who want to preserve backward compatibility.
>> I haven't looked into the details and hence not fully understood why a
>> change to the SCSI ID/SN code would be necessary, but I don't think we
>> can silently (logging a deprecation warning is just that) change the
>> behaviour.
>
> Well the change to the SCSI ID code in target_disk.c was percipitated
> from the need to provide SCSI 8h type ID to be SPC-3 compliant so
> Windows 2008 would recognize IET as supporting PR.

That's exactly what is unclear to me: is there really a need? How do the other OSS targets handle that? It's been a while since I went looking for inspiration in other targets ;) but I can't seem to remember seeing support for that.

Reading the thread that spawned this discussion, I'm under the impression that it's just a warning?

> It's all in SPC-3 7.6.3

Could you pls. quote the relevant part that states that PR support requires these ID types? That would really help me a lot.

> In looking at the code at the spec for SCSI name string identifiers,
> section 7.6.3.11, I had mistakenly thought that the logical unit
> name extension had to be world-wide unique by itself which I took
> to mean we would need to calculate a separate 64-bit hash and was
> looking at the existing code to see how best to perform this and
> went off in a tangent. Turns out this 64-bit hex number can just
> be the LUN so long as the IQN + LUN is unique.
>
> I must say though that there really is no need for a ScsiID and
> ScsiSN when it's only the ScsiSN that matters, so I think phasing
> it out over time would remove some unneeded code, but that isn't
> important here.

We can not kill ScsiId as it's in use today, and several tools (udev's scsi_id, ...) use it. I also don't see a need to remove it. It should be possible to maintain the old ID/SN behaviour even if a new ID type needs to be introduced at the expense of slightly more / uglier code - no big deal IMHO.

Cheers,
Arne

> Here is a rough draft of the patch, not compile tested
> or anything.
>
>
> Index: target_disk.c
> ===================================================================
> --- target_disk.c       (revision 467)
> +++ target_disk.c       (working copy)
> @@ -238,21 +238,43 @@
>                        tio_set(tio, len + 4, 0);
>                        err = 0;
>                } else if (scb[2] == 0x83) {
> -                       u32 len = SCSI_ID_LEN + 8;
> +                       u16 len = 0, len1, len8, offset;
> +                       char buffer[256];
>
>                        data[1] = 0x83;
> -                       data[3] = len + 4;
> -                       data[4] = 0x1;
> -                       data[5] = 0x1;
> -                       data[7] = len;
> +
>                        if (cmnd->lun) { /* We need this ? */
> +                               len1 = SCSI_ID_LEN + 8;
> +                               snprintf(buffer, 256, "%.223s,L,0x%016x",
> +                                       cmnd->lun->target->name, cmnd->lun->lun);
> +                               len8 = ((strlen(buffer) + 1) + 3) & -4;
> +                               len = len1 + len8 + 8;
> +                       }
> +
> +                       len = cpu_to_be16(len);
> +                       memcpy(data + 2, &len, 2);
> +                       len = be16_to_cpu(len);
> +
> +                       if (cmnd->lun) { /* We need this ? */
> +                               offset = 8 + len1;
> +
> +                               data[4] = 0x1;
> +                               data[5] = 0x1;
> +                               data[7] = len1;
> +
> +                               /* Below should be spaces 0x20 per the spec */
>                                memset(data + 8, 0x00, 8);
>                                memcpy(data + 8, VENDOR_ID,
>                                        min_t(size_t, strlen(VENDOR_ID), 8));
>                                memcpy(data + 16, cmnd->lun->scsi_id,
>                                                                SCSI_ID_LEN);
> +
> +                               data[offset++] = 0x3;
> +                               data[offset++] = 0x8;
> +                               data[++offset++] = len8;
> +                               memcpy(data + offset, buffer, strlen(buffer));
>                        }
> -                       tio_set(tio, len + 8, 0);
> +                       tio_set(tio, len + 4, 0);
>                        err = 0;
>                }
>        }
> ______________________________________________________________________
> This e-mail, and any attachments thereto, is intended only for use by
> the addressee(s) named herein and may contain legally privileged
> and/or confidential information. If you are not the intended recipient
> of this e-mail, you are hereby notified that any dissemination,
> distribution or copying of this e-mail, and any attachments thereto,
> is strictly prohibited. If you have received this e-mail in error,
> please immediately notify the sender and permanently delete the
> original and any copy or printout thereof.