Re: [PATCH][RFC] iSCSI transport class

[Mike Christie <michaelc@cs.wisc.edu>]

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