On Wed, 13 Apr 2005, Christoph Hellwig wrote:
> > atomic_set(&fcport->state, FCS_ONLINE);
> > + if (ha->flags.init_done)
> > + qla2x00_reg_remote_port(ha, fcport);
> > }
>
> ...
>
> > - goto probe_failed;
> > + goto probe_alloc_failed;
> > }
> >
> > + pci_set_drvdata(pdev, ha);
> > + host->this_id = 255;
> > + host->cmd_per_lun = 3;
> > + host->unique_id = ha->instance;
> > + host->max_cmd_len = MAX_CMDSZ;
> > + host->max_channel = ha->ports - 1;
> > + host->max_id = ha->max_targets;
> > + host->max_lun = ha->max_luns;
> > + host->transportt = qla2xxx_transport_template;
> > + if (scsi_add_host(host, &pdev->dev))
> > + goto probe_alloc_failed;
> > +
> > + qla2x00_alloc_sysfs_attr(ha);
> > +
> > if (qla2x00_initialize_adapter(ha) &&
> > !(ha->device_flags & DFLG_NO_CABLE)) {
>
> Now this I don't undersant. You're moving the host registration earlier,
> maybe too earlier but I haven't checked that yet,
>
Yeah, that hunk is a residual of some other (trashy) changes I made
during early fc_rport integration and really should be reverted back
to the original...
> why do you still need
> the special case for delaying registration of the targets?
>
Ok, this is actually a concern I have with the current fc_rport
implementation (and yes, I realize I'm a day late) -- the auto-scan
logic employed during the creation of the fc_rport (via
fc_remote_port_add()).
Before I get into that though, let me answer your question -- the
reason I post-register the ports is because the driver is not ready
(during init-time) to process I/O originating from queuecommand().
This topic was discussed briefly during the implementation of
fc_rport -- some suggestions, if I recall correctly, were to either
block the host or post-register the rport at an appropriate time.
Each options has its caveats -- issuing and fc_remote_port_add() while
the host is blocked of course forces the user to perform a manual
rescan at some later time. Post-registration on the otherhand still
requires some 'external' port structure (fc_port in the case of
qla2xxx) to exist until fc_rport creation and thus negating the
benefits and purpose of fc_rport->dd_data. And again, since scanning
occurs immediately for 'target-type' (roles) fc_rports, the
fc_rport->dd_data which one would like to use at slave_alloc() time is
unset (NULL).
Perhaps I'm missing something fundamental -- as I've glanced through
lpfc and see slave_alloc() doing linear scans through its known lists
of ports trying to determine the proper lpfc_nodelist, whereas a:
rport = fc_remote_port_add() -- would create and fc_rport
given the rport_identifiers.
rport->dd_data = <internal port data> -- caller populates
rport->dd_data with
internal driver
structure (i.e. fc_port)
fc_remote_port_scan(rport) -- initiate scan.
qla2xxx_slave_alloc() implementation would be something like:
struct fc_rport *rport = starget_to_rport(scsi_target(sdev));
fc_port_t *fcport;
if (!rport)
return -ENXIO;
fcport = (struct fc_port *) rport->dd_data;
if (!fcport)
return -ENXIO;
...
One possibility (as to not break current functionality) is to add an
addition role modifier, say FC_RPORT_ROLE_NO_SCAN and | it into
rport_ids.roles prior to fc_remote_port_add() to indicate no scanning
should take place during the call. Then once ready, the driver could
issue the corresponding fc_remote_port_scan() on the rport.
Looking ahead, this may also be useful in the case of port
authentication (i.e. FC-SP).
Does this sound like a reasonable extension? I'll code up something
in the morning if others are interested?
> Also please propagate the full error that scsi_add_host returned.
>
Sure, I post another set of patches to address this and the early
host-addition code.
Thanks,
AV
-
: 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
