Re: [PATCH 5/6] firewire: SBP-2 highlevel driver

[Stefan Richter <stefanr@s5r6.in-berlin.de>]

Kristian Høgsberg wrote:
> Christoph Hellwig wrote:
>>> +static int add_scsi_devices(struct fw_unit *unit)
>>> +{
>>> +    struct sbp2_device *sd = unit->device.driver_data;
>>> +    int retval, lun;
>>> +
>>> +    if (sd->scsi_host != NULL)
>>> +        return 0;
>>> +
>>> +    sd->scsi_host = scsi_host_alloc(&scsi_driver_template,
>>> +                    sizeof(unsigned long));
>>> +    if (sd->scsi_host == NULL) {
>>> +        fw_error("failed to register scsi host\n");
>>> +        return -1;
>>> +    }
>>> +
>>> +    sd->scsi_host->hostdata[0] = (unsigned long)unit;
>>
>> Please take a look ar ther other scsi drivers how this is supposed
>> to be used.
> 
> I was trying to be clever and only allocate the host once the device had
> been discovered and initialized.  I have now changed the code to just
> allocate the host up front and use the hostdata mechanism for the
> sbp2_device struct, which also addresses the host life cycle comments
> below.

I have doubts.  IMO the previous code is 100% correct as long as 1 SBP-2
target LU maps to 1 Scsi_Host.

The lifetime of the Scsi_Host would only be longer than that of the LU
if all LUs (or all LUs at the same initiator port) would be added beneath
the same instance of Scsi_Host.  Then the lifetime of the Scsi_Host would
be that of the fw-sbp2 driver, or that of fw-sbp2's representation of a
FireWire bus.

However, I'll see how it turns out in your update patch.
-- 
Stefan Richter
-=====-=-=== -=-= -=--=
http://arcgraph.de/sr/