Re: [dm-devel] rdac.c patch not quite right.

[Christophe Varoqui <christophe varoqui gmail com>]

On lun., 2010-11-29 at 19:03 -0500, Chaskiel Grundman wrote:
> commit 362d2e5f215894818b52a0d03b723b75917390fb
> added the following block to rdac.c:
> 
>         } else if ((inq.PQ_PDT & 0x20) || (inq.PQ_PDT & 0x7f)) {
>                 /* LUN not connected*/
>                 ret = PATH_DOWN;
>                 goto done;
>         }
> 
> I don't know what the intended effect actually was, but since
> (inq.PQ_PDT & 0x20) will be true if (inq.PQ_PDT & 0x7f), the code is
> incorrect as-is. I have some dell RDAC devices, but I cannot delete
> targets on any of them to test this.
> 
> Update, after reading some scsi docs: If RDAC is using the standard
> peripheral qualifier semantics, I think the following is what should be
> used instead
> 
> 
>         } else if ((inq.PQ_PDT & 0xE0) == 0x20 || inq.PQ_PDT == 0x7f) {
> 
> This form checks for the PQ bits being 001 or 011, and if 011, also
> that the device type has the standard value for an unsupported
> lun.
> 
> Alternatively,
> 
>         } else if ((inq.PQ_PDT & 0xC0) == 0x20) {
> 
> does not validate the device type bits in case the PQ is 011
> 
As the original author of the aforementioned change, do you ack this
patch ?

Best regards,
cvaroqui