Re: [PATCH] ibmvscsis: Move to struct ibmvscsis_vdev usage in fabric configfs handlers

["Nicholas A. Bellinger" <nab@linux-iscsi.org>]

On Tue, 2010-11-02 at 06:05 +0900, FUJITA Tomonori wrote:
> On Sat, 30 Oct 2010 16:13:54 -0700
> "Nicholas A. Bellinger" <nab@linux-iscsi.org> wrote:
> 
> > From: Nicholas Bellinger <nab@linux-iscsi.org>
> > 
> > This patch takes tomo-san original commit 94fdb0196151 and changes a handful
> > of important items wrt to the fabric configfs logic.
> > 
> > Firstly, this patch introduces struct ibmvscsis_vdev and converts the
> > VIO ibmvscsis_probe() and ibmvscsi_remove() callers to allocate/free
> > struct ibmvscsis_vdev instead of the original usage of struct ibmvscsis_tpg
> > which is intended to be allocated/freed respectively in ibmvscsis_make_tpg()
> > and ibmvscsis_drop_tpg() fabric configfs handlers.
> 
> What happens if an initiator sends a crq command before an user
> creates a tpg? Or what happens if an initiator sends a crq command
> after removing a tpg?
> 

Hmmm, good point.  So this would require ibmvscsis_queuecommand() to
check internal some ibmvscsi_tpg state to determine availability, and
reject incoming I/O otherwise.  The other operation would be to just
move crq queue creation/release for individual ibmvscsi_vdev into
ibmvscsis_make_tpg() and ibmvscsis_drop_tpg().

I am suspecting the latter would bit slightly cleaner..

> 
> > Secondly, this patch changes the metadata that is used to determine the
> > fabric WWN and TPGT (collectively the TCM VIO SRP endpoint) layout for
> > /sys/kernel/config/target/ibmvscsis/$WWN/tpgt_$TPGT/.  This patch will
> > now use struct vio_dev->unit_address for $WWN, and dev_name(vio_dev->dev)
> > for $TPGT.
> 
> I don't care much about this but vio_dev->unit_address ==
> dev_name(vio_dev->dev), I think. See vio_register_device_node() in
> arch/powerpc/kernel/vio.c. So it's odd a bit.
> 

Hmmmm..

> ibmvscsis model doesn't strictly follow the SRP SCSI model; no port,
> wwpn, etc. 
> 

Correct, which means we need to find a value to build a WWPN that is
both unique to the individual POWER machine, and is persisent across
reports..

brking and Co, does the struct vio_dev->unit_address meet these two
criteria for a TCM WWPN..?  If not, is there anything else we could use
(say the system_id + guest specific ID) to emulate this..?

> 
> >  The values passed in via configff for $WWN and $TPGT must match
> > in ibmvscsis_make_tport() and ibmvscsis_make_tpg() against what has been
> > automatically setup for each struct ibmvscsis_vdev in the VIO ibmvscsis_probe()
> > callback.
> 
> As I said before, it's much better to create them automatically
> instead of creating them via sysfs. The driver knows all nexuses to
> handle.

Point well taken here, but as discussed there is not a sane way within
configfs to do this and I really do not think we loose anything by
driving the configuration via entirely in userspace.

>From the perspective of the enduser of this code we will ultimately want
to hide all these multiple userspace driven configfs ops.  Eg, present a
list to enduser of the available struct vio_devs through either current
sysfs attributes or /sys/kernel/config/target/ibvscsis/available_vdevs
that intrepreted userspace code, and is able to explictly enable the
specific TCM subsystem backend storage for each target mode VIO LPAR
regision for TCM/ibmvscsis operation.

Thanks!

--nab