List Info

Thread: scsi_dh: Add 'dh_state' sysfs attribute




scsi_dh: Add 'dh_state' sysfs attribute
country flaguser name
Germany
2008-05-20 09:05:30
Implement a 'dh_state' sdev attribute for dynamic device
handler
manipulation. A read on the attribute will return the name
of
the currently attached device handler or 'detached' if no
handler
is attached.
The attribute allows the following strings to be written:
- The name of the device handler to be attached if the state
is
  'detached'.
- 'activate' to trigger path activation if a device handler
  is attached.
- 'detach' to detach the currently attached device handler.

Signed-off-by: Hannes Reinecke <haresuse.de>
---
 drivers/scsi/device_handler/scsi_dh.c |  107
++++++++++++++++++++++++++++++++-
 1 files changed, 105 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh.c
b/drivers/scsi/device_handler/scsi_dh.c
index 0b5c457..b80fae7 100644
--- a/drivers/scsi/device_handler/scsi_dh.c
+++ b/drivers/scsi/device_handler/scsi_dh.c
 -79,6
+79,98  static void scsi_dh_handler_detach(struct
scsi_device *sdev)
 }
 
 /*
+ * Functions for sysfs attribute 'dh_state'
+ */
+static ssize_t
+store_dh_state(struct device *dev, struct device_attribute
*attr,
+	       const char *buf, size_t count)
+{
+	struct scsi_device *sdev = to_scsi_device(dev);
+	struct scsi_device_handler *scsi_dh;
+	int err = -EINVAL;
+
+	if (!sdev->scsi_dh_data) {
+		/*
+		 * Attach to a device handler
+		 */
+		if (!(scsi_dh = get_device_handler(buf)))
+			return err;
+		err = scsi_dh_handler_attach(sdev, scsi_dh);
+	} else {
+		if (!strncmp(buf, "detach", 6)) {
+			/*
+			 * Detach from a device handler
+			 */
+			scsi_dh_handler_detach(sdev);
+			err = 0;
+		} else if (!strncmp(buf, "activate", 8)) {
+			/*
+			 * Activate a device handler
+			 */
+			scsi_dh = sdev->scsi_dh_data->scsi_dh;
+			if (scsi_dh->activate)
+				err = scsi_dh->activate(sdev);
+			else
+				err = 0;
+		}
+	}
+
+	return err<0?err:count;
+}
+
+static ssize_t
+show_dh_state(struct device *dev, struct device_attribute
*attr, char *buf)
+{
+	struct scsi_device *sdev = to_scsi_device(dev);
+
+	if (!sdev->scsi_dh_data)
+		return snprintf(buf, 20, "detachedn");
+
+	return snprintf(buf, 20, "%sn",
sdev->scsi_dh_data->scsi_dh->name);
+}
+
+static struct device_attribute scsi_dh_state_attr =
+	__ATTR(dh_state, S_IRUGO | S_IWUSR, show_dh_state,
+	       store_dh_state);
+
+/*
+ * scsi_dh_sysfs_attr_add - Callback for scsi_init_dh
+ */
+static int scsi_dh_sysfs_attr_add(struct device *dev, void
*data)
+{
+	struct scsi_device *sdev;
+	int err;
+
+	if (!scsi_is_sdev_device(dev))
+		return 0;
+
+	sdev = to_scsi_device(dev);
+
+	err = device_create_file(&sdev->sdev_gendev,
+				 &scsi_dh_state_attr);
+
+	return 0;
+}
+
+/*
+ * scsi_dh_sysfs_attr_remove - Callback for scsi_exit_dh
+ */
+static int scsi_dh_sysfs_attr_remove(struct device *dev,
void *data)
+{
+	struct scsi_device *sdev;
+
+	if (!scsi_is_sdev_device(dev))
+		return 0;
+
+	sdev = to_scsi_device(dev);
+
+	device_remove_file(&sdev->sdev_gendev,
+			   &scsi_dh_state_attr);
+
+	return 0;
+}
+
+/*
  * scsi_dh_notifier - notifier chain callback
  */
 static int scsi_dh_notifier(struct notifier_block *nb,
 -86,7
+178,7  static int scsi_dh_notifier(struct notifier_block
*nb,
 {
 	struct device *dev = data;
 	struct scsi_device *sdev;
-	int i, err = 0;
+	int i;
 	struct scsi_device_handler *tmp, *devinfo = NULL;
 
 	if (!scsi_is_sdev_device(dev))
 -114,14
+206,19  static int scsi_dh_notifier(struct notifier_block
*nb,
 		goto out;
 
 	if (action == BUS_NOTIFY_ADD_DEVICE) {
+		int err;
+
 		err = scsi_dh_handler_attach(sdev, devinfo);
+		if (!err)
+			err = device_create_file(dev, &scsi_dh_state_attr);
 	} else if (action == BUS_NOTIFY_DEL_DEVICE) {
 		if (sdev->scsi_dh_data == NULL)
 			goto out;
+		device_remove_file(dev, &scsi_dh_state_attr);
 		scsi_dh_handler_detach(sdev);
 	}
 out:
-	return err;
+	return 0;
 }
 
 /*
 -275,11
+372,17  static int __init scsi_dh_init(void)
 
 	r = bus_register_notifier(&scsi_bus_type,
&scsi_dh_nb);
 
+	if (!r)
+		bus_for_each_dev(&scsi_bus_type, NULL, NULL,
+				 scsi_dh_sysfs_attr_add);
+
 	return r;
 }
 
 static void __exit scsi_dh_exit(void)
 {
+	bus_for_each_dev(&scsi_bus_type, NULL, NULL,
+			 scsi_dh_sysfs_attr_remove);
 	bus_unregister_notifier(&scsi_bus_type,
&scsi_dh_nb);
 }
 
-- 
1.5.2.4

--
dm-devel mailing list
dm-develredhat.com
http
s://www.redhat.com/mailman/listinfo/dm-devel

Re: scsi_dh: Add 'dh_state' sysfs attribute
country flaguser name
United States
2008-05-22 21:08:20
On Tue, 2008-05-20 at 16:05 +0200, Hannes Reinecke wrote:
> Implement a 'dh_state' sdev attribute for dynamic
device handler
> manipulation. A read on the attribute will return the
name of
> the currently attached device handler or 'detached' if
no handler
> is attached.
> The attribute allows the following strings to be
written:
> - The name of the device handler to be attached if the
state is
>   'detached'.
> - 'activate' to trigger path activation if a device
handler
>   is attached.
> - 'detach' to detach the currently attached device
handler.
> 
> Signed-off-by: Hannes Reinecke <haresuse.de>
> ---
>  drivers/scsi/device_handler/scsi_dh.c |  107
++++++++++++++++++++++++++++++++-
>  1 files changed, 105 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh.c
b/drivers/scsi/device_handler/scsi_dh.c
> index 0b5c457..b80fae7 100644
> --- a/drivers/scsi/device_handler/scsi_dh.c
> +++ b/drivers/scsi/device_handler/scsi_dh.c
>  -79,6 +79,98  static void scsi_dh_handler_detach(struct
scsi_device *sdev)
>  }
> 
>  /*
> + * Functions for sysfs attribute 'dh_state'
> + */
> +static ssize_t
> +store_dh_state(struct device *dev, struct
device_attribute *attr,
> +	       const char *buf, size_t count)
> +{
> +	struct scsi_device *sdev = to_scsi_device(dev);
> +	struct scsi_device_handler *scsi_dh;
> +	int err = -EINVAL;
> +
> +	if (!sdev->scsi_dh_data) {
> +		/*
> +		 * Attach to a device handler
> +		 */
> +		if (!(scsi_dh = get_device_handler(buf)))
> +			return err;
> +		err = scsi_dh_handler_attach(sdev, scsi_dh);
> +	} else {
> +		if (!strncmp(buf, "detach", 6)) {
> +			/*
> +			 * Detach from a device handler
> +			 */
> +			scsi_dh_handler_detach(sdev);
> +			err = 0;
> +		} else if (!strncmp(buf, "activate", 8))
{
> +			/*
> +			 * Activate a device handler
> +			 */
> +			scsi_dh = sdev->scsi_dh_data->scsi_dh;
> +			if (scsi_dh->activate)
> +				err = scsi_dh->activate(sdev);
> +			else
> +				err = 0;

Sorry for the late comment. Why do we want to give the user
to be able
to activate a path ? (I am assuming user as we do not want
to be calling
this from multipath tools).

> +		}
> +	}
> +
> +	return err<0?err:count;
> +}
> +
> +static ssize_t
> +show_dh_state(struct device *dev, struct
device_attribute *attr, char *buf)
> +{
> +	struct scsi_device *sdev = to_scsi_device(dev);
> +
> +	if (!sdev->scsi_dh_data)
> +		return snprintf(buf, 20, "detachedn");
> +
> +	return snprintf(buf, 20, "%sn",
sdev->scsi_dh_data->scsi_dh->name);
> +}
> +
> +static struct device_attribute scsi_dh_state_attr =
> +	__ATTR(dh_state, S_IRUGO | S_IWUSR, show_dh_state,
> +	       store_dh_state);
> +
> +/*
> + * scsi_dh_sysfs_attr_add - Callback for scsi_init_dh
> + */
> +static int scsi_dh_sysfs_attr_add(struct device *dev,
void *data)
> +{
> +	struct scsi_device *sdev;
> +	int err;
> +
> +	if (!scsi_is_sdev_device(dev))
> +		return 0;
> +
> +	sdev = to_scsi_device(dev);
> +
> +	err = device_create_file(&sdev->sdev_gendev,
> +				 &scsi_dh_state_attr);
> +
> +	return 0;
> +}
> +
> +/*
> + * scsi_dh_sysfs_attr_remove - Callback for
scsi_exit_dh
> + */
> +static int scsi_dh_sysfs_attr_remove(struct device
*dev, void *data)
> +{
> +	struct scsi_device *sdev;
> +
> +	if (!scsi_is_sdev_device(dev))
> +		return 0;
> +
> +	sdev = to_scsi_device(dev);
> +
> +	device_remove_file(&sdev->sdev_gendev,
> +			   &scsi_dh_state_attr);
> +
> +	return 0;
> +}
> +
> +/*
>   * scsi_dh_notifier - notifier chain callback
>   */
>  static int scsi_dh_notifier(struct notifier_block
*nb,
>  -86,7 +178,7  static int scsi_dh_notifier(struct
notifier_block *nb,
>  {
>  	struct device *dev = data;
>  	struct scsi_device *sdev;
> -	int i, err = 0;
> +	int i;
>  	struct scsi_device_handler *tmp, *devinfo = NULL;
> 
>  	if (!scsi_is_sdev_device(dev))
>  -114,14 +206,19  static int
scsi_dh_notifier(struct notifier_block *nb,
>  		goto out;
> 
>  	if (action == BUS_NOTIFY_ADD_DEVICE) {
> +		int err;
> +
>  		err = scsi_dh_handler_attach(sdev, devinfo);
> +		if (!err)
> +			err = device_create_file(dev,
&scsi_dh_state_attr);

we don't want to detach when we fail creating the file ? I
am ok with
it, but just wanted to make sure that is what you intended.

>  	} else if (action == BUS_NOTIFY_DEL_DEVICE) {
>  		if (sdev->scsi_dh_data == NULL)
>  			goto out;
> +		device_remove_file(dev, &scsi_dh_state_attr);
>  		scsi_dh_handler_detach(sdev);
>  	}
>  out:
> -	return err;
> +	return 0;

why are we not returning err ?
>  }
> 
>  /*
>  -275,11 +372,17  static int __init
scsi_dh_init(void)
> 
>  	r = bus_register_notifier(&scsi_bus_type,
&scsi_dh_nb);
> 
> +	if (!r)
> +		bus_for_each_dev(&scsi_bus_type, NULL, NULL,
> +				 scsi_dh_sysfs_attr_add);
> +
>  	return r;
>  }
> 
>  static void __exit scsi_dh_exit(void)
>  {
> +	bus_for_each_dev(&scsi_bus_type, NULL, NULL,
> +			 scsi_dh_sysfs_attr_remove);
>  	bus_unregister_notifier(&scsi_bus_type,
&scsi_dh_nb);
>  }
> 

--
dm-devel mailing list
dm-develredhat.com
http
s://www.redhat.com/mailman/listinfo/dm-devel

Re: scsi_dh: Add 'dh_state' sysfs attribute
country flaguser name
Germany
2008-05-23 06:23:58
On Thu, May 22, 2008 at 07:08:20PM -0700, Chandra
Seetharaman wrote:
> 
> On Tue, 2008-05-20 at 16:05 +0200, Hannes Reinecke
wrote:
> > Implement a 'dh_state' sdev attribute for dynamic
device handler
> > manipulation. A read on the attribute will return
the name of
> > the currently attached device handler or
'detached' if no handler
> > is attached.
> > The attribute allows the following strings to be
written:
> > - The name of the device handler to be attached if
the state is
> >   'detached'.
> > - 'activate' to trigger path activation if a
device handler
> >   is attached.
> > - 'detach' to detach the currently attached device
handler.
> > 
> > Signed-off-by: Hannes Reinecke <haresuse.de>
> > ---
> >  drivers/scsi/device_handler/scsi_dh.c |  107
++++++++++++++++++++++++++++++++-
> >  1 files changed, 105 insertions(+), 2
deletions(-)
> > 
> > diff --git a/drivers/scsi/device_handler/scsi_dh.c
b/drivers/scsi/device_handler/scsi_dh.c
> > index 0b5c457..b80fae7 100644
> > --- a/drivers/scsi/device_handler/scsi_dh.c
> > +++ b/drivers/scsi/device_handler/scsi_dh.c
> >  -79,6 +79,98  static void scsi_dh_handler_detach(struct
scsi_device *sdev)
> >  }
> > 
> >  /*
> > + * Functions for sysfs attribute 'dh_state'
> > + */
> > +static ssize_t
> > +store_dh_state(struct device *dev, struct
device_attribute *attr,
> > +	       const char *buf, size_t count)
> > +{
> > +	struct scsi_device *sdev = to_scsi_device(dev);
> > +	struct scsi_device_handler *scsi_dh;
> > +	int err = -EINVAL;
> > +
> > +	if (!sdev->scsi_dh_data) {
> > +		/*
> > +		 * Attach to a device handler
> > +		 */
> > +		if (!(scsi_dh = get_device_handler(buf)))
> > +			return err;
> > +		err = scsi_dh_handler_attach(sdev, scsi_dh);
> > +	} else {
> > +		if (!strncmp(buf, "detach", 6)) {
> > +			/*
> > +			 * Detach from a device handler
> > +			 */
> > +			scsi_dh_handler_detach(sdev);
> > +			err = 0;
> > +		} else if (!strncmp(buf, "activate",
8)) {
> > +			/*
> > +			 * Activate a device handler
> > +			 */
> > +			scsi_dh = sdev->scsi_dh_data->scsi_dh;
> > +			if (scsi_dh->activate)
> > +				err = scsi_dh->activate(sdev);
> > +			else
> > +				err = 0;
> 
> Sorry for the late comment. Why do we want to give the
user to be able
> to activate a path ? (I am assuming user as we do not
want to be calling
> this from multipath tools).
> 
This is quite a handy feature if you want to test failover
manually (ie
if the commands are being sent correctly and the array
reacts properly).
And, of course, there might be scenarios where you'd want to
trigger
a failover manually.

[ .. ]
> >  -114,14 +206,19  static int
scsi_dh_notifier(struct notifier_block *nb,
> >  		goto out;
> > 
> >  	if (action == BUS_NOTIFY_ADD_DEVICE) {
> > +		int err;
> > +
> >  		err = scsi_dh_handler_attach(sdev, devinfo);
> > +		if (!err)
> > +			err = device_create_file(dev,
&scsi_dh_state_attr);
> 
> we don't want to detach when we fail creating the file
? I am ok with
> it, but just wanted to make sure that is what you
intended.
> 
The 'dh_state' attribute is just an add-on and not necessary
for proper
functionality. So I thought we could just ignore this
error.
But seeing that this indicates some general error condition
it's probably
better to at least return this error.

> >  	} else if (action == BUS_NOTIFY_DEL_DEVICE) {
> >  		if (sdev->scsi_dh_data == NULL)
> >  			goto out;
> > +		device_remove_file(dev,
&scsi_dh_state_attr);
> >  		scsi_dh_handler_detach(sdev);
> >  	}
> >  out:
> > -	return err;
> > +	return 0;
> 
> why are we not returning err ?
See above. 

Fixed in the scsi_dh branch.

Cheers,

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

--
dm-devel mailing list
dm-develredhat.com
http
s://www.redhat.com/mailman/listinfo/dm-devel

[1-3]

about | contact  Other archives ( Real Estate discussion Medical topics )