Re: [PATCH]: create sysfs file, dh_state for SCSI devices even if they are not in the inteernal lists

[Hannes Reinecke <hare@suse.de>]

Chandra Seetharaman wrote:
> Hi Hannes,
> 
> Can you comment on this please.
> 
> If you are convinced with my justification, please send an ACK.
> 
> Thanks & Regards,
> 
> chandra
> On Mon, 2009-06-29 at 12:30 -0700, Chandra Seetharaman wrote:
>> On Mon, 2009-06-29 at 09:40 +0200, Hannes Reinecke wrote:
>>> Chandra Seetharaman wrote:
>>>> Create the sysfs file, dh_state even if the new SCSI device is not
>>>> in the any of the device handler's internal lists.
>>>>
>>>> Signed-Off-by: Chandra Seetharaman <sekharan@us.ibm.com>
>>>> ---
>>>>  drivers/scsi/device_handler/scsi_dh.c |   11 ++++-------
>>>>  1 file changed, 4 insertions(+), 7 deletions(-)
>>>>
>>>> Index: linux-2.6.31-rc1/drivers/scsi/device_handler/scsi_dh.c
>>>> ===================================================================
>>>> --- linux-2.6.31-rc1.orig/drivers/scsi/device_handler/scsi_dh.c
>>>> +++ linux-2.6.31-rc1/drivers/scsi/device_handler/scsi_dh.c
>>>> @@ -304,18 +304,15 @@ static int scsi_dh_notifier(struct notif
>>>>  	sdev = to_scsi_device(dev);
>>>>  
>>>>  	if (action == BUS_NOTIFY_ADD_DEVICE) {
>>>> +		err = device_create_file(dev, &scsi_dh_state_attr);
>>>> +		/* don't care about err */
>>>>  		devinfo = device_handler_match(NULL, sdev);
>>>> -		if (!devinfo)
>>>> -			goto out;
>>>> -
>>>> -		err = scsi_dh_handler_attach(sdev, devinfo);
>>>> -		if (!err)
>>>> -			err = device_create_file(dev, &scsi_dh_state_attr);
>>>> +		if (devinfo)
>>>> +			err = scsi_dh_handler_attach(sdev, devinfo);
>>>>  	} else if (action == BUS_NOTIFY_DEL_DEVICE) {
>>>>  		device_remove_file(dev, &scsi_dh_state_attr);
>>>>  		scsi_dh_handler_detach(sdev, NULL);
>>>>  	}
>>>> -out:
>>>>  	return err;
>>>>  }
>>>>  
>>>>
>>>>
>>>> --
>>>> dm-devel mailing list
>>>> dm-devel@redhat.com
>>>> https://www.redhat.com/mailman/listinfo/dm-devel
>>> NACK.
>>>
>>> This will create sysfs attributes even if the attach()
>>> failed for other reason like a generic error. So we'll end
>>> up with device handler attributes and no device handler attached.
>> we do not have to worry about if attach failed or succeeded. We will
>> have "detached" if there is no device handler is attached to a device. 
>>
>> Basically, existence of this file simply means that scsi_dh module is
>> active, only the contents of this file will indicate if any device
>> handler is attached or not.
>>
>> We are just creating the file dh_state for each SCSI device that exists
>> and that is the only way non-in-built devices can be attached to a
>> handler by the end-user.
>>
>> Do note that we do create this file for all the existing scsi devices
>> when the scsi_dh module is inserted (see scsi_dh_init()).
>>
Right. So you're changing the semantics from the 'dh_state' attribute
to mean 'scsi_dh' is active, not 'a scsi_dh module has been attached'.

Ok, that seems reasonable.

Acked-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)