From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Nicholas A. Bellinger" Subject: Re: [PATCH] ibmvscsis: Move to struct ibmvscsis_vdev usage in fabric configfs handlers Date: Mon, 01 Nov 2010 14:37:42 -0700 Message-ID: <1288647462.7708.31.camel@haakon2.linux-iscsi.org> References: <1288480434-29670-1-git-send-email-nab@linux-iscsi.org> <20101102060030Z.fujita.tomonori@lab.ntt.co.jp> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Return-path: Received: from mail.linux-iscsi.org ([67.23.28.174]:36606 "EHLO linux-iscsi.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750754Ab0KAVmt (ORCPT ); Mon, 1 Nov 2010 17:42:49 -0400 In-Reply-To: <20101102060030Z.fujita.tomonori@lab.ntt.co.jp> Sender: linux-scsi-owner@vger.kernel.org List-Id: linux-scsi@vger.kernel.org To: FUJITA Tomonori Cc: linux-scsi@vger.kernel.org, brking@linux.vnet.ibm.com, lxie@us.ibm.com, rcjenn@us.ibm.com, michaelc@cs.wisc.edu, James.Bottomley@suse.de, Joel Becker On Tue, 2010-11-02 at 06:05 +0900, FUJITA Tomonori wrote: > On Sat, 30 Oct 2010 16:13:54 -0700 > "Nicholas A. Bellinger" wrote: > > > From: Nicholas Bellinger > > > > 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