[RFC] [PATCH 0/3] dm-multipath: New features

[RFC] [PATCH 0/3] dm-multipath: New features

[Hannes Reinecke]

This patchset implements two new features, 'no_partitions'
and 'default_hw_handler'.
The 'no_partition' feature is just a marker for kpartx
to not attempt partitions scanning on these devices.

The 'default_hw_handler' feature allows for a 'default'
hardware handler. Problem is that multipath-tools only has
a static setting for the hardware handler of a device. The
kernel, however, is able to distinguish between various
possible hardware handler and might already have attached
a different one.
So by specifying the feature 'default_hw_handler'
the dm-multipath module is instructed to use the
currently selected one, and not the one specified
during table creation.

This is an alternative implementation to Mike Snitzers
'default' hardware handler implementation.

Hannes Reinecke (3):
  scsi_dh: Allow NULL hardware handler name in scsi_dh_attach()
  dm-multipath: disable partition scan feature
  dm-multipath: 'default_hw_handler' feature

 drivers/md/dm-mpath.c                 |   36 ++++++++++++++++++++++++++++++--
 drivers/scsi/device_handler/scsi_dh.c |    8 +++++-
 2 files changed, 39 insertions(+), 5 deletions(-)

[PATCH 1/3] scsi_dh: Allow NULL hardware handler name in scsi_dh_attach()

[Hannes Reinecke]

This patch allows to pass in a NULL hardware handler to
scsi_dh_attach(), causing the reference count of the existing
hardware handler to be increased.
An error will be returned if no hardware handler is attached.

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

diff --git a/drivers/scsi/device_handler/scsi_dh.c b/drivers/scsi/device_handler/scsi_dh.c
index 48e46f5..9820f1f 100644
--- a/drivers/scsi/device_handler/scsi_dh.c
+++ b/drivers/scsi/device_handler/scsi_dh.c
@@ -475,10 +475,14 @@ int scsi_dh_attach(struct request_queue *q, const char *name)
 {
 	unsigned long flags;
 	struct scsi_device *sdev;
-	struct scsi_device_handler *scsi_dh;
+	struct scsi_device_handler *scsi_dh = NULL;
 	int err = 0;
 
-	scsi_dh = get_device_handler(name);
+	if (name) {
+		scsi_dh = get_device_handler(name);
+	} else if (sdev && sdev->scsi_dh_data) {
+		scsi_dh = sdev->scsi_dh_data->scsi_dh;
+	}
 	if (!scsi_dh)
 		return -EINVAL;
 
-- 
1.6.0.2

[PATCH 2/3] dm-multipath: disable partition scan feature

[Hannes Reinecke]

When multipath devices are being used as disks for VM Guests
any partition scanning / setup should be done within the VM Guest,
not from host. So we need to switch off partitions scanning via
kpartx there.
For this I've implemented a new feature 'no_partitions' which
just serves as a notifier to kpartx to _not_ create partitions
on these devices.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/md/dm-mpath.c |   15 +++++++++++++--
 1 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 922a338..6d3f2a8 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -56,6 +56,8 @@ struct priority_group {
 	struct list_head pgpaths;
 };
 
+#define FEATURE_NO_PARTITIONS 1
+
 /* Multipath context */
 struct multipath {
 	struct list_head list;
@@ -87,6 +89,7 @@ struct multipath {
 	unsigned pg_init_retries;	/* Number of times to retry pg_init */
 	unsigned pg_init_count;		/* Number of times pg_init called */
 	unsigned pg_init_delay_msecs;	/* Number of msecs before pg_init retry */
+	unsigned features;		/* Additional selected features */
 
 	struct work_struct process_queued_ios;
 	struct list_head queued_ios;
@@ -758,7 +761,7 @@ static int parse_features(struct dm_arg_set *as, struct multipath *m)
 	const char *arg_name;
 
 	static struct dm_arg _args[] = {
-		{0, 5, "invalid number of feature args"},
+		{0, 6, "invalid number of feature args"},
 		{1, 50, "pg_init_retries must be between 1 and 50"},
 		{0, 60000, "pg_init_delay_msecs must be between 0 and 60000"},
 	};
@@ -779,6 +782,11 @@ static int parse_features(struct dm_arg_set *as, struct multipath *m)
 			continue;
 		}
 
+		if (!strcasecmp(arg_name, "no_partitions")) {
+			m->features |= FEATURE_NO_PARTITIONS;
+			continue;
+		}
+
 		if (!strcasecmp(arg_name, "pg_init_retries") &&
 		    (argc >= 1)) {
 			r = dm_read_arg(_args + 1, as, &m->pg_init_retries, &ti->error);
@@ -1362,13 +1370,16 @@ static int multipath_status(struct dm_target *ti, status_type_t type,
 	else {
 		DMEMIT("%u ", m->queue_if_no_path +
 			      (m->pg_init_retries > 0) * 2 +
-			      (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT) * 2);
+			      (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT) * 2 +
+			      (m->features & FEATURE_NO_PARTITIONS));
 		if (m->queue_if_no_path)
 			DMEMIT("queue_if_no_path ");
 		if (m->pg_init_retries)
 			DMEMIT("pg_init_retries %u ", m->pg_init_retries);
 		if (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT)
 			DMEMIT("pg_init_delay_msecs %u ", m->pg_init_delay_msecs);
+		if (m->features & FEATURE_NO_PARTITIONS)
+			DMEMIT("no_partitions ");
 	}
 
 	if (!m->hw_handler_name || type == STATUSTYPE_INFO)
-- 
1.6.0.2

[PATCH 3/3] dm-multipath: 'default_hw_handler' feature

[Hannes Reinecke]

This patch introduces a 'default_hw_handler' feature for
dm-mpath. When specifying the feature 'default_hw_handler'
dm-multipath will be using the currently attached hardware
handler instead of trying to re-attach with the one
specified during table creation.
If no hardware handler is attached the specified hardware
handler will be used.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/md/dm-mpath.c |   25 ++++++++++++++++++++++---
 1 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 6d3f2a8..c1ef41d 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -57,6 +57,7 @@ struct priority_group {
 };
 
 #define FEATURE_NO_PARTITIONS 1
+#define FEATURE_DEFAULT_HW_HANDLER 2
 
 /* Multipath context */
 struct multipath {
@@ -589,14 +590,24 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps
 
 	if (m->hw_handler_name) {
 		struct request_queue *q = bdev_get_queue(p->path.dev->bdev);
+		const char *hw_handler_name = m->hw_handler_name;
 
-		r = scsi_dh_attach(q, m->hw_handler_name);
+		if (m->features & FEATURE_DEFAULT_HW_HANDLER)
+			hw_handler_name = NULL;
+
+		r = scsi_dh_attach(q, hw_handler_name);
 		if (r == -EBUSY) {
 			/*
 			 * Already attached to different hw_handler,
 			 * try to reattach with correct one.
 			 */
 			scsi_dh_detach(q);
+			r = scsi_dh_attach(q, hw_handler_name);
+		} else if (r == -EINVAL) {
+			/*
+			 * No hardware handler attached, use
+			 * the specified one.
+			 */
 			r = scsi_dh_attach(q, m->hw_handler_name);
 		}
 
@@ -761,7 +772,7 @@ static int parse_features(struct dm_arg_set *as, struct multipath *m)
 	const char *arg_name;
 
 	static struct dm_arg _args[] = {
-		{0, 6, "invalid number of feature args"},
+		{0, 7, "invalid number of feature args"},
 		{1, 50, "pg_init_retries must be between 1 and 50"},
 		{0, 60000, "pg_init_delay_msecs must be between 0 and 60000"},
 	};
@@ -787,6 +798,11 @@ static int parse_features(struct dm_arg_set *as, struct multipath *m)
 			continue;
 		}
 
+		if (!strcasecmp(arg_name, "default_hw_handler")) {
+			m->features |= FEATURE_DEFAULT_HW_HANDLER;
+			continue;
+		}
+
 		if (!strcasecmp(arg_name, "pg_init_retries") &&
 		    (argc >= 1)) {
 			r = dm_read_arg(_args + 1, as, &m->pg_init_retries, &ti->error);
@@ -1371,7 +1387,8 @@ static int multipath_status(struct dm_target *ti, status_type_t type,
 		DMEMIT("%u ", m->queue_if_no_path +
 			      (m->pg_init_retries > 0) * 2 +
 			      (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT) * 2 +
-			      (m->features & FEATURE_NO_PARTITIONS));
+			      (m->features & FEATURE_NO_PARTITIONS) +
+			      (m->features & FEATURE_DEFAULT_HW_HANDLER));
 		if (m->queue_if_no_path)
 			DMEMIT("queue_if_no_path ");
 		if (m->pg_init_retries)
@@ -1380,6 +1397,8 @@ static int multipath_status(struct dm_target *ti, status_type_t type,
 			DMEMIT("pg_init_delay_msecs %u ", m->pg_init_delay_msecs);
 		if (m->features & FEATURE_NO_PARTITIONS)
 			DMEMIT("no_partitions ");
+		if (m->features & FEATURE_DEFAULT_HW_HANDLER)
+			DMEMIT("default_hw_handler ");
 	}
 
 	if (!m->hw_handler_name || type == STATUSTYPE_INFO)
-- 
1.6.0.2

Re: [PATCH 3/3] dm-multipath: 'default_hw_handler' feature

[Mike Snitzer]

On Tue, May 08 2012 at 10:18am -0400,
Hannes Reinecke <hare@suse.de> wrote:

> This patch introduces a 'default_hw_handler' feature for
> dm-mpath. When specifying the feature 'default_hw_handler'
> dm-multipath will be using the currently attached hardware
> handler instead of trying to re-attach with the one
> specified during table creation.
> If no hardware handler is attached the specified hardware
> handler will be used.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/md/dm-mpath.c |   25 ++++++++++++++++++++++---
>  1 files changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> index 6d3f2a8..c1ef41d 100644
> --- a/drivers/md/dm-mpath.c
> +++ b/drivers/md/dm-mpath.c
> @@ -57,6 +57,7 @@ struct priority_group {
>  };
>  
>  #define FEATURE_NO_PARTITIONS 1
> +#define FEATURE_DEFAULT_HW_HANDLER 2
>  
>  /* Multipath context */
>  struct multipath {
> @@ -589,14 +590,24 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps
>  
>  	if (m->hw_handler_name) {
>  		struct request_queue *q = bdev_get_queue(p->path.dev->bdev);
> +		const char *hw_handler_name = m->hw_handler_name;
>  
> -		r = scsi_dh_attach(q, m->hw_handler_name);
> +		if (m->features & FEATURE_DEFAULT_HW_HANDLER)
> +			hw_handler_name = NULL;
> +
> +		r = scsi_dh_attach(q, hw_handler_name);
>  		if (r == -EBUSY) {
>  			/*
>  			 * Already attached to different hw_handler,
>  			 * try to reattach with correct one.
>  			 */
>  			scsi_dh_detach(q);
> +			r = scsi_dh_attach(q, hw_handler_name);
> +		} else if (r == -EINVAL) {
> +			/*
> +			 * No hardware handler attached, use
> +			 * the specified one.
> +			 */
>  			r = scsi_dh_attach(q, m->hw_handler_name);
>  		}

I like what you've done with the 'default_hw_handler' feature.  But
you're not establishing m->hw_handler_name.  As such the rest of the
dm-mpath.c code that keys off of m->hw_handler_name (e.g. reinstate_path,
pg_init_done, free_pgpaths) will not work.

Would you be OK with a hybrid of both our approaches?  Use your
'default_hw_handler' feature and, rather than pass a NULL name to
scsi_dh_attach, use scsi_dh_attached_handler_name like I provided?

(I don't see a problem with altering m->hw_handler_name to reflect the
scsi_dh that is used by the path.. Babu agreed with this too).

I'd be happy to merge our dm-mpath patches and attribute authorship to
you.

Mike

Re: [PATCH 3/3] dm-multipath: 'default_hw_handler' feature

[Hannes Reinecke]

On 05/08/2012 04:46 PM, Mike Snitzer wrote:
> On Tue, May 08 2012 at 10:18am -0400,
> Hannes Reinecke<hare@suse.de>  wrote:
>
>> This patch introduces a 'default_hw_handler' feature for
>> dm-mpath. When specifying the feature 'default_hw_handler'
>> dm-multipath will be using the currently attached hardware
>> handler instead of trying to re-attach with the one
>> specified during table creation.
>> If no hardware handler is attached the specified hardware
>> handler will be used.
>>
>> Signed-off-by: Hannes Reinecke<hare@suse.de>
>> ---
>>   drivers/md/dm-mpath.c |   25 ++++++++++++++++++++++---
>>   1 files changed, 22 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
>> index 6d3f2a8..c1ef41d 100644
>> --- a/drivers/md/dm-mpath.c
>> +++ b/drivers/md/dm-mpath.c
>> @@ -57,6 +57,7 @@ struct priority_group {
>>   };
>>
>>   #define FEATURE_NO_PARTITIONS 1
>> +#define FEATURE_DEFAULT_HW_HANDLER 2
>>
>>   /* Multipath context */
>>   struct multipath {
>> @@ -589,14 +590,24 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps
>>
>>   	if (m->hw_handler_name) {
>>   		struct request_queue *q = bdev_get_queue(p->path.dev->bdev);
>> +		const char *hw_handler_name = m->hw_handler_name;
>>
>> -		r = scsi_dh_attach(q, m->hw_handler_name);
>> +		if (m->features&  FEATURE_DEFAULT_HW_HANDLER)
>> +			hw_handler_name = NULL;
>> +
>> +		r = scsi_dh_attach(q, hw_handler_name);
>>   		if (r == -EBUSY) {
>>   			/*
>>   			 * Already attached to different hw_handler,
>>   			 * try to reattach with correct one.
>>   			 */
>>   			scsi_dh_detach(q);
>> +			r = scsi_dh_attach(q, hw_handler_name);
>> +		} else if (r == -EINVAL) {
>> +			/*
>> +			 * No hardware handler attached, use
>> +			 * the specified one.
>> +			 */
>>   			r = scsi_dh_attach(q, m->hw_handler_name);
>>   		}
>
> I like what you've done with the 'default_hw_handler' feature.  But
> you're not establishing m->hw_handler_name.  As such the rest of the
> dm-mpath.c code that keys off of m->hw_handler_name (e.g. reinstate_path,
> pg_init_done, free_pgpaths) will not work.
>
Ah. True.

> Would you be OK with a hybrid of both our approaches?  Use your
> 'default_hw_handler' feature and, rather than pass a NULL name to
> scsi_dh_attach, use scsi_dh_attached_handler_name like I provided?
>
Yes, sure. That sounds like the best approach here.

I'm not _that_ keen on my 'NULL' hw handler name approach, so your idea
of having a function for that looks like a better idea.

> (I don't see a problem with altering m->hw_handler_name to reflect the
> scsi_dh that is used by the path.. Babu agreed with this too).
>
Yep.

> I'd be happy to merge our dm-mpath patches and attribute authorship to
> you.
>
Oh, cool.
Please do.

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)

Re: [PATCH 3/3] dm-multipath: 'default_hw_handler' feature

[Moger, Babu]

> -----Original Message-----
> From: Hannes Reinecke [mailto:hare@suse.de]
> Sent: Tuesday, May 08, 2012 12:30 PM
> To: Mike Snitzer
> Cc: dm-devel@redhat.com; Moger, Babu
> Subject: Re: [PATCH 3/3] dm-multipath: 'default_hw_handler' feature
> 
> On 05/08/2012 04:46 PM, Mike Snitzer wrote:
> > On Tue, May 08 2012 at 10:18am -0400,
> > Hannes Reinecke<hare@suse.de>  wrote:
> >
> >> This patch introduces a 'default_hw_handler' feature for
> >> dm-mpath. When specifying the feature 'default_hw_handler'
> >> dm-multipath will be using the currently attached hardware
> >> handler instead of trying to re-attach with the one
> >> specified during table creation.
> >> If no hardware handler is attached the specified hardware
> >> handler will be used.
> >>
> >> Signed-off-by: Hannes Reinecke<hare@suse.de>
> >> ---
> >>   drivers/md/dm-mpath.c |   25 ++++++++++++++++++++++---
> >>   1 files changed, 22 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> >> index 6d3f2a8..c1ef41d 100644
> >> --- a/drivers/md/dm-mpath.c
> >> +++ b/drivers/md/dm-mpath.c
> >> @@ -57,6 +57,7 @@ struct priority_group {
> >>   };
> >>
> >>   #define FEATURE_NO_PARTITIONS 1
> >> +#define FEATURE_DEFAULT_HW_HANDLER 2
> >>
> >>   /* Multipath context */
> >>   struct multipath {
> >> @@ -589,14 +590,24 @@ static struct pgpath *parse_path(struct
> dm_arg_set *as, struct path_selector *ps
> >>
> >>   	if (m->hw_handler_name) {
> >>   		struct request_queue *q = bdev_get_queue(p->path.dev-
> >bdev);
> >> +		const char *hw_handler_name = m->hw_handler_name;
> >>
> >> -		r = scsi_dh_attach(q, m->hw_handler_name);
> >> +		if (m->features&  FEATURE_DEFAULT_HW_HANDLER)
> >> +			hw_handler_name = NULL;
> >> +
> >> +		r = scsi_dh_attach(q, hw_handler_name);
> >>   		if (r == -EBUSY) {
> >>   			/*
> >>   			 * Already attached to different hw_handler,
> >>   			 * try to reattach with correct one.
> >>   			 */
> >>   			scsi_dh_detach(q);
> >> +			r = scsi_dh_attach(q, hw_handler_name);
> >> +		} else if (r == -EINVAL) {
> >> +			/*
> >> +			 * No hardware handler attached, use
> >> +			 * the specified one.
> >> +			 */
> >>   			r = scsi_dh_attach(q, m->hw_handler_name);
> >>   		}
> >
> > I like what you've done with the 'default_hw_handler' feature.  But
> > you're not establishing m->hw_handler_name.  As such the rest of the
> > dm-mpath.c code that keys off of m->hw_handler_name (e.g.
> reinstate_path,
> > pg_init_done, free_pgpaths) will not work.
> >
> Ah. True.
> 
> > Would you be OK with a hybrid of both our approaches?  Use your
> > 'default_hw_handler' feature and, rather than pass a NULL name to
> > scsi_dh_attach, use scsi_dh_attached_handler_name like I provided?
> >
> Yes, sure. That sounds like the best approach here.
> 
> I'm not _that_ keen on my 'NULL' hw handler name approach, so your idea
> of having a function for that looks like a better idea.
> 
> > (I don't see a problem with altering m->hw_handler_name to reflect the
> > scsi_dh that is used by the path.. Babu agreed with this too).
> >
> Yep.
> 
> > I'd be happy to merge our dm-mpath patches and attribute authorship to
> > you.
> >
> Oh, cool.
> Please do.

I am little bit lost with all the patches. From what I understand so far.
Multipath tools will pass following parameters..

hardware_handler "1 alua"  
features "1 default_hw_handler"

On seeing the "features", dm will try to detect default handler and will initialize
m-> hw_handler_name. If the feature is not specified then old behavior should
take effect.  Or if the dm cannot determine the default handler then also old behavior
should take effect. 
If the dm CAN  find default handler then parse_hw_handler will skip all the processing.
Is that the summary.  Or Did I miss something here.

Re: [PATCH 3/3] dm-multipath: 'default_hw_handler' feature

[Chandra Seetharaman]

I concur with the idea of combining both patches. 

Chandra
On Tue, 2012-05-08 at 19:30 +0200, Hannes Reinecke wrote:
> On 05/08/2012 04:46 PM, Mike Snitzer wrote:
> > On Tue, May 08 2012 at 10:18am -0400,
> > Hannes Reinecke<hare@suse.de>  wrote:
> >
> >> This patch introduces a 'default_hw_handler' feature for
> >> dm-mpath. When specifying the feature 'default_hw_handler'
> >> dm-multipath will be using the currently attached hardware
> >> handler instead of trying to re-attach with the one
> >> specified during table creation.
> >> If no hardware handler is attached the specified hardware
> >> handler will be used.
> >>
> >> Signed-off-by: Hannes Reinecke<hare@suse.de>
> >> ---
> >>   drivers/md/dm-mpath.c |   25 ++++++++++++++++++++++---
> >>   1 files changed, 22 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> >> index 6d3f2a8..c1ef41d 100644
> >> --- a/drivers/md/dm-mpath.c
> >> +++ b/drivers/md/dm-mpath.c
> >> @@ -57,6 +57,7 @@ struct priority_group {
> >>   };
> >>
> >>   #define FEATURE_NO_PARTITIONS 1
> >> +#define FEATURE_DEFAULT_HW_HANDLER 2
> >>
> >>   /* Multipath context */
> >>   struct multipath {
> >> @@ -589,14 +590,24 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps
> >>
> >>   	if (m->hw_handler_name) {
> >>   		struct request_queue *q = bdev_get_queue(p->path.dev->bdev);
> >> +		const char *hw_handler_name = m->hw_handler_name;
> >>
> >> -		r = scsi_dh_attach(q, m->hw_handler_name);
> >> +		if (m->features&  FEATURE_DEFAULT_HW_HANDLER)
> >> +			hw_handler_name = NULL;
> >> +
> >> +		r = scsi_dh_attach(q, hw_handler_name);
> >>   		if (r == -EBUSY) {
> >>   			/*
> >>   			 * Already attached to different hw_handler,
> >>   			 * try to reattach with correct one.
> >>   			 */
> >>   			scsi_dh_detach(q);
> >> +			r = scsi_dh_attach(q, hw_handler_name);
> >> +		} else if (r == -EINVAL) {
> >> +			/*
> >> +			 * No hardware handler attached, use
> >> +			 * the specified one.
> >> +			 */
> >>   			r = scsi_dh_attach(q, m->hw_handler_name);
> >>   		}
> >
> > I like what you've done with the 'default_hw_handler' feature.  But
> > you're not establishing m->hw_handler_name.  As such the rest of the
> > dm-mpath.c code that keys off of m->hw_handler_name (e.g. reinstate_path,
> > pg_init_done, free_pgpaths) will not work.
> >
> Ah. True.
> 
> > Would you be OK with a hybrid of both our approaches?  Use your
> > 'default_hw_handler' feature and, rather than pass a NULL name to
> > scsi_dh_attach, use scsi_dh_attached_handler_name like I provided?
> >
> Yes, sure. That sounds like the best approach here.
> 
> I'm not _that_ keen on my 'NULL' hw handler name approach, so your idea
> of having a function for that looks like a better idea.
> 
> > (I don't see a problem with altering m->hw_handler_name to reflect the
> > scsi_dh that is used by the path.. Babu agreed with this too).
> >
> Yep.
> 
> > I'd be happy to merge our dm-mpath patches and attribute authorship to
> > you.
> >
> Oh, cool.
> Please do.
> 
> Cheers,
> 
> Hannes

Re: [PATCH 1/3] scsi_dh: Allow NULL hardware handler name in scsi_dh_attach()

[Mike Snitzer]

On Tue, May 08 2012 at 10:18am -0400,
Hannes Reinecke <hare@suse.de> wrote:

> This patch allows to pass in a NULL hardware handler to
> scsi_dh_attach(), causing the reference count of the existing
> hardware handler to be increased.
> An error will be returned if no hardware handler is attached.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/scsi/device_handler/scsi_dh.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh.c b/drivers/scsi/device_handler/scsi_dh.c
> index 48e46f5..9820f1f 100644
> --- a/drivers/scsi/device_handler/scsi_dh.c
> +++ b/drivers/scsi/device_handler/scsi_dh.c
> @@ -475,10 +475,14 @@ int scsi_dh_attach(struct request_queue *q, const char *name)
>  {
>  	unsigned long flags;
>  	struct scsi_device *sdev;
> -	struct scsi_device_handler *scsi_dh;
> +	struct scsi_device_handler *scsi_dh = NULL;
>  	int err = 0;
>  
> -	scsi_dh = get_device_handler(name);
> +	if (name) {
> +		scsi_dh = get_device_handler(name);
> +	} else if (sdev && sdev->scsi_dh_data) {
> +		scsi_dh = sdev->scsi_dh_data->scsi_dh;
> +	}

Like the first time you posted this, sdev is not initialized where
you're trying to use it (the sdev initialization happens later in
scsi_dh_attach).  And you have extraneous curly braces.