Re: [dm-devel] [PATCH v2 25/29] libmultipath: introduce symbolic values for logsink

2020-10-16 Thread Benjamin Marzinski
On Fri, Oct 16, 2020 at 12:44:57PM +0200, mwi...@suse.com wrote: > From: Martin Wilck Git complains about this patch adding new blank line at EOF in libmultipath/debug.h Otherwise, it looks good. -Ben > > Signed-off-by: Martin Wilck > --- > libmultipath/debug.c | 4 ++-- >

Re: [dm-devel] [PATCH] libmultipath: fix memory leaks in coalesce_paths

2020-10-16 Thread Benjamin Marzinski
On Fri, Oct 16, 2020 at 02:23:58PM +0800, lixiaokeng wrote: > When multipath -F are executed firstly and multipath -v2 or > -d are executed later, asan will warn memory leaks. The > reason is that the mpp allocated in coalesce_paths isn't > freed. Here we add newmp in configure(multipath) to store

Re: [dm-devel] [PATCH 19/23] multipathd: move cleanup_{prio, checkers, foreign} to libmultipath_exit

2020-09-29 Thread Benjamin Marzinski
On Tue, Sep 29, 2020 at 11:31:06AM +0200, Martin Wilck wrote: > On Mon, 2020-09-28 at 15:26 -0500, Benjamin Marzinski wrote: > > On Thu, Sep 24, 2020 at 03:40:50PM +0200, mwi...@suse.com wrote: > > > > > > /* > > > * We don't support re-initializati

Re: [dm-devel] [PATCH v2 11/21] libmultipath: protect racy libdevmapper calls with a mutex

2020-09-28 Thread Benjamin Marzinski
mevents unit tests. dm_task_run is no longer called in dmevents.c. Intead, its only called in devmapper.c, so this needs to be in dmevents-test_OBJDEPS > Cc: lixiaok...@huawei.com > Reviewed-by: Benjamin Marzinski > Signed-off-by: Martin Wilck > --- > libmu

Re: [dm-devel] [PATCH 00/23] libmultipath: improve cleanup on exit

2020-09-28 Thread Benjamin Marzinski
ile the bulk of the series is the cleanup handling, it also contains > some bug fixes for issues that I found while working on this. > > Regards > Martin > Reviewed-by: Benjamin Marzinski For all patches except 1, 7, 14, 18, & 23 > Martin Wilck (23): > multipathd: uxlsnr:

Re: [dm-devel] [PATCH 19/23] multipathd: move cleanup_{prio, checkers, foreign} to libmultipath_exit

2020-09-28 Thread Benjamin Marzinski
On Thu, Sep 24, 2020 at 03:40:50PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > Signed-off-by: Martin Wilck > --- > libmpathpersist/mpath_persist.c | 2 -- > libmultipath/config.c | 4 > libmultipath/libmultipath.version | 5 + > multipathd/main.c

Re: [dm-devel] [PATCH 18/23] libmultipath: fix log_thread startup and teardown

2020-09-28 Thread Benjamin Marzinski
On Thu, Sep 24, 2020 at 03:40:49PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > This fixes several issues with the log_thread. First, the running > flag logq_running should be set by the thread itself, not by > log_thread_start()/_stop(). Second, the thread was both cancelled and >

Re: [dm-devel] [PATCH 14/23] libmultipath: add libmp_dm_exit()

2020-09-28 Thread Benjamin Marzinski
On Thu, Sep 24, 2020 at 03:40:45PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > This function prepares for calling dm_lib_exit() on program exit. > It undoes changes to libdm internals done by libmultipath. > It doesn't call dm_lib_exit(), as the caller may want to keep > libdm active.

Re: [dm-devel] [PATCH 07/23] multipathd: move conf destruction into separate function

2020-09-28 Thread Benjamin Marzinski
On Thu, Sep 24, 2020 at 03:40:38PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > Also removing the comment about dlog() and dm_write_log(). > dlog() can cope with get_multipath_config() returning NULL, > and dm_write_log() hasn't accessed the configuration for a while. > >

Re: [dm-devel] [PATCH v2 13/21] libmultipath: provide defaults for {get, put}_multipath_config

2020-09-26 Thread Benjamin Marzinski
On Sat, Sep 26, 2020 at 11:43:46AM +0200, Martin Wilck wrote: > On Fri, 2020-09-25 at 16:57 -0500, Benjamin Marzinski wrote: > > On Fri, Sep 25, 2020 at 10:00:10PM +0200, Martin Wilck wrote: > > > > > > I suggest to track the verbosity independently in a different >

Re: [dm-devel] [PATCH 01/23] multipathd: uxlsnr: avoid deadlock on exit

2020-09-25 Thread Benjamin Marzinski
On Thu, Sep 24, 2020 at 03:40:32PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > The uxlsnr wouldn't always release the client lock when cancelled, > causing a deadlock in uxsock_cleanup(). While this hasn't been > caused by commit 3d611a2, the deadlock seems to have become much > more

Re: [dm-devel] [PATCH v2 21/21] multipathd: remove logsink and udev

2020-09-25 Thread Benjamin Marzinski
On Thu, Sep 24, 2020 at 03:37:16PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > We can use the symbols from libmultipath now. > Reviewed-by: Benjamin Marzinski > Signed-off-by: Martin Wilck > --- > multipathd/main.c | 9 +++-- > 1 file changed, 3 in

Re: [dm-devel] [PATCH v2 20/21] mpathpersist: remove logsink and udev

2020-09-25 Thread Benjamin Marzinski
On Thu, Sep 24, 2020 at 03:37:15PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > We can use libmultipath's internal symbols now. The libmultipath > initialization is taken care of by libmpathpersist_init(). > Reviewed-by: Benjamin Marzinski > Signed-of

Re: [dm-devel] [PATCH v2 19/21] libmpathpersist: call libmultipath_{init, exit}()

2020-09-25 Thread Benjamin Marzinski
On Thu, Sep 24, 2020 at 03:37:14PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > Have libmpathpersist_{init,exit} do the udev initialization, too. > Reviewed-by: Benjamin Marzinski > Signed-off-by: Martin Wilck > --- > libmpathpersist

Re: [dm-devel] [PATCH v2 10/21] libmultipath: devmapper: refactor libdm version determination

2020-09-25 Thread Benjamin Marzinski
On Thu, Sep 24, 2020 at 03:37:05PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > As one step towards bundling all possibly racy libdm init calls into a single > function, split the code for determining and checking versions of > libdm and kernel components. Provide a generic helper >

Re: [dm-devel] [PATCH 10/11] libmpathpersist: add linker version script

2020-09-25 Thread Benjamin Marzinski
On Sat, Sep 26, 2020 at 01:07:34AM +0200, Martin Wilck wrote: > On Fri, 2020-09-25 at 17:32 -0500, Benjamin Marzinski wrote: > > On Fri, Sep 25, 2020 at 05:10:22PM -0500, Benjamin Marzinski wrote: > > > On Fri, Sep 25, 2020 at 09:52:51PM +0200, Martin Wilck wrote: > > >

Re: [dm-devel] [PATCH v2 17/21] libmultipath: add udev and logsink symbols

2020-09-25 Thread Benjamin Marzinski
On Thu, Sep 24, 2020 at 03:37:12PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > With these symbols added, applications using libmultipath don't > need to define global variables "udev" and "logsink" any more. > This comes at the cost of having to call an init function. > Currently,

Re: [dm-devel] [PATCH 10/11] libmpathpersist: add linker version script

2020-09-25 Thread Benjamin Marzinski
On Fri, Sep 25, 2020 at 09:52:51PM +0200, Martin Wilck wrote: > On Thu, 2020-09-24 at 23:00 -0500, Benjamin Marzinski wrote: > > On Thu, Sep 24, 2020 at 03:36:43PM +0200, mwi...@suse.com wrote: > > > > > > --- /dev/null > > > +++ b/libmpathpersist/libmpat

Re: [dm-devel] [PATCH v2 13/21] libmultipath: provide defaults for {get, put}_multipath_config

2020-09-25 Thread Benjamin Marzinski
On Fri, Sep 25, 2020 at 10:00:10PM +0200, Martin Wilck wrote: > On Thu, 2020-09-24 at 23:34 -0500, Benjamin Marzinski wrote: > > > > This causes problems with the libmpathvalid library code I > > wrote. The > > issue is that right now, when you run _init_config()

Re: [dm-devel] [PATCH 02/11] libmultipath: copy mpp->hwe from pp->hwe

2020-09-25 Thread Benjamin Marzinski
On Fri, Sep 25, 2020 at 06:01:26PM +0200, Martin Wilck wrote: > On Thu, 2020-09-24 at 15:12 -0500, Benjamin Marzinski wrote: > > On Thu, Sep 24, 2020 at 03:36:35PM +0200, mwi...@suse.com wrote: > > > From: Martin Wilck > > > > > > Since f0462f0 ("libmul

Re: [dm-devel] [RFC PATCH v2 0/3] add library to check if device is a valid path

2020-09-25 Thread Benjamin Marzinski
On Fri, Sep 25, 2020 at 10:01:01AM +, Martin Wilck wrote: > On Thu, 2020-09-24 at 20:08 -0500, Benjamin Marzinski wrote: > > On Thu, Sep 24, 2020 at 07:22:21PM +, Martin Wilck wrote: > > > > > > So, SID will call into libmultipath via libmpathva

Re: [dm-devel] [PATCH v2 13/21] libmultipath: provide defaults for {get, put}_multipath_config

2020-09-24 Thread Benjamin Marzinski
On Thu, Sep 24, 2020 at 03:37:08PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > Add an implementation of get_multipath_config() and put_multipath_config() > to libmultipath. The linker's symbol lookup rules will make sure that > applications can override these functions if they need

Re: [dm-devel] [PATCH 10/11] libmpathpersist: add linker version script

2020-09-24 Thread Benjamin Marzinski
On Thu, Sep 24, 2020 at 03:36:43PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > This defines the ABI of libmpathpersist in the current state. > --- > libmpathpersist/Makefile| 6 -- > libmpathpersist/libmpathpersist.version | 20 > 2 files

Re: [dm-devel] [RFC PATCH v2 0/3] add library to check if device is a valid path

2020-09-24 Thread Benjamin Marzinski
On Thu, Sep 24, 2020 at 07:22:21PM +, Martin Wilck wrote: > On Thu, 2020-09-24 at 11:30 -0500, Benjamin Marzinski wrote: > > On Thu, Sep 24, 2020 at 08:18:02AM +, Martin Wilck wrote: > > > On Wed, 2020-09-23 at 23:59 -0500, Benjamin Marzinski wrote: > > > >

Re: [dm-devel] [PATCH 08/11] libmultipath: create separate .so for unit tests

2020-09-24 Thread Benjamin Marzinski
On Thu, Sep 24, 2020 at 03:36:41PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > The unit tests use a lot of internal symbols that we don't want > to add to the ABI if we don't have to. As long as we don't > have to make incompatible changes to functions, we can work around > that by

Re: [dm-devel] [PATCH 23/23] multipath-tools: mpath-tools.supp: file with valgrind suppressions

2020-09-24 Thread Benjamin Marzinski
On Thu, Sep 24, 2020 at 03:40:54PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > Git complains about the extra blank line at the end of the file third-party/valgrind/mpath-tools.supp:33: new blank line at EOF. -Ben > These leaks are caused by other libraries (libsystemd, glibc, >

Re: [dm-devel] [PATCH 06/11] libmultipath: checkers/prio: allow non-lazy .so loading

2020-09-24 Thread Benjamin Marzinski
warning if any expected chekers/priotitizers are missing, so Reviewed-by: Benjamin Marzinski > --- > libmultipath/checkers.c | 17 + > libmultipath/prio.c | 22 ++ > 2 files changed, 39 insertions(+) > > diff --git a/libmultipath/checkers.c

Re: [dm-devel] [PATCH 02/11] libmultipath: copy mpp->hwe from pp->hwe

2020-09-24 Thread Benjamin Marzinski
On Thu, Sep 24, 2020 at 03:36:35PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > Since f0462f0 ("libmultipath: use vector for for pp->hwe and mp->hwe"), > we've been trying to fix issues caused by paths getting freed and mpp->hwe > dangling. This approach couldn't work because we need

Re: [dm-devel] [RFC PATCH v2 0/3] add library to check if device is a valid path

2020-09-24 Thread Benjamin Marzinski
On Thu, Sep 24, 2020 at 08:18:02AM +, Martin Wilck wrote: > On Wed, 2020-09-23 at 23:59 -0500, Benjamin Marzinski wrote: > > The main part of the this patchset is the first patch, which adds a > > new library interface to check whether devices are valid paths. This > >

Re: [dm-devel] [RFC PATCH v2 3/3] libmultipath: change log level for null uid_attribute

2020-09-24 Thread Benjamin Marzinski
On Thu, Sep 24, 2020 at 08:06:41AM +, Martin Wilck wrote: > On Wed, 2020-09-23 at 23:59 -0500, Benjamin Marzinski wrote: > > If uid_attribute is explicitly set to an empty string, multipath > > should > > log the uid at the default log level, since us

[dm-devel] [RFC PATCH v2 2/3] libmultipath: add uid failback for dasd devices

2020-09-23 Thread Benjamin Marzinski
Add failback code to get the uid for dasd devices from sysfs. Copied from dasdinfo Reviewed-by: Martin Wilck Signed-off-by: Benjamin Marzinski --- libmultipath/defaults.h | 1 + libmultipath/discovery.c | 37 - 2 files changed, 37 insertions(+), 1 deletion

[dm-devel] [RFC PATCH v2 0/3] add library to check if device is a valid path

2020-09-23 Thread Benjamin Marzinski
always use the fallback getuid code was unnecessary. It just makes a uid_attribute of "" log at normal levels. Benjamin Marzinski (3): multipath: add libmpathvalid library libmultipath: add uid failback for dasd devices libmultipath: change log level for null uid_attribute M

[dm-devel] [RFC PATCH v2 3/3] libmultipath: change log level for null uid_attribute

2020-09-23 Thread Benjamin Marzinski
If uid_attribute is explicitly set to an empty string, multipath should log the uid at the default log level, since using the fallback code is the expected behavior. Signed-off-by: Benjamin Marzinski --- libmultipath/discovery.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff

[dm-devel] [RFC PATCH v2 1/3] multipath: add libmpathvalid library

2020-09-23 Thread Benjamin Marzinski
-by: Benjamin Marzinski --- Makefile| 3 +- libmpathvalid/Makefile | 38 ++ libmpathvalid/libmpathvalid.version | 10 ++ libmpathvalid/mpath_valid.c | 199 libmpathvalid/mpath_valid.h | 61 + 5 files

Re: [dm-devel] [PATCH 17/19] libmultipath: add udev and logsink symbols

2020-09-23 Thread Benjamin Marzinski
On Wed, Sep 23, 2020 at 10:16:48AM +0200, Martin Wilck wrote: > On Mon, 2020-09-21 at 15:10 -0500, Benjamin Marzinski wrote: > > > > After calling libmultipath_exit(), you can never reinitialized the > > udev > > device. That seems fine, but it should pr

Re: [dm-devel] [PATCH] libmultipath: Allow discovery of USB devices

2020-09-22 Thread Benjamin Marzinski
On Tue, Sep 22, 2020 at 09:59:36PM +0200, Martin Wilck wrote: > On Tue, 2020-09-22 at 20:34 +0200, Frank Meinl wrote: > > This change adds a new configuration option allow_usb_devices. It is > > disabled by default, so that the behavior of existing setups is not > > changed. If enabled (via

Re: [dm-devel] [PATCH] libmultipath: fix memory leak in _check_bindings_file

2020-09-21 Thread Benjamin Marzinski
On Wed, Sep 16, 2020 at 06:26:58PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > We must pass "" to the cleanup function, not "line". Reviewed-by: Benjamin Marzinski > Fixes: "libmultipath: add consistency check for alias se

Re: [dm-devel] [PATCH 00/19] multipath-tools: shutdown, libdevmapper races, globals

2020-09-21 Thread Benjamin Marzinski
bmultipath. This way > callers won't have to bother with defining these global symbols any > more in the future (but they still can). Thanks for doing this. I really like these cleanups. I'll be resending my libmpathvalid library code on top of this set. Reviewed-by: Benjamin Marzinski For

Re: [dm-devel] [PATCH 19/19] mpathpersist: remove logsink and udev

2020-09-21 Thread Benjamin Marzinski
On Wed, Sep 16, 2020 at 05:37:18PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > We can use libmultipath's internal symbols now. > > Signed-off-by: Martin Wilck > --- > mpathpersist/main.c | 10 -- > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git

Re: [dm-devel] [PATCH 17/19] libmultipath: add udev and logsink symbols

2020-09-21 Thread Benjamin Marzinski
On Wed, Sep 16, 2020 at 05:37:16PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > With these symbols added, applications using libmultipath don't > need to define global variables "udev" and "logsink" any more. > This comes at the cost of having to call an init function. > Currently,

Re: [dm-devel] [PATCH 13/19] libmultipath: provide defaults for {get, put}_multipath_config

2020-09-21 Thread Benjamin Marzinski
On Wed, Sep 16, 2020 at 05:37:12PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > Add an implementation of get_multipath_config() and put_multipath_config() > to libmultipath. The linker's symbol lookup rules will make sure that > applications can override these functions if they need

Re: [dm-devel] [PATCH 10/19] libmultipath: devmapper: refactor libdm version determination

2020-09-19 Thread Benjamin Marzinski
On Wed, Sep 16, 2020 at 05:37:09PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > As one step towards bundling all possibly racy libdm init calls into a single > function, split the code for determining and checking versions of > libdm and kernel components. Provide a generic helper >

Re: [dm-devel] [PATCH 07/19] multipathd: set_config_state(): avoid code duplication

2020-09-19 Thread Benjamin Marzinski
only tangentially related to this patch, but it's possible for set_conf_state() to timeout, and we'd don't always retry it. That's fine, be we don't always check for failure and notify the user that the reconfigure isn't happening, and we probably should. But the patch itself is fine. Reviewed-by:

Re: [dm-devel] [PATCH 0/6] libmultipath: check udev* func return value

2020-09-17 Thread Benjamin Marzinski
> return value to avoid dereference NULL. > > repo: openSUSE/multipath-tools > repo link: https://github.com/openSUSE/multipath-tools > branch: upstream-queue > For the set Reviewed-by: Benjamin Marzinski > lixiaokeng (6): > libmultipath: check uedv* return value in s

Re: [dm-devel] [PATCH 1/3] multipath: add libmpathvalid library

2020-09-17 Thread Benjamin Marzinski
On Wed, Sep 16, 2020 at 02:18:49PM +, Martin Wilck wrote: > On Tue, 2020-09-15 at 16:45 -0500, Benjamin Marzinski wrote: > > This library allows other programs to check if a path should be > > claimed > > by multipath. It exports an init and exit function, a pointer t

Re: [dm-devel] [PATCH 3/3] libmultipath: add ignore_udev_uid option

2020-09-17 Thread Benjamin Marzinski
On Wed, Sep 16, 2020 at 02:46:18PM +, Martin Wilck wrote: > Hi Ben, > > On Tue, 2020-09-15 at 16:45 -0500, Benjamin Marzinski wrote: > > Setting this option to yes will force multipath to get the uid by > > using > > the fallback sysfs methods, inst

[dm-devel] [PATCH 2/3] libmultipath: add uid failback for dasd devices

2020-09-15 Thread Benjamin Marzinski
Add failback code to get the uid for dasd devices from sysfs. Copied from dasdinfo Signed-off-by: Benjamin Marzinski --- libmultipath/defaults.h | 1 + libmultipath/discovery.c | 37 - 2 files changed, 37 insertions(+), 1 deletion(-) diff --git

[dm-devel] [PATCH 1/3] multipath: add libmpathvalid library

2020-09-15 Thread Benjamin Marzinski
the caller to pass in an array of the already known path wwids, and checks if the current path matches any of those. The library also doesn't set up the device-mapper log fuctions. It leaves this up to the caller. Signed-off-by: Benjamin Marzinski --- Makefile| 3

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

2020-09-15 Thread Benjamin Marzinski
being resent for device that failed to get a WWID, although I'm on the fence about the benefit of this. Signed-off-by: Benjamin Marzinski --- libmultipath/config.h | 1 + libmultipath/dict.c| 4 libmultipath/discovery.c | 17 +++-- libmultipath/discovery.h | 8

[dm-devel] [PATCH 0/3] add library to check if device is a valid path

2020-09-15 Thread Benjamin Marzinski
, I understand that there's not much reason to set this outside of SID. I have a git branch that is Martin's upstream-queue branch with these patches added, that I people can use if this patch isn't acceptable. https://github.com/bmarzins/rh-multipath-tools/tree/sid-patches Benjamin Marzinski (3

Re: [dm-devel] [PATCH v2] libmultipath: setup_map(): don't break multipath attributes

2020-09-15 Thread Benjamin Marzinski
ecause of an error in this setup_map() > invocation. As these properties aren't likely to change during an update > operation, saving and restoring them is better than leaving the map > improperly initialized. > Reviewed-by: Benjamin Marzinski > Signed-off-by: Martin

Re: [dm-devel] [PATCH 0/3] Fixes for bitfield unit tests

2020-09-15 Thread Benjamin Marzinski
s: fix bitfield tests for small fields > > multipath-tools tests: fix bitfield tests for big endian > > multipath-tools tests: fix small bitfield tests for big endian > > > > tests/util.c | 146 ++--- > > -- > > 1 fi

Re: [dm-devel] [PATCH V4 09/14] libmultipath: check whether mpp->features is NUll in setup_map

2020-09-10 Thread Benjamin Marzinski
no noticeable changes in behavior. Ideally we should stick to a "do no harm" policy when updating the device. It seems better to have a device structure that's outdated than one that's invalid. But regardless of what we do in setup_map, the assemble_map() part of this

Re: [dm-devel] [PATCH 09/14] libmultipath: check whether mp->features is NULl in, assemble_map

2020-09-08 Thread Benjamin Marzinski
On Tue, Sep 08, 2020 at 06:35:45PM +0200, Martin Wilck wrote: > On Tue, 2020-09-08 at 10:45 -0500, Benjamin Marzinski wrote: > > On Mon, Sep 07, 2020 at 08:21:28PM +0800, lixiaokeng wrote: > > > > > @@ -86,9 +86,12 @@ assemble_map (struct multipath * mp, char * &

Re: [dm-devel] [PATCH 09/14] libmultipath: check whether mp->features is NULl in, assemble_map

2020-09-08 Thread Benjamin Marzinski
On Mon, Sep 07, 2020 at 08:21:28PM +0800, lixiaokeng wrote: > > >> @@ -86,9 +86,12 @@ assemble_map (struct multipath * mp, char * params, int > >> len) > >>get_linux_version_code() < KERNEL_VERSION(4, 3, 0)) > >>add_feature(>features, retain_hwhandler); > >> > >> - f =

Re: [dm-devel] [PATCH 14/14] multipathpersist: delete unused variable in handle_args

2020-09-04 Thread Benjamin Marzinski
On Wed, Sep 02, 2020 at 03:26:29PM +0800, lixiaokeng wrote: > In handle_args, the tmp isn't used. We delete it. > Reviewed-by: Benjamin Marzinski > Signed-off-by: Lixiaokeng > Signed-off-by: Zhiqiang Liu > Signed-off-by: Linfeilong > --- > mpathpersist/main.c | 1 -

Re: [dm-devel] [PATCH 13/14] multipath: use update_multipath_table/status in, check_useable_paths

2020-09-04 Thread Benjamin Marzinski
On Wed, Sep 02, 2020 at 03:25:28PM +0800, lixiaokeng wrote: > The return values of dm_get_map, disassemble_map,dm_get_status > and disassemble_status in check_usable_paths were not checked. > Use update_multipath_table/status to instead of them. > Reviewed-by: Benjamin Marzinski

Re: [dm-devel] [PATCH 12/14] libmultipathpersist: use update_multipath_table/status, in get_mpvec

2020-09-04 Thread Benjamin Marzinski
On Wed, Sep 02, 2020 at 03:24:33PM +0800, lixiaokeng wrote: > The return values of dm_get_map, disassemble_map in get_mpvec > were not checked. Use update_multipath_table/status to instead > of them. > Looks mostly good. I agree that we should be checking the results of getting the raw data

Re: [dm-devel] [PATCH 11/14] mpathpersist: check whether malloc paramp->trnptid_list, fails in handle_args func

2020-09-04 Thread Benjamin Marzinski
On Wed, Sep 02, 2020 at 03:23:42PM +0800, lixiaokeng wrote: > In handle_args func, we donot check whether malloc paramp and > each paramp->trnptid_list[j] fails before using them, it may > cause access NULL pointer. > > Here, we add alloc_prout_param_descriptor to allocate and init > paramp, and

Re: [dm-devel] [PATCH 10/14] util/tests: use assert_non_null to ensure malloc, returns non-null pointer

2020-09-04 Thread Benjamin Marzinski
On Wed, Sep 02, 2020 at 03:22:37PM +0800, lixiaokeng wrote: > In tests/util.c, we should use assert_non_null to ensure > malloc() returns non-null pointer in both test_strlcpy_5 > and test_strlcpy_6 func. > Reviewed-by: Benjamin Marzinski > Signed-off-by: Zhiqiang Li

Re: [dm-devel] [PATCH 09/14] libmultipath: check whether mp->features is NULl in, assemble_map

2020-09-04 Thread Benjamin Marzinski
On Wed, Sep 02, 2020 at 03:21:15PM +0800, lixiaokeng wrote: > In assemble_map func, if add_feature fails and mp->features is > default value (NULL), STRDUP(mp->features) will cause a seg-fault. > In addition, f = STRDUP(mp->features) is just used for APPEND(). > We can directly pass mp->features

Re: [dm-devel] [PATCH 08/14] libmultipath: donot free *dst if REALLOC fails in, merge_words

2020-09-04 Thread Benjamin Marzinski
On Wed, Sep 02, 2020 at 03:20:29PM +0800, lixiaokeng wrote: > In merge_words func, if REALLOC() fails, the input *dst will > be freed. If so, mpp->hwhandler| mpp->features|mpp->selector > may be set to NULL after calling merge_words func in > disassemble_map func. This may cause accessing freed

Re: [dm-devel] [PATCH 07/14] libmultipath: check return value of dm_mapname in, sysfs_check_holders

2020-09-04 Thread Benjamin Marzinski
it may cause a segmentation fault in dm_task_set_name. > > Here, we will check whether dm_mapname func returns NULL before > using it. Reviewed-by: Benjamin Marzinski > > Signed-off-by: Zhiqiang Liu > Signed-off-by: Lixiaokeng > --- > libmultipath/sysfs.c | 6 +- > 1 fi

Re: [dm-devel] [PATCH 00/14] multipath-tools series: some cleanups and fixes checked by codedex tool

2020-09-04 Thread Benjamin Marzinski
On Thu, Sep 03, 2020 at 10:08:53PM +0200, Martin Wilck wrote: > Hello Lixiaokeng, > > On Wed, 2020-09-02 at 14:40 +0800, lixiaokeng wrote: > > Hi: > > Now, we check multipath-tools codes with codedex tool. Here > > are some some cleanups and fixes. > > Thank you. However I'm going to nack

Re: [dm-devel] [PATCH 05/14] multipathd: use MALLOC and check return value in, cli_getprkey func

2020-09-04 Thread Benjamin Marzinski
On Thu, Sep 03, 2020 at 09:13:04PM +0200, Martin Wilck wrote: > On Wed, 2020-09-02 at 15:18 +0800, lixiaokeng wrote: > > In cli_getprkey func, we use MALLOC instead of malloc, and check > > the return value of MALLOC. > > > > Signed-off-by: Zhiqiang Liu > > Signed-off-by: Lixiaokeng > >

Re: [dm-devel] [PATCH v2 00/10] multipath-tools: valgrind tests & fixes

2020-08-26 Thread Benjamin Marzinski
nt yesterday ([PATCH 1/5] multipath-tools tests: > fix memory leak in alias test), but in a different order / numbering, > which seems more appropriate now. > For the set Reviewed-by: Benjamin Marzinski > Regards, > Martin > > Martin Wilck (10): > multipath-t

Re: [dm-devel] [RFC PATCH 4/6] multipathd: cancel threads early during shutdown

2020-08-21 Thread Benjamin Marzinski
0, Martin Wilck wrote: > > On Wed, 2019-01-16 at 17:40 -0600, Benjamin Marzinski wrote: > > > On Fri, Jan 04, 2019 at 06:59:12PM +0100, Martin Wilck wrote: > > > > Cancel the other threads before taking vecs->lock. This avoids > > > > delays during shutdown

Re: [dm-devel] [PATCH 0/3] discard broken maps in get_dm_mpvec

2020-08-21 Thread Benjamin Marzinski
devel/2020-August/msg00246.html). > > As discussed before, the idea is to discard broken / incompletely > initialized maps in get_dm_mpvec(). > > Regards > Martin > Reviewed-by: Benjamin Marzinski > Martin Wilck (3): > libmultipath: update_multipath_table(): add flags

Re: [dm-devel] [PATCH v3 87/87] libmultipath: fix a -Wformat-truncation warning from gcc 10

2020-08-19 Thread Benjamin Marzinski
On Wed, Aug 19, 2020 at 03:18:19PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > This fixes a warning seen with gcc 10 on x86 (32 bit). > Fix it by checking the snprintf() return value. > Reviewed-by: Benjamin Marzinski > Signed-off-by: Martin Wilck > --- >

Re: [dm-devel] [PATCH v3 85/87] libmultipath: alias.c: use strtok_r() instead of strtok()

2020-08-19 Thread Benjamin Marzinski
On Wed, Aug 19, 2020 at 03:18:17PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > ... for thread-safety. > Reviewed-by: Benjamin Marzinski > Suggested-by: Benjamin Marzinski > Signed-off-by: Martin Wilck > --- > libmultipath/alias.c | 20 ++

Re: [dm-devel] [PATCH v3 86/87] libmultipath: adopt_paths(): set pp->mpp only on success

2020-08-19 Thread Benjamin Marzinski
On Wed, Aug 19, 2020 at 03:18:18PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > Make sure that pp->mpp is only set for paths that have been > successfully added to mpp->paths. > > Suggested-by: Benjamin Marzinki Reviewed-by: Benjamin Marzinski >

Re: [dm-devel] [PATCH v3 84/87] libmultipath: add consistency check for alias settings

2020-08-19 Thread Benjamin Marzinski
es. > Reviewed-by: Benjamin Marzinski > Signed-off-by: Martin Wilck > --- > libmultipath/alias.c | 265 +++ > libmultipath/alias.h | 3 + > multipath/main.c | 3 + > multipathd/main.c| 3 + > 4 files changed, 274 insertions(+)

Re: [dm-devel] [PATCH v3 76/80] libmultipath: select_action(): force udev reload for uninitialized maps

2020-08-19 Thread Benjamin Marzinski
e the multipath layer. > However, if the map was previously uninitialized, we have to force > udev to reload. > Better now. Reviewed-by: Benjamin Marzinski > Signed-off-by: Martin Wilck > --- > libmultipath/configure.c | 62 > 1 file cha

Re: [dm-devel] [PATCH v3 77/80] libmultipath: log dm_task_run() errors

2020-08-19 Thread Benjamin Marzinski
On Wed, Aug 19, 2020 at 03:17:47PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > Log the ioctl error messages from libdm. > Reviewed-by: Benjamin Marzinski > Signed-off-by: Martin Wilck > --- > libmultipath/devmapper.c | 61 ---

Re: [dm-devel] [PATCH 4/6] libmultipath: check blist before calling MALLOC in alloc_ble_device func

2020-08-18 Thread Benjamin Marzinski
gt; Ben, Christophe, what's your take on this matter? While I'm not really a fan of whitespace tweaking patches, I'm fine with this. All things being equal, I really do prefer it when functions check their arguments first, instead of doing possibly unnecessary work. Reviewed-by: Benjamin Marzinsk

Re: [dm-devel] [PATCH v2 84/84] libmultipath: add consistency check for alias settings

2020-08-17 Thread Benjamin Marzinski
On Wed, Aug 12, 2020 at 01:36:01PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > A typo in a config file, assigning the same alias to multiple WWIDs, > can cause massive confusion and even data corruption. Check and > if possible fix the bindings file in such cases. > > Signed-off-by:

Re: [dm-devel] [PATCH v2 82/84] libmultipath: free pp if store_path fails in disassemble_map

2020-08-17 Thread Benjamin Marzinski
l call free_path to free pp when store_path fails. > Reviewed-by: Benjamin Marzinski > Signed-off-by: Zhiqiang Liu > Signed-off-by: lixiaokeng > Signed-off-by: Martin Wilck > --- > libmultipath/dmparser.c | 12 > 1 file changed, 8 insertions(+), 4 deletions(-

Re: [dm-devel] [PATCH v2 83/84] libmultipath: alias: static const variable for BINDINGS_FILE_HEADER

2020-08-17 Thread Benjamin Marzinski
On Wed, Aug 12, 2020 at 01:36:00PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > ... and fixup the header file. > Reviewed-by: Benjamin Marzinski > Signed-off-by: Martin Wilck > --- > libmultipath/alias.c | 17 ++--- > libmultipath/alias.h | 12

Re: [dm-devel] [PATCH v2 81/84] multipath: check_path_valid(): eliminate some failure modes

2020-08-17 Thread Benjamin Marzinski
On Wed, Aug 12, 2020 at 01:35:58PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > The memory allocations can fail, and pathvec is not needed until the > path_discovery() call. Eliminate the failure modes by not setting up > pathvec before it's actually needed. > Rev

Re: [dm-devel] [PATCH v2 77/80] libmultipath: log dm_task_run() errors

2020-08-17 Thread Benjamin Marzinski
On Wed, Aug 12, 2020 at 01:35:41PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > Log the ioctl error messages from libdm. > I assume that the configure.c code from this patch belongs in the last one. -Ben > Reviewed-by: Benjamin Marzinski > Signed-of

Re: [dm-devel] [PATCH v2 76/80] libmultipath: select_action(): force udev reload for uninitialized maps

2020-08-17 Thread Benjamin Marzinski
pp from select_action, not mpp. I'm pretty sure that the next patch makes everything work o.k. again. -Ben > > Reviewed-by: Benjamin Marzinski > Signed-off-by: Martin Wilck > --- > libmultipath/configure.c | 61 > 1 file changed, 37 inserti

Re: [dm-devel] [PATCH v2 71/74] multipath: use update_pathvec_from_dm()

2020-08-17 Thread Benjamin Marzinski
On Wed, Aug 12, 2020 at 01:35:11PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > The multipath-specific function update_paths() can now be replaced with > a call to update_pathvec_from_dm(). > Reviewed-by: Benjamin Marzinski > Signed-off-by: Martin Wilck >

Re: [dm-devel] [PATCH v2 66/74] libmultipath: update_pathvec_from_dm: handle pp->mpp mismatch

2020-08-13 Thread Benjamin Marzinski
On Wed, Aug 12, 2020 at 01:35:10PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > Treat this like a WWID mismatch. > Reviewed-by: Benjamin Marzinski > Signed-off-by: Martin Wilck > --- > libmultipath/structs_vec.c | 33 +

Re: [dm-devel] [PATCH v2 65/74] libmultipath: add update_pathvec_from_dm()

2020-08-13 Thread Benjamin Marzinski
; set from the map WWID, which I consider not perfect, but no worse > than what we did before. > Reviewed-by: Benjamin Marzinski > Signed-off-by: Martin Wilck > --- > libmultipath/structs_vec.c | 136 + > libmultipath/structs_vec.h | 2 +

Re: [dm-devel] [PATCH v2 63/74] multipathd: deal with INIT_REMOVED during path addition

2020-08-13 Thread Benjamin Marzinski
ev_add_path() and cli_add_path(). > Reviewed-by: Benjamin Marzinski > Signed-off-by: Martin Wilck > --- > multipathd/cli_handlers.c | 54 +++-- > multipathd/main.c | 57 --- > 2 files changed, 1

Re: [dm-devel] [PATCH v2 64/74] libmultipath: orphan_paths(): avoid BUG message

2020-08-13 Thread Benjamin Marzinski
On Wed, Aug 12, 2020 at 01:35:08PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > Since c44d769, we print a BUG message when we orphan a path that > holds the mpp->hwe pointer. But if this called via orphan_paths(), > this is expected and we shouldn't warn. >

Re: [dm-devel] [PATCH v2 61/74] libmultipath: adopt_paths(): skip removed paths

2020-08-13 Thread Benjamin Marzinski
On Wed, Aug 12, 2020 at 01:35:06PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > If we don't do this, pathinfo() will fail on these paths, causing > adopt_paths() to fail. > Reviewed-by: Benjamin Marzinski > Signed-off-by: Martin Wilck > --- > libmul

Re: [dm-devel] [PATCH v2 46/54] libmultipath: path_discover(): explain pathinfo flags

2020-08-13 Thread Benjamin Marzinski
On Wed, Aug 12, 2020 at 01:34:29PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > Add a comment explaining why we use different flags for "new" and > existing paths. > Reviewed-by: Benjamin Marzinski > Signed-off-by: Martin Wilck > --- > libmultipa

Re: [dm-devel] [PATCH v2 44/54] libmultipath: adopt_paths(): don't bail out on single path failure

2020-08-13 Thread Benjamin Marzinski
ath device. But since that bug was already there we can fix it in a seperate patch, so Reviewed-by: Benjamin Marzinski > Signed-off-by: Martin Wilck > --- > libmultipath/structs_vec.c | 17 +++-- > multipathd/main.c | 3 ++- > 2 files changed, 13 ins

Re: [dm-devel] [PATCH v2 41/42] libmultipath: refuse reloading an existing map with different WWID

2020-08-13 Thread Benjamin Marzinski
se it has a wrong alias, which messes with the device that we were supposed to reload (I assume this makes the code easier). Does that sound like a reasonable solution? Reviewed-by: Benjamin Marzinski > Signed-off-by: Martin Wilck > --- > libmultipath/configure.c | 19 +++--

Re: [dm-devel] [PATCH v2 40/42] libmultipath: free_multipath(): remove mpp references

2020-08-13 Thread Benjamin Marzinski
On Wed, Aug 12, 2020 at 01:34:04PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > If free_paths is false, make sure no references to the dropped > multipath remain. Otherwise multipathd may crash later when > trying to access these. > Reviewed-by: Benjamin Marzi

Re: [dm-devel] [PATCH v2 40/42] libmultipath: free_multipath(): remove mpp references

2020-08-13 Thread Benjamin Marzinski
On Wed, Aug 12, 2020 at 01:34:04PM +0200, mwi...@suse.com wrote: > From: Martin Wilck > > If free_paths is false, make sure no references to the dropped > multipath remain. Otherwise multipathd may crash later when > trying to access these. > Reviewed-by: Benjamin Marzi

Re: [dm-devel] [PATCH v2 12/35] libmultipath: strlcpy()/strlcat(): use restrict qualifier

2020-08-12 Thread Benjamin Marzinski
other implementations, so it's fine. Reviewed-by: Benjamin Marzinski > Signed-off-by: Martin Wilck > --- > libmultipath/util.c | 28 > libmultipath/util.h | 4 ++-- > 2 files changed, 14 insertions(+), 18 deletions(-) > > diff --git a/libmultipat

Re: [dm-devel] [PATCH v2 08/35] libmultipath: create bitfield abstraction

2020-08-12 Thread Benjamin Marzinski
e with overflow detection and a > find_first_set() method. > > Use this in coalesce_paths(), and adapt the unit tests. Also, add > unit tests for "odd" bitfield sizes; so far we tested only multiples > of 64. > Reviewed-by: Benjamin Marzinski > Signed-off-b

Re: [dm-devel] [PATCH v2 7/8] multipathd: unset mpp->hwe when removing map

2020-08-12 Thread Benjamin Marzinski
On Wed, Aug 12, 2020 at 09:28:14AM +, Martin Wilck wrote: > On Tue, 2020-08-11 at 16:58 -0500, Benjamin Marzinski wrote: > > If the map doesn't unset its hwe pointer before orphaning all the > > paths, > > multipathd will print a warning message in orphan_path()

[dm-devel] [PATCH v2 8/8] multipathd: log reason for calling domap()

2020-08-11 Thread Benjamin Marzinski
When multipathd calls domap(), it should also print the reason on log level 2, it already does this on every code path except when domap is called by the path_checker. Also, if __setup_multipath deletes the device, it should log that. Signed-off-by: Benjamin Marzinski --- multipathd/main.c

[dm-devel] [PATCH v2 5/8] libmultipath: deal with flushing no maps

2020-08-11 Thread Benjamin Marzinski
dm_flush_maps() was failing if there were no device-mapper devices at all, instead of returning success, since there is nothing to do. Fixes: "libmultipath: make dm_flush_maps only return 0 on success" Reviewed-by: Martin Wilck Signed-off-by: Benjamin Marzinski --- libmultipath/devma

[dm-devel] [PATCH v2 3/8] libmultipath: refactor path counting

2020-08-11 Thread Benjamin Marzinski
use count_active_paths() in mpath_persist. Signed-off-by: Benjamin Marzinski --- libmpathpersist/mpath_persist.c | 4 ++-- libmultipath/structs.c | 31 +-- libmultipath/structs.h | 1 - 3 files changed, 19 insertions(+), 17 deletions(-) diff --git

[dm-devel] [PATCH v2 0/8] multipath cleanups

2020-08-11 Thread Benjamin Marzinski
aths(), as suggested by Martin Wilck - Added patches 0007 & 0008, which are both unrelated minor changes. Benjamin Marzinski (8): Makefile.inc: trim extra information from systemd version kpartx: fix -Wsign-compare error libmultipath: refactor path counting libmultipath: count pending paths a

[dm-devel] [PATCH v2 6/8] multipath: deal with delegation failures correctly

2020-08-11 Thread Benjamin Marzinski
; Reviewed-by: Martin Wilck Signed-off-by: Benjamin Marzinski --- multipath/main.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/multipath/main.c b/multipath/main.c index 4c43314e..3da692dc 100644 --- a/multipath/main.c +++ b/multipath/main.c @@ -861,9 +861

<    4   5   6   7   8   9   10   11   12   13   >