Multipath blacklist exceptions issues

Multipath blacklist exceptions issues

[Benjamin Marzinski]

[-- Attachment #1: Type: text/plain, Size: 1320 bytes --]

I've noticed some issues with how the blacklist exceptions are currently
working.  One is an obvious bug. Multipathd is treating paths that have
blacklist exceptions the same as paths that are blacklisted. The other
is also incorrect, but I'm not exactly sure what was intended.

If I recall correctly, blacklist exceptions were originally proposed as
a solution to the problem of not being able to remove the bl_product
lines from the default device configuration, without reentering the
whole configuration for that device.  They were specifically designed so
that you could still blacklist the device by wwid and devnode rules.

That is not currently the case.

Right now if you have in your multipath.conf file

blacklist {
	wwid "*"
}

blacklist_exceptions {
	devnode "sda"
}

You will create a multipath device on top of /dev/sda. This makes it
look like exceptions are now being used as a whitelist. However, if you
have

blacklist {
        devnode "*"
}

blacklist_exceptions {
        wwid "3600d0230000000000e13955cc3757800"
}

You will not create any multipath devices.  This doesn't make any sense,
if the earlier example was supposed to create a device. The attached
patch fixes the multipathd issue, and makes blacklist exceptions work
like they originally did, which I assume is what was intended.

-Ben

[-- Attachment #2: blacklist_exceptions_fix.patch --]
[-- Type: text/plain, Size: 1477 bytes --]

diff -urpN a/libmultipath/blacklist.c b/libmultipath/blacklist.c
--- a/libmultipath/blacklist.c	2007-10-01 13:23:59.000000000 -0500
+++ b/libmultipath/blacklist.c	2007-11-08 17:57:05.000000000 -0600
@@ -297,14 +297,14 @@ _filter_path (struct config * conf, stru
 	int r;
 
 	r = _filter_devnode(conf->blist_devnode, conf->elist_devnode,pp->dev);
-	if (r)
+	if (r == MATCH_WWID_BLIST)
 		return r;
 	r = _filter_wwid(conf->blist_wwid, conf->elist_wwid, pp->wwid);
-	if (r)
+	if (r == MATCH_WWID_BLIST)
 		return r;
 	r = _filter_device(conf->blist_device, conf->elist_device,
 		 	   pp->vendor_id, pp->product_id);
-	if (r)
+	if (r == MATCH_WWID_BLIST)
 		return r;
 	return 0;
 }
diff -urpN a/multipathd/main.c b/multipathd/main.c
--- a/multipathd/main.c	2007-08-15 17:18:27.000000000 -0500
+++ b/multipathd/main.c	2007-11-08 17:57:05.000000000 -0600
@@ -368,7 +368,7 @@ ev_add_path (char * devname, struct vect
 		condlog(0, "%s: failed to get path uid", devname);
 		return 1; /* leave path added to pathvec */
 	}
-	if (filter_path(conf, pp)){
+	if (filter_path(conf, pp) > 0){
 		int i = find_slot(vecs->pathvec, (void *)pp);
 		if (i != -1)
 			vector_del_slot(vecs->pathvec, i);
@@ -1062,7 +1062,7 @@ configure (struct vectors * vecs, int st
 	path_discovery(vecs->pathvec, conf, DI_ALL);
 
 	vector_foreach_slot (vecs->pathvec, pp, i){
-		if (filter_path(conf, pp)){
+		if (filter_path(conf, pp) > 0){
 			vector_del_slot(vecs->pathvec, i);
 			free_path(pp);
 			i--;

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: Multipath blacklist exceptions issues

[Stefan Bader]

[-- Attachment #1.1: Type: text/plain, Size: 1635 bytes --]

2007/11/9, Benjamin Marzinski <bmarzins@redhat.com>:
>
> <snip>.
>
> If I recall correctly, blacklist exceptions were originally proposed as
> a solution to the problem of not being able to remove the bl_product
> lines from the default device configuration, without reentering the
> whole configuration for that device.  They were specifically designed so
> that you could still blacklist the device by wwid and devnode rules.


The blacklist exceptions originally  were introduced as  both: a solution to
remove a product blacklist without having to specify the other pre-defined
settings and a way to enable some devices specifically without having to
create a blacklist with holes in it.
The way this currently works is a bit surprising. Although there are
comments in the annotated config file these are easily missed and this
probably should be improved. At the moment devnode black and whitelist has a
higher priority than wwid and wwid a higher priority than product blacklist.
This means, if you have a devnode blacklist that matches a device, the wwid
black and whitelist is never considered.
What would work is to blacklist all wwid's and then whitelist the ones that
should be used. Usings wwid's instead of the devnodes has the advantage that
these are consistent, regardless of the order the devices are added.
I am not quite sure which way is better. Should we change the code so a
whitelisting is always preceding a blacklist entry or is there a way to
point out that there is a certain workflow and mixing devnode and wwid black
and whitelists is not working well? Personally I would go for the first
option.


Stefan

[-- Attachment #1.2: Type: text/html, Size: 1948 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Multipath blacklist exceptions issues

[Benjamin Marzinski]

On Fri, Nov 09, 2007 at 03:20:08PM +0100, Stefan Bader wrote:
>    2007/11/9, Benjamin Marzinski <[1]bmarzins@redhat.com>:
> 
>      <snip>.
> 
>      If I recall correctly, blacklist exceptions were originally proposed as
>      a solution to the problem of not being able to remove the bl_product
>      lines from the default device configuration, without reentering the
>      whole configuration for that device.  They were specifically designed so
>      that you could still blacklist the device by wwid and devnode rules.
> 
>    The blacklist exceptions originally  were introduced as  both: a solution
>    to remove a product blacklist without having to specify the other
>    pre-defined settings and a way to enable some devices specifically without
>    having to create a blacklist with holes in it.
>    The way this currently works is a bit surprising. Although there are
>    comments in the annotated config file these are easily missed and this
>    probably should be improved.

The comments from the annotated config file say
## name    : blacklist_exceptions
## scope   : multipath & multipathd
## desc    : list of device names to be treated as multipath candidates
##           even if they are on the blacklist.
##           Note: blacklist exceptions are only valid in the same class.
##           It is not possible to blacklist devices using the devnode keyword
##           and to exclude some devices of them using the wwid keyword.

My patch changes the behavior back to match this description.

>    At the moment devnode black and whitelist has
>    a higher priority than wwid and wwid a higher priority than product
>    blacklist. This means, if you have a devnode blacklist that matches a
>    device, the wwid black and whitelist is never considered.
>    What would work is to blacklist all wwid's and then whitelist the ones
>    that should be used. Usings wwid's instead of the devnodes has the
>    advantage that these are consistent, regardless of the order the devices
>    are added.
>    I am not quite sure which way is better. Should we change the code so a
>    whitelisting is always preceding a blacklist entry or is there a way to
>    point out that there is a certain workflow and mixing devnode and wwid
>    black and whitelists is not working well? Personally I would go for the
>    first option.

Personally, I'd love to see the blacklisting system redesigned to make
it more powerful and initutive

The two big drawbacks with the current system are:
1. That it encourages people to blacklist by devnode, when it's possible
that their devices might come up with different device nodes on future
reboots.
2. If you have many devices but only want to multipath a few of them, it's
more confusing to set up than it needs to be.

One solution would be the following:

1. Drop the existing blacklist config options (blacklist,
blacklist_exceptions). Alternatively, deprecate them, and disallow
mixing the old style and the new style.

2. Add two new sections to the top level of multipath.conf, "invalid_types"
and "blklist" (or "blacklist", if you want to completely drop support
for the current style).

invalid_types {
	type "str"
}

Where "str" is a string of all the device types that multipath will not
do anything with.  This is only used to remove whole types of devices.
Ideally, each line would be a comma seperated list of device types, with
no regular expressions used at all. Something like

invald_types {
	type "ram,raw,loop,fd,md,dm-,sr,sdc,st,hd"
}

When multipath or multipathd sees a device of one of these types, it
just skips it, with no further work, like devnode blacklisted devices
are treated now.

blklist {
	type "str"
}

Where "str" is a string of all the device types to blacklist.  I think
that this string should also only be used to remove whole types of
devices, and not allow regular expressions if possible. Blacklisting a
single device node name, for example "sdc", is really not a good idea,
because sdc is not a persistent name.

When multipath encounters a device of a type listed in this section, it
would continue to collect path information on it, to see if it should
really be blacklisted. This means that multipath would need to run path
discovery on devices which were previously just blacklisted by devnode,
but this should be o.k.

3. Add two new subsections to the devices section, "blklist" and
"whitelist"

devices {
	blklist {
		device {
			vendor "regexp"
			product "regexp"
		}
	}
	whitelist {
		device {
			vendor "regexp"
			product "regexp"
		}
	}
}

Where "regexp" is a regular expression like we currently have for all
the blacklist rules. The devices whitelist overrides the devices
blklist, and both rules override the top level blklist. The
product_blacklist lines (which are retained in this design) will be
converted by multipath to a devices blklist.

4. Add two new subsections to the multipaths section, "blklist" and
"whitelist"

multipaths {
	blklist {
		wwid "regexp"
	}
	whitelist {
		wwid "regexp"
	}
}

Where "regexp" is a regular expression. The multipaths whitelist
overrides the multipaths blacklist, and both rules override the device
rules and the top level rules.


Doing things this way means that blacklisting devices works the same as
the other configuration parameters do, with the multipaths section rules
taking precedence over the devices section rules, and the devices
section rules taking precedence over the default rules.

It easily allows you to do things like

blklist {
	type "sd"
}

devices {
	whitelist {
		vendor "WINSYS"
		product "SF2372"
	}
}

multipaths {
	blklist {
		wwid "3600d0230000000000e13955cc3757803"
	}
}

which blacklists all scsi devices except ones a vendor string of
"WINSYS" and a product string of "SF2372".  From those, only the device
with the wwid of 3600d0230000000000e13955cc3757803 is blacklisted.

The only loss of functionality is that without blacklist_exceptions, you
can't simply negate a product_blacklist line. You will instead be
whitelisting that device, which is not what you might want.  However, I
think that there is a better solution to this problem than
blacklist_exceptions.  Instead, multipath could have a new parameter in
the devices section, "keep_defaults". If users have

keep_defaults	yes

in their multipath.conf device configuration, and there is already a
default device configuration with matching "vendor" and "product"
values, only the fields that the users specificially set will be
changed. The other fields will retain their default values. So, if a
user had the configuration

devices {
	device {
		keep_defaults		yes
		vendor			"IBM"
		product			"S/390 DASD ECKD"
		product_blacklist	""
	}
}

That would only remove the existing product_blacklist, and leave the
rest of the configuration the same.

Any thoughts on this? Good Idea? Worth doing?

-Ben

>    Stefan
> 
> References
> 
>    Visible links
>    1. mailto:bmarzins@redhat.com

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

Re: Multipath blacklist exceptions issues

[Stefan Bader]

[-- Attachment #1.1: Type: text/plain, Size: 1419 bytes --]

>
> Any thoughts on this? Good Idea? Worth doing?
>

To be honest, I do not see the real simplification in that many changes.
Spreading black- and/or white-lists over so many places seems rather
confusion to me. I agree that using devnode names is not ideal for the
reasons you mentioned. That was done mainly to have an internal blacklist of
known devices that are known not to work. Potentially this could be a device
type (= driver name?). But would this not also be possible as a new element
of the blacklist? E.g.:

blacklist {
    driver fd
    driver device-mapper
    ...
}

The problem with the current implementation, in my thinking, is that we have
a dependency between two sections (blacklist and blacklist_exceptions) and
the keywords within. Without reading any further
documentation it seems awkward that it is not possible to blacklist device
nodes and punch holes by certain wwid numbers. When I think about it, I
guess (any other opinions welcome) that a exception is what is really
intended to be used. So that should always have more priority than a
blacklist. So if the filter finds a matching entry in the
blacklist_exceptions section, the device should be used.

So my proposal would be:

1. process the blacklist_exceptions (any match enables the device)
2. process the blacklist
3. any device dropping through is also used.

Additionally I like the idea of a match-by-driver extension.

Stefan

[-- Attachment #1.2: Type: text/html, Size: 1690 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Multipath blacklist exceptions issues

[Christophe Varoqui]

[-- Attachment #1: Type: text/plain, Size: 3406 bytes --]

Indeed the current situation is fishy. Ben pointed a true braino in the
code I introduced when restructuring the blacklist lib :

in _filter_path(), I test each _filter_*() for r!=0 , where I intented
to check for r>0.

r==0 implements  : "exit on first blacklist or exception match".
 r>0 implements  : "exit on first blacklist match".

With this later behaviour I can set things like that for max safety and
efficiency :

blacklist {
        devnode .*
        device {
                vendor .*
                product .*
        }
        wwid .*
}
blacklist_exceptions {
        devnode sd.*
        device {
                vendor IET.*
                product .*
        }
        wwid "1646561646265.*"
}

or pragmatically :

blacklist {
        devnode .*
        wwid .*
}
blacklist_exceptions {
        devnode sd.*
        wwid "1646561646265.*"
}



Working that out, I also realized there may be another small
misbehaviour :

First, a little background on path discovery operations :

1) /sys/block parsing shows devnode names
2) devnode names examination shows device identification strings
3) these strings help us choose a getuid helper, which finally shows
wwids

Meaning we want the devnode blacklisting to prevail over device and
wwid, in case we know we don't have device strings available (loop, dm-,
raw, ...)

Similarily, we want the device blacklist to prevail over wwid, in case
we know we don't have getuid callout available. I have no example for
this case though, so it shouldn't be as important as the previous one.

Problem is we challenge _filter_device() after _filter_wwid().
This can be easily shufled around.


So, I submit this (attached) patch to your review.

regards,
cvaroqui

	
Le samedi 10 novembre 2007 à 00:20 +0100, Stefan Bader a écrit :
> 
>         
>         Any thoughts on this? Good Idea? Worth doing?
> 
> To be honest, I do not see the real simplification in that many
> changes. Spreading black- and/or white-lists over so many places seems
> rather confusion to me. I agree that using devnode names is not ideal
> for the reasons you mentioned. That was done mainly to have an
> internal blacklist of known devices that are known not to work.
> Potentially this could be a device type (= driver name?). But would
> this not also be possible as a new element of the blacklist? E.g.:
> 
> blacklist {
>     driver fd
>     driver device-mapper
>     ...
> }
> 
> The problem with the current implementation, in my thinking, is that
> we have a dependency between two sections (blacklist and
> blacklist_exceptions) and the keywords within. Without reading any
> further 
> documentation it seems awkward that it is not possible to blacklist
> device nodes and punch holes by certain wwid numbers. When I think
> about it, I guess (any other opinions welcome) that a exception is
> what is really intended to be used. So that should always have more
> priority than a blacklist. So if the filter finds a matching entry in
> the blacklist_exceptions section, the device should be used. 
> 
> So my proposal would be:
> 
> 1. process the blacklist_exceptions (any match enables the device)
> 2. process the blacklist
> 3. any device dropping through is also used.
> 
> Additionally I like the idea of a match-by-driver extension. 
> 
> Stefan

[-- Attachment #2: blacklist-misbehave.patch --]
[-- Type: text/x-patch, Size: 676 bytes --]

diff --git a/libmultipath/blacklist.c b/libmultipath/blacklist.c
index 9a058f7..6297516 100644
--- a/libmultipath/blacklist.c
+++ b/libmultipath/blacklist.c
@@ -297,16 +297,14 @@ _filter_path (struct config * conf, struct path * pp)
 	int r;
 
 	r = _filter_devnode(conf->blist_devnode, conf->elist_devnode,pp->dev);
-	if (r)
-		return r;
-	r = _filter_wwid(conf->blist_wwid, conf->elist_wwid, pp->wwid);
-	if (r)
+	if (r > 0)
 		return r;
 	r = _filter_device(conf->blist_device, conf->elist_device,
 		 	   pp->vendor_id, pp->product_id);
-	if (r)
+	if (r > 0)
 		return r;
-	return 0;
+	r = _filter_wwid(conf->blist_wwid, conf->elist_wwid, pp->wwid);
+	return r;
 }
 
 int

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: Multipath blacklist exceptions issues

[Benjamin Marzinski]

On Sat, Nov 10, 2007 at 12:20:31AM +0100, Stefan Bader wrote:
>      Any thoughts on this? Good Idea? Worth doing?
> 
>    To be honest, I do not see the real simplification in that many changes.
>    Spreading black- and/or white-lists over so many places seems rather
>    confusion to me. I agree that using devnode names is not ideal for the
>    reasons you mentioned. That was done mainly to have an internal blacklist
>    of known devices that are known not to work. Potentially this could be a
>    device type (= driver name?). But would this not also be possible as a new
>    element of the blacklist? E.g.:
> 
>    blacklist {
>        driver fd
>        driver device-mapper
>        ...
>    }
> 
>    The problem with the current implementation, in my thinking, is that we
>    have a dependency between two sections (blacklist and
>    blacklist_exceptions) and the keywords within. Without reading any further
>    documentation it seems awkward that it is not possible to blacklist device
>    nodes and punch holes by certain wwid numbers. When I think about it, I
>    guess (any other opinions welcome) that a exception is what is really
>    intended to be used. So that should always have more priority than a
>    blacklist. So if the filter finds a matching entry in the
>    blacklist_exceptions section, the device should be used.
> 
>    So my proposal would be:
> 
>    1. process the blacklist_exceptions (any match enables the device)
>    2. process the blacklist
>    3. any device dropping through is also used.
> 
>    Additionally I like the idea of a match-by-driver extension.

The issue I see with doing this is it messes with the one of the reasons
for having the blacklist_exceptions in the first place.

IBM asked for the ability to let customers do

blacklist_exceptions {
       device {
               vendor  "IBM"
               product "S/390.*"
       }
}

So that they could undo the product_blacklist line in the
"IBM:S/390 DASD ECKD" default device configuration.  With the method you
propose, customers will be able to undo that blacklist line. However,
they will either have to use an exception like the one above, which
makes it impossible to then blacklist any of those devices, or they will
have to use an exceptions like

blacklist_exceptions {
	wwid "foo"
	wwid "bar"
	wwid "xyzzy"
	...
}

To manually allow each device. If you have lots of devices, and you want
almost but not all of them multipathed, you no longer have to freedom to
simply blacklist the ones you don't want. With the original method, the
blacklist_exception simply makes it like the product_blacklist line
didn't exist. I'm not sure how much of a bid deal this is.

I agree that it seems wrong that you can't blacklist devices by
devnode, and then punch some holes through by wwid.  However, you are
perfectly able to blacklist devices by wwid and then puch holes through
by wwid, for instance

blacklist {
	wwid ".*"
}

blacklist_exceptions {
	wwid "foo"
}


Another possible redesign, which can do even more complex filtering than
my last method, does away with any need for product_blacklist lines, and
also keeps all the filtering information in one place is to simply have
a two top level sections in the config file, "invalid_drivers" and
"filter"

invalid_drivers {
	driver "string"
}

filter {
	blacklist_driver "string"
	blacklist_wwid "regexp"
	blacklist_device {
		vendor "regexp"
		product "regexp"
	}
	whitelist_driver "string"
	whitelist_wwid "regexp"
	whitelist_device {
		vendor "regexp"
		product "regexp"
	}
}

devices whose driver matches a rule in "invalid_drivers" are totally
ignored by multipath.

In the "filter" section, the blacklist_* rules blacklist devices just
like you would expect, and the whitelist_* rules whitelist them.  The
important thing is that the rules are checked in order, and the first
one to match is used.  The existing "product_blacklist" lines just turn
into "blacklist_device" lines that are checked after all the lines in
multipath.conf. If a device doesn't match any rules, it's allowed.

This works like the lvm.conf filter line, and makes it easy to either
blacklist a group of devices and then whitelist a few of them, or
whitelist a group devices which were blacklisted by default and then
blacklist a few of them. The only downside is that, with this method,
order suddenly matters. Hopefully a helpful comment in the config file
will keep this from confusing people.

# blacklist all scsi devices except the device with wwid "foo"
filter {
	whitelist_wwid "foo"
	blacklist_driver "sd"
}

# Remove the default blacklist for "IBM:S/390 DASD ECKD" devices, and
# then blacklist only the device named "bar"
filter {
	blacklist_wwid "bar"
	whitelist_device {
		vendor "IBM"
		product "S/390.*"
	}
}

Or we can just leave well enough alone.

-Ben

>    Stefan

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

Re: Multipath blacklist exceptions issues

[Stefan Bader]

[-- Attachment #1.1: Type: text/plain, Size: 5070 bytes --]

The flaw in Christophe's approach that I see is, that it makes it hard to
see why a device is blacklisted or not. Because the filter checks group
black- and whitelist for each of the keywords. A user does not know which is
the internal sequence of checking.

The issue I see with doing this is it messes with the one of the reasons
> for having the blacklist_exceptions in the first place.

IBM asked for the ability to let customers do
>
> blacklist_exceptions {
>        device {
>                vendor  "IBM"
>                product "S/390.*"
>        }
> }
>
> So that they could undo the product_blacklist line in the
> "IBM:S/390 DASD ECKD" default device configuration.  With the method you


Yes,  without any exceptions it was impossible to get DASDs working without
an extra device section. And this required knowledge of the complete
required parameters. Which renders the internal hardware table useless.

propose, customers will be able to undo that blacklist line. However,
> they will either have to use an exception like the one above, which
> makes it impossible to then blacklist any of those devices, or they will
> have to use an exceptions like
>
> blacklist_exceptions {
>         wwid "foo"
>         wwid "bar"
>         wwid "xyzzy"
>         ...
> }
>
> To manually allow each device. If you have lots of devices, and you want
> almost but not all of them multipathed, you no longer have to freedom to
> simply blacklist the ones you don't want. With the original method, the
>
blacklist_exception simply makes it like the product_blacklist line
> didn't exist. I'm not sure how much of a bid deal this is.


I think this is a real nutshell. At least the device blacklist for DASDs was
not introduced because there is no UID callout but as an easy way to exclude
the devices by default (probably like the driver idea). This was required
because in most cases DASDs are not used for multipathing.
It would be sufficient, if wwid whitelisting could be used to enable some
DASD devices. The UID for them follows a certain scheme and together with
wwid being a regular expression, it would be an acceptable effort to do.

Another possible redesign, which can do even more complex filtering than
> my last method, does away with any need for product_blacklist lines, and
> also keeps all the filtering information in one place is to simply have
> a two top level sections in the config file, "invalid_drivers" and
> "filter"
>
> invalid_drivers {
>         driver "string"
> }
>
> filter {
>         blacklist_driver "string"
>         blacklist_wwid "regexp"
>         blacklist_device {
>                 vendor "regexp"
>                 product "regexp"
>         }
>         whitelist_driver "string"
>         whitelist_wwid "regexp"
>         whitelist_device {
>                 vendor "regexp"
>                 product "regexp"
>         }
> }


I do not understand why the invalid_driver section is required. Would it not
be the same as putting blacklist_driver entries at the beginning of the
filter section? Otherwise I like that idea because it clearly defines the
sequence of filtering visible to the user.

devices whose driver matches a rule in "invalid_drivers" are totally
> ignored by multipath.
>
> In the "filter" section, the blacklist_* rules blacklist devices just
> like you would expect, and the whitelist_* rules whitelist them.  The
> important thing is that the rules are checked in order, and the first
> one to match is used.  The existing "product_blacklist" lines just turn
> into "blacklist_device" lines that are checked after all the lines in
> multipath.conf. If a device doesn't match any rules, it's allowed.
>
> This works like the lvm.conf filter line, and makes it easy to either
> blacklist a group of devices and then whitelist a few of them, or
> whitelist a group devices which were blacklisted by default and then
> blacklist a few of them. The only downside is that, with this method,
> order suddenly matters. Hopefully a helpful comment in the config file
> will keep this from confusing people.
>

In some way order already mattered before. Just not visible to everybody. A
wwid blacklist entry could not blacklist a certain device for which the
device node was whitelisted. I think with the filter idea it would be
possible to get it all together: having an internal filter, the ability to
prevent accessing devices that do not work and also the ability to override
the internal filter if that is desired. Internal filter rules would just be
appended to the user specified list. The only trick might be that a caller
to filter function can specify which specific filters to use.
For example when deciding which devices from /sys/block should be processed
further, only the driver an devnode entries would be evaluated and the first
match is used. Later, if the device vendor and model are known the list of
remaining devices would be again filtered. This time by the device entries.
And finally, if there is a wwid known for each remaining device, filter
those that match a wwid entry.

Stefan

[-- Attachment #1.2: Type: text/html, Size: 7437 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Multipath blacklist exceptions issues

[Benjamin Marzinski]

On Sun, Nov 11, 2007 at 01:25:51AM +0100, Stefan Bader wrote:
>      Another possible redesign, which can do even more complex filtering than
>      my last method, does away with any need for product_blacklist lines, and
>      also keeps all the filtering information in one place is to simply have
>      a two top level sections in the config file, "invalid_drivers" and
>      "filter"
> 
>      invalid_drivers {
>              driver "string"
>      }
> 
>      filter {
>              blacklist_driver "string"
>              blacklist_wwid "regexp"
>              blacklist_device {
>                      vendor "regexp"
>                      product "regexp"
>              }
>              whitelist_driver "string"
>              whitelist_wwid "regexp"
>              whitelist_device {
>                      vendor "regexp"
>                      product "regexp"
>              }
>      }
> 
>    I do not understand why the invalid_driver section is required. Would it
>    not be the same as putting blacklist_driver entries at the beginning of
>    the filter section? Otherwise I like that idea because it clearly defines
>    the sequence of filtering visible to the user.

Um. Mostly because I just thought up this idea to address the issues you
had with my first idea.  I though it would be nice if multipath had a
clearly defined set of devices it didn't need to bother checking, and
then it would only need to look at the filter rules after all the path
information had been gathered.  However, you are correct.  Multipath can
just check the blacklist_driver rules at the beginning of the filter
section before gathering path information, and the rest of the rules
afterwards, to achieve the same effect. As a side note, blacklist_driver
should also allow the special string "*", which blacklists all devices.
Also, whitelist_driver rules don't make any sense.

There are some minor issues that crop up, if we do away with
invalid_drivers.

Without the invalid_drivers section, the current default devnode
blacklist rules would turn into blacklist_driver rules that were
automatically appended to the beginning of the filter section.  The
users' inability to whitelist certain devices seems like it would be
less obvious in this situation than it would be if there was a specific
section for invalid devices. But this does seem to be a pretty minor
fault. As a side note, the product_blacklist rules would still need to
become blacklist_device rules that are appended to the end of the filter
section, so it is possible to override them.

There is also the issue of how to keep multipath from doing anything.
Currently, you need to use

blacklist {
	devnode "*"
}

to keep multipath from doing anything to your devices. Unfortunately, if
you use the following

blacklist {
	wwid "*"
}

it doesn't keep multipath from touching your devices (for instance, the
getuid callout is still run on them), although customers might naively
think that it should. With invalid_drivers, it is obvious how to keep
multipath from touching the devices. Without it, you are forced to do

filter {
	blacklist_driver "*"
}

>      devices whose driver matches a rule in "invalid_drivers" are totally
>      ignored by multipath.
> 
>      In the "filter" section, the blacklist_* rules blacklist devices just
>      like you would expect, and the whitelist_* rules whitelist them.  The
>      important thing is that the rules are checked in order, and the first
>      one to match is used.  The existing "product_blacklist" lines just turn
>      into "blacklist_device" lines that are checked after all the lines in
>      multipath.conf. If a device doesn't match any rules, it's allowed.
> 
>      This works like the lvm.conf filter line, and makes it easy to either
>      blacklist a group of devices and then whitelist a few of them, or
>      whitelist a group devices which were blacklisted by default and then
>      blacklist a few of them. The only downside is that, with this method,
>      order suddenly matters. Hopefully a helpful comment in the config file
>      will keep this from confusing people.
> 
>    In some way order already mattered before. Just not visible to everybody.
>    A wwid blacklist entry could not blacklist a certain device for which the
>    device node was whitelisted. I think with the filter idea it would be
>    possible to get it all together: having an internal filter, the ability to
>    prevent accessing devices that do not work and also the ability to
>    override the internal filter if that is desired. Internal filter rules
>    would just be appended to the user specified list. The only trick might be
>    that a caller to filter function can specify which specific filters to
>    use.
>    For example when deciding which devices from /sys/block should be
>    processed further, only the driver an devnode entries would be evaluated
>    and the first match is used. Later, if the device vendor and model are
>    known the list of remaining devices would be again filtered. This time by
>    the device entries. And finally, if there is a wwid known for each
>    remaining device, filter those that match a wwid entry.

If I understand what your idea is, I'm not sure that it's what we want
to do. In my mind, any of the following filters should allow you to
create a multipath device for the device with the wwid "foo"

filter {
	whitelist_wwid "foo"		# foo is a scsi device
	blacklist_driver "sd"
}

filter {
	whitelist_wwid "foo"		# foo is an ACME:WIDGET device
	blacklist_device {
		vendor		"ACME"
		product		"WIDGET"
	}
}

filter {
	whitelist_wwid "foo"	# foo is an "IBM:S/390 DASD ECKD" device
}

If you only looked at driver entries when deciding whether to continue
processing a devices in /sys/block, or only looked at device entries
once the vendor/product info was known, you would never get to the wwid.

I think a better way is to have multipath only apply the
blacklist_driver filters at the beginning of the filter section, when it
is deciding whether to process a device in /sys/block. If the device is
not blacklisted by the time multipath hits a non-"blacklist_driver"
rule, then multipath goes ahead and gathers path information.

If we wanted, we could do the same type of thing after multipath gets
the vendor and product information, however multipath doesn't currently
check if the device is blacklisted at this point. But again, I think we
should only apply the blacklist_device rules at the beginning of the
filter list (after any blacklist_driver rules, of course).

The only problem with just checking the beginning rules, is in has the
potential to surprise users.  For instance, if a user changes their
configuration from 

filter {
	blacklist_driver "cciss"
}

to

filter {
	whitelist_wwid "foo"	# foo is NOT a "cciss" device
	blacklist_driver "cciss"
}

suddenly, the user's cciss devices are going to be checked. They will
stay blacklisted, but if the getuid callout wasn't working on them,
multipath will suddenly start spitting out warning messages.  This is
another issue that would be solved by having an invalid_drivers section.


Finally, you mentioned devnode rules.  Do you think that they are
necessary?  The purpose of the driver rules is to replace the devnode
rules with ones that don't allow users to do things like

blacklist {
	devnode "sdc"
}

which can be done other ways, and might end up doing the wrong thing
later if their device names change.

-Ben

> 
>    Stefan

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

Re: Multipath blacklist exceptions issues

[Benjamin Marzinski]

On Sat, Nov 10, 2007 at 02:17:45AM +0100, Christophe Varoqui wrote:
> Indeed the current situation is fishy. Ben pointed a true braino in the
> code I introduced when restructuring the blacklist lib :
> 
> in _filter_path(), I test each _filter_*() for r!=0 , where I intented
> to check for r>0.
> 
> r==0 implements  : "exit on first blacklist or exception match".
>  r>0 implements  : "exit on first blacklist match".
> 
> With this later behaviour I can set things like that for max safety and
> efficiency :
> 
> blacklist {
>         devnode .*
>         device {
>                 vendor .*
>                 product .*
>         }
>         wwid .*
> }
> blacklist_exceptions {
>         devnode sd.*
>         device {
>                 vendor IET.*
>                 product .*
>         }
>         wwid "1646561646265.*"
> }
> 
> or pragmatically :
> 
> blacklist {
>         devnode .*
>         wwid .*
> }
> blacklist_exceptions {
>         devnode sd.*
>         wwid "1646561646265.*"
> }
> 
> 
> 
> Working that out, I also realized there may be another small
> misbehaviour :
> 
> First, a little background on path discovery operations :
> 
> 1) /sys/block parsing shows devnode names
> 2) devnode names examination shows device identification strings
> 3) these strings help us choose a getuid helper, which finally shows
> wwids
> 
> Meaning we want the devnode blacklisting to prevail over device and
> wwid, in case we know we don't have device strings available (loop, dm-,
> raw, ...)
> 
> Similarily, we want the device blacklist to prevail over wwid, in case
> we know we don't have getuid callout available. I have no example for
> this case though, so it shouldn't be as important as the previous one.
> 
> Problem is we challenge _filter_device() after _filter_wwid().
> This can be easily shufled around.
> 
> 
> So, I submit this (attached) patch to your review.

Looks fine to me.

-Ben
 
> regards,
> cvaroqui
> 
> 	
> Le samedi 10 novembre 2007 à 00:20 +0100, Stefan Bader a écrit :
> > 
> >         
> >         Any thoughts on this? Good Idea? Worth doing?
> > 
> > To be honest, I do not see the real simplification in that many
> > changes. Spreading black- and/or white-lists over so many places seems
> > rather confusion to me. I agree that using devnode names is not ideal
> > for the reasons you mentioned. That was done mainly to have an
> > internal blacklist of known devices that are known not to work.
> > Potentially this could be a device type (= driver name?). But would
> > this not also be possible as a new element of the blacklist? E.g.:
> > 
> > blacklist {
> >     driver fd
> >     driver device-mapper
> >     ...
> > }
> > 
> > The problem with the current implementation, in my thinking, is that
> > we have a dependency between two sections (blacklist and
> > blacklist_exceptions) and the keywords within. Without reading any
> > further 
> > documentation it seems awkward that it is not possible to blacklist
> > device nodes and punch holes by certain wwid numbers. When I think
> > about it, I guess (any other opinions welcome) that a exception is
> > what is really intended to be used. So that should always have more
> > priority than a blacklist. So if the filter finds a matching entry in
> > the blacklist_exceptions section, the device should be used. 
> > 
> > So my proposal would be:
> > 
> > 1. process the blacklist_exceptions (any match enables the device)
> > 2. process the blacklist
> > 3. any device dropping through is also used.
> > 
> > Additionally I like the idea of a match-by-driver extension. 
> > 
> > Stefan

> diff --git a/libmultipath/blacklist.c b/libmultipath/blacklist.c
> index 9a058f7..6297516 100644
> --- a/libmultipath/blacklist.c
> +++ b/libmultipath/blacklist.c
> @@ -297,16 +297,14 @@ _filter_path (struct config * conf, struct path * pp)
>  	int r;
>  
>  	r = _filter_devnode(conf->blist_devnode, conf->elist_devnode,pp->dev);
> -	if (r)
> -		return r;
> -	r = _filter_wwid(conf->blist_wwid, conf->elist_wwid, pp->wwid);
> -	if (r)
> +	if (r > 0)
>  		return r;
>  	r = _filter_device(conf->blist_device, conf->elist_device,
>  		 	   pp->vendor_id, pp->product_id);
> -	if (r)
> +	if (r > 0)
>  		return r;
> -	return 0;
> +	r = _filter_wwid(conf->blist_wwid, conf->elist_wwid, pp->wwid);
> +	return r;
>  }
>  
>  int

Re: Multipath blacklist exceptions issues

[Kiyoshi Ueda]

Hi Ben,

On Tue, 13 Nov 2007 14:18:04 -0600, Benjamin Marzinski <bmarzins@redhat.com> wrote:
> multipath will suddenly start spitting out warning messages.  This is
> another issue that would be solved by having an invalid_drivers section.

I agree with separating those filters.
Those filters are used for different purposes, one is to avoid
the getuid callout, the other is to decide whether use the device
for multipath.
Merging those different purpose filters into one structure will mess
things up.

I think that the following rule which you proposed is simple enough:
  o unneeded devices are dropped first by the invalid_drivers section
  o First match of the filter section is used for each device like LVM2

 
> Finally, you mentioned devnode rules.  Do you think that they are
> necessary?  The purpose of the driver rules is to replace the devnode
> rules with ones that don't allow users to do things like
> 
> blacklist {
> 	devnode "sdc"
> }
> 
> which can be done other ways, and might end up doing the wrong thing
> later if their device names change.

Although I'm not sure it's necessary, I prefer to leave it.
I sometimes use devnode rules just for testing, like creating a map
including only a part of paths (not all-paths).
That sort of mapping is not possible without devnode rule now.

Thanks,
Kiyoshi Ueda

Re: Multipath blacklist exceptions issues

[Stefan Bader]

[-- Attachment #1.1: Type: text/plain, Size: 4338 bytes --]

>
>
> Um. Mostly because I just thought up this idea to address the issues you
> had with my first idea.  I though it would be nice if multipath had a
> clearly defined set of devices it didn't need to bother checking, and
> then it would only need to look at the filter rules after all the path
> information had been gathered.  However, you are correct.  Multipath can
> just check the blacklist_driver rules at the beginning of the filter
> section before gathering path information, and the rest of the rules
> afterwards, to achieve the same effect. As a side note, blacklist_driver
> should also allow the special string "*", which blacklists all devices.
> Also, whitelist_driver rules don't make any sense.


I tried to avoid having multiple filter-defining sections but I must admit
having everything in one section might be confusing as well. The problem is,
that there are three stages at which a decission has to be made to continue
or not. First by driver or node name (Kiyoshi has a valid argument there).
Every match at this early stage prevents useless work. Whitelist rules for
driver and devnode make sense if internal rules have to be reversed (e.g.
internal blacklist uses blacklist_driver "*").
Second stage is to check whether it makes sense to continue with a certain
class of devices (from vendor, model). At this point I do not think it makes
sense to look at the driver and devnode rules again.
And finally, if the UID information has been gathered, there would be the
moment to check which of them should be used.

When the current code does the final filtering it goes again through all
stacked filters and by that a whitelist for a devnode(range) cannot be
limited by a wwid blacklist.

There are some minor issues that crop up, if we do away with
> invalid_drivers.
>
> Without the invalid_drivers section, the current default devnode
> blacklist rules would turn into blacklist_driver rules that were
> automatically appended to the beginning of the filter section.  The


Not appended at the beginning, because that could never be changed by
user-specified  rules. Everything pre-defined would have to get appended
after the user defined list.

users' inability to whitelist certain devices seems like it would be
> less obvious in this situation than it would be if there was a specific
> section for invalid devices. But this does seem to be a pretty minor
> fault. As a side note, the product_blacklist rules would still need to
> become blacklist_device rules that are appended to the end of the filter
> section, so it is possible to override them.


Yes.

There is also the issue of how to keep multipath from doing anything.
> Currently, you need to use
>
> blacklist {
>         devnode "*"
> }
>
> to keep multipath from doing anything to your devices. Unfortunately, if
> you use the following
>
> blacklist {
>         wwid "*"
> }
>
> it doesn't keep multipath from touching your devices (for instance, the
> getuid callout is still run on them), although customers might naively
> think that it should. With invalid_drivers, it is obvious how to keep
> multipath from touching the devices. Without it, you are forced to do


Not if devnode (and driver) rules are checked before the other rules.

filter {
>         blacklist_driver "*"
> }





> If I understand what your idea is, I'm not sure that it's what we want
> to do. In my mind, any of the following filters should allow you to
> create a multipath device for the device with the wwid "foo"


I admit my idea had a major flaw. It is just not compatible with the way
device detection is run. It might turn out that the only thing that has to
be changed is to change which filter rules are used at a certain point of
the detection.

Decide which device to ignore at all:
- look at the whitelist entries for devnode (and driver), continue to check
if found
- look at the blacklist entries for devnode (and driver), do not use if
found
- no match, continue
Decide for which devices to call getuid:
- look at the whitelist entries for device, continue to check if found
- look at the blacklist entries for device, do not use if found
- no match, continue
Decide which UIDs should be used:
- look at the whitelist entries for wwid, use if found
- look at the blacklist entries for wwid, do not use if found
- no match, use device


Stefan

[-- Attachment #1.2: Type: text/html, Size: 5685 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Multipath blacklist exceptions issues

[Kiyoshi Ueda]

Hi Stefan,

On Wed, 14 Nov 2007 00:17:25 +0100, "Stefan Bader" <sbader3@googlemail.com> wrote:
> > Um. Mostly because I just thought up this idea to address the issues you
> > had with my first idea.  I though it would be nice if multipath had a
> > clearly defined set of devices it didn't need to bother checking, and
> > then it would only need to look at the filter rules after all the path
> > information had been gathered.  However, you are correct.  Multipath can
> > just check the blacklist_driver rules at the beginning of the filter
> > section before gathering path information, and the rest of the rules
> > afterwards, to achieve the same effect. As a side note, blacklist_driver
> > should also allow the special string "*", which blacklists all devices.
> > Also, whitelist_driver rules don't make any sense.
> 
> 
> I tried to avoid having multiple filter-defining sections but I must admit
> having everything in one section might be confusing as well. The problem is,
> that there are three stages at which a decission has to be made to continue
> or not. First by driver or node name (Kiyoshi has a valid argument there).
> Every match at this early stage prevents useless work. Whitelist rules for
> driver and devnode make sense if internal rules have to be reversed (e.g.
> internal blacklist uses blacklist_driver "*").
> Second stage is to check whether it makes sense to continue with a certain
> class of devices (from vendor, model). At this point I do not think it makes
> sense to look at the driver and devnode rules again.
> And finally, if the UID information has been gathered, there would be the
> moment to check which of them should be used.
> 
> When the current code does the final filtering it goes again through all
> stacked filters and by that a whitelist for a devnode(range) cannot be
> limited by a wwid blacklist.
...
snip
...
> I admit my idea had a major flaw. It is just not compatible with the way
> device detection is run. It might turn out that the only thing that has to
> be changed is to change which filter rules are used at a certain point of
> the detection.

Before discussing details, I would like to make my standpoint clear.
  - Minimize the number of sections user has to modify as possible
  - Don't confuse users by pretending too much flexibility

If we could do with only one section, it would be ideal.
However, wwid can be obtained only by getuid callout, which has
side effects of accessing devices and yielding unpleasant kernel
warning messages, etc.
So the filtering has to be splitted into 2 stages at least:
  before running getuid callout and after that
and we can't use wwid in the 1st stage.
Not to pretend that mixing wwid filtering and others is possible,
it's natural to have 2 different filtering sections.
 

> Decide which device to ignore at all:
> - look at the whitelist entries for devnode (and driver), continue to check
> if found
> - look at the blacklist entries for devnode (and driver), do not use if
> found
> - no match, continue
> Decide for which devices to call getuid:
> - look at the whitelist entries for device, continue to check if found
> - look at the blacklist entries for device, do not use if found
> - no match, continue
> Decide which UIDs should be used:
> - look at the whitelist entries for wwid, use if found
> - look at the blacklist entries for wwid, do not use if found
> - no match, use device

Why do you need to separate the first stage and the second stage?
I don't think we need to separate them.
I think the point is whether execute getuid callout or not.
So we should need just 2 stages like below:
(The keyword "pre_getuid_filter" is not so good, just a example.)

----------------------- /etc/multipath.conf -------------------------
# First stage filter.  Drop unneeded devices on this stage
pre_getuid_filter {
	whitelist_driver "scsi|cciss"
	whitelist_device {
		vendor	"IBM"
		model	"S/390 DASD ECKD"
	}
	blacklist_driver "*"
}

# Second stage filter
filter {
	blacklist_devnode "sda"
	whitelist_wwid "012345"
	blacklist_wwid "*"
	# only wwid filter may be enough in this stage
}
---------------------------------------------------------------------

---------------------------- multipathd -----------------------------
for_each_devnode in /sys/block {
    o gather all sysfs information including vendor and model
    o decide whether it is needed using the first stage filter
      (the matching rule is that first match is used)
    o store it in pathvec if needed
}

for_each_remained_devnode in pathvec {
    o run getuid callout
    o decide whether it should be used for map by the second stage filter
      (the matching rule is that first match is used)
}
---------------------------------------------------------------------

What do you think?

Thanks,
Kiyoshi Ueda

Re: Multipath blacklist exceptions issues

[Stefan Bader]

[-- Attachment #1.1: Type: text/plain, Size: 2315 bytes --]

2007/11/15, Kiyoshi Ueda <k-ueda@ct.jp.nec.com>:
>
> Hi Stefan,
>
> Did you drop dm-devel from Cc meaningly?


Oops, no. Just me accidentally using reply instead of reply-all

On Wed, 14 Nov 2007 23:11:55 +0100, "Stefan Bader" <sbader3@googlemail.com>
> wrote:
> > > Before discussing details, I would like to make my standpoint clear.
> > >   - Minimize the number of sections user has to modify as possible
> > >   - Don't confuse users by pretending too much flexibility
> > >
> > > If we could do with only one section, it would be ideal.
> > > However, wwid can be obtained only by getuid callout, which has
> > > side effects of accessing devices and yielding unpleasant kernel
> > > warning messages, etc.
> > > So the filtering has to be splitted into 2 stages at least:
> > >   before running getuid callout and after that
> > > and we can't use wwid in the 1st stage.
> > > Not to pretend that mixing wwid filtering and others is possible,
> > > it's natural to have 2 different filtering sections.
> >
> >
> > The behavior that is not that well expected is that if you can not do
> the
> > following:
> >
> > blacklist {
> >     devnode ".*"
> >     wwid ".*"
> > }
> > blacklist_execptions {
> >     devnode "^sda[0-9+]"
> >     wwid "foo"
> > }
>
> Sorry, I don't understand what you mean.  Could you elaborate that?
> Do you mean that my proposal doesn't work for the case above?
> Or do you mean that current code doesn't work as expected although
> we can specify like above, but my proposal removes the confusion?


I am sorry, I was a bit unclear. The example was for the current code, which
allows to write a blacklist like in the example but does not give the
expected result. Your proposal would (in my opinion) remove the confusion
because both stages would apply the rules in the order they are written.
Which is not the case in the current code.

In the current code there are, from my point of view, two problems:
1. the rules are not checked in the order they are written but in the order
devnode, wwid, device.
2. the first match in the order above is used. There is no way to change
this. For example (current code):

blacklist {
    devnode ".*"
}
blacklist_exception {
    wwid "foo"
    devnode "dasda"
}

This will only use dasda. The wwid entries are just useless.

Regards,
Stefan

[-- Attachment #1.2: Type: text/html, Size: 3253 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Multipath blacklist exceptions issues

[Kiyoshi Ueda]

On Thu, 15 Nov 2007 16:07:35 +0100, "Stefan Bader" <Stefan.Bader@de.ibm.com> wrote:
> > Did you drop dm-devel from Cc meaningly?
> 
> 
> Oops, no. Just me accidentally using reply instead of reply-all

OK.  I attached the Stefan's reply which was sent to only me
so that dm-devel can see it.


-------------------------------------------------------------------
Hi Kiyoshi,

2007/11/14, Kiyoshi Ueda <k-ueda@ct.jp.nec.com>:
>
>
> Before discussing details, I would like to make my standpoint clear.
>   - Minimize the number of sections user has to modify as possible
>   - Don't confuse users by pretending too much flexibility
>
> If we could do with only one section, it would be ideal.
> However, wwid can be obtained only by getuid callout, which has
> side effects of accessing devices and yielding unpleasant kernel
> warning messages, etc.
> So the filtering has to be splitted into 2 stages at least:
>   before running getuid callout and after that
> and we can't use wwid in the 1st stage.
> Not to pretend that mixing wwid filtering and others is possible,
> it's natural to have 2 different filtering sections.


The behavior that is not that well expected is that if you can not do the
following:

blacklist {
    devnode ".*"
    wwid ".*"
}
blacklist_execptions {
    devnode "^sda[0-9+]"
    wwid "foo"
}

> Decide which device to ignore at all:
> > - look at the whitelist entries for devnode (and driver), continue to
> check
> > if found
> > - look at the blacklist entries for devnode (and driver), do not use if
> > found
> > - no match, continue
> > Decide for which devices to call getuid:
> > - look at the whitelist entries for device, continue to check if found
> > - look at the blacklist entries for device, do not use if found
> > - no match, continue
> > Decide which UIDs should be used:
> > - look at the whitelist entries for wwid, use if found
> > - look at the blacklist entries for wwid, do not use if found
> > - no match, use device
>
> Why do you need to separate the first stage and the second stage?
> I don't think we need to separate them.


There just might be block devices that should be ignored right away. Not all
drivers  have a vendor and model attribute. Though, such devices could be
ignored as well.

I think the point is whether execute getuid callout or not.
> So we should need just 2 stages like below:
> (The keyword "pre_getuid_filter" is not so good, just a example.)
>
> ----------------------- /etc/multipath.conf -------------------------
> # First stage filter.  Drop unneeded devices on this stage
> pre_getuid_filter {
>         whitelist_driver "scsi|cciss"
>         whitelist_device {
>                 vendor  "IBM"
>                 model   "S/390 DASD ECKD"
>         }
>         blacklist_driver "*"
> }

# Second stage filter
> filter {
>         blacklist_devnode "sda"
>         whitelist_wwid "012345"
>         blacklist_wwid "*"
>         # only wwid filter may be enough in this stage
> }
> ---------------------------------------------------------------------


It clearly shows that there are different stages in filtering. Good as long
there are only two stages.

---------------------------- multipathd -----------------------------
> for_each_devnode in /sys/block {
>     o gather all sysfs information including vendor and model
>     o decide whether it is needed using the first stage filter
>       (the matching rule is that first match is used)
>     o store it in pathvec if needed
> }


Basically split the filter rules into two distinct sets: driver and device
rules for pre-filtering and devnode and wwid for post-getuid-filtering.
As I said, this looks fine, if those two stages are enough for everyone.

Regards,
Stefan

Re: Multipath blacklist exceptions issues

[Kiyoshi Ueda]

Hi Stefan,

On Thu, 15 Nov 2007 16:07:35 +0100, "Stefan Bader" <Stefan.Bader@de.ibm.com> wrote:
> > > > Before discussing details, I would like to make my standpoint clear.
> > > >   - Minimize the number of sections user has to modify as possible
> > > >   - Don't confuse users by pretending too much flexibility
> > > >
> > > > If we could do with only one section, it would be ideal.
> > > > However, wwid can be obtained only by getuid callout, which has
> > > > side effects of accessing devices and yielding unpleasant kernel
> > > > warning messages, etc.
> > > > So the filtering has to be splitted into 2 stages at least:
> > > >   before running getuid callout and after that
> > > > and we can't use wwid in the 1st stage.
> > > > Not to pretend that mixing wwid filtering and others is possible,
> > > > it's natural to have 2 different filtering sections.
> > >
> > >
> > > The behavior that is not that well expected is that if you can not do
> > > the following:
> > >
> > > blacklist {
> > >     devnode ".*"
> > >     wwid ".*"
> > > }
> > > blacklist_execptions {
> > >     devnode "^sda[0-9+]"
> > >     wwid "foo"
> > > }
> >
> > Sorry, I don't understand what you mean.  Could you elaborate that?
> > Do you mean that my proposal doesn't work for the case above?
> > Or do you mean that current code doesn't work as expected although
> > we can specify like above, but my proposal removes the confusion?
> 
> 
> I am sorry, I was a bit unclear. The example was for the current code, which
> allows to write a blacklist like in the example but does not give the
> expected result. Your proposal would (in my opinion) remove the confusion
> because both stages would apply the rules in the order they are written.
> Which is not the case in the current code.
> 
> In the current code there are, from my point of view, two problems:
> 1. the rules are not checked in the order they are written but in the order
> devnode, wwid, device.
> 2. the first match in the order above is used. There is no way to change
> this. For example (current code):
> 
> blacklist {
>     devnode ".*"
> }
> blacklist_exception {
>     wwid "foo"
>     devnode "dasda"
> }
> 
> This will only use dasda. The wwid entries are just useless.

Thank you for the detailed explanation.
I understand that you agree with my proposal.

Thanks,
Kiyoshi Ueda

Re: Multipath blacklist exceptions issues

[Benjamin Marzinski]

On Wed, Nov 14, 2007 at 03:44:56PM -0500, Kiyoshi Ueda wrote:
> 
> Why do you need to separate the first stage and the second stage?
> I don't think we need to separate them.
> I think the point is whether execute getuid callout or not.
> So we should need just 2 stages like below:
> (The keyword "pre_getuid_filter" is not so good, just a example.)
> 
> ----------------------- /etc/multipath.conf -------------------------
> # First stage filter.  Drop unneeded devices on this stage
> pre_getuid_filter {
> 	whitelist_driver "scsi|cciss"
> 	whitelist_device {
> 		vendor	"IBM"
> 		model	"S/390 DASD ECKD"
> 	}
> 	blacklist_driver "*"
> }
> 
> # Second stage filter
> filter {
> 	blacklist_devnode "sda"
> 	whitelist_wwid "012345"
> 	blacklist_wwid "*"
> 	# only wwid filter may be enough in this stage
> }
> ---------------------------------------------------------------------
> 
> ---------------------------- multipathd -----------------------------
> for_each_devnode in /sys/block {
>     o gather all sysfs information including vendor and model
>     o decide whether it is needed using the first stage filter
>       (the matching rule is that first match is used)
>     o store it in pathvec if needed
> }
> 
> for_each_remained_devnode in pathvec {
>     o run getuid callout
>     o decide whether it should be used for map by the second stage filter
>       (the matching rule is that first match is used)
> }
> ---------------------------------------------------------------------
> 
> What do you think?

This method has a lot going for it.  I like the ability to override the
default blacklisted drivers, even if it's almost never used, which you
couldn't do with an "invalid_drivers" section. I also like the finer
granularity on which devices to run the getuid callout on.

On the other hand, I agree with Stefan that there will be issues with
trying to gather the sysfs information for devices that just don't have
it.  Like he said, you can ignore them, but I wonder if this might cause
problems for the rare, but still possible, case where you expect that
a device should properly provide all its sysfs information, but for some
reason, you can't get it. In this case the device would most likely be
silently ignored.  Obviously, we can provide the information if you run
the commands with a higher verbosity.

The bigger issue I have is that there should be a way to tell multipath,
"Don't do anything."  Many people have a multipath package installed on
their systems that they don't even know about, and they don't want it to
do anything.  Right now, if you have

blacklist {
	devnode ".*"
}

You pretty much get that behaviour. If anything on their system happens
to run the multipath command, nothing much really happens.  With this
method, your adding more things that multipath has to do before if
realizes that it shouldn't be doing anything. Of course this could
easily be solved by adding the parameter to the defaults section of
multipath.conf.

defaults {
	disable yes
}

This would allow multipath to quit immediately after reading the config
file, and it would seperate the choice to disable multipath from the
blacklist setup.

-Ben

> Thanks,
> Kiyoshi Ueda
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel

Re: Multipath blacklist exceptions issues

[Stefan Bader]

[-- Attachment #1.1: Type: text/plain, Size: 2054 bytes --]

2007/11/15, Benjamin Marzinski <bmarzins@redhat.com>:
>
>
> > What do you think?
>
> On the other hand, I agree with Stefan that there will be issues with
> trying to gather the sysfs information for devices that just don't have
> it.  Like he said, you can ignore them, but I wonder if this might cause
> problems for the rare, but still possible, case where you expect that
> a device should properly provide all its sysfs information, but for some
> reason, you can't get it. In this case the device would most likely be
> silently ignored.  Obviously, we can provide the information if you run
> the commands with a higher verbosity.


I think this wouls be acceptable. I am not sure whether any useful device
that is used for multipathing is not required to provide a /sys/block/device
link and a vendor/model attributes there. As long as the reason for which a
device is dropped can be seen with a higher verbosity level this should be
fine.

The bigger issue I have is that there should be a way to tell multipath,
> "Don't do anything."  Many people have a multipath package installed on
> their systems that they don't even know about, and they don't want it to
> do anything.  Right now, if you have
>
> blacklist {
>         devnode ".*"
> }
>
> You pretty much get that behaviour. If anything on their system happens
> to run the multipath command, nothing much really happens.  With this
> method, your adding more things that multipath has to do before if
> realizes that it shouldn't be doing anything. Of course this could
> easily be solved by adding the parameter to the defaults section of
> multipath.conf.
>
> defaults {
>         disable yes
> }
>
> This would allow multipath to quit immediately after reading the config
> file, and it would seperate the choice to disable multipath from the
> blacklist setup.


Would it really hurt that much if multipath has to go through some devices
before doing nothing? I think it should not take that much time to go
through /sys/block and gather that initial information.

Regards,
Stefan

[-- Attachment #1.2: Type: text/html, Size: 2699 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Multipath blacklist exceptions issues

[Kiyoshi Ueda]

Hi Ben,

On Thu, 15 Nov 2007 13:24:28 -0600, Benjamin Marzinski <bmarzins@redhat.com> wrote:
> On Wed, Nov 14, 2007 at 03:44:56PM -0500, Kiyoshi Ueda wrote:
> > 
> > Why do you need to separate the first stage and the second stage?
> > I don't think we need to separate them.
> > I think the point is whether execute getuid callout or not.
> > So we should need just 2 stages like below:
> > (The keyword "pre_getuid_filter" is not so good, just a example.)
> > 
> > ----------------------- /etc/multipath.conf -------------------------
> > # First stage filter.  Drop unneeded devices on this stage
> > pre_getuid_filter {
> > 	whitelist_driver "scsi|cciss"
> > 	whitelist_device {
> > 		vendor	"IBM"
> > 		model	"S/390 DASD ECKD"
> > 	}
> > 	blacklist_driver "*"
> > }
> > 
> > # Second stage filter
> > filter {
> > 	blacklist_devnode "sda"
> > 	whitelist_wwid "012345"
> > 	blacklist_wwid "*"
> > 	# only wwid filter may be enough in this stage
> > }
> > ---------------------------------------------------------------------
> > 
> > ---------------------------- multipathd -----------------------------
> > for_each_devnode in /sys/block {
> >     o gather all sysfs information including vendor and model
> >     o decide whether it is needed using the first stage filter
> >       (the matching rule is that first match is used)
> >     o store it in pathvec if needed
> > }
> > 
> > for_each_remained_devnode in pathvec {
> >     o run getuid callout
> >     o decide whether it should be used for map by the second stage filter
> >       (the matching rule is that first match is used)
> > }
> > ---------------------------------------------------------------------
> > 
> > What do you think?
> 
> This method has a lot going for it.  I like the ability to override the
> default blacklisted drivers, even if it's almost never used, which you
> couldn't do with an "invalid_drivers" section. I also like the finer
> granularity on which devices to run the getuid callout on.
> 
> On the other hand, I agree with Stefan that there will be issues with
> trying to gather the sysfs information for devices that just don't have
> it.  Like he said, you can ignore them, but I wonder if this might cause
> problems for the rare, but still possible, case where you expect that
> a device should properly provide all its sysfs information, but for some
> reason, you can't get it. In this case the device would most likely be
> silently ignored.  Obviously, we can provide the information if you run
> the commands with a higher verbosity.

I think it's no problem because of the reason below.

It depends on subsystem type whether vendor/model information exist
in sysfs or not.
Multipath-tools already has sysfs scanners for each subsystem type
(e.g. scsi_sysfs_pathinfo, ccw_sysfs_pathinfo, cciss_sysfs_pathinfo).
When there is no vendor/model information, each scanner can make
appropriate decision: for example,
  - scsi scanner will take it as error and drop the device
  - cciss scanner won't care about it and use the device

After that, devices without vendor/model information don't match
any "device" rules, either blacklist nor whitelist.
They can match other rules like "devnode" and "driver".

Thanks,
Kiyoshi Ueda