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

[Chandra Seetharaman <sekharan@us.ibm.com>]

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()).

> 
> Not a good idea.

I do not agree, and we do need to have this fix for dh_state file to be
functionally useful.

> 
> Cheers,
> 
> Hannes