[PATCH] Add 'tpgs' sysfs attribute for SCSI devices

[PATCH] Add 'tpgs' sysfs attribute for SCSI devices

[Hannes Reinecke]

We need to export the 'TGPS' setting of the
inquiry data to properly support ALUA.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/scsi_scan.c   |    1 +
 drivers/scsi/scsi_sysfs.c  |    2 ++
 include/scsi/scsi_device.h |    3 ++-
 3 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index a14d245..d1619ed 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -836,6 +836,7 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
 	sdev->inq_periph_qual = (inq_result[0] >> 5) & 7;
 	sdev->lockable = sdev->removable;
 	sdev->soft_reset = (inq_result[7] & 1) && ((inq_result[3] & 7) == 2);
+	sdev->tpgs = (inq_result[5] >> 4) & 3;
 
 	if (sdev->scsi_level >= SCSI_3 ||
 			(sdev->inquiry_len > 56 && inq_result[56] & 0x04))
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index fa4711d..7513ce5 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -560,6 +560,7 @@ sdev_rd_attr (scsi_level, "%d\n");
 sdev_rd_attr (vendor, "%.8s\n");
 sdev_rd_attr (model, "%.16s\n");
 sdev_rd_attr (rev, "%.4s\n");
+sdev_rd_attr (tpgs, "%d\n");
 
 /*
  * TODO: can we make these symlinks to the block layer ones?
@@ -745,6 +746,7 @@ static struct attribute *scsi_sdev_attrs[] = {
 	&dev_attr_vendor.attr,
 	&dev_attr_model.attr,
 	&dev_attr_rev.attr,
+	&dev_attr_tpgs.attr,
 	&dev_attr_rescan.attr,
 	&dev_attr_delete.attr,
 	&dev_attr_state.attr,
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 3f566af..22aaf72 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -96,7 +96,8 @@ struct scsi_device {
 	void *hostdata;		/* available to low-level driver */
 	char type;
 	char scsi_level;
-	char inq_periph_qual;	/* PQ from INQUIRY data */	
+	char inq_periph_qual;	/* PQ from INQUIRY data */
+	char tpgs;		/* Target port group support */
 	unsigned char inquiry_len;	/* valid bytes in 'inquiry' */
 	unsigned char * inquiry;	/* INQUIRY response data */
 	const char * vendor;		/* [back_compat] point into 'inquiry' ... */
-- 
1.5.3.2

Re: [PATCH] Add 'tpgs' sysfs attribute for SCSI devices

[James Bottomley]

On Wed, 2009-03-18 at 13:02 +0100, Hannes Reinecke wrote:
> We need to export the 'TGPS' setting of the
> inquiry data to properly support ALUA.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/scsi/scsi_scan.c   |    1 +
>  drivers/scsi/scsi_sysfs.c  |    2 ++
>  include/scsi/scsi_device.h |    3 ++-
>  3 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index a14d245..d1619ed 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -836,6 +836,7 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
>  	sdev->inq_periph_qual = (inq_result[0] >> 5) & 7;
>  	sdev->lockable = sdev->removable;
>  	sdev->soft_reset = (inq_result[7] & 1) && ((inq_result[3] & 7) == 2);
> +	sdev->tpgs = (inq_result[5] >> 4) & 3;

As far as I can tell, this is a read only parameter, isn't it?

In that case, why not just do what we do for the SPI inquiry data and
have a wrapper pulling it out of sdev->inquiry instead of a new flag in
sdev?

James

Re: [PATCH] Add 'tpgs' sysfs attribute for SCSI devices

[Hannes Reinecke]

James Bottomley wrote:
> On Wed, 2009-03-18 at 13:02 +0100, Hannes Reinecke wrote:
>> We need to export the 'TGPS' setting of the
>> inquiry data to properly support ALUA.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>  drivers/scsi/scsi_scan.c   |    1 +
>>  drivers/scsi/scsi_sysfs.c  |    2 ++
>>  include/scsi/scsi_device.h |    3 ++-
>>  3 files changed, 5 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
>> index a14d245..d1619ed 100644
>> --- a/drivers/scsi/scsi_scan.c
>> +++ b/drivers/scsi/scsi_scan.c
>> @@ -836,6 +836,7 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
>>  	sdev->inq_periph_qual = (inq_result[0] >> 5) & 7;
>>  	sdev->lockable = sdev->removable;
>>  	sdev->soft_reset = (inq_result[7] & 1) && ((inq_result[3] & 7) == 2);
>> +	sdev->tpgs = (inq_result[5] >> 4) & 3;
> 
> As far as I can tell, this is a read only parameter, isn't it?
> 
> In that case, why not just do what we do for the SPI inquiry data and
> have a wrapper pulling it out of sdev->inquiry instead of a new flag in
> sdev?
> 
Because we're evaluating it later on from the device_handler.
But honestly I didn't inquire in that direction.
I'll be checking.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
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

Re: [PATCH] Add 'tpgs' sysfs attribute for SCSI devices

[James Bottomley]

On Wed, 2009-03-18 at 15:02 +0100, Hannes Reinecke wrote:
> James Bottomley wrote:
> > On Wed, 2009-03-18 at 13:02 +0100, Hannes Reinecke wrote:
> >> We need to export the 'TGPS' setting of the
> >> inquiry data to properly support ALUA.
> >>
> >> Signed-off-by: Hannes Reinecke <hare@suse.de>
> >> ---
> >>  drivers/scsi/scsi_scan.c   |    1 +
> >>  drivers/scsi/scsi_sysfs.c  |    2 ++
> >>  include/scsi/scsi_device.h |    3 ++-
> >>  3 files changed, 5 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> >> index a14d245..d1619ed 100644
> >> --- a/drivers/scsi/scsi_scan.c
> >> +++ b/drivers/scsi/scsi_scan.c
> >> @@ -836,6 +836,7 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
> >>  	sdev->inq_periph_qual = (inq_result[0] >> 5) & 7;
> >>  	sdev->lockable = sdev->removable;
> >>  	sdev->soft_reset = (inq_result[7] & 1) && ((inq_result[3] & 7) == 2);
> >> +	sdev->tpgs = (inq_result[5] >> 4) & 3;
> > 
> > As far as I can tell, this is a read only parameter, isn't it?
> > 
> > In that case, why not just do what we do for the SPI inquiry data and
> > have a wrapper pulling it out of sdev->inquiry instead of a new flag in
> > sdev?
> > 
> Because we're evaluating it later on from the device_handler.
> But honestly I didn't inquire in that direction.
> I'll be checking.

As long as you have access to the sdev, you have the inquiry data.

The idea would just be to have a static function

static int scsi_device_tpgs(struct scsi_device *sdev)
{
	return (sdev->inquiry[5] >> 4) &3;
}

We have a few others like this in scsi_device.h (in scsi-misc,
otherwise, I think they're in scsi.h)

James