Hi Hannes,
Thank you for reviewing patches. Please find responses inline.
On 13/04/15 7:59 pm, "Hannes Reinecke" <hare@xxxxxxx> 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@xxxxxxx> 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@xxxxxxx +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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
