Re: [PATCH v4 4/9] snic:Add snic target discovery

["Narsimhulu Musini (nmusini)" <nmusini@cisco.com>]

Hi Hannes,
    Thank you for reviewing patches. Please find responses inline.

On 13/04/15 7:59 pm, "Hannes Reinecke" <hare@suse.de> wrote:

>On 04/13/2015 07:25 AM, Narsimhulu Musini (nmusini) wrote:
>> Hi Hannes,
>> 
>>   Thank you for reviewing patches. Please find responses inline.
>> 
>> 
>> 
>> On 09/04/15 6:29 pm, "Hannes Reinecke" <hare@suse.de> wrote:
>> 
>>>
>>> On 04/09/2015 01:49 PM, Narsimhulu Musini wrote:
>[ .. ]
>>>> +/* snic_tgt_create: checks for existence of snic_tgt, if it doesn't
>>>> + * it creates one.
>>>> + */
>>>> +static struct snic_tgt *
>>>> +snic_tgt_create(struct snic *snic, struct snic_tgt_id *tgtid)
>>>> +{
>>>> +	struct snic_tgt *tgt = NULL;
>>>> +	unsigned long flags;
>>>> +	int ret;
>>>> +
>>>> +	tgt = snic_tgt_lookup(snic, tgtid);
>>>> +	if (tgt) {
>>>> +		/* update the information if required */
>>>> +		return tgt;
>>>> +	}
>>>> +
>>>> +	tgt = kzalloc(sizeof(*tgt), GFP_KERNEL);
>>>> +	if (!tgt) {
>>>> +		SNIC_HOST_ERR(snic->shost, "Failure to allocate snic_tgt.\n");
>>>> +		ret = -ENOMEM;
>>>> +
>>>> +		return tgt;
>>>> +	}
>>>> +
>>>> +	INIT_LIST_HEAD(&tgt->list);
>>>> +	tgt->id = tgtid->tgt_id;
>>>> +	tgt->channel = 0;
>>>> +
>>>> +	SNIC_BUG_ON(tgtid->tgt_type > SNIC_TGT_SAN);
>>>> +	tgt->tdata.typ = tgtid->tgt_type;
>>>> +
>>>> +	/*
>>>> +	 * Plugging into SML Device Tree
>>>> +	 */
>>>> +	tgt->tdata.disc_id = 0;
>>>> +	tgt->state = SNIC_TGT_STAT_INIT;
>>>> +	device_initialize(&tgt->dev);
>>>> +	tgt->dev.parent = get_device(&snic->shost->shost_gendev);
>>>> +	tgt->dev.release = snic_tgt_dev_release;
>>> Why do you use your own scsi target instantiation here?
>>> If it's equivalent to the scsi target than you should rather use the
>>> 'scsi_target' structure here and attach driver-specific information
>>> to either hostdata or starget_data.
>> I got your idea, we followed an approach similar to fc
>>(fc_rport_create()
>> in scsi_transport_fc.c).
>> Both are valid approaches. We opted for current approach, and gone
>>through
>> good amount of testing.
>> I would like to keep the way it is. Please share your thoughts.
>
>scsi_transport_fc() creates its own sysfs hierarchy for the remote
>port, which is inserted between scsi_host and scsi_target:
>
>/sys/devices/pci0000:00/0000:00:03.0/0000:07:00.3/host2/rport-2:0-13/targe
>t2:0:11/2:0:11:7
>
>here, the fc_rport structure corresponds to the FC sysfs hierarchy,
>containing all FC-related sysfs attributes. And as such makes sense
>have a distinct object.
>
>In your case no such hiearchy exists, and there are no additional
>sysfs attributes which would warrant the creation of a distinct object.
We will add sysfs attributes for targets in future patches. And about
hierarchy, The current approach provides the option, which we would like
to avail in future.
>
>Cheers,
>
>Hannes
>-- 
>Dr. Hannes Reinecke		               zSeries & Storage
>hare@suse.de			               +49 911 74053 688
>SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
>GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
>HRB 21284 (AG Nürnberg)
Thanks
simha
>

--
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