* Re: [PATCH 2/2] dm mpath: allow table load with 0 priority groups
2011-01-31 16:14 ` [PATCH 2/2] dm mpath: allow table load with 0 priority groups Mike Snitzer
@ 2011-01-31 18:09 ` Moger, Babu
2011-01-31 18:34 ` [RFC PATCH] 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) Mike Snitzer
2011-02-01 15:40 ` [PATCH 2/2] dm mpath: allow table load with 0 priority groups Hannes Reinecke
2011-02-10 21:06 ` [PATCH 2/2 v2] " Mike Snitzer
2 siblings, 1 reply; 10+ messages in thread
From: Moger, Babu @ 2011-01-31 18:09 UTC (permalink / raw)
To: Mike Snitzer, dm-devel
Looks good to me.
> -----Original Message-----
> From: Mike Snitzer [mailto:snitzer@redhat.com]
> Sent: Monday, January 31, 2011 10:14 AM
> To: dm-devel@redhat.com
> 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).
>
> 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..
>
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> Cc: Babu Moger <babu.moger@lsi.com>
> ---
> drivers/md/dm-mpath.c | 13 ++++++++++---
> 1 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> index 97842c6..bc824bd 100644
> --- a/drivers/md/dm-mpath.c
> +++ b/drivers/md/dm-mpath.c
> @@ -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);
>
> --
> 1.7.3.4
^ permalink raw reply [flat|nested] 10+ messages in thread* [RFC PATCH] 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)
2011-01-31 18:09 ` Moger, Babu
@ 2011-01-31 18:34 ` Mike Snitzer
2011-02-01 15:38 ` [RFC PATCH] multipathd: use 0 for initial pg if nr_priority_groups=0 Hannes Reinecke
0 siblings, 1 reply; 10+ messages in thread
From: Mike Snitzer @ 2011-01-31 18:34 UTC (permalink / raw)
To: Moger, Babu; +Cc: dm-devel
On Mon, Jan 31 2011 at 1:09pm -0500,
Moger, Babu <Babu.Moger@lsi.com> wrote:
> Looks good to me.
>
> > -----Original Message-----
> > From: Mike Snitzer [mailto:snitzer@redhat.com]
> > Sent: Monday, January 31, 2011 10:14 AM
> > To: dm-devel@redhat.com
> > 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).
> >
> > 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):
diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c
index 1ef3aad..97b6420 100644
--- a/libmultipath/dmparser.c
+++ b/libmultipath/dmparser.c
@@ -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");
diff --git a/libmultipath/switchgroup.c b/libmultipath/switchgroup.c
index 025a95d..82c30ad 100644
--- a/libmultipath/switchgroup.c
+++ b/libmultipath/switchgroup.c
@@ -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)
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [RFC PATCH] multipathd: use 0 for initial pg if nr_priority_groups=0
2011-01-31 18:34 ` [RFC PATCH] 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) Mike Snitzer
@ 2011-02-01 15:38 ` Hannes Reinecke
2011-02-10 22:00 ` Mike Snitzer
0 siblings, 1 reply; 10+ messages in thread
From: Hannes Reinecke @ 2011-02-01 15:38 UTC (permalink / raw)
To: device-mapper development; +Cc: Mike Snitzer
On 01/31/2011 07:34 PM, Mike Snitzer wrote:
> On Mon, Jan 31 2011 at 1:09pm -0500,
> Moger, Babu <Babu.Moger@lsi.com> wrote:
>
>> Looks good to me.
>>
>>> -----Original Message-----
>>> From: Mike Snitzer [mailto:snitzer@redhat.com]
>>> Sent: Monday, January 31, 2011 10:14 AM
>>> To: dm-devel@redhat.com
>>> 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).
>>>
>>> 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):
>
> diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c
> index 1ef3aad..97b6420 100644
> --- a/libmultipath/dmparser.c
> +++ b/libmultipath/dmparser.c
> @@ -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");
> diff --git a/libmultipath/switchgroup.c b/libmultipath/switchgroup.c
> index 025a95d..82c30ad 100644
> --- a/libmultipath/switchgroup.c
> +++ b/libmultipath/switchgroup.c
> @@ -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
--
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)
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC PATCH] multipathd: use 0 for initial pg if nr_priority_groups=0
2011-02-01 15:38 ` [RFC PATCH] multipathd: use 0 for initial pg if nr_priority_groups=0 Hannes Reinecke
@ 2011-02-10 22:00 ` Mike Snitzer
2011-02-11 15:44 ` Hannes Reinecke
0 siblings, 1 reply; 10+ messages in thread
From: Mike Snitzer @ 2011-02-10 22:00 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: device-mapper development
On Tue, Feb 01 2011 at 10:38am -0500,
Hannes Reinecke <hare@suse.de> wrote:
> On 01/31/2011 07:34 PM, Mike Snitzer wrote:
> > On Mon, Jan 31 2011 at 1:09pm -0500,
> > Moger, Babu <Babu.Moger@lsi.com> wrote:
> >
> >> Looks good to me.
> >>
> >>> -----Original Message-----
> >>> From: Mike Snitzer [mailto:snitzer@redhat.com]
> >>> Sent: Monday, January 31, 2011 10:14 AM
> >>> To: dm-devel@redhat.com
> >>> 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).
> >>>
> >>> 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):
> >
> > diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c
> > index 1ef3aad..97b6420 100644
> > --- a/libmultipath/dmparser.c
> > +++ b/libmultipath/dmparser.c
> > @@ -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");
> > diff --git a/libmultipath/switchgroup.c b/libmultipath/switchgroup.c
> > index 025a95d..82c30ad 100644
> > --- a/libmultipath/switchgroup.c
> > +++ b/libmultipath/switchgroup.c
> > @@ -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.
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?
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC PATCH] multipathd: use 0 for initial pg if nr_priority_groups=0
2011-02-10 22:00 ` Mike Snitzer
@ 2011-02-11 15:44 ` Hannes Reinecke
0 siblings, 0 replies; 10+ messages in thread
From: Hannes Reinecke @ 2011-02-11 15:44 UTC (permalink / raw)
To: Mike Snitzer; +Cc: device-mapper development
On 02/10/2011 11:00 PM, Mike Snitzer wrote:
> On Tue, Feb 01 2011 at 10:38am -0500,
> Hannes Reinecke <hare@suse.de> wrote:
>
>> On 01/31/2011 07:34 PM, Mike Snitzer wrote:
>>> On Mon, Jan 31 2011 at 1:09pm -0500,
>>> Moger, Babu <Babu.Moger@lsi.com> wrote:
>>>
>>>> Looks good to me.
>>>>
>>>>> -----Original Message-----
>>>>> From: Mike Snitzer [mailto:snitzer@redhat.com]
>>>>> Sent: Monday, January 31, 2011 10:14 AM
>>>>> To: dm-devel@redhat.com
>>>>> 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).
>>>>>
>>>>> 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):
>>>
>>> diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c
>>> index 1ef3aad..97b6420 100644
>>> --- a/libmultipath/dmparser.c
>>> +++ b/libmultipath/dmparser.c
>>> @@ -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");
>>> diff --git a/libmultipath/switchgroup.c b/libmultipath/switchgroup.c
>>> index 025a95d..82c30ad 100644
>>> --- a/libmultipath/switchgroup.c
>>> +++ b/libmultipath/switchgroup.c
>>> @@ -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.
>
> 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
--
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)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] dm mpath: allow table load with 0 priority groups
2011-01-31 16:14 ` [PATCH 2/2] dm mpath: allow table load with 0 priority groups Mike Snitzer
2011-01-31 18:09 ` Moger, Babu
@ 2011-02-01 15:40 ` Hannes Reinecke
2011-02-10 21:06 ` [PATCH 2/2 v2] " Mike Snitzer
2 siblings, 0 replies; 10+ messages in thread
From: Hannes Reinecke @ 2011-02-01 15:40 UTC (permalink / raw)
To: device-mapper development; +Cc: Mike Snitzer
On 01/31/2011 05:14 PM, Mike Snitzer wrote:
> 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).
>
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.
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> Cc: Babu Moger <babu.moger@lsi.com>
Acked-by: Hannes Reinecke <hare@suse.de>
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)
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2 v2] dm mpath: allow table load with 0 priority groups
2011-01-31 16:14 ` [PATCH 2/2] dm mpath: allow table load with 0 priority groups Mike Snitzer
2011-01-31 18:09 ` Moger, Babu
2011-02-01 15:40 ` [PATCH 2/2] dm mpath: allow table load with 0 priority groups Hannes Reinecke
@ 2011-02-10 21:06 ` Mike Snitzer
2011-02-10 21:13 ` Mike Snitzer
2 siblings, 1 reply; 10+ messages in thread
From: Mike Snitzer @ 2011-02-10 21:06 UTC (permalink / raw)
To: dm-devel
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 <snitzer@redhat.com>
Acked-by: Babu Moger <babu.moger@lsi.com>
---
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
Index: linux-2.6/drivers/md/dm-mpath.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-mpath.c
+++ 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,
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 2/2 v2] dm mpath: allow table load with 0 priority groups
2011-02-10 21:06 ` [PATCH 2/2 v2] " Mike Snitzer
@ 2011-02-10 21:13 ` Mike Snitzer
0 siblings, 0 replies; 10+ messages in thread
From: Mike Snitzer @ 2011-02-10 21:13 UTC (permalink / raw)
To: dm-devel
On Thu, Feb 10 2011 at 4:06pm -0500,
Mike Snitzer <snitzer@redhat.com> wrote:
> 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 <snitzer@redhat.com>
> Acked-by: Babu Moger <babu.moger@lsi.com>
Forgot Hannes':
Acked-by: Hannes Reinecke <hare@suse.de>
^ permalink raw reply [flat|nested] 10+ messages in thread