Re: [dm-devel] [PATCH 0/2] multipathd: Add 'sysfs' prioritizer

2016-06-03 Thread Benjamin Marzinski
On Fri, Jun 03, 2016 at 10:34:20AM +0200, Hannes Reinecke wrote: > Hi all, > > hardly necessary to send an entire patchset, but there you go. > I've now modified the sysfs prioritizer to generate the same > priority the alua one would do, including the 'exclusive_pref_bit' > setting. > So this

Re: [dm-devel] [PATCH] multipathd: update defaults

2016-06-07 Thread Benjamin Marzinski
On Fri, Jun 03, 2016 at 09:14:32AM +0200, Hannes Reinecke wrote: > For years I've been telling our customers to use the 'tur' checker > as the current default 'directio' will cause spurious path failures > under high load. > > And for several versions (years, even) the linux kernel has the

[dm-devel] [PATCH 4/7] kpartx.rules: respect skip_kpartx flag

2016-05-31 Thread Benjamin Marzinski
Check if DM_SUBSYSTEM_UDEV_FLAG1 is set, and if so, don't run kpartx. If the event was not generated by device-mapper, just use the existing value of DM_SUBSYSTEM_UDEV_FLAG1. Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com> --- kpartx/kpartx.rules | 2 ++ 1 file changed, 2 inse

[dm-devel] [PATCH 3/7] libmultipath: add skip_kpartx option

2016-05-31 Thread Benjamin Marzinski
the remove, and if not, sets the DM_SUBSYSTEM_UDEV_FLAG1 on the resume after failure, so that no partitions will be generated. Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com> --- libmultipath/config.c | 2 ++ libmultipath/config.h | 3 +++ libmultipath/co

[dm-devel] [PATCH 0/7] multpath patch sync

2016-05-31 Thread Benjamin Marzinski
This series is a resend of my previous "handler fixes" patch, along with some bug fixes, a new default configuration, and a new attempt to make multipath able to skip kpartx handling on devices. Benjamin Marzinski (7): multipathd: handler fixes libmultipath: remove calls to dm_ude

[dm-devel] [PATCH 6/7] libmultipath: add Huawei Storage default config

2016-05-31 Thread Benjamin Marzinski
Add a default device config for the Huawei XSG1 arrary. This config comes from Huawei. Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com> --- libmultipath/hwtable.c | 9 + 1 file changed, 9 insertions(+) diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c index 8

[dm-devel] [PATCH 7/7] kpartx: fix dos partition rollover error

2016-05-31 Thread Benjamin Marzinski
by the sector size multiplier, it can rollover before it gets stored in 64 bit slice sector count. This patch just changes the multiplier to a 64 bit uint to match the slice sector count, and avoid the rollover. Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com> --- kpartx/dos.c | 2 +-

[dm-devel] [PATCH 1/7] multipathd: handler fixes

2016-05-31 Thread Benjamin Marzinski
the vecs->lock when it calls pthread_mutex_timedlock, we can't call unlock() on it, because unlocking a mutex you haven't locked causes undefined behviour. So we need to only execute the handler if didn't timeout trying to acquire the lock. Signed-off-by: Benjamin Marzinski <bmarz...@redh

[dm-devel] [PATCH 5/7] multipath: validate devmap names

2016-05-31 Thread Benjamin Marzinski
are not allowed to have a "/" in them, multipath now checks the identifier for this before assuming that is is a device alias, and if there is a "/", it errors out with a helpful message. Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com> --- libmultipath/alias.c

Re: [dm-devel] [LSF/MM ATTEND] multipath redesign and dm blk-mq issues

2016-01-29 Thread Benjamin Marzinski
On Fri, Jan 29, 2016 at 07:59:09AM +0100, Hannes Reinecke wrote: > On 01/28/2016 10:23 PM, Benjamin Marzinski wrote: > > I'd like to attend LSF/MM 2016 to participate in any discussions about > > redesigning how device-mapper multipath operates. I spend a significant > >

Re: [dm-devel] [PATCH v2] dm pref-path: provides preferred path load balance policy

2016-01-30 Thread Benjamin Marzinski
On Sat, Jan 30, 2016 at 09:32:53AM +0100, Hannes Reinecke wrote: > On 01/29/2016 06:50 PM, Benjamin Marzinski wrote: > >On Fri, Jan 29, 2016 at 02:10:52PM +, Nalla, Ravikanth wrote: > >>Hi Mike, Hannes, Ben > >>>This seems like a problem that has alread

Re: [dm-devel] [PATCH v2] dm pref-path: provides preferred path load balance policy

2016-01-30 Thread Benjamin Marzinski
On Sat, Jan 30, 2016 at 02:25:25PM -0600, Benjamin Marzinski wrote: > Before this commit, it always used the pref bit. Again, like I said > before, I'm saying that this was the wrong thing to do. The Spec is Oops. I meant: "I'm NOT saying that this was the wrong thing to do".

Re: [dm-devel] [PATCH]multipath-tools: prevent unnecessary reinstate of stand-by paths with implicit tpgs mode and no active array paths.

2016-02-25 Thread Benjamin Marzinski
On Sat, Feb 20, 2016 at 08:23:29PM +, Shiva Krishna wrote: I understand your problem, but this isn't the right patch to fix it. For one this check + if (newstate != PATH_GHOST || pp->mpp->nr_active > 0 || + pp->tpgs != TPGS_IMPLICIT) { is pretty problematic.

Re: [dm-devel] [PATCH]multipath-tools: prevent unnecessary reinstate of stand-by paths with implicit tpgs mode and no active array paths.

2016-02-26 Thread Benjamin Marzinski
On Fri, Feb 26, 2016 at 12:32:51AM +, Shiva Krishna wrote: > > > On 2/25/16, 12:49 PM, "Benjamin Marzinski" <bmarz...@redhat.com> wrote: > > >On Sat, Feb 20, 2016 at 08:23:29PM +, Shiva Krishna wrote: > > > >I understand your problem, but th

Re: [dm-devel] [PATCH V2]multipath-tools: prevent unnecessary reinstate of stand-by paths with implicit tpgs mode and no active array paths

2016-02-26 Thread Benjamin Marzinski
On Fri, Feb 26, 2016 at 02:25:08AM +, Shiva Krishna wrote: > --- > libmultipath/propsel.c |2 +- > libmultipath/structs.h |1 + > multipathd/main.c | 19 --- > 3 files changed, 18 insertions(+), 4 deletions(-) > > diff --git a/libmultipath/propsel.c

Re: [dm-devel] dm-multipath low performance with blk-mq

2016-01-25 Thread Benjamin Marzinski
On Mon, Jan 25, 2016 at 04:40:16PM -0500, Mike Snitzer wrote: > On Tue, Jan 19 2016 at 5:45P -0500, > Mike Snitzer wrote: > > > On Mon, Jan 18 2016 at 7:04am -0500, > > Sagi Grimberg wrote: > > Would still appreciate answers to my 2 questions

Re: [dm-devel] dm-multipath low performance with blk-mq

2016-01-26 Thread Benjamin Marzinski
On Tue, Jan 26, 2016 at 11:03:24AM -0500, Mike Snitzer wrote: > > Christoph, any chance you could rebase your 'nvme-loop.2' on v4.5-rc1? > > Or point me to a branch that is more current... Failing that, you could try using the null_blk device directly. It doesn't provide enough information for

[dm-devel] [PATCH v2 02/18] retrigger uevents to try and get the uid through udev

2016-04-07 Thread Benjamin Marzinski
uld already be taken care of by the blacklists, so it would be always a good idea to recheck devices on change events. What would be ideal is if udev would let us know when it had problems or timed out when processing a uevent, so we would know if retiggering the uevent would be useful. Signed-off-by

[dm-devel] [PATCH v2 00/18] Multipath patch sync

2016-04-07 Thread Benjamin Marzinski
essage and re-enable reloads as if it had received the uevent. I also added a new patch that adds a new keyword to the weightedpath prioritizer, so that you can refer to paths in a persistent way. Benjamin Marzinski (18): multipathd: use /run instead of /var/run retrigger uevents to try and get

[dm-devel] [PATCH v2 01/18] multipathd: use /run instead of /var/run

2016-04-07 Thread Benjamin Marzinski
always be available before multipathd is started, so multipath should just write there directly, instead of through the symlink. If /var/run is not a symlink, continue using it. Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com> --- Makefile.inc

[dm-devel] [PATCH v2 15/18] multipath: check partitions unused before removing

2016-04-07 Thread Benjamin Marzinski
If kpartx partition devices are in-use, multipath will not be able to perform a non-deferred remove of the multipath device. So, before starting the remove, multipath should verify that none of the partition devices are currently in-use. Signed-off-by: Benjamin Marzinski <bmarz...@redhat.

[dm-devel] [PATCH v2 04/18] Add libmpathcmd library and use it internally

2016-04-07 Thread Benjamin Marzinski
. Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com> --- Makefile | 1 + Makefile.inc | 2 + libmpathcmd/Makefile | 30 +++ libmpathcmd/mpath_cmd.c | 178 +++ libmpathcmd/mpath

[dm-devel] [PATCH v2 09/18] libmultipath: check correct function for define

2016-04-07 Thread Benjamin Marzinski
fixes the Makefile define. Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com> --- libmultipath/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libmultipath/Makefile b/libmultipath/Makefile index 11750c2..1ee968e 100644 --- a/libmultipath/Makefile +++ b/libmul

[dm-devel] [PATCH v2 06/18] libmultipath: fix PAD and PRINT macros

2016-04-07 Thread Benjamin Marzinski
The PAD and PRINT macros are multi-line macros that aren't enclosed in braces. This means that if they are used as single line code blocks with no braces, they won't work correctly. This is currently happening with the PAD macro, but should be fixed in both. Signed-off-by: Benjamin Marzinski

Re: [dm-devel] [PATCH v2 10/18] multipathd: delay reloads during creation

2016-04-08 Thread Benjamin Marzinski
On Fri, Apr 08, 2016 at 10:36:07AM +0200, Zdenek Kabelac wrote: > However now the multipathd *kills* this assumption - since the current udev > rules implementation for multipath devices targets only for the initial scan > and all subsequent RESUMES are supposed to be ignored as it's believed the

[dm-devel] [PATCH v2 13/18] multipath: add exclusive_pref_bit for alua prio

2016-04-07 Thread Benjamin Marzinski
Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com> --- libmultipath/prioritizers/alua.c | 20 +++- multipath/multipath.conf.5 | 19 --- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/libmultipath/prioritizers/alua.c b/libmultipath/pri

[dm-devel] [PATCH v2 12/18] kpartx: verify partition devices

2016-04-07 Thread Benjamin Marzinski
it, causing all sorts of problems. This patch makes kpartx check the uuid to verify that the device it is modifying really is a partition device for the correct dm device. Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com> --- kpartx/devmapper.c | 17 ++--- kpartx/devmapper.

[dm-devel] [PATCH v2 18/18] multipath: add wwn keyword to weightedpath prio

2016-04-07 Thread Benjamin Marzinski
with the iscsi target name of iqn.2009-10.com.redhat.msp.lab.ask-06 (which multipath has always treated as the same as the FC Target WWNN) to a priority of 10 - and all other paths to a priority of 0 Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com> --- libmultipath/print.c

[dm-devel] [PATCH v2 16/18] multipathd.service: remove blk-availability Requires

2016-04-07 Thread Benjamin Marzinski
multipathd.service can start up and run just fine without blk-availability.service. It is only necessary to coordinate shutdown order in certain multipath setups (over iscsi for instance). Thus, instead of "Requires", multipathd.service should use "Wants" Signed-off-by: Benja

[dm-devel] [PATCH 04/17] Add libmpathcmd library and use it internally

2016-03-28 Thread Benjamin Marzinski
. Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com> --- Makefile | 1 + Makefile.inc | 2 + libmpathcmd/Makefile | 30 +++ libmpathcmd/mpath_cmd.c | 178 +++ libmpathcmd/mpath

[dm-devel] [PATCH 11/17] multipath: Fix minor text issues

2016-03-28 Thread Benjamin Marzinski
The multipath.conf man page gave the incorrect default for queue_without_daemon. The multipath usage output listed the -p option twice. And multipath was misspelled in an mpathpersist error message. This patch fixes these issues. Signed-off-by: Benjamin Marzinski <bmarz...@redhat.

[dm-devel] [PATCH 15/17] multipath: check partitions unused before removing

2016-03-28 Thread Benjamin Marzinski
If kpartx partition devices are in-use, multipath will not be able to perform a non-deferred remove of the multipath device. So, before starting the remove, multipath should verify that none of the partition devices are currently in-use. Signed-off-by: Benjamin Marzinski <bmarz...@redhat.

[dm-devel] [PATCH 01/17] multipathd: use /run instead of /var/run

2016-03-28 Thread Benjamin Marzinski
always be available before multipathd is started, so multipath should just write there directly, instead of through the symlink. If /var/run is not a symlink, continue using it. Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com> --- Makefile.inc

[dm-devel] [PATCH 02/17] retrigger uevents to try and get the uid through udev

2016-03-28 Thread Benjamin Marzinski
uld already be taken care of by the blacklists, so it would be always a good idea to recheck devices on change events. What would be ideal is if udev would let us know when it had problems or timed out when processing a uevent, so we would know if retiggering the uevent would be useful. Signed-off-by

[dm-devel] [PATCH 10/17] multipathd: delay reloads during creation

2016-03-28 Thread Benjamin Marzinski
seconds. Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com> --- libmultipath/config.c | 1 + libmultipath/config.h | 2 + libmultipath/configure.c | 4 ++ libmultipath/defaults.h| 1 + libmultipath/dict.c| 4 ++ libmultipath/structs.h | 2 + multip

[dm-devel] [PATCH 07/17] libmultipath: Cut down on alua prioritizer ioctls

2016-03-28 Thread Benjamin Marzinski
when get_target_port_group() fails, to provide a more useful error message. Also, before doing an SGIO ioctl to get the vpd83 data, multipath first tries to read it from sysfs. Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com> --- libmultipath/discovery.h | 2 + libmul

[dm-devel] [PATCH 00/17] Multipath patch sync

2016-03-28 Thread Benjamin Marzinski
of the patches are new. Please apply. Thanks. Benjamin Marzinski (17): multipathd: use /run instead of /var/run retrigger uevents to try and get the uid through udev Fix issues with user_friendly_names initramfs bindings Add libmpathcmd library and use it internally libmultipath: add

[dm-devel] [PATCH 06/17] libmultipath: fix PAD and PRINT macros

2016-03-28 Thread Benjamin Marzinski
The PAD and PRINT macros are multi-line macros that aren't enclosed in braces. This means that if they are used as single line code blocks with no braces, they won't work correctly. This is currently happening with the PAD macro, but should be fixed in both. Signed-off-by: Benjamin Marzinski

[dm-devel] [PATCH 08/17] multipathd: fail if pidfile can't be created

2016-03-28 Thread Benjamin Marzinski
multipathd if the create fails. Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com> --- multipathd/main.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/multipathd/main.c b/multipathd/main.c index 8f4fb58..06876b9 100644 --- a/multipathd/main.c +++ b/m

[dm-devel] [PATCH 05/17] libmultipath: add ignore_new_boot_devs option

2016-03-28 Thread Benjamin Marzinski
dy in the wwids file. This means that only devices that are claimed by "multipath -c" will be used by multipathd. Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com> --- libmultipath/config.h| 1 + libmultipath/configure.c | 3 +-- libmultipath/wwids.c | 19 --

[dm-devel] [PATCH 14/17] multipathd: print "fail" when remove fails

2016-03-28 Thread Benjamin Marzinski
When multipathd fails to remove a path or a map, it was printing "ok" instead of "fail", and exitting with a 0 return code. Now it prints "fail" and exits with a 1, like it does when other commands fail. Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com>

[dm-devel] [PATCH 13/17] multipath: add exclusive_pref_bit for alua prio

2016-03-28 Thread Benjamin Marzinski
Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com> --- libmultipath/prioritizers/alua.c | 20 +++- multipath/multipath.conf.5 | 19 --- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/libmultipath/prioritizers/alua.c b/libmultipath/pri

[dm-devel] [PATCH 17/17] multipathd: use 64-bit int for command key

2016-03-28 Thread Benjamin Marzinski
on 32-bit systems. This patch switches all the keys to use uint64_t instead. Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com> --- multipathd/cli.c | 22 +++--- multipathd/cli.h | 20 +++- 2 files changed, 22 insertions(+), 20 deletions(-) diff

Re: [dm-devel] [PATCH 10/17] multipathd: delay reloads during creation

2016-03-29 Thread Benjamin Marzinski
On Tue, Mar 29, 2016 at 10:02:41AM -0400, John Stoffel wrote: > >>>>> "Benjamin" == Benjamin Marzinski <bmarz...@redhat.com> writes: > > Benjamin> lvm needs PV devices to not be suspended while the udev > Benjamin> rules are running, for them to be

Re: [dm-devel] [PATCH 01/17] multipathd: use /run instead of /var/run

2016-03-29 Thread Benjamin Marzinski
gt; > Benjamin> If /var/run is not a symlink, continue using it. > > Benjamin> Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com> > Benjamin> --- > Benjamin> Makefile.inc| 10 +- > Benjamin> libmultipath/defaults.

Re: [dm-devel] multipath-0.5.0 still provides broken udev rules

2016-04-25 Thread Benjamin Marzinski
On Mon, Apr 25, 2016 at 02:56:35PM +0200, Christophe Varoqui wrote: >Hi, >Those example udev rules are indeed unmaintained and should be removed not >to confuse distributors. >Distributors can't be asked to agree on a common udev ruleset. Ben, >Hannes, Xosé, Peter are you ok

Re: [dm-devel] multipath-0.5.0 still provides broken udev rules

2016-04-27 Thread Benjamin Marzinski
On Tue, Apr 26, 2016 at 07:53:48AM +0200, Hannes Reinecke wrote: > On 04/25/2016 10:14 PM, Xose Vazquez Perez wrote: > > On 04/25/2016 02:56 PM, Christophe Varoqui wrote: > > > >> Those example udev rules are indeed unmaintained and should be > >> removed not to confuse distributors. > >> > >>

Re: [dm-devel] multipath-0.5.0 still provides broken udev rules

2016-04-28 Thread Benjamin Marzinski
On Thu, Apr 28, 2016 at 08:23:44AM +0200, Hannes Reinecke wrote: > On 04/28/2016 12:46 AM, Benjamin Marzinski wrote: > > Like I said, Red Hat doesn't use them. I'll post our multipath.rules > > shortly. > > > Which would be cool. > I was actually hoping to me

Re: [dm-devel] multipath-0.5.0 still provides broken udev rules

2016-04-28 Thread Benjamin Marzinski
On Thu, Apr 28, 2016 at 08:23:44AM +0200, Hannes Reinecke wrote: > On 04/28/2016 12:46 AM, Benjamin Marzinski wrote: > > Aside from dropping the socket, it checks that /etc/multipath.conf > > exists, and that the kernel wasn't started with "nompath". Also it runs >

[dm-devel] [PATCH] multipathd: handler fixes

2016-05-20 Thread Benjamin Marzinski
the vecs->lock when it calls pthread_mutex_timedlock, we can't call unlock() on it, because unlocking a mutex you haven't locked causes undefined behviour. So we need to only execute the handler if didn't timeout trying to acquire the lock. Signed-off-by: Benjamin Marzinski <bmarz...@redh

Re: [dm-devel] multipathd: Add 'sysfs' prioritizer

2016-05-24 Thread Benjamin Marzinski
On Mon, May 23, 2016 at 12:20:28PM +0200, Hannes Reinecke wrote: > Recent kernels have an 'access_state' attribute which allows > us to read the asymmetric access state directly from sysfs. Neat. Just some thoughts. I'm not sure if you want to add a prioritizer option to be able to change what

Re: [dm-devel] libmultipath: use poll() in uevent_listen()

2016-05-11 Thread Benjamin Marzinski
On Wed, May 11, 2016 at 12:35:53PM +0200, Hannes Reinecke wrote: > As we're not modifying the signal mask anymore we can switch > back to normal 'poll' instead of 'ppoll'. ACK -Ben > > Signed-off-by: Hannes Reinecke > --- > libmultipath/uevent.c | 9 +++-- > 1 file

Re: [dm-devel] multipathd: move 'filter_devnode' under vector lock

2016-05-11 Thread Benjamin Marzinski
On Wed, May 11, 2016 at 12:35:41PM +0200, Hannes Reinecke wrote: > Ben Marzinski pointed out that filter_devnode() is used > without any lock or configuration settings in uev_trigger(), > and hence might be invalid when processing events during > reconfiguration. > So move it into the individual

Re: [dm-devel] [PATCH v3 1/1] add display of map information in JSON format

2016-05-13 Thread Benjamin Marzinski
On Fri, May 13, 2016 at 10:39:00AM -0400, Todd Gill wrote: ACK -Ben > The patch adds these commands: > > multipathd show maps json > multipathd show map $map json > > Each command will output the requested map(s) in JSON. > > For the "show maps json" command, the patch pre-allocates >

Re: [dm-devel] [PATCH 20/57] multipathd: Do not update the paths vec when removing paths

2016-04-29 Thread Benjamin Marzinski
On Wed, Apr 27, 2016 at 01:10:21PM +0200, Hannes Reinecke wrote: > When we remove a path it's totally pointless to add it to > the path list first; it'll be removed on the next step anyway. > And we should be cleaning up the comments while we're at it. This one causes problems. The easiest way to

Re: [dm-devel] [PATCH 55/57] multipathd: asynchronous configuration

2016-05-03 Thread Benjamin Marzinski
On Wed, Apr 27, 2016 at 01:10:56PM +0200, Hannes Reinecke wrote: > For initial configuration multipathd waits until it has synchronized > with the existing setup. On larger systems this takes up quite > some time (I've measured 80 seconds on a system with 1024 paths) > causing systemd to stall and

Re: [dm-devel] [PATCH 55/57] multipathd: asynchronous configuration

2016-05-03 Thread Benjamin Marzinski
On Wed, Apr 27, 2016 at 01:10:56PM +0200, Hannes Reinecke wrote: Oh, I did have one other thought. If you're not changing the signal mask in uevent_listen, can't you just use poll? Otherwise people are going to try and figure out what's going on with the signals that they're missing (at least

Re: [dm-devel] [PATCH 49/57] multipathd: do not flush maps on startup

2016-05-02 Thread Benjamin Marzinski
On Wed, Apr 27, 2016 at 01:10:50PM +0200, Hannes Reinecke wrote: > When the daemon is started prior to udev the paths are not present > (yet). However, the maps themselves will be read from device-mapper. > This causes existing maps to be dropped in coalesce_maps(), only > to be reinstated later

Re: [dm-devel] [PATCH 56/57] multipathd: push down lock in checkerloop()

2016-05-03 Thread Benjamin Marzinski
On Wed, Apr 27, 2016 at 01:10:57PM +0200, Hannes Reinecke wrote: > Instead of grabbing the lock at the start of the checkerloop > and releasing it at the end we should be holding it only > during the time when we actually need it. I'm pretty sure that this can cause crashes if multipathd

Re: [dm-devel] [PATCH 28/57] libmultipath: use a shared lock to co-operate with udev

2016-05-03 Thread Benjamin Marzinski
On Tue, May 03, 2016 at 04:31:19PM +0100, Germano Percossi wrote: > Hi, > > Sorry for jumping in the middle of patch review > > On 05/03/2016 03:27 PM, Benjamin Marzinski wrote: > >On Tue, May 03, 2016 at 07:57:01AM +0200, Hannes Reinecke wrote: > >>On 05/02/201

Re: [dm-devel] [PATCH 36/57] libmultipath: pass in 'cookie' as argument for dm_addmap()

2016-05-03 Thread Benjamin Marzinski
On Tue, May 03, 2016 at 11:31:19AM +0200, Hannes Reinecke wrote: > On 05/03/2016 12:23 AM, Benjamin Marzinski wrote: > > On Wed, Apr 27, 2016 at 01:10:37PM +0200, Hannes Reinecke wrote: > >> Instead of generating the cookie internally we should be > >> passing

Re: [dm-devel] [PATCH 36/57] libmultipath: pass in 'cookie' as argument for dm_addmap()

2016-05-03 Thread Benjamin Marzinski
On Tue, May 03, 2016 at 04:43:01PM +0200, Hannes Reinecke wrote: > On 05/03/2016 04:39 PM, Benjamin Marzinski wrote: > > On Tue, May 03, 2016 at 11:31:19AM +0200, Hannes Reinecke wrote: > >> On 05/03/2016 12:23 AM, Benjamin Marzinski wrote: > >>> On Wed, Apr 27, 20

Re: [dm-devel] [PATCH 28/57] libmultipath: use a shared lock to co-operate with udev

2016-05-03 Thread Benjamin Marzinski
On Tue, May 03, 2016 at 09:27:32AM -0500, Benjamin Marzinski wrote: > On Tue, May 03, 2016 at 07:57:01AM +0200, Hannes Reinecke wrote: > > On 05/02/2016 06:26 PM, Benjamin Marzinski wrote: > > > On Wed, Apr 27, 2016 at 01:10:29PM +0200, Hannes Reinecke wrote: > > >&

Re: [dm-devel] [PATCH 56/57] multipathd: push down lock in checkerloop()

2016-05-03 Thread Benjamin Marzinski
On Tue, May 03, 2016 at 05:17:34PM -0500, Benjamin Marzinski wrote: > On Wed, Apr 27, 2016 at 01:10:57PM +0200, Hannes Reinecke wrote: > > Instead of grabbing the lock at the start of the checkerloop > > and releasing it at the end we should be holding it only > > during the

Re: [dm-devel] [PATCH 57/57] Allow specific CLI commands to run unlocked

2016-05-03 Thread Benjamin Marzinski
On Wed, Apr 27, 2016 at 01:10:58PM +0200, Hannes Reinecke wrote: > When multipath is busy with checking paths or processing udev > events it'll take the vector lock, causing the CLI > to become unresponsive. > This patch allows certain CLI commands to not wait for the vector > lock, so that those

Re: [dm-devel] [PATCH 00/57] SLES resync

2016-05-03 Thread Benjamin Marzinski
On Wed, Apr 27, 2016 at 01:10:01PM +0200, Hannes Reinecke wrote: > Hi all, > > as promised several times, here's now a patchset with all the patches > queued up in my SLES repository. ACK on all the patches I haven't commented on. -Ben > > The first bits are pretty much uncontroversial (I

Re: [dm-devel] [PATCH 2/6] libmultipath: pass in 'cookie' as argument for dm_addmap()

2016-05-06 Thread Benjamin Marzinski
On Wed, May 04, 2016 at 07:57:26AM +0200, Hannes Reinecke wrote: > Instead of generating the cookie internally we should be > passing in the cookie to dm_addmap(). Like I said in my comment to the last patch, you should never reuse a cookie value once you've waited for it. The only reason that

Re: [dm-devel] [PATCH 6/6] libmultipath: fixup dm_rename to complete cookie on failure

2016-05-06 Thread Benjamin Marzinski
On Wed, May 04, 2016 at 07:57:30AM +0200, Hannes Reinecke wrote: > >From my understanding we should be calling udev_complete() on > a cookie if dm_task_set_cookie() failed. I was wrong when I said that this will sometimes be helpful to us. It is true that it won't hurt things. But that is because

Re: [dm-devel] [PATCH 5/6] devmapper: do not flush I/O for DM_DEVICE_CREATE

2016-05-06 Thread Benjamin Marzinski
On Wed, May 04, 2016 at 07:57:29AM +0200, Hannes Reinecke wrote: > DM_DEVICE_CREATE loads a new table, so there cannot be any > I/O pending. Hence we should be setting the 'no flush' > and 'skip lockfs' flag to avoid delays during creation. ACK -Ben > > Signed-off-by: Hannes Reinecke

Re: [dm-devel] [PATCH 1/6] libmultipath: pass in cookie as argument for dm_simplecmd()

2016-05-06 Thread Benjamin Marzinski
On Wed, May 04, 2016 at 07:57:25AM +0200, Hannes Reinecke wrote: > Instead of generating the cookie internally we should be passing > it in as an argument; that will allow for cookie reuse. While this one doesn't break anything, it seems unhelpful. We are always dealing with the cookies inside

Re: [dm-devel] [PATCH 6/6] libmultipath: fixup dm_rename to complete cookie on failure

2016-05-06 Thread Benjamin Marzinski
On Fri, May 06, 2016 at 10:59:01PM +0100, Alasdair G Kergon wrote: > The library functions and return states are supposed to be well-defined. > > If you think you've found a cookie leak on an error path within a library > function, we can investigate that and fix the library if need be. In

Re: [dm-devel] [PATCH 22/57] multipath: use option '-i' when called from udev

2016-05-02 Thread Benjamin Marzinski
On Wed, Apr 27, 2016 at 01:10:23PM +0200, Hannes Reinecke wrote: > multipath should be using the option '-i' to ignore the wwids > file when called from udev. Otherwise we might run into a race > condition with systemd and the system might not boot up correctly. The race condition being? Are you

Re: [dm-devel] [PATCH 20/57] multipathd: Do not update the paths vec when removing paths

2016-05-02 Thread Benjamin Marzinski
On Mon, May 02, 2016 at 07:48:50AM +0200, Hannes Reinecke wrote: > On 04/30/2016 12:39 AM, Benjamin Marzinski wrote: > > On Wed, Apr 27, 2016 at 01:10:21PM +0200, Hannes Reinecke wrote: > >> When we remove a path it's totally pointless to add it to > >> the path l

Re: [dm-devel] [PATCH 19/57] multipathd: Do not print misleading message 'not found in pathvec'

2016-05-02 Thread Benjamin Marzinski
On Wed, Apr 27, 2016 at 01:10:20PM +0200, Hannes Reinecke wrote: > When looking up a path in the existing configuration it is perfectly > possible for the path not to be present. > This should not generate a message as it might be errorneously > interpreted as an error. > Do you feel really

Re: [dm-devel] [PATCH 23/57] multipath: remove warning 'failed to get wwid'

2016-05-02 Thread Benjamin Marzinski
On Wed, Apr 27, 2016 at 01:10:24PM +0200, Hannes Reinecke wrote: Again. I'd prefer leaving the messages in either at a higher log level or with less alarming text. -Ben > Signed-off-by: Hannes Reinecke > --- > multipath/main.c | 1 - > 1 file changed, 1 deletion(-) > > diff

Re: [dm-devel] [PATCH 28/57] libmultipath: use a shared lock to co-operate with udev

2016-05-02 Thread Benjamin Marzinski
On Wed, Apr 27, 2016 at 01:10:29PM +0200, Hannes Reinecke wrote: > udev since v214 is placing a shared lock on the device node > whenever it's processing the event. This introduces a race > condition with multipathd, as multipathd is processing the > event for the block device at the same time as

Re: [dm-devel] [PATCH 36/57] libmultipath: pass in 'cookie' as argument for dm_addmap()

2016-05-02 Thread Benjamin Marzinski
On Wed, Apr 27, 2016 at 01:10:37PM +0200, Hannes Reinecke wrote: > Instead of generating the cookie internally we should be > passing in the cookie to dm_addmap(). These look like they could actually cause problems. With dm_addmap_create and dm_addmap_reload, you could be in a situation where you

Re: [dm-devel] [PATCH 37/57] Remove 'udev_sync' argument from dm_simplecmd()

2016-05-02 Thread Benjamin Marzinski
On Wed, Apr 27, 2016 at 01:10:38PM +0200, Hannes Reinecke wrote: > The 'udev_sync' attribute is pointless without a cookie, so we > can as well use the existence of the 'cookie' argument for > the same function. ACK. But I do worry that dm_simplecmd is still going to confuse someone later. Right

Re: [dm-devel] [PATCH 28/57] libmultipath: use a shared lock to co-operate with udev

2016-05-03 Thread Benjamin Marzinski
On Tue, May 03, 2016 at 07:57:01AM +0200, Hannes Reinecke wrote: > On 05/02/2016 06:26 PM, Benjamin Marzinski wrote: > > On Wed, Apr 27, 2016 at 01:10:29PM +0200, Hannes Reinecke wrote: > >> udev since v214 is placing a shared lock on the device node > >> wheneve

Re: [dm-devel] [PATCH 22/57] multipath: use option '-i' when called from udev

2016-05-03 Thread Benjamin Marzinski
g version of the SUSE rules file really benefits nobody. I'm in favor of this going it. -Ben >Thanks, >Christophe >On Tue, May 3, 2016 at 7:44 AM, Hannes Reinecke <[1]h...@suse.de> wrote: > > On 05/02/2016 05:31 PM, Benjamin Marzinski wrote: > > On W

Re: [dm-devel] [PATCH 53/57] multipathd: implement 'show map $map format $fmt'

2016-05-03 Thread Benjamin Marzinski
On Wed, Apr 27, 2016 at 01:10:54PM +0200, Hannes Reinecke wrote: > Similar to the existing 'show map $map topology', but allowing > formatted content. This touches on a patch I've thought about writing, but never got around to. The issue is that with show_maps_topology() we call

Re: [dm-devel] [PATCH 4/6] libmultipath: Fixup 'DM_DEVICE_RELOAD' handling

2016-05-09 Thread Benjamin Marzinski
On Mon, May 09, 2016 at 12:26:36PM +0200, Hannes Reinecke wrote: > On 05/06/2016 10:12 PM, Benjamin Marzinski wrote: > > On Wed, May 04, 2016 at 07:57:28AM +0200, Hannes Reinecke wrote: > >> libdevmapper has the 'quirk' that DM_DEVICE_CREATE is translated > >> internall

Re: [dm-devel] [PATCH 4/7] devmapper: wait for udev in dm_simplecmd_noflush()

2016-05-09 Thread Benjamin Marzinski
On Mon, May 09, 2016 at 12:53:02PM +0200, Hannes Reinecke wrote: > When calling dm_simplecmd_noflush() with udev_flags set we > need to set the 'need_sync' flag otherwise the udev flags > will never be set. The other possibility here would be to temporarily disable udev sync support, and then

Re: [dm-devel] [PATCH 5/7] multipathd: wait for udev in cli_resume()

2016-05-09 Thread Benjamin Marzinski
On Mon, May 09, 2016 at 12:53:03PM +0200, Hannes Reinecke wrote: > When calling 'cli_resume()' we should be waiting for udev to > ensure the device really has been resumed. Since multipathd has udev sync support disabled, this change doesn't do anything (which is fine, since I don't see any need

Re: [dm-devel] [PATCHv2 0/7] multipathd 'cookie' handling rework

2016-05-09 Thread Benjamin Marzinski
On Mon, May 09, 2016 at 12:52:58PM +0200, Hannes Reinecke wrote: > Hi all, > > here's now the second attempt to fixup the 'cookie' handling > in multipath-tools. > I've removed the patch to pass in a cookie as an argument > for dm_addmap_reload() and dm_simplecmd(). > And I've removed all

Re: [dm-devel] [PATCH 0/6] Delete attributes with default values

2016-07-13 Thread Benjamin Marzinski
On Sat, Jul 09, 2016 at 10:17:05AM +0200, Christophe Varoqui wrote: >I'll wait for comments from distribution maintainers on this patchset. >Because, though we have already deleted the checker_name settings when >equal to the default value, it should be clear that with this patchset >

Re: [dm-devel] [PATCH] multipathd: restore paths after reconfigure

2016-07-21 Thread Benjamin Marzinski
On Thu, Jul 21, 2016 at 09:29:37AM -0700, Bart Van Assche wrote: > On 07/01/2016 02:46 PM, Benjamin Marzinski wrote: > >multipathd has code to finish gathering the information of paths that > >were not active at the time they were discovered. When the checker loop > >

[dm-devel] [PATCH] multipathd: fix typo that breaks failure path

2016-07-21 Thread Benjamin Marzinski
There wasn't supposed to be a semicolon after the "else" when checking paths in checkerloop(). Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com> --- multipathd/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/multipathd/main.c b/multipathd/main

Re: [dm-devel] dm-mq and end_clone_request()

2016-07-27 Thread Benjamin Marzinski
On Wed, Jul 27, 2016 at 10:08:28AM -0400, Mike Snitzer wrote: > On Tue, Jul 26 2016 at 6:51pm -0400, > Bart Van Assche wrote: > > > On 07/25/2016 06:15 PM, Mike Snitzer wrote: > > > Please try this patch to see if it fixes your issue, thanks. > > > > > > diff --git

Re: [dm-devel] [PATCH V7 2/4] multipath-tools: New way to limit the IPC command length.

2016-08-12 Thread Benjamin Marzinski
On Fri, Aug 12, 2016 at 08:12:38PM +0800, Gris Ge wrote: Perhaps a better way would be to keep your v6 code, and just make the client ignore the SIGPIPE signals. Personally, I'm fine with always ignoring them, since the writes will still fail with EPIPE, and the multipathd client code should be

Re: [dm-devel] [PATCH V6 1/3] multipath-tools: New way to limit the IPC command length.

2016-08-12 Thread Benjamin Marzinski
On Fri, Aug 12, 2016 at 08:57:51AM -0700, Bart Van Assche wrote: > On 07/15/2016 02:35 PM, Benjamin Marzinski wrote: > >On Tue, Jul 12, 2016 at 02:50:36PM +0800, Gris Ge wrote: > > > >The only thing that I wonder about with this patch is, when previously > >the multi

Re: [dm-devel] [PATCH V6 1/3] multipath-tools: New way to limit the IPC command length.

2016-07-15 Thread Benjamin Marzinski
On Tue, Jul 12, 2016 at 02:50:36PM +0800, Gris Ge wrote: The only thing that I wonder about with this patch is, when previously the multipath client code would have failed with EPIPE, and (at least in some cases) spit out a semi-useful message, the program will now terminate because of the

Re: [dm-devel] [PATCH V6 0/3] Introducing multipath C API

2016-07-15 Thread Benjamin Marzinski
On Tue, Jul 12, 2016 at 02:50:35PM +0800, Gris Ge wrote: ACK, with my comment on patch 1/3 -Ben > Changes since V5: > * Fix commit message typo of patch 1/3: > 'EINVA vs EINVAL' and 'dedicate vs dedicated' > * Use $(LN) and $(RM) in Makefile in patch 3/3. > * Rebased to current

Re: [dm-devel] Subject: [PATCH 1/1] multipath-tools: fix dm- device filtering

2016-07-08 Thread Benjamin Marzinski
On Fri, Jul 08, 2016 at 08:55:38AM +0200, Christophe Varoqui wrote: ACK -Ben >Hannes, Ben, do you ack this one ? >On Thu, Jun 23, 2016 at 11:06 PM, Dragan Stancevic ><[1]dragan.stance...@canonical.com> wrote: > > Hi Christophe, > can you please take a look at the attached

Re: [dm-devel] [PATCH] multipathd: fix waiter_attr resource leak

2016-07-08 Thread Benjamin Marzinski
On Fri, Jul 08, 2016 at 09:59:18AM +0200, Christophe Varoqui wrote: ACK -Ben >Ben, Hannes, I'd appreciate your ack on this patch. >Thanks. >On Wed, Jul 6, 2016 at 10:08 AM, <[1]zhang.ka...@zte.com.cn> wrote: > > As a global variable waiter_attr of thread attribute was set up

Re: [dm-devel] [Patch] multipath-tools: Check if multipathd is running or not and print a warning

2016-07-29 Thread Benjamin Marzinski
On Thu, Jul 28, 2016 at 04:35:21PM +0200, Hannes Reinecke wrote: > On 07/28/2016 01:48 PM, Milan P. Gandhi wrote: > > Hello, > > > > With this patch dm-multipath commands e.g. multipath -v2, > > multipath -ll etc. now checks if there are multipath > > device maps created, and multipathd service

[dm-devel] [PATCH] multipathd: restore paths after reconfigure

2016-07-01 Thread Benjamin Marzinski
that if you reconfigure multipathd while paths are down, they will no longer be usable. This patch makes sure that check_path will actually rerun pathinfo to finish setting up the path, so that after the path comes back up, it will be usable again. Signed-off-by: Benjamin Marzinski <bm

Re: [dm-devel] [PATCH 02/26] Use 'mptable' as argument for find_mpe() and get_mpe_wwid()

2016-07-01 Thread Benjamin Marzinski
On Mon, Jun 20, 2016 at 10:08:49AM +0200, Hannes Reinecke wrote: > Signed-off-by: Hannes Reinecke > --- > libmultipath/config.c | 8 > libmultipath/config.h | 4 ++-- > libmultipath/configure.c | 2 +- > libmultipath/propsel.c | 2 +- >

Re: [dm-devel] [PATCH 00/26] Userspace-RCU for config accesses

2016-07-01 Thread Benjamin Marzinski
On Mon, Jun 20, 2016 at 10:08:47AM +0200, Hannes Reinecke wrote: > Hi all, ACK on all patches besides 2 and 17 (which I've commented on). -Ben > > as Benjamin Marzinski pointed out the 'config' structure is > not protected, but it will be re-allocated whenever we do a >

Re: [dm-devel] [PATCH 17/26] libmultipath: use 'struct config' as argument for pathinfo()

2016-07-01 Thread Benjamin Marzinski
On Mon, Jun 20, 2016 at 10:09:04AM +0200, Hannes Reinecke wrote: > pathinfo() requires access to the entire configuration, not just > hwtable. So don't pretend this is the case. > > Signed-off-by: Hannes Reinecke > --- > libmpathpersist/mpath_persist.c | 6 +++--- >

  1   2   3   4   5   6   7   8   >