[PATCH][RFC] iSCSI transport class

[PATCH][RFC] iSCSI transport class

[Mike Christie]

[-- Attachment #1: Type: text/plain, Size: 604 bytes --]

Attached is the begining of an iSCSI transport class. Currently,
the Cisco driver has some sysfs issues, so the patch in the next
mail will convert their print_info attribute to use this class
instead.

This patch was built against 2.6.5-rc2. And, here is a tree. Not
much yet, so please send comments. Maybe it should be per session
instead of per device?

[mc@minna 1:0:0:0]$ pwd
/sys/class/iscsi_transport/1:0:0:0
[mc@minna 1:0:0:0]$ tree
.
|-- device -> ../../../devices/platform/iscsi/1:0:0:0
|-- driver -> ../../../bus/scsi/drivers/sd
|-- target_addr
|-- target_name
`-- target_port

Mike Christie

[-- Attachment #2: iscsi-transport-class.patch --]
[-- Type: text/plain, Size: 7205 bytes --]

diff -Naurp linux-2.6.5-rc2/drivers/scsi/Kconfig linux-2.6.5-rc2-iscsi/drivers/scsi/Kconfig
--- linux-2.6.5-rc2/drivers/scsi/Kconfig	2004-03-19 16:11:42.000000000 -0800
+++ linux-2.6.5-rc2-iscsi/drivers/scsi/Kconfig	2004-03-25 01:26:32.000000000 -0800
@@ -214,6 +214,14 @@ config SCSI_FC_ATTRS
 	  each attached FiberChannel device to sysfs, say Y.
 	  Otherwise, say N.
 
+config SCSI_ISCSI_ATTRS
+	tristate "iSCSI Transport Attributes"
+	depends on SCSI
+	help
+	  If you wish to export transport-specific information about
+	  each attached iSCSI device to sysfs, say Y.
+	  Otherwise, say N.
+
 endmenu
 
 menu "SCSI low-level drivers"
diff -Naurp linux-2.6.5-rc2/drivers/scsi/Makefile linux-2.6.5-rc2-iscsi/drivers/scsi/Makefile
--- linux-2.6.5-rc2/drivers/scsi/Makefile	2004-03-19 16:11:40.000000000 -0800
+++ linux-2.6.5-rc2-iscsi/drivers/scsi/Makefile	2004-03-25 01:34:52.000000000 -0800
@@ -28,6 +28,7 @@ obj-$(CONFIG_SCSI)		+= scsi_mod.o
 # --------------------------
 obj-$(CONFIG_SCSI_SPI_ATTRS)	+= scsi_transport_spi.o
 obj-$(CONFIG_SCSI_FC_ATTRS) 	+= scsi_transport_fc.o
+obj-$(CONFIG_SCSI_ISCSI_ATTRS)	+= scsi_transport_iscsi.o
 
 
 obj-$(CONFIG_SCSI_AMIGA7XX)	+= amiga7xx.o	53c7xx.o
diff -Naurp linux-2.6.5-rc2/drivers/scsi/scsi_transport_iscsi.c linux-2.6.5-rc2-iscsi/drivers/scsi/scsi_transport_iscsi.c
--- linux-2.6.5-rc2/drivers/scsi/scsi_transport_iscsi.c	1969-12-31 16:00:00.000000000 -0800
+++ linux-2.6.5-rc2-iscsi/drivers/scsi/scsi_transport_iscsi.c	2004-03-25 01:26:26.000000000 -0800
@@ -0,0 +1,133 @@
+/* 
+ * iSCSI sysfs attributes.
+ *
+ * Copyright (C) IBM
+ *
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <scsi/scsi.h>
+#include <scsi/scsi_device.h>
+#include <scsi/scsi_host.h>
+#include <scsi/scsi_transport.h>
+#include <scsi/scsi_transport_iscsi.h>
+
+#define ISCSI_NUM_ATTRS 3	/* increase this if you add attributes */
+
+struct iscsi_internal {
+	struct scsi_transport_template t;
+	struct iscsi_function_template *f;
+	/* The actual attributes */
+	struct class_device_attribute private_attrs[ISCSI_NUM_ATTRS];
+	/* The array of null terminated pointers to attributes 
+	 * needed by scsi_sysfs.c */
+	struct class_device_attribute *attrs[ISCSI_NUM_ATTRS + 1];
+};
+
+#define to_iscsi_internal(tmpl) container_of(tmpl, struct iscsi_internal, t)
+
+static void transport_class_release(struct class_device *class_dev)
+{
+	struct scsi_device *sdev = transport_class_to_sdev(class_dev);
+	put_device(&sdev->sdev_gendev);
+}
+
+struct class iscsi_transport_class = {
+	.name = "iscsi_transport",
+	.release = transport_class_release,
+};
+
+#define iscsi_transport_show_function(field)				\
+									\
+static ssize_t								\
+show_iscsi_transport_##field(struct class_device *cdev, char *buf)	\
+{									\
+	struct scsi_device *sdev = transport_class_to_sdev(cdev);	\
+	struct iscsi_internal *i = to_iscsi_internal(sdev->host->transportt); \
+	return i->f->get_##field(sdev, buf);				\
+}
+
+#define iscsi_transport_rd_attr(field)					\
+	iscsi_transport_show_function(field)				\
+static CLASS_DEVICE_ATTR(field, S_IRUGO, show_iscsi_transport_##field, NULL)
+
+iscsi_transport_rd_attr(target_port);
+iscsi_transport_rd_attr(target_name);
+iscsi_transport_rd_attr(target_addr);
+
+#define SETUP_ATTRIBUTE(ft, field)					\
+	if (ft->get_##field) {						\
+		i->private_attrs[count] = class_device_attr_##field;	\
+		i->attrs[count] = &i->private_attrs[count];		\
+		count++;						\
+	}
+
+struct scsi_transport_template *
+iscsi_attach_transport(struct iscsi_function_template *ft)
+{
+	struct iscsi_internal *i = kmalloc(sizeof(struct iscsi_internal),
+					   GFP_KERNEL);
+	int count = 0;
+	if (unlikely(!i))
+		return NULL;
+
+	memset(i, 0, sizeof(struct iscsi_internal));
+
+	i->t.attrs = &i->attrs[0];
+	i->t.class = &iscsi_transport_class;
+	i->t.size = 0;
+	i->f = ft;
+
+	SETUP_ATTRIBUTE(ft, target_port);
+	SETUP_ATTRIBUTE(ft, target_name);
+	SETUP_ATTRIBUTE(ft, target_addr);
+	i->attrs[count] = NULL;
+
+	return &i->t;
+}
+EXPORT_SYMBOL(iscsi_attach_transport);
+
+void iscsi_release_transport(struct scsi_transport_template *t)
+{
+	struct iscsi_internal *i = to_iscsi_internal(t);
+
+	kfree(i);
+}
+EXPORT_SYMBOL(iscsi_release_transport);
+
+
+static __init int iscsi_transport_init(void)
+{
+	return class_register(&iscsi_transport_class);
+}
+
+static void __exit iscsi_transport_exit(void)
+{
+	class_unregister(&iscsi_transport_class);
+}
+
+module_init(iscsi_transport_init);
+module_exit(iscsi_transport_exit);
+
+MODULE_AUTHOR("Mike Christie");
+MODULE_DESCRIPTION("iSCSI Transport Attributes");
+MODULE_LICENSE("GPL");
+
diff -Naurp linux-2.6.5-rc2/include/scsi/scsi_transport_iscsi.h linux-2.6.5-rc2-iscsi/include/scsi/scsi_transport_iscsi.h
--- linux-2.6.5-rc2/include/scsi/scsi_transport_iscsi.h	1969-12-31 16:00:00.000000000 -0800
+++ linux-2.6.5-rc2-iscsi/include/scsi/scsi_transport_iscsi.h	2004-03-25 01:26:02.000000000 -0800
@@ -0,0 +1,41 @@
+/* 
+ * iSCSI sysfs attributes.
+ *
+ * Copyright (C) IBM
+ *
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#ifndef SCSI_TRANSPORT_ISCSI_H
+#define SCSI_TRANSPORT_ISCSI_H
+
+#include <linux/config.h>
+
+struct scsi_transport_template;
+
+/* The functions by which the transport class and the driver communicate */
+struct iscsi_function_template {
+	ssize_t (*get_target_port)(struct scsi_device *, char *);
+	ssize_t (*get_target_addr)(struct scsi_device *, char *);
+	ssize_t (*get_target_name)(struct scsi_device *, char *);
+};
+
+struct scsi_transport_template *iscsi_attach_transport(struct iscsi_function_template *);
+void iscsi_release_transport(struct scsi_transport_template *t);
+
+#endif /* SCSI_TRANSPORT_ISCSI_H */

RE: [PATCH][RFC] iSCSI transport class

[Surekha.PC]

 Hi Mike,

 I have reviewed your changes for iSCSI transport class. The changes look
fine.
 I have few suggestions on further improvements. The attributes can be
organised
 under iSCSI transport class in the following structure.

# ls /sys/class/iscsi_transport/*

  /* iSCSI host specific attributes */
  |-- shutdown
  |-- host_id
  |-- ....

  /* iSCSI target specific attributes */
  |-- 1:0:1/
           |-- target_addr
           |-- target_name
           |-- target_port
           |-- ....

  /* lun specific attributes */
  |-- 1:0:1:0/
             |-- device -> ../../../devices/platform/iscsi/1:0:0:0
             |-- driver -> ../../../bus/scsi/drivers/sd
             |-- lun_status
             |-- ....

 This way the relevant attributes can be used from respective paths.
  
Thanks,
surekha

> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org 
> [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Mike Christie
> Sent: Saturday, March 27, 2004 2:21 AM
> To: SCSI Mailing List; linux-iscsi-devel@lists.sourceforge.net
> Subject: [PATCH][RFC] iSCSI transport class
> 
> 
> Attached is the begining of an iSCSI transport class. 
> Currently, the Cisco driver has some sysfs issues, so the 
> patch in the next mail will convert their print_info 
> attribute to use this class instead.
> 
> This patch was built against 2.6.5-rc2. And, here is a tree. 
> Not much yet, so please send comments. Maybe it should be per 
> session instead of per device?
> 
> [mc@minna 1:0:0:0]$ pwd
> /sys/class/iscsi_transport/1:0:0:0
> [mc@minna 1:0:0:0]$ tree
> .
> |-- device -> ../../../devices/platform/iscsi/1:0:0:0
> |-- driver -> ../../../bus/scsi/drivers/sd
> |-- target_addr
> |-- target_name
> `-- target_port
> 
> Mike Christie
> 

Re: [PATCH][RFC] iSCSI transport class

[Mike Christie]

Hey Surekha,

Thanks for the comments.

Surekha.PC wrote:
>  Hi Mike,
> 
>  I have reviewed your changes for iSCSI transport class. The changes look
> fine.
>  I have few suggestions on further improvements. The attributes can be
> organised
>  under iSCSI transport class in the following structure.
> 
> # ls /sys/class/iscsi_transport/*
> 
>   /* iSCSI host specific attributes */
>   |-- shutdown
>   |-- host_id
>   |-- ....

The examples you placed here are not tranport specific. They are
just HBA values, and should stay under the host attributes.

Also, the host layout here does not look right. What happens when I
have multiple HBAs?

>   /* iSCSI target specific attributes */
>   |-- 1:0:1/
>            |-- target_addr
>            |-- target_name
>            |-- target_port
>            |-- ....

I agree these would be nice under a target attribute. Did
you have something to post since the last time we talked,
or did you write the example by hand?


>   /* lun specific attributes */
>   |-- 1:0:1:0/
>              |-- device -> ../../../devices/platform/iscsi/1:0:0:0
>              |-- driver -> ../../../bus/scsi/drivers/sd
>              |-- lun_status
>              |-- ....

I don't think anything in lun_status is iscsi specific.

>  This way the relevant attributes can be used from respective paths.
> 

I get your point (you just chose the above attributes based on
name to accentuate this right). However, in your example it seems
like you would want to show the parent-children relationships like
here:

/sys/class/iscsi_transport_host
|-- host1
|   |-- a_host_attribute
|   |-- 1:0:1
|   |   |-- target_name
|   |   |-- 1:0:1:0
|   |   |   |- some_lun_attribute

Or you could also do something like

/sys/class/iscsi_transport_host
|-- host1
|   |- a_host_attribute
|   |- 1:0:1 -> ../iscsi_transport_target/1:0:1
|   `- 1:0:2 -> ../iscsi_transport_target/1:0:2
/sys/class/iscsi_transport_target
|-- 1:0:1
|   |- target_name
|   |- ../iscsi_transport_lun/1:0:1:0
........

Making /sys/class/iscsi_transport the parent for everything just
does not look right.


Mike

RE: [PATCH][RFC] iSCSI transport class

[Surekha.PC]

 Hi Mike,

  Sorry for the delay.

  The structure which I represented was meant to have iSCSI transport
attributes
  under their respective category. Right now, in our iSCSI driver all
attributes are defined
  in `iscsi-attr.c` and `iscsi-debug-attr.c` which need to be either per lun
or per target.

> >  I have reviewed your changes for iSCSI transport class. 
> The changes 
> > look fine.  I have few suggestions on further improvements. The 
> > attributes can be organised
> >  under iSCSI transport class in the following structure.
> > 
> > # ls /sys/class/iscsi_transport/*
> > 
> >   /* iSCSI host specific attributes */
> >   |-- shutdown
> >   |-- host_id
> >   |-- ....
> 
> The examples you placed here are not tranport specific. They 
> are just HBA values, and should stay under the host attributes.

AFAIU Transport class is basically to hold all private attributes of the
iSCSI driver.
Since iSCSI driver has attributes which are hba/target/lun specific it would
be good to have them in a place away from the generic scsi attributes.

For eg, the "shutdown" attribute is a iSCSI attribute to shutdown the iSCSI
hba.
Similarly host_id is iSCSI transport specific and gives the host number.

> Also, the host layout here does not look right. What happens 
> when I have multiple HBAs?

Right now iSCSI driver supports single HBA, hence this representation is OK.
However in multiple HBA case, it can be represented as below.

 /sys/class/iscsi_transport/
   |--host1/
       |-- shutdown_hba
       |-- show_hba_id
   |--host2/
       |-- ---"----
       |-- ---"----

  
> 
> >   /* iSCSI target specific attributes */
> >   |-- 1:0:1/
> >            |-- target_addr
> >            |-- target_name
> >            |-- target_port
> >            |-- ....
> 
> I agree these would be nice under a target attribute. Did
> you have something to post since the last time we talked,
> or did you write the example by hand?

I have written the example by hand, I am working towards implementing it.
 
 
> >   /* lun specific attributes */
> >   |-- 1:0:1:0/
> >              |-- device -> ../../../devices/platform/iscsi/1:0:0:0
> >              |-- driver -> ../../../bus/scsi/drivers/sd
> >              |-- lun_status
> >              |-- ....
> 
> I don't think anything in lun_status is iscsi specific.

We have lun specific attributes defined in iSCSI driver some of which can be
used in debug mode for testing purposes e.g initiate command timeout on a
lun, set lun queue as busy, query the lun status etc.
I think since they are private to iSCSI, this is the right place holder.

> >  This way the relevant attributes can be used from respective paths.
> I get your point (you just chose the above attributes based 
> on name to accentuate this right). However, in your example 
> it seems like you would want to show the parent-children 
> relationships like
> here:
> 
> /sys/class/iscsi_transport_host
> |-- host1
> |   |-- a_host_attribute
> |   |-- 1:0:1
> |   |   |-- target_name
> |   |   |-- 1:0:1:0
> |   |   |   |- some_lun_attribute
> 
> Or you could also do something like
> 
> /sys/class/iscsi_transport_host
> |-- host1
> |   |- a_host_attribute
> |   |- 1:0:1 -> ../iscsi_transport_target/1:0:1
> |   `- 1:0:2 -> ../iscsi_transport_target/1:0:2
> /sys/class/iscsi_transport_target
> |-- 1:0:1
> |   |- target_name
> |   |- ../iscsi_transport_lun/1:0:1:0
> ........
> 
> Making /sys/class/iscsi_transport the parent for everything 
> just does not look right.

In the above example do you mean that iscsi_transport_target is registered
as 
another transport class along with iscsi_transport_host? Is this OK ?

Thanks,
surekha

Re: [PATCH][RFC] iSCSI transport class

[Mike Christie]

Hey,

Surekha.PC wrote:
>  Hi Mike,
> 
>   Sorry for the delay.
>   The structure which I represented was meant to have iSCSI transport
> attributes
>   under their respective category. Right now, in our iSCSI driver all
> attributes are defined
>   in `iscsi-attr.c` and `iscsi-debug-attr.c` which need to be either per lun
> or per target.
>>> I have reviewed your changes for iSCSI transport class. 
>>
>>The changes 
>>
>>>look fine.  I have few suggestions on further improvements. The 
>>>attributes can be organised
>>> under iSCSI transport class in the following structure.
>>>
>>># ls /sys/class/iscsi_transport/*
>>>
>>>  /* iSCSI host specific attributes */
>>>  |-- shutdown
>>>  |-- host_id
>>>  |-- ....
>>
>>The examples you placed here are not tranport specific. They 
>>are just HBA values, and should stay under the host attributes.
> 
> 
> AFAIU Transport class is basically to hold all private attributes of the
> iSCSI driver.

That is wrong. Look at the qla2xxx or 53c700 driver. The transport
classes represent transport specifics. They are not there for a
driver that is a particular transport to place all its attributes, and
each driver that is a particular transport does not implement its
own class.

> Since iSCSI driver has attributes which are hba/target/lun specific it would
> be good to have them in a place away from the generic scsi attributes.
> For eg, the "shutdown" attribute is a iSCSI attribute to shutdown the iSCSI
> hba.

shutdown is a hba thing, and has nothing to do with iscsi itself.

> Similarly host_id is iSCSI transport specific and gives the host number.

How is the struct scsi_host's host_no related to the transport?
It is assigned from the SCSI layer.

>>Also, the host layout here does not look right. What happens 
>>when I have multiple HBAs?
> 
> Right now iSCSI driver supports single HBA, hence this representation is OK.
> However in multiple HBA case, it can be represented as below.

Again, only specific to your driver.

>  /sys/class/iscsi_transport/
>    |--host1/
>        |-- shutdown_hba
>        |-- show_hba_id
>    |--host2/
>        |-- ---"----
>        |-- ---"----
> 
>   
> 
>>>  /* iSCSI target specific attributes */
>>>  |-- 1:0:1/
>>>           |-- target_addr
>>>           |-- target_name
>>>           |-- target_port
>>>           |-- ....
>>
>>I agree these would be nice under a target attribute. Did
>>you have something to post since the last time we talked,
>>or did you write the example by hand?
> 
> 
> I have written the example by hand, I am working towards implementing it.
>>>  /* lun specific attributes */
>>>  |-- 1:0:1:0/
>>>             |-- device -> ../../../devices/platform/iscsi/1:0:0:0
>>>             |-- driver -> ../../../bus/scsi/drivers/sd
>>>             |-- lun_status
>>>             |-- ....
>>
>>I don't think anything in lun_status is iscsi specific.
> 
> 
> We have lun specific attributes defined in iSCSI driver some of which can be
> used in debug mode for testing purposes e.g initiate command timeout on a
> lun, set lun queue as busy, query the lun status etc.
> I think since they are private to iSCSI, this is the right place holder.

Again, specific to your particular iscsi driver vs transport.

>>> This way the relevant attributes can be used from respective paths.
>>
>>I get your point (you just chose the above attributes based 
>>on name to accentuate this right). However, in your example 
>>it seems like you would want to show the parent-children 
>>relationships like
>>here:
>>
>>/sys/class/iscsi_transport_host
>>|-- host1
>>|   |-- a_host_attribute
>>|   |-- 1:0:1
>>|   |   |-- target_name
>>|   |   |-- 1:0:1:0
>>|   |   |   |- some_lun_attribute
>>
>>Or you could also do something like
>>
>>/sys/class/iscsi_transport_host
>>|-- host1
>>|   |- a_host_attribute
>>|   |- 1:0:1 -> ../iscsi_transport_target/1:0:1
>>|   `- 1:0:2 -> ../iscsi_transport_target/1:0:2
>>/sys/class/iscsi_transport_target
>>|-- 1:0:1
>>|   |- target_name
>>|   |- ../iscsi_transport_lun/1:0:1:0
>>........
>>
>>Making /sys/class/iscsi_transport the parent for everything 
>>just does not look right.
> 
> 
> In the above example do you mean that iscsi_transport_target is registered
> as 
> another transport class along with iscsi_transport_host? Is this OK ?
> 

Your proposed layout is messy. That is all I am saying.


Mike

Re: [PATCH][RFC] iSCSI transport class

[Mike Christie]

Mike Christie wrote:

> Hey,
> 
> Surekha.PC wrote:
> 
>>  Hi Mike,
>>
>>   Sorry for the delay.
>>   The structure which I represented was meant to have iSCSI transport
>> attributes
>>   under their respective category. Right now, in our iSCSI driver all
>> attributes are defined
>>   in `iscsi-attr.c` and `iscsi-debug-attr.c` which need to be either 
>> per lun
>> or per target.
>>
>>>> I have reviewed your changes for iSCSI transport class. 
>>>
>>>
>>> The changes
>>>
>>>> look fine.  I have few suggestions on further improvements. The 
>>>> attributes can be organised
>>>> under iSCSI transport class in the following structure.
>>>>
>>>> # ls /sys/class/iscsi_transport/*
>>>>
>>>>  /* iSCSI host specific attributes */
>>>>  |-- shutdown
>>>>  |-- host_id
>>>>  |-- ....
>>>
>>>
>>> The examples you placed here are not tranport specific. They are just 
>>> HBA values, and should stay under the host attributes.
>>
>>
>>
>> AFAIU Transport class is basically to hold all private attributes of the
>> iSCSI driver.
> 
> 
> That is wrong. Look at the qla2xxx or 53c700 driver. The transport
> classes represent transport specifics. They are not there for a
> driver that is a particular transport to place all its attributes, and
> each driver that is a particular transport does not implement its
> own class.

Let me clarify what I wrote. I am just saying becuase Cisco's software 
iscsi driver has some attribute which a fc or spi or whatever driver 
lacks does not necessarily mean it is transport related. Some of your 
attributes are HBA or device related, and some are a result of your 
implmentation, and some are iscsi related.

>> Since iSCSI driver has attributes which are hba/target/lun specific it 
>> would
>> be good to have them in a place away from the generic scsi attributes.
>> For eg, the "shutdown" attribute is a iSCSI attribute to shutdown the 
>> iSCSI
>> hba.
> 
> 
> shutdown is a hba thing, and has nothing to do with iscsi itself.
> 
>> Similarly host_id is iSCSI transport specific and gives the host number.
> 
> 
> How is the struct scsi_host's host_no related to the transport?
> It is assigned from the SCSI layer.
> 
>>> Also, the host layout here does not look right. What happens when I 
>>> have multiple HBAs?
>>
>>
>> Right now iSCSI driver supports single HBA, hence this representation 
>> is OK.
>> However in multiple HBA case, it can be represented as below.
> 
> 
> Again, only specific to your driver.
> 
>>  /sys/class/iscsi_transport/
>>    |--host1/
>>        |-- shutdown_hba
>>        |-- show_hba_id
>>    |--host2/
>>        |-- ---"----
>>        |-- ---"----
>>
>>  
>>
>>>>  /* iSCSI target specific attributes */
>>>>  |-- 1:0:1/
>>>>           |-- target_addr
>>>>           |-- target_name
>>>>           |-- target_port
>>>>           |-- ....
>>>
>>>
>>> I agree these would be nice under a target attribute. Did
>>> you have something to post since the last time we talked,
>>> or did you write the example by hand?
>>
>>
>>
>> I have written the example by hand, I am working towards implementing it.
>>
>>>>  /* lun specific attributes */
>>>>  |-- 1:0:1:0/
>>>>             |-- device -> ../../../devices/platform/iscsi/1:0:0:0
>>>>             |-- driver -> ../../../bus/scsi/drivers/sd
>>>>             |-- lun_status
>>>>             |-- ....
>>>
>>>
>>> I don't think anything in lun_status is iscsi specific.
>>
>>
>>
>> We have lun specific attributes defined in iSCSI driver some of which 
>> can be
>> used in debug mode for testing purposes e.g initiate command timeout on a
>> lun, set lun queue as busy, query the lun status etc.
>> I think since they are private to iSCSI, this is the right place holder.
> 
> 
> Again, specific to your particular iscsi driver vs transport.

Just another clarification. I am not saying everying you are doing at 
the lun level is not iscsi specific.

RE: [PATCH][RFC] iSCSI transport class

[Heiko Carstens]

Hi,

> > I get your point (you just chose the above attributes based 
> > on name to accentuate this right). However, in your example 
> > it seems like you would want to show the parent-children 
> > relationships like
> > here:
> > 
> > /sys/class/iscsi_transport_host
> > |-- host1
> > |   |-- a_host_attribute
> > |   |-- 1:0:1
> > |   |   |-- target_name
> > |   |   |-- 1:0:1:0
> > |   |   |   |- some_lun_attribute
> > 
> > Or you could also do something like
> > 
> > /sys/class/iscsi_transport_host
> > |-- host1
> > |   |- a_host_attribute
> > |   |- 1:0:1 -> ../iscsi_transport_target/1:0:1
> > |   `- 1:0:2 -> ../iscsi_transport_target/1:0:2
> > /sys/class/iscsi_transport_target
> > |-- 1:0:1
> > |   |- target_name
> > |   |- ../iscsi_transport_lun/1:0:1:0

Looking at all the different transport classes recently
introduced, I don't think this is the way to go. The
sysfs layout for scsi devices gets too complicated in my
opinion.

To give you another example for an ugly sysfs layout,
caused by the lack of a generic solution, here is what
we do within the zfcp lldd:

/sys/devices/css0/0.0.000e/0.0.50dc
|-- <zfcp lldd hba specific attributes>
|-- host0
|   |- <host_attributes>
|   |- 0:0:1:0
|   .  |- <lun attributes>
|   .
|
|-- <wwpn0> (this and stuff below created by zfcp lldd)
|   |-- <fc port attributes>
|   |-- <zfcp lldd specific fc port attributes>
|   |-- <fcp_lun0>
|   .    |-- <fcp lun attributes>
|   .    |-- <zfcp lldd specific fcp lun attributes>
|
|-- <wwpn1>
.   .
.   .

So with using the fc transport class we would end up
with even another place where one has to look for things:

/sys/class/fc_transport/0:0:1:0


Now that there are transport classes available, the fc port
and lun attributes should move there. But we still have quite
a bunch of zfcp lldd specific fc port and lun attributes that
don't fit into any other place. It would be nice to have a
common place for all the different attributes, i.e. lldd
attributes, midlayer attributes and transport attributes.

>From my point of view it would be better to go for a simple
hierarchical structure like this:

/sys/devices/<hba0>
             |-- <host0>
             .   |- <scsi midlayer attributes>
             .   |- <transport attributes>
             .   |- <lldd attributes>
                 |- <bus0>
                 .  |- <scsi midlayer attributes>
                 .  |- <transport attributes>
                 .  |- <lldd attributes>
                    |- <target0>
                    .  |- <scsi midlayer attributes>
                    .  |- <transport attributes>
                    .  |- <lldd attributes>
                       |- <lun0>
                       .  |- <scsi midlayer attributes>
                       .  |- <transport attributes>
                       .  |- <lldd attributes>

In addition I can think of getting rid of hostnumber,
or is it really needed for anything?

Any comments/flames?

Heiko