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" <h...@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/target2: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.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@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)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
