Re: [PATCH 1/2] scsi_transport_fc: implement 'disable_target_scan' module parameter

[Hannes Reinecke <hare@suse.de>]

On 03/01/2016 09:12 PM, Christoph Hellwig wrote:
> On Mon, Feb 22, 2016 at 08:51:01AM +0100, Hannes Reinecke wrote:
>>  /*
>> + * disable_target_scan: Disable target scan per default
>> + *   useful on larger installations where only a small
>> + *   number of LUNs are required for booting.
>> + */
>> +static bool fc_disable_target_scan;
>> +
>> +module_param_named(disable_target_scan, fc_disable_target_scan,
>> +		   bool, S_IRUGO|S_IWUSR);
>> +MODULE_PARM_DESC(disable_target_scan,
>> +		 "Disable target scan on remote ports (default=0)");
>> +
>> +/*
>>   * Redefine so that we can have same named attributes in the
>>   * sdev/starget/host objects.
>>   */
>> @@ -3272,10 +3284,14 @@ fc_scsi_scan_rport(struct work_struct *work)
>>  	struct Scsi_Host *shost = rport_to_shost(rport);
>>  	struct fc_internal *i = to_fc_internal(shost->transportt);
>>  	unsigned long flags;
>> +	bool disable_target_scan;
>> +
>> +	disable_target_scan = fc_disable_target_scan ?
>> +		fc_disable_target_scan : i->f->disable_target_scan;
>>  
>>  	if ((rport->port_state == FC_PORTSTATE_ONLINE) &&
>>  	    (rport->roles & FC_PORT_ROLE_FCP_TARGET) &&
>> -	    !(i->f->disable_target_scan)) {
>> +	    !disable_target_scan) {
> 
> Wouldn't this be simpler by just writing:
> 
> if ((rport->port_state == FC_PORTSTATE_ONLINE) &&
>     (rport->roles & FC_PORT_ROLE_FCP_TARGET) &&
>     !i->f->disable_target_scan && !disable_target_scan) {
> 
> that being said I don't understand why we need this in the FC transport
> class.  What is so special about FC that we do it here and not globally?
> 
Main reasons being that
a) it's most commonly on FC where you end up having thousands of LUNs;
SAS domains are of finite size and you'd need to string quite a few SAS
expanders together to achieve a similar setup. And iSCSI is ... well.
b) FC already has the required infrastructure in place where you easily
can suppress a target scan, making the patch really trivial.

So rather than trying to come up with a generic solution and an overly
complicated patch I opted for the simpler version :-)

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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