[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]