dm mpath: fail message ioctl if specified path is not valid

PATCH In review Mike Snitzer 3 participants 10 messages Jan 31, 2011 – Feb 11, 2011 Device mapper

Share & tools

Mike Snitzer

author

2011-01-31 16:14 UTC #0 lore raw

PATCH 1/2 dm mpath: fail message ioctl if specified path is not valid

The message ioctl would succeed for the 'reinistate_path' and 'fail_path' messages even if action was not taken because the specified device was not a valid path of the multipath device.

before:

$ dmsetup message mpathb 0 reinstate_path /dev/vdb
$ echo $?
0

after:

$ dmsetup message mpathb 0 reinstate_path /dev/vdb
device-mapper: message ioctl failed: Invalid argument
Command failed
$ echo $?
1

Signed-off-by Mike Snitzer <[email protected]>

---
 drivers/md/dm-mpath.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/md/dm-mpath.c +1 -1

index b82d288..97842c6 100644
@@ -1065,7 +1065,7 @@ out:
 static int action_dev(struct multipath *m, struct dm_dev *dev,
 		      action_fn action)
 {
-	int r = 0;
+	int r = -EINVAL;
 	struct pgpath *pgpath;
 	struct priority_group *pg;

Signature · 1 line

1.7.3.4

Mike Snitzer

author

2011-01-31 16:14 UTC #1 lore raw

PATCH 2/2 dm mpath: allow table load with 0 priority groups

If an mpath device is held open when all paths in the last priority group have failed userspace multipathd will attempt to reload the associated DM table to reflect that the device no longer has any priority groups. But the reload attempt always failed because the multipath target did not allow 0 priority groups.

Adjust multipath target to allow a table with both 0 priority groups and 0 for the initial priority group number.

All multipath target messages related to priority group (enable_group, disable_group, switch_group) will properly handle a priority group of 0 (will cause error).

When reloading a multipath table with 0 priority groups, userspace multipathd must be updated to specify an initial priority group number of 0 (rather than 1).

Signed-off-by Mike Snitzer <[email protected]>

Cc Babu Moger <[email protected]>

---
 drivers/md/dm-mpath.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

drivers/md/dm-mpath.c +10 -3

index 97842c6..bc824bd 100644
@@ -844,8 +844,8 @@ static int multipath_ctr(struct dm_target *ti, unsigned int argc,
 {
 	/* target parameters */
 	static struct param _params[] = {
-		{1, 1024, "invalid number of priority groups"},
-		{1, 1024, "invalid initial priority group number"},
+		{0, 1024, "invalid number of priority groups"},
+		{0, 1024, "invalid initial priority group number"},
 	};
 
 	int r;

@@ -879,6 +879,13 @@ static int multipath_ctr(struct dm_target *ti, unsigned int argc,
 	if (r)
 		goto bad;
 
+	if ((!m->nr_priority_groups && next_pg_num) ||
+	    (m->nr_priority_groups && !next_pg_num)) {
+		ti->error = "invalid initial priority group";
+		r = -EINVAL;
+		goto bad;
+	}
+
 	/* parse the priority groups */
 	while (as.argc) {
 		struct priority_group *pg;

@@ -1417,7 +1424,7 @@ static int multipath_status(struct dm_target *ti, status_type_t type,
 	else if (m->current_pg)
 		pg_num = m->current_pg->pg_num;
 	else
-			pg_num = 1;
+		pg_num = (m->nr_priority_groups ? 1 : 0);
 
 	DMEMIT("%u ", pg_num);

Signature · 1 line

1.7.3.4

Moger, Babu

2011-01-31 18:09 UTC #2 lore raw

Re: [PATCH 2/2] dm mpath: allow table load with 0 priority groups

Looks good to me.

Quoted 16 lines

-----Original Message-----
From: Mike Snitzer [mailto:[email protected]]
Sent: Monday, January 31, 2011 10:14 AM
To: [email protected]
Cc: Mike Snitzer; Moger, Babu
Subject: [PATCH 2/2] dm mpath: allow table load with 0 priority groups
If an mpath device is held open when all paths in the last priority group have failed userspace multipathd will attempt to reload the associated DM table to reflect that the device no longer has any priority groups. But the reload attempt always failed because the multipath target did not allow 0 priority groups.
Adjust multipath target to allow a table with both 0 priority groups and 0 for the initial priority group number.

Quoted 7 lines

All multipath target messages related to priority group (enable_group, disable_group, switch_group) will properly handle a priority group of 0 (will cause error).
When reloading a multipath table with 0 priority groups, userspace multipathd must be updated to specify an initial priority group number of 0 (rather than 1).

Looks like we still have some action from multipath tool. CCing Ben..

Quoted 51 lines

Signed-off-by Mike Snitzer <[email protected]>

Cc Babu Moger <[email protected]>

---
 drivers/md/dm-mpath.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

drivers/md/dm-mpath.c +2 -2

index 97842c6..bc824bd 100644
@@ -844,8 +844,8 @@ static int multipath_ctr(struct dm_target *ti,
 unsigned int argc,
 {
 	/* target parameters */
 	static struct param _params[] = {
-		{1, 1024, "invalid number of priority groups"},
-		{1, 1024, "invalid initial priority group number"},
+		{0, 1024, "invalid number of priority groups"},
+		{0, 1024, "invalid initial priority group number"},
 	};

drivers/md/dm-mpath.c +7 -0

 	int r;

@@ -879,6 +879,13 @@ static int multipath_ctr(struct dm_target *ti,
 unsigned int argc,
 	if (r)
 		goto bad;
 
+	if ((!m->nr_priority_groups && next_pg_num) ||
+	    (m->nr_priority_groups && !next_pg_num)) {
+		ti->error = "invalid initial priority group";
+		r = -EINVAL;
+		goto bad;
+	}
+
 	/* parse the priority groups */
 	while (as.argc) {

drivers/md/dm-mpath.c +1 -2

 		struct priority_group *pg;

@@ -1417,7 +1424,7 @@ static int multipath_status(struct dm_target *ti,
 status_type_t type,
 	else if (m->current_pg)
 		pg_num = m->current_pg->pg_num;
 	else
-			pg_num = 1;
+		pg_num = (m->nr_priority_groups ? 1 : 0);
 
 	DMEMIT("%u ", pg_num);
 
--

1.7.3.4

Mike Snitzer

author

2011-01-31 18:34 UTC #3 lore raw

RFC PATCH 1/1 multipathd: use 0 for initial pg if nr_priority_groups=0 (was: Re: [PATCH 2/2] dm mpath: allow table load with 0 priority groups)

Quoted 22 lines · Babu

Looks good to me.
Quoted 20 lines
-----Original Message-----
From: Mike Snitzer [mailto:[email protected]]
Sent: Monday, January 31, 2011 10:14 AM
To: [email protected]
Cc: Mike Snitzer; Moger, Babu
Subject: [PATCH 2/2] dm mpath: allow table load with 0 priority groups
If an mpath device is held open when all paths in the last priority group have failed userspace multipathd will attempt to reload the associated DM table to reflect that the device no longer has any priority groups. But the reload attempt always failed because the multipath target did not allow 0 priority groups.
Adjust multipath target to allow a table with both 0 priority groups and 0 for the initial priority group number.
All multipath target messages related to priority group (enable_group, disable_group, switch_group) will properly handle a priority group of 0 (will cause error).

Quoted 5 lines

Quoted 3 lines
When reloading a multipath table with 0 priority groups, userspace multipathd must be updated to specify an initial priority group number of 0 (rather than 1).
Looks like we still have some action from multipath tool. CCing Ben..

Right, I looked in to the multipath-tools change. Here is an RFC patch.

Inlined "FIXME"s pose the relevant questions. I held off on having select_path_group() return 0 when !mpp->pg because I wasn't confident that I wouldn't break multipathd in some unintuitive way.

I did audit the various callers (indirect through setup_map call):

update_path_groups
ev_add_path
ev_remove_path

(all above first call setup_map)

setup_map()
- mpp->bestpg = select_path_group()
- assemble_map() -- establishes mp->params

do_map()

Even though I verified that much I wasn't completely confident in changing select_path_group()'s defualt return of 1 -- so I localized the change to where I _knew_ it was safe (assemble_map):

libmultipath/dmparser.c +12 -1

index 1ef3aad..97b6420 100644
@@ -52,6 +52,7 @@ assemble_map (struct multipath * mp)
 	int i, j;
 	int shift, freechar;
 	int minio;
+	int nr_priority_groups, initial_pg_nr;
 	char * p;
 	struct pathgroup * pgp;
 	struct path * pp;

@@ -60,9 +61,19 @@ assemble_map (struct multipath * mp)
 	p = mp->params;
 	freechar = sizeof(mp->params);
 
+	nr_priority_groups = VECTOR_SIZE(mp->pg);
+	/*
+	 * FIXME: make this conditional on multipath target version?
+	 * - but no existing code is conditional on multipath target version,
+	 *   nor does dm_drvprereq() store the version for code to do so.
+	 * - thing is older multipath never allowed nr_priority_groups=0 so
+	 *   it doesn't _really_ matter if initial_pg_nr=0 here...
+	 */
+	initial_pg_nr = (nr_priority_groups ? mp->bestpg : 0);
+
 	shift = snprintf(p, freechar, "%s %s %i %i",
 			 mp->features, mp->hwhandler,
-			 VECTOR_SIZE(mp->pg), mp->bestpg);
+			 nr_priority_groups, initial_pg_nr);
 
 	if (shift >= freechar) {
 		fprintf(stderr, "mp->params too small\n");

libmultipath/switchgroup.c +1 -1

index 025a95d..82c30ad 100644
@@ -41,7 +41,7 @@ select_path_group (struct multipath * mpp)
 	struct pathgroup * pgp;
 
 	if (!mpp->pg)
-		return 1;
+		return 1; /* FIXME: return 0 here? */
 
 	vector_foreach_slot (mpp->pg, pgp, i) {
 		if (!pgp->paths)

Hannes Reinecke

2011-02-01 15:38 UTC #4 lore raw

Re: [RFC PATCH] multipathd: use 0 for initial pg if nr_priority_groups=0

Quoted 94 lines · Mike Snitzer

Quoted 22 lines · Babu
Looks good to me.
Quoted 20 lines
-----Original Message-----
From: Mike Snitzer [mailto:[email protected]]
Sent: Monday, January 31, 2011 10:14 AM
To: [email protected]
Cc: Mike Snitzer; Moger, Babu
Subject: [PATCH 2/2] dm mpath: allow table load with 0 priority groups
If an mpath device is held open when all paths in the last priority group have failed userspace multipathd will attempt to reload the associated DM table to reflect that the device no longer has any priority groups. But the reload attempt always failed because the multipath target did not allow 0 priority groups.
Adjust multipath target to allow a table with both 0 priority groups and 0 for the initial priority group number.
All multipath target messages related to priority group (enable_group, disable_group, switch_group) will properly handle a priority group of 0 (will cause error).
Quoted 5 lines
Quoted 3 lines
When reloading a multipath table with 0 priority groups, userspace multipathd must be updated to specify an initial priority group number of 0 (rather than 1).
Looks like we still have some action from multipath tool. CCing Ben..
Right, I looked in to the multipath-tools change. Here is an RFC patch.
Inlined "FIXME"s pose the relevant questions. I held off on having select_path_group() return 0 when !mpp->pg because I wasn't confident that I wouldn't break multipathd in some unintuitive way.
I did audit the various callers (indirect through setup_map call):
update_path_groups
ev_add_path
ev_remove_path
(all above first call setup_map)
setup_map()
- mpp->bestpg = select_path_group()
- assemble_map() -- establishes mp->params
do_map()
Even though I verified that much I wasn't completely confident in changing select_path_group()'s defualt return of 1 -- so I localized the change to where I _knew_ it was safe (assemble_map):
libmultipath/dmparser.c +12 -1
index 1ef3aad..97b6420 100644
@@ -52,6 +52,7 @@ assemble_map (struct multipath * mp)
 	int i, j;
 	int shift, freechar;
 	int minio;
+	int nr_priority_groups, initial_pg_nr;
 	char * p;
 	struct pathgroup * pgp;
 	struct path * pp;
@@ -60,9 +61,19 @@ assemble_map (struct multipath * mp)
 	p = mp->params;
 	freechar = sizeof(mp->params);
 
+	nr_priority_groups = VECTOR_SIZE(mp->pg);
+	/*
+	 * FIXME: make this conditional on multipath target version?
+	 * - but no existing code is conditional on multipath target version,
+	 *   nor does dm_drvprereq() store the version for code to do so.
+	 * - thing is older multipath never allowed nr_priority_groups=0 so
+	 *   it doesn't _really_ matter if initial_pg_nr=0 here...
+	 */
+	initial_pg_nr = (nr_priority_groups ? mp->bestpg : 0);
+
 	shift = snprintf(p, freechar, "%s %s %i %i",
 			 mp->features, mp->hwhandler,
-			 VECTOR_SIZE(mp->pg), mp->bestpg);
+			 nr_priority_groups, initial_pg_nr);
 
 	if (shift >= freechar) {
 		fprintf(stderr, "mp->params too small\n");
libmultipath/switchgroup.c +0 -0
index 025a95d..82c30ad 100644

libmultipath/switchgroup.c select_path_group (struct multipath * mpp) · line 47

Quoted 8 lines

libmultipath/switchgroup.c +1 -1

@@ -41,7 +41,7 @@ select_path_group (struct multipath * mpp)
 	struct pathgroup * pgp;
 
 	if (!mpp->pg)
-		return 1;
+		return 1; /* FIXME: return 0 here? */
 
 	vector_foreach_slot (mpp->pg, pgp, i) {
 		if (!pgp->paths)

I have a similar patchset in my multipath-tools tree, to allow maps with zero paths, too.
However, there are quite a few places which required checking, as we're basically _never_ check if a pointer is NULL before accessing it. With a bit of luck I'll find some time to send out the patchset.

Otherwise you could have a look at

git://git.kernel.org/pub/scm/linux/kernel/git/hare/multipath-tools.git
branch sles11-sp1 will have the most recent stuff.

Cheers,

Hannes

Signature · 4 lines

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

Mike Snitzer

author

2011-02-10 22:00 UTC #5 lore raw

Re: [RFC PATCH] multipathd: use 0 for initial pg if nr_priority_groups=0

Quoted 109 lines · Hannes Reinecke

Quoted 94 lines · Mike Snitzer
Quoted 22 lines · Babu
Looks good to me.
Quoted 20 lines
-----Original Message-----
From: Mike Snitzer [mailto:[email protected]]
Sent: Monday, January 31, 2011 10:14 AM
To: [email protected]
Cc: Mike Snitzer; Moger, Babu
Subject: [PATCH 2/2] dm mpath: allow table load with 0 priority groups
If an mpath device is held open when all paths in the last priority group have failed userspace multipathd will attempt to reload the associated DM table to reflect that the device no longer has any priority groups. But the reload attempt always failed because the multipath target did not allow 0 priority groups.
Adjust multipath target to allow a table with both 0 priority groups and 0 for the initial priority group number.
All multipath target messages related to priority group (enable_group, disable_group, switch_group) will properly handle a priority group of 0 (will cause error).
Quoted 5 lines
Quoted 3 lines
When reloading a multipath table with 0 priority groups, userspace multipathd must be updated to specify an initial priority group number of 0 (rather than 1).
Looks like we still have some action from multipath tool. CCing Ben..
Right, I looked in to the multipath-tools change. Here is an RFC patch.
Inlined "FIXME"s pose the relevant questions. I held off on having select_path_group() return 0 when !mpp->pg because I wasn't confident that I wouldn't break multipathd in some unintuitive way.
I did audit the various callers (indirect through setup_map call):
update_path_groups
ev_add_path
ev_remove_path
(all above first call setup_map)
setup_map()
- mpp->bestpg = select_path_group()
- assemble_map() -- establishes mp->params
do_map()
Even though I verified that much I wasn't completely confident in changing select_path_group()'s defualt return of 1 -- so I localized the change to where I _knew_ it was safe (assemble_map):
libmultipath/dmparser.c +12 -1
index 1ef3aad..97b6420 100644
@@ -52,6 +52,7 @@ assemble_map (struct multipath * mp)
 	int i, j;
 	int shift, freechar;
 	int minio;
+	int nr_priority_groups, initial_pg_nr;
 	char * p;
 	struct pathgroup * pgp;
 	struct path * pp;
@@ -60,9 +61,19 @@ assemble_map (struct multipath * mp)
 	p = mp->params;
 	freechar = sizeof(mp->params);
 
+	nr_priority_groups = VECTOR_SIZE(mp->pg);
+	/*
+	 * FIXME: make this conditional on multipath target version?
+	 * - but no existing code is conditional on multipath target version,
+	 *   nor does dm_drvprereq() store the version for code to do so.
+	 * - thing is older multipath never allowed nr_priority_groups=0 so
+	 *   it doesn't _really_ matter if initial_pg_nr=0 here...
+	 */
+	initial_pg_nr = (nr_priority_groups ? mp->bestpg : 0);
+
 	shift = snprintf(p, freechar, "%s %s %i %i",
 			 mp->features, mp->hwhandler,
-			 VECTOR_SIZE(mp->pg), mp->bestpg);
+			 nr_priority_groups, initial_pg_nr);
 
 	if (shift >= freechar) {
 		fprintf(stderr, "mp->params too small\n");
libmultipath/switchgroup.c +0 -0
index 025a95d..82c30ad 100644
libmultipath/switchgroup.c select_path_group (struct multipath * mpp) · line 47
Quoted 8 lines
libmultipath/switchgroup.c +1 -1
@@ -41,7 +41,7 @@ select_path_group (struct multipath * mpp)
 	struct pathgroup * pgp;
 
 	if (!mpp->pg)
-		return 1;
+		return 1; /* FIXME: return 0 here? */
 
 	vector_foreach_slot (mpp->pg, pgp, i) {
 		if (!pgp->paths)
I have a similar patchset in my multipath-tools tree, to allow maps with zero paths, too.
However, there are quite a few places which required checking, as we're basically _never_ check if a pointer is NULL before accessing it. With a bit of luck I'll find some time to send out the patchset.

Quoted 4 lines

Otherwise you could have a look at
git://git.kernel.org/pub/scm/linux/kernel/git/hare/multipath-tools.git
branch sles11-sp1 will have the most recent stuff.

I found the following commit but it doesn't have any meaningful changes:
1e53326 Allow zero paths for device-mapper strings

Could you be more explicit on which commits you're thinking of?

Hannes Reinecke

2011-02-11 15:44 UTC #6 lore raw

Re: [RFC PATCH] multipathd: use 0 for initial pg if nr_priority_groups=0

Quoted 114 lines · Mike Snitzer

Quoted 111 lines · Hannes Reinecke
Quoted 94 lines · Mike Snitzer
Quoted 22 lines · Babu
Looks good to me.
Quoted 20 lines
-----Original Message-----
From: Mike Snitzer [mailto:[email protected]]
Sent: Monday, January 31, 2011 10:14 AM
To: [email protected]
Cc: Mike Snitzer; Moger, Babu
Subject: [PATCH 2/2] dm mpath: allow table load with 0 priority groups
If an mpath device is held open when all paths in the last priority group have failed userspace multipathd will attempt to reload the associated DM table to reflect that the device no longer has any priority groups. But the reload attempt always failed because the multipath target did not allow 0 priority groups.
Adjust multipath target to allow a table with both 0 priority groups and 0 for the initial priority group number.
All multipath target messages related to priority group (enable_group, disable_group, switch_group) will properly handle a priority group of 0 (will cause error).
Quoted 5 lines
Quoted 3 lines
When reloading a multipath table with 0 priority groups, userspace multipathd must be updated to specify an initial priority group number of 0 (rather than 1).
Looks like we still have some action from multipath tool. CCing Ben..
Right, I looked in to the multipath-tools change. Here is an RFC patch.
Inlined "FIXME"s pose the relevant questions. I held off on having select_path_group() return 0 when !mpp->pg because I wasn't confident that I wouldn't break multipathd in some unintuitive way.
I did audit the various callers (indirect through setup_map call):
update_path_groups
ev_add_path
ev_remove_path
(all above first call setup_map)
setup_map()
- mpp->bestpg = select_path_group()
- assemble_map() -- establishes mp->params
do_map()
Even though I verified that much I wasn't completely confident in changing select_path_group()'s defualt return of 1 -- so I localized the change to where I _knew_ it was safe (assemble_map):
libmultipath/dmparser.c +12 -1
index 1ef3aad..97b6420 100644
@@ -52,6 +52,7 @@ assemble_map (struct multipath * mp)
 	int i, j;
 	int shift, freechar;
 	int minio;
+	int nr_priority_groups, initial_pg_nr;
 	char * p;
 	struct pathgroup * pgp;
 	struct path * pp;
@@ -60,9 +61,19 @@ assemble_map (struct multipath * mp)
 	p = mp->params;
 	freechar = sizeof(mp->params);
 
+	nr_priority_groups = VECTOR_SIZE(mp->pg);
+	/*
+	 * FIXME: make this conditional on multipath target version?
+	 * - but no existing code is conditional on multipath target version,
+	 *   nor does dm_drvprereq() store the version for code to do so.
+	 * - thing is older multipath never allowed nr_priority_groups=0 so
+	 *   it doesn't _really_ matter if initial_pg_nr=0 here...
+	 */
+	initial_pg_nr = (nr_priority_groups ? mp->bestpg : 0);
+
 	shift = snprintf(p, freechar, "%s %s %i %i",
 			 mp->features, mp->hwhandler,
-			 VECTOR_SIZE(mp->pg), mp->bestpg);
+			 nr_priority_groups, initial_pg_nr);
 
 	if (shift >= freechar) {
 		fprintf(stderr, "mp->params too small\n");
libmultipath/switchgroup.c +0 -0
index 025a95d..82c30ad 100644
libmultipath/switchgroup.c select_path_group (struct multipath * mpp) · line 47
Quoted 8 lines
libmultipath/switchgroup.c +1 -1
@@ -41,7 +41,7 @@ select_path_group (struct multipath * mpp)
 	struct pathgroup * pgp;
 
 	if (!mpp->pg)
-		return 1;
+		return 1; /* FIXME: return 0 here? */
 
 	vector_foreach_slot (mpp->pg, pgp, i) {
 		if (!pgp->paths)
I have a similar patchset in my multipath-tools tree, to allow maps with zero paths, too.
However, there are quite a few places which required checking, as we're basically _never_ check if a pointer is NULL before accessing it. With a bit of luck I'll find some time to send out the patchset.
Otherwise you could have a look at

Quoted 7 lines

Quoted 2 lines
git://git.kernel.org/pub/scm/linux/kernel/git/hare/multipath-tools.git
branch sles11-sp1 will have the most recent stuff.
I found the following commit but it doesn't have any meaningful changes:
1e53326 Allow zero paths for device-mapper strings
Could you be more explicit on which commits you're thinking of?

Ah, I just rechecked the code. There used to be quite a few missing NULL pointer checks, especially if mpp->pg or mpp->paths are NULL. But they seem to have been fixed now.

So ignore my objection here.

Cheers,

Hannes

Signature · 4 lines

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

Hannes Reinecke

Acked-by on v1 2/2

2011-02-01 15:40 UTC #7 lore raw

Re: [PATCH 2/2] dm mpath: allow table load with 0 priority groups

In reply to Mike Snitzer

Quoted 9 lines · Mike Snitzer

If an mpath device is held open when all paths in the last priority group have failed userspace multipathd will attempt to reload the associated DM table to reflect that the device no longer has any priority groups. But the reload attempt always failed because the multipath target did not allow 0 priority groups.
Adjust multipath target to allow a table with both 0 priority groups and 0 for the initial priority group number.

Quoted 8 lines

All multipath target messages related to priority group (enable_group, disable_group, switch_group) will properly handle a priority group of 0 (will cause error).
When reloading a multipath table with 0 priority groups, userspace multipathd must be updated to specify an initial priority group number of 0 (rather than 1).

I actually send a similar patch way back, but Alasdair refused to take it as he still had to mull them over.

And, as mentioned in my other mail, to properly support maps with no paths there are quite a few changes required in multipath-tools itself.

But other than that: Congrats. One patchset less in my queue.

Quoted 2 lines

Signed-off-by Mike Snitzer <[email protected]>
Cc Babu Moger <[email protected]>

Acked-by Hannes Reinecke <[email protected]>

Cheers,

Hannes

Signature · 4 lines

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

Mike Snitzer

author

2011-02-10 21:06 UTC #8 lore raw

PATCH v2 2/2 dm mpath: allow table load with 0 priority groups

In reply to Mike Snitzer

Adjust multipath target to allow a table with both 0 priority groups and 0 for the initial priority group number.

All multipath target messages related to priority group (enable_group, disable_group, switch_group) will properly handle a priority group of 0 (will cause error).

When reloading a multipath table with 0 priority groups, userspace multipathd must be updated to specify an initial priority group number of 0 (rather than 1).

Signed-off-by Mike Snitzer <[email protected]>

Acked-by Babu Moger <[email protected]>

---
 drivers/md/dm-mpath.c |   15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

v2: bumped multipath target version to 1.3.0, added babu's acked-by

drivers/md/dm-mpath.c +11 -4

Index: linux-2.6/drivers/md/dm-mpath.c
===================================================================
@@ -844,8 +844,8 @@ static int multipath_ctr(struct dm_targe
 {
 	/* target parameters */
 	static struct param _params[] = {
-		{1, 1024, "invalid number of priority groups"},
-		{1, 1024, "invalid initial priority group number"},
+		{0, 1024, "invalid number of priority groups"},
+		{0, 1024, "invalid initial priority group number"},
 	};
 
 	int r;

@@ -879,6 +879,13 @@ static int multipath_ctr(struct dm_targe
 	if (r)
 		goto bad;
 
+	if ((!m->nr_priority_groups && next_pg_num) ||
+	    (m->nr_priority_groups && !next_pg_num)) {
+		ti->error = "invalid initial priority group";
+		r = -EINVAL;
+		goto bad;
+	}
+
 	/* parse the priority groups */
 	while (as.argc) {
 		struct priority_group *pg;

@@ -1417,7 +1424,7 @@ static int multipath_status(struct dm_ta
 	else if (m->current_pg)
 		pg_num = m->current_pg->pg_num;
 	else
-			pg_num = 1;
+		pg_num = (m->nr_priority_groups ? 1 : 0);
 
 	DMEMIT("%u ", pg_num);

@@ -1671,7 +1678,7 @@ out:
  *---------------------------------------------------------------*/
 static struct target_type multipath_target = {
 	.name = "multipath",
-	.version = {1, 2, 0},
+	.version = {1, 3, 0},
 	.module = THIS_MODULE,
 	.ctr = multipath_ctr,
 	.dtr = multipath_dtr,

Mike Snitzer

author Acked-by Hannes Reinecke on v2 2/2

2011-02-10 21:13 UTC #9 lore raw

Re: [PATCH 2/2 v2] dm mpath: allow table load with 0 priority groups

Quoted 13 lines · Mike Snitzer

If an mpath device is held open when all paths in the last priority group have failed userspace multipathd will attempt to reload the associated DM table to reflect that the device no longer has any priority groups. But the reload attempt always failed because the multipath target did not allow 0 priority groups.
Adjust multipath target to allow a table with both 0 priority groups and 0 for the initial priority group number.
All multipath target messages related to priority group (enable_group, disable_group, switch_group) will properly handle a priority group of 0 (will cause error).

Quoted 6 lines

When reloading a multipath table with 0 priority groups, userspace multipathd must be updated to specify an initial priority group number of 0 (rather than 1).
Signed-off-by Mike Snitzer <[email protected]>
Acked-by Babu Moger <[email protected]>

Forgot Hannes':

Acked-by Hannes Reinecke <[email protected]>