The scsi_dh attribute allows for dynamic manipulation of the device
handler state. It supports manual attaching, detaching, and activating
of the device handler.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/device_handler/scsi_dh.c | 88 +++++++++++++++++++++++++++++++++
1 files changed, 88 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh.c b/drivers/scsi/device_handler/scsi_dh.c
index 55e5fd2..2dbf84b 100644
--- a/drivers/scsi/device_handler/scsi_dh.c
+++ b/drivers/scsi/device_handler/scsi_dh.c
@@ -54,6 +54,55 @@ static struct scsi_device_handler *get_device_handler(const char *name)
return found;
}

+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 {
+ /*
+ * Detach from a device handler
+ */
+ if (!strncmp(buf, "detach", 6)) {
+ err = scsi_dh_handler_detach(sdev);
+ } else if (!strncmp(buf, "activate", 8)) {
+ scsi_dh = sdev->scsi_dh_data->scsi_dh;
+
+ err = scsi_dh->activate(sdev);
+ }
+ }
+
+ 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);
+ struct scsi_device_handler *scsi_dh;
+
+ if (!sdev->scsi_dh_data)
+ return snprintf(buf, 20, "detached
");
+
+ scsi_dh = sdev->scsi_dh_data->scsi_dh;
+
+ return snprintf(buf, 20, "%s
", 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_handler_attach - Attach a device handler to a device
* @sdev - SCSI device the device handler should attach to
@@ -115,9 +164,11 @@ static int scsi_dh_notifier(struct notifier_block *nb,

if (action == BUS_NOTIFY_ADD_DEVICE) {
scsi_dh_handler_attach(sdev, devinfo->handler);
+ 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:
@@ -298,17 +349,54 @@ static struct notifier_block scsi_dh_nb = {
.notifier_call = scsi_dh_notifier
};

+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;
+}
+
static int __init scsi_dh_init(void)
{
int r;

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 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;
+}
+
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-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

On Wed, 2008-05-14 at 16:43 +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 <hare@suse.de>
> ---

All the sysfs functions can be grouped together. Helps in readability.

> drivers/scsi/device_handler/scsi_dh.c | 88 +++++++++++++++++++++++++++++++++
> 1 files changed, 88 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/scsi/device_handler/scsi_dh.c b/drivers/scsi/device_handler/scsi_dh.c
> index 55e5fd2..2dbf84b 100644
> --- a/drivers/scsi/device_handler/scsi_dh.c
> +++ b/drivers/scsi/device_handler/scsi_dh.c
> @@ -54,6 +54,55 @@ static struct scsi_device_handler *get_device_handler(const char *name)
> return found;
> }
>
> +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 {
> + /*
> + * Detach from a device handler
> + */
> + if (!strncmp(buf, "detach", 6)) {
> + err = scsi_dh_handler_detach(sdev);
> + } else if (!strncmp(buf, "activate", 8)) {
> + scsi_dh = sdev->scsi_dh_data->scsi_dh;
> +
> + err = scsi_dh->activate(sdev);
> + }
> + }
> +
> + 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);
> + struct scsi_device_handler *scsi_dh;
> +
> + if (!sdev->scsi_dh_data)
> + return snprintf(buf, 20, "detached
");
> +
> + scsi_dh = sdev->scsi_dh_data->scsi_dh;

we can use it directly instead of this variable ?

> +
> + return snprintf(buf, 20, "%s
", 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_handler_attach - Attach a device handler to a device
> * @sdev - SCSI device the device handler should attach to
> @@ -115,9 +164,11 @@ static int scsi_dh_notifier(struct notifier_block *nb,
>
> if (action == BUS_NOTIFY_ADD_DEVICE) {
> scsi_dh_handler_attach(sdev, devinfo->handler);
> + device_create_file(dev, &scsi_dh_state_attr);

A later patch checks for the return status of scsi_dh_handler_attach(),
should be moved here.

> } 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:
> @@ -298,17 +349,54 @@ static struct notifier_block scsi_dh_nb = {
> .notifier_call = scsi_dh_notifier
> };
>
> +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;
> +}
> +
> static int __init scsi_dh_init(void)
> {
> int r;
>
> 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 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;
> +}
> +
> 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-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Chandra Seetharaman wrote:

On Wed, 2008-05-14 at 16:43 +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 <hare@suse.de>
---


All the sysfs functions can be grouped together. Helps in readability.


?? But I did ... Or thought so, anyway. Checking.


drivers/scsi/device_handler/scsi_dh.c | 88 +++++++++++++++++++++++++++++++++
1 files changed, 88 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh.c b/drivers/scsi/device_handler/scsi_dh.c
index 55e5fd2..2dbf84b 100644
--- a/drivers/scsi/device_handler/scsi_dh.c
+++ b/drivers/scsi/device_handler/scsi_dh.c
@@ -54,6 +54,55 @@ static struct scsi_device_handler *get_device_handler(const char *name)
return found;
}

+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 {
+ /*
+ * Detach from a device handler
+ */
+ if (!strncmp(buf, "detach", 6)) {
+ err = scsi_dh_handler_detach(sdev);
+ } else if (!strncmp(buf, "activate", 8)) {
+ scsi_dh = sdev->scsi_dh_data->scsi_dh;
+
+ err = scsi_dh->activate(sdev);
+ }
+ }
+
+ 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);
+ struct scsi_device_handler *scsi_dh;
+
+ if (!sdev->scsi_dh_data)
+ return snprintf(buf, 20, "detached
");
+
+ scsi_dh = sdev->scsi_dh_data->scsi_dh;


we can use it directly instead of this variable ?


Yes.


+
+ return snprintf(buf, 20, "%s
", 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_handler_attach - Attach a device handler to a device
* @sdev - SCSI device the device handler should attach to
@@ -115,9 +164,11 @@ static int scsi_dh_notifier(struct notifier_block *nb,

if (action == BUS_NOTIFY_ADD_DEVICE) {
scsi_dh_handler_attach(sdev, devinfo->handler);
+ device_create_file(dev, &scsi_dh_state_attr);


A later patch checks for the return status of scsi_dh_handler_attach(),
should be moved here.


Ok.

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)

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

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 <hare@suse.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, "detached
");
+
+ return snprintf(buf, 20, "%s
", 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-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

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 <hare@suse.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, "detached
");
> +
> + return snprintf(buf, 20, "%s
", 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-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

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 <hare@suse.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
hare@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N�g
GF: Markus Rex, HRB 16746 (AG N�g)

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

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 <hare@suse.de>
Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
---
drivers/scsi/device_handler/scsi_dh.c | 103 ++++++++++++++++++++++++++++++++-
1 files changed, 102 insertions(+), 1 deletions(-)

Index: linux-2.6.26-git5/drivers/scsi/device_handler/scsi_dh.c
================================================== =================
--- linux-2.6.26-git5.orig/drivers/scsi/device_handler/scsi_dh.c
+++ linux-2.6.26-git5/drivers/scsi/device_handler/scsi_dh.c
@@ -84,7 +84,7 @@ static int scsi_dh_handler_attach(struct
* @scsi_dh - Device handler to be detached
*
* Detach from a device handler. If a device handler is specified,
- * only detach if the currently attached handler is equal to it.
+ * only detach if the currently attached handler matches @scsi_dh.
*/
static void scsi_dh_handler_detach(struct scsi_device *sdev,
struct scsi_device_handler *scsi_dh)
@@ -103,6 +103,98 @@ static void scsi_dh_handler_detach(struc
}

/*
+ * 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 {
+ scsi_dh = sdev->scsi_dh_data->scsi_dh;
+ if (!strncmp(buf, "detach", 6)) {
+ /*
+ * Detach from a device handler
+ */
+ scsi_dh_handler_detach(sdev, scsi_dh);
+ err = 0;
+ } else if (!strncmp(buf, "activate", 8)) {
+ /*
+ * Activate a device handler
+ */
+ 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, "detached
");
+
+ return snprintf(buf, 20, "%s
", 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,
@@ -132,7 +224,10 @@ static int scsi_dh_notifier(struct notif

if (action == BUS_NOTIFY_ADD_DEVICE) {
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) {
+ device_remove_file(dev, &scsi_dh_state_attr);
scsi_dh_handler_detach(sdev, NULL);
}
out:
@@ -284,11 +379,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-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel