multipath-tools/.git
5 months agoRename the README to README.md master 0.8.5
Christophe Varoqui [Thu, 12 Nov 2020 08:06:34 +0000 (09:06 +0100)]
Rename the README to README.md

As it is the github expectation for README with markdown content.

5 months agoUpdate the README content to reflect to new github project hosting
Christophe Varoqui [Thu, 12 Nov 2020 08:05:20 +0000 (09:05 +0100)]
Update the README content to reflect to new github project hosting

And use markdown notations.

5 months agoBump version to 0.8.5
Christophe Varoqui [Mon, 9 Nov 2020 09:25:53 +0000 (10:25 +0100)]
Bump version to 0.8.5

5 months agolibmultipath: call udev_device_unref before return
lixiaokeng [Thu, 22 Oct 2020 07:30:13 +0000 (15:30 +0800)]
libmultipath: call udev_device_unref before return

There is a bug in commit acff7d4. Before return, we should call
udev_device_unref(hostdev) in sysfs_get_tgt_nodenam.

Signed-off_by:lixiaokeng <lxiiaokeng@huawei.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
5 months agomultipath-tools: replace leading spaces with tabs at LENOVO/DE_Series
Xose Vazquez Perez [Thu, 22 Oct 2020 19:24:39 +0000 (21:24 +0200)]
multipath-tools: replace leading spaces with tabs at LENOVO/DE_Series

Cc: Martin Wilck <mwilck@suse.com>
Cc: Benjamin Marzinski <bmarzins@redhat.com>
Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: DM-DEVEL ML <dm-devel@redhat.com>
Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
5 months agomultipath-tools: add MacroSAN arrays to hwtable
Xose Vazquez Perez [Thu, 22 Oct 2020 19:24:38 +0000 (21:24 +0200)]
multipath-tools: add MacroSAN arrays to hwtable

Based on:
http://case.macrosan.com/webdoc/view/Pub40288112616a02f101617abd864a0fe9.html
http://case.macrosan.com/webdoc/view/Pub402881126078307e0160c15456491369.html
http://case.macrosan.com/webdoc/view/Pub4028811263cdeeb40165d7150dce19e6.html
http://case.macrosan.com/webdoc/view/Pub40288112661f1dee016a95cb58f5504c.html

Cc: Martin Wilck <mwilck@suse.com>
Cc: Benjamin Marzinski <bmarzins@redhat.com>
Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: DM-DEVEL ML <dm-devel@redhat.com>
Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
5 months agolibmultipath tests: delete test-log.d during "make clean"
Martin Wilck [Fri, 30 Oct 2020 20:32:57 +0000 (21:32 +0100)]
libmultipath tests: delete test-log.d during "make clean"

test-log.d was not cleaned out. Fix it.

5 months agolibmultipath: dm_task_get_errno() is not always available
Martin Wilck [Wed, 28 Oct 2020 22:13:28 +0000 (23:13 +0100)]
libmultipath: dm_task_get_errno() is not always available

The function was added in LVM2 2.02.122. The DM version is
1.02.99. Fall back to errno if dm_task_get_errno() doesn't exist.

5 months agomultipath-tools tests: fix for busybox
Martin Wilck [Tue, 27 Oct 2020 22:34:29 +0000 (23:34 +0100)]
multipath-tools tests: fix for busybox

busybox' "ln" command doesn't support the "-t" syntax of GNU ln.

5 months agolibmultipath tests: fix strerror() difference between musl and glibc
Martin Wilck [Tue, 27 Oct 2020 22:32:59 +0000 (23:32 +0100)]
libmultipath tests: fix strerror() difference between musl and glibc

If an error occurs with errno=0, strerror() on musl returns a different
string than "Success". Make sure the test doesn't fail for that reason.

5 months agolibmultipath: always use glibc basename()
Martin Wilck [Tue, 27 Oct 2020 22:28:28 +0000 (23:28 +0100)]
libmultipath: always use glibc basename()

Our code relies on the non-destructive behavior of glibc's basename().
Fortunately that function is very simple, and our unit test makes
it easy to verify its functionality.

5 months agomultipath-tools: fix -Wformat errors with musl libc
Martin Wilck [Tue, 27 Oct 2020 22:36:06 +0000 (23:36 +0100)]
multipath-tools: fix -Wformat errors with musl libc

rlim_t type is different on musl libc. A cast to unsigned long
should be fine. Also, in musl, pthread_t is a pointer.

5 months agolibmultipath: make sure __GLIBC_PREREQ() is defined
Martin Wilck [Tue, 27 Oct 2020 22:30:26 +0000 (23:30 +0100)]
libmultipath: make sure __GLIBC_PREREQ() is defined

Otherwise complilation fails on non-glibc systems.

6 months agomultipath-tools: add info about the historical-service-time path selector to man...
Xose Vazquez Perez [Mon, 5 Oct 2020 19:28:06 +0000 (21:28 +0200)]
multipath-tools: add info about the historical-service-time path selector to man page

Just minimal info.

Cc: Khazhismel Kumykov <khazhy@google.com>
Cc: Gabriel Krisman Bertazi <krisman@collabora.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Benjamin Marzinski <bmarzins@redhat.com>
Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: DM-DEVEL ML <dm-devel@redhat.com>
Reviewed-by: Gabriel Krisman Bertazi <krisman@collabora.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
6 months agolibmultipath: Allow discovery of USB devices
Frank Meinl [Mon, 28 Sep 2020 19:09:57 +0000 (21:09 +0200)]
libmultipath: Allow discovery of USB devices

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 multipath.conf), USB devices will be found
during device discovery and can be used for multipath setups.

Without this option, multipath currently ignores all USB drives, which
is convenient for most users. (refer also to commit 51957eb)

However, it can be beneficial to use the multipath dm-module even if
there is only a single path to an USB attached storage. In combination
with the option 'queue_if_no_path', such a setup survives a temporary
device disconnect without any severe consequences for the running
applications. All I/O is queued until the device reappears.

Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Frank Meinl <frank.meinl@live.de>
6 months agolibmultipath: update_pathvec_from_dm: always check WWID
Martin Wilck [Fri, 25 Sep 2020 13:26:45 +0000 (15:26 +0200)]
libmultipath: update_pathvec_from_dm: always check WWID

The current code checks the WWID of "new" paths discovered by
disassemble_map() only if pp->udev is NULL. But a path could
have been removed and re-added with a different pp->udev. To
be certain, check the WWID in this case, too.

7 months agolibmultipath: fix memory leak in _check_bindings_file
Martin Wilck [Wed, 16 Sep 2020 16:23:10 +0000 (18:23 +0200)]
libmultipath: fix memory leak in _check_bindings_file

We must pass "&line" to the cleanup function, not "line".

Fixes: "libmultipath: add consistency check for alias settings"
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
7 months agolibmultipath: check udev_device_get_* return value to avoid segfault
lixiaokeng [Mon, 21 Sep 2020 04:00:39 +0000 (12:00 +0800)]
libmultipath: check udev_device_get_* return value to avoid segfault

The udev_device_get_* function may return NULL, and it will be
deregerenced in str* and sscanf func. We check the return value
to avoid segfault. Fix all.

Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by:Lixiaokeng<lixiaokeng@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: Linfeilong <linfeilong@huawei.com>
7 months agomultipathpersist: delete unused variable in handle_args
lixiaokeng [Thu, 10 Sep 2020 10:54:30 +0000 (18:54 +0800)]
multipathpersist: delete unused variable in handle_args

In handle_args, the tmp isn't used. We delete it.

Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Lixiaokeng <lixiaokeng@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: Linfeilong <linfeilong@huawei.com>
7 months agolibmultipathpersist: use update_multipath_table/status, in get_mpvec
lixiaokeng [Thu, 10 Sep 2020 10:53:39 +0000 (18:53 +0800)]
libmultipathpersist: use update_multipath_table/status, in get_mpvec

The return values of dm_get_map, disassemble_map in get_mpvec
were not checked. Use update_multipath_table/status to instead
of them. If these function fail, call remove_map

Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Lixiaokeng <lixiaokeng@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: Linfeilong <linfeilong@huawei.com>
7 months agomultipath: use update_multipath_table/status in, check_useable_paths
lixiaokeng [Wed, 2 Sep 2020 07:25:28 +0000 (15:25 +0800)]
multipath: use update_multipath_table/status in, check_useable_paths

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 <bmarzins@redhat.com>
Signed-off-by: Lixiaokeng <lixiaokeng@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: Linfeilong <linfeilong@huawei.com>
7 months agompathpersist: check whether malloc paramp->trnptid_list fails in handle_args func
lixiaokeng [Sat, 19 Sep 2020 10:28:36 +0000 (18:28 +0800)]
mpathpersist: check whether malloc paramp->trnptid_list fails in handle_args func

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 we add free_prout_param_descriptor to free paramp
and each paramp->trnptid_list[j].

We change num_transport to num_transportids to combine them.

Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
7 months agoutil/tests: use assert_non_null to ensure malloc, returns non-null pointer
lixiaokeng [Wed, 2 Sep 2020 07:22:37 +0000 (15:22 +0800)]
util/tests: use assert_non_null to ensure malloc, returns non-null pointer

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: Martin Wilck <mwilck@suse.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
Signed-off-by: Linfeilong <linfeilong@huawei.com>
7 months agolibmultipath: check whether mpp->features is NUll in setup_map
lixiaokeng [Fri, 11 Sep 2020 07:21:41 +0000 (15:21 +0800)]
libmultipath: check whether mpp->features is NUll in setup_map

In assemble_map func, f = STRDUP(mp->features) is just used
for APPEND(). We can directly pass mp->features to APPEND().
The mpp->features, hwhandler and selector got form strdup
should be check after select* function.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: Lixiaokeng <lixiaokeng@huawei.com>
7 months agolibmultipath: check return value of dm_mapname in, sysfs_check_holders
lixiaokeng [Wed, 2 Sep 2020 07:19:51 +0000 (15:19 +0800)]
libmultipath: check return value of dm_mapname in, sysfs_check_holders

In sysfs_check_holders func, table_name is obtained by calling
dm_mapname func, and then call dm_reassign_table for reassigning
table. However, we donnot check whether dm_mapname func returns
NULL, and then 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 <bmarzins@redhat.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: Lixiaokeng <lixiaokeng@huawei.com>
7 months agokpartx: use xmalloc to instead of malloc in main func
lixiaokeng [Thu, 10 Sep 2020 10:50:36 +0000 (18:50 +0800)]
kpartx: use xmalloc to instead of malloc in main func

In main func of kpartx.c, we should check return value of
malloc before using it. So we use xmalloc to instead of
malloc.

Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: Lixiaokeng <lixiaokeng@huawei.com>
7 months agomultipathd: check return value of malloc in cli_getprkey func
lixiaokeng [Thu, 10 Sep 2020 10:49:27 +0000 (18:49 +0800)]
multipathd: check return value of malloc in cli_getprkey func

In cli_getprkey func, we check the return value of malloc.

Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: Lixiaokeng <lixiaokeng@huawei.com>
Signed-off-by: Linfeilong <linfeilong@huawei.com>
7 months agomultipathd: check MALLOC return value in mpath_pr_event_handler_fn
lixiaokeng [Thu, 10 Sep 2020 10:48:35 +0000 (18:48 +0800)]
multipathd: check MALLOC return value in mpath_pr_event_handler_fn

In  mpath_pr_event_handler_fn, we use MALLOC instead of malloc, and check
the return value of MALLOC. And we delete seting ret when jump to out

Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Lixiaokeng <lixiaokeng@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: Linfeilong <linfeilong@huawei.com>
7 months agolibmultipath: use map instead of dm_task_get_name
lixiaokeng [Wed, 2 Sep 2020 07:17:27 +0000 (15:17 +0800)]
libmultipath: use map instead of dm_task_get_name

In dm_mapname, dm_task_get_name(dmt) has been called and the return value
has been stored in map. Use map instead of second  dm_task_get_name.

Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Lixiaokeng <lixiaokeng@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: Linfeilong <linfeilong@huawei.com>
7 months agolibmultipath: change malloc to calloc in print_foreign_topology
lixiaokeng [Thu, 10 Sep 2020 10:47:34 +0000 (18:47 +0800)]
libmultipath: change malloc to calloc in print_foreign_topology

We chanege malloc to calloc.

Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Lixiaokeng <lixiaokeng@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: Linfeilong <linfeilong@huawei.com>
7 months agomultipathd: initialize major and minor in cli_add_map
lixiaokeng [Thu, 10 Sep 2020 10:46:37 +0000 (18:46 +0800)]
multipathd: initialize major and minor in cli_add_map

If dm_get_major_minor failed, log with major and minor should not
be printed to avoid major and minor used before initialization.

Here, we set major and minor to -1 at begining

Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Lixiaokeng <lixiaokeng@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: Linfeilong <linfeilong@huawei.com>
7 months agolibmultipath: setup_map(): don't break multipath attributes
Martin Wilck [Thu, 10 Sep 2020 19:38:37 +0000 (21:38 +0200)]
libmultipath: setup_map(): don't break multipath attributes

setup_map() is called both for new maps (e.g. from coalesce_paths())
and existing maps (e.g. from reload_map(), resize_map()). In the former
case, the map will be removed from global data structures, so incomplete
initialization is not an issue. But In the latter case, removal isn't
generally possible. We expect that mpp->features, mpp->hwhandler,
mpp->selector have been initialized and are are non-NULL. We must make sure
not to break this assumption because 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.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
7 months agomultipath-tools tests: fix small bitfield tests for big endian
Martin Wilck [Fri, 21 Aug 2020 22:23:27 +0000 (00:23 +0200)]
multipath-tools tests: fix small bitfield tests for big endian

This is the opposite case of the previous patch: 64 bit bitfield_t,
tested with 32 bit words.

Fixes: "libmultipath: create bitfield abstraction"
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
7 months agomultipath-tools tests: fix bitfield tests for big endian
Martin Wilck [Fri, 21 Aug 2020 21:33:19 +0000 (23:33 +0200)]
multipath-tools tests: fix bitfield tests for big endian

On big endian systems, the 32bit words need to be swapped in
the test code to get the byte ordering right.

Fixes: "libmultipath: create bitfield abstraction"
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
7 months agomultipath-tools tests: fix bitfield tests for small fields
Martin Wilck [Fri, 21 Aug 2020 15:34:17 +0000 (17:34 +0200)]
multipath-tools tests: fix bitfield tests for small fields

The bitmask tests may fail if sizeof(bitfield_t) is 32, depending
on compiler options and other circumstances.
This is because allocation of the bitfield with calloc() only zeroes
out the actual length of the bitfield rounded to 32, and thus
the assertion *((uint64_t *)bf->bits) == 0 may fail.

Use uint32_t in the tests instead of uint64_t.

Fixes: "libmultipath: create bitfield abstraction"
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
7 months agomultipath-tools: add HPE MSA 1060/2060 to hwtable
Xose Vazquez Perez [Wed, 9 Sep 2020 15:48:05 +0000 (17:48 +0200)]
multipath-tools: add HPE MSA 1060/2060 to hwtable

Cc: Martin Wilck <mwilck@suse.com>
Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: DM-DEVEL ML <dm-devel@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
7 months agomultipathd: use daemon_status_msg to construct sd notify msg in do_sd_notify func
Zhiqiang Liu [Mon, 31 Aug 2020 11:37:11 +0000 (19:37 +0800)]
multipathd: use daemon_status_msg to construct sd notify msg in do_sd_notify func

sd_notify_status() is very similar with daemon_status(), except
DAEMON_IDLE and DAEMON_RUNNING state. As suggested by Martin,
we can create the sd notification string in a dynamic buffer,
and treat DAEMON_IDLE and DAEMON_RUNNING cases first. Then,
we can use daemon_status_msg[state] for other cases.

V3->V4:
- put "STATUS=" in format string to avoid "prefix" var as suggested by
Martin.

V2->V3:
- set MSG_SIZE to 32 and use safe_sprintf as suggested by Martin.

Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
7 months agomultipathd: adopt static char* arr in daemon_status
Zhiqiang Liu [Sat, 29 Aug 2020 03:03:04 +0000 (11:03 +0800)]
multipathd: adopt static char* arr in daemon_status

We adopt static char* array (demon_status_msg) in daemon_status func,
so it looks more simpler and easier to expand.

V2->V3:
- add default case in sd_notify_status to fix compile error
V1->V2:
- use "int" as the type of "status" (suggested by Martin)

Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
7 months agomultipath: get_dm_mpvec: discard broken maps
Martin Wilck [Fri, 21 Aug 2020 10:22:25 +0000 (12:22 +0200)]
multipath: get_dm_mpvec: discard broken maps

Use the same logic as map_discovery() to discard maps that
couldn't be parsed successfully. If map parsing fails,
certain vital fields of the mpp, like features or hwhandler,
will not be set, which might cause multipath to crash later on.

7 months agolibmultipath: remove_map(): separate pathvec and mpvec arguments
Martin Wilck [Fri, 21 Aug 2020 10:19:49 +0000 (12:19 +0200)]
libmultipath: remove_map(): separate pathvec and mpvec arguments

This will enable us to use remove_map() from multipath.
Also, pass symbolic arguments for remove_map() everywhere.

7 months agolibmultipath: update_multipath_table(): add flags argument
Martin Wilck [Fri, 21 Aug 2020 09:40:31 +0000 (11:40 +0200)]
libmultipath: update_multipath_table(): add flags argument

... to be passed to update_pathvec_from_dm().

7 months agolibmultipath: validate_config_strvec(): avoid out-of-bounds access
Martin Wilck [Wed, 26 Aug 2020 09:27:53 +0000 (11:27 +0200)]
libmultipath: validate_config_strvec(): avoid out-of-bounds access

Always check the length of strvec before accessing elements.

7 months agolibmultipath: alloc_strvec: NULL-initialize strvec elements
Martin Wilck [Wed, 26 Aug 2020 09:26:20 +0000 (11:26 +0200)]
libmultipath: alloc_strvec: NULL-initialize strvec elements

The statement assigning a real token to a strvec element may
never be reached. Thus set the element to NULL beforehand to
make sure later code can recognized the non-properly initialized
element.

7 months agolibmultipath: fix invalid memory access in is_token()
Martin Wilck [Wed, 26 Aug 2020 09:20:25 +0000 (11:20 +0200)]
libmultipath: fix invalid memory access in is_token()

memcmp() must always be passed memory areas that are valid for the
full length given by the size argument.

See e.g. https://trust-in-soft.com/blog/2015/12/21/memcmp-requires-pointers-to-fully-valid-buffers/

Fixes: 7d95fb6 ("libmultipath: config parser: fix corner case for double quotes")

7 months agolibmultipath: fix enable_foreign memory leak
Martin Wilck [Tue, 25 Aug 2020 21:11:48 +0000 (23:11 +0200)]
libmultipath: fix enable_foreign memory leak

enable_foreign wasn't freed in free_config(). Do it, and make
sure it's always a malloc'd string.

7 months agolibmultipath: fix memory leak in ble handlers
Martin Wilck [Tue, 25 Aug 2020 20:53:25 +0000 (22:53 +0200)]
libmultipath: fix memory leak in ble handlers

Since patch "libmultipath fix a memory leak in set_ble_device",
strings are strdup'd in set_ble_device() and store_ble(). The
passed string must therefore be freed in the handlers in dict.c.

Fixes: ("libmultipath fix a memory leak in set_ble_device")
Cc: lixiaokeng <lixiaokeng@huawei.com>
7 months agomultipath-tools tests: fix memory leak in vpd test
Martin Wilck [Tue, 25 Aug 2020 21:21:26 +0000 (23:21 +0200)]
multipath-tools tests: fix memory leak in vpd test

regfree() wasn't called on the re used in subst_spaces().

7 months agomultipath-tools tests: fix memory leak in hwtable test
Martin Wilck [Tue, 25 Aug 2020 21:12:32 +0000 (23:12 +0200)]
multipath-tools tests: fix memory leak in hwtable test

7 months agomultipath-tools tests: fix memory leak in alias test
Martin Wilck [Tue, 25 Aug 2020 20:40:11 +0000 (22:40 +0200)]
multipath-tools tests: fix memory leak in alias test

7 months agomultipath-tools: Makefile: add "valgrind-test" target
Martin Wilck [Wed, 26 Aug 2020 09:29:58 +0000 (11:29 +0200)]
multipath-tools: Makefile: add "valgrind-test" target

The new target "valgrind-test" (or "valgrind" in the tests/
subdirectory) allows running the unit tests under valgrind.

7 months agomultipath-tools: Makefile.inc: fix compilation with gcc 4.x
Martin Wilck [Wed, 26 Aug 2020 08:13:23 +0000 (10:13 +0200)]
multipath-tools: Makefile.inc: fix compilation with gcc 4.x

GCC 4.x doesn't enable the "gnu99" standard by default, which is
required by the multipath-tools code. Tested with gcc 4, 7, 8,
9, 10 and clang 3.9, 7, 8, 9, 10.

8 months agolibmultipath: fix a -Wformat-truncation warning from gcc 10
Martin Wilck [Wed, 19 Aug 2020 08:55:08 +0000 (10:55 +0200)]
libmultipath: fix a -Wformat-truncation warning from gcc 10

This fixes a warning seen with gcc 10 on x86 (32 bit).
Fix it by checking the snprintf() return value.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: adopt_paths(): set pp->mpp only on success
Martin Wilck [Wed, 19 Aug 2020 08:40:45 +0000 (10:40 +0200)]
libmultipath: adopt_paths(): set pp->mpp only on success

Make sure that pp->mpp is only set for paths that have been
successfully added to mpp->paths.

Suggested-by: Benjamin Marzinki <bmarzins@redhat.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: alias.c: use strtok_r() instead of strtok()
Martin Wilck [Tue, 18 Aug 2020 09:11:16 +0000 (11:11 +0200)]
libmultipath: alias.c: use strtok_r() instead of strtok()

... for thread-safety.

Suggested-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: add consistency check for alias settings
Martin Wilck [Fri, 7 Aug 2020 20:50:06 +0000 (22:50 +0200)]
libmultipath: add consistency check for alias settings

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.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: alias: static const variable for BINDINGS_FILE_HEADER
Martin Wilck [Mon, 10 Aug 2020 19:55:28 +0000 (21:55 +0200)]
libmultipath: alias: static const variable for BINDINGS_FILE_HEADER

... and fixup the header file.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: free pp if store_path fails in disassemble_map
Zhiqiang Liu [Fri, 24 Jul 2020 01:40:18 +0000 (09:40 +0800)]
libmultipath: free pp if store_path fails in disassemble_map

In disassemble_map func, one pp will be allocated and stored in
pgp->paths. However, if store_path fails, pp will not be freed,
then memory leak problem occurs.

Here, we will call free_path to free pp when store_path fails.

Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agomultipath: check_path_valid(): eliminate some failure modes
Martin Wilck [Wed, 5 Aug 2020 12:37:34 +0000 (14:37 +0200)]
multipath: check_path_valid(): eliminate some failure modes

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.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: select_action(): don't drop map if alias clashes
Martin Wilck [Fri, 3 Jul 2020 13:17:09 +0000 (15:17 +0200)]
libmultipath: select_action(): don't drop map if alias clashes

If for a given map, if we find that the requested alias is already
used by a map with different WWID, while the map's own WWID is
not used yet, give up the alias and use the WWID instead. This
is safer than trying to destroy the existing map, which is likely
to fail.

This allows us to make use const for the "curmp" parameter.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agomultipathd: rename update_path_groups() -> reload_and_sync_map()
Martin Wilck [Fri, 3 Jul 2020 13:32:09 +0000 (15:32 +0200)]
multipathd: rename update_path_groups() -> reload_and_sync_map()

This function doesn't just update the path groups. It completely
rebuilds the multipath, reloads the kernel map, and syncs path
states. That should be reflected in the function name, which should
use the term "map", like all other functions that modify kernel state.

Todo: there's a large functional overlap between this function
and update_map(). Perhaps we should unify the two.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: move reload_map() to multipathd
Martin Wilck [Fri, 3 Jul 2020 13:15:33 +0000 (15:15 +0200)]
libmultipath: move reload_map() to multipathd

reload_map() is only used by multipathd. We don't have less exported
symbols though, because select_action() now needs to be exported.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: log dm_task_run() errors
Martin Wilck [Tue, 30 Jun 2020 11:36:45 +0000 (13:36 +0200)]
libmultipath: log dm_task_run() errors

Log the ioctl error messages from libdm.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: select_action(): force udev reload for uninitialized maps
Martin Wilck [Mon, 29 Jun 2020 22:22:47 +0000 (00:22 +0200)]
libmultipath: select_action(): force udev reload for uninitialized maps

If we are in the reconfigure() code path, and we encounter maps to
be reloaded, we usually set the DM_SUBSYSTEM_UDEV_FLAG0 flag to tell
udev not to repeat device detection steps above the multipath layer.
However, if the map was previously uninitialized, we have to force
udev to reload.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agomultipathd: uev_trigger(): handle incomplete ADD events
Martin Wilck [Mon, 29 Jun 2020 16:15:24 +0000 (18:15 +0200)]
multipathd: uev_trigger(): handle incomplete ADD events

Udev may be killed after handling the ADD event for a multipath map,
but before handling the subsequent CHANGE event that populates the
udev data base with the device properties (e.g. during initrd processing).
If this happens, the ADD uevent sent during coldplug will only provide a
subset of the device properties. We need another CHANGE event to make the map
available to the system.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: dmparser: constify function arguments
Martin Wilck [Wed, 8 Jul 2020 07:12:55 +0000 (09:12 +0200)]
libmultipath: dmparser: constify function arguments

With the previous change that avoids additions to pathvec,
the pathvec argument to disassemble_map() is const now.
Also use const for the string arguments where possible.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: decrease loglevel in store_path()
Martin Wilck [Tue, 7 Jul 2020 22:16:39 +0000 (00:16 +0200)]
libmultipath: decrease loglevel in store_path()

"Empty device name" in store_path() can happen regularly and
shouldn't be logged at -v2.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmpathpersist: use update_pathvec_from_dm()
Martin Wilck [Tue, 7 Jul 2020 20:44:08 +0000 (22:44 +0200)]
libmpathpersist: use update_pathvec_from_dm()

The libmpathpersist-specific function updatepaths() can be replaced
with the generic function.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agomultipath: use update_pathvec_from_dm()
Martin Wilck [Tue, 7 Jul 2020 20:41:35 +0000 (22:41 +0200)]
multipath: use update_pathvec_from_dm()

The multipath-specific function update_paths() can now be replaced with
a call to update_pathvec_from_dm().

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: disassemble_map(): do not change pathvec and WWIDs
Martin Wilck [Sun, 7 Jun 2020 20:00:54 +0000 (22:00 +0200)]
libmultipath: disassemble_map(): do not change pathvec and WWIDs

After introducing update_pathvec_from_dm() in a predecessor patch,
the "layer violations" in disassemble_map() can now be removed.
I hope this clarifies program logic a little bit.

Callers need to call update_pathvec_from_dm() after disassemble_map().

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: disassemble_map(): get rid of "is_daemon" argument
Martin Wilck [Sat, 6 Jun 2020 23:28:20 +0000 (01:28 +0200)]
libmultipath: disassemble_map(): get rid of "is_daemon" argument

The reason for the is_daemon parameter in disassemble_map() lies
deep in multipath-tools' past, in b96dead ("[multipathd] remove the
retry login in uev_remove_path()"): By not adding paths from
disassembled maps to the pathvec, we avoided to re-add removed paths on
future map reloads (logic in update_mpp_paths()). As we can handle this with
INIT_REMOVED now, we don't need to distinguish daemon and non-daemon
mode any more. This fixes a memory leak, because only paths which are in
pathvec will be freed on program exit.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: disassemble_map(): require non-NULL pathvec
Martin Wilck [Sat, 13 Jun 2020 19:10:01 +0000 (21:10 +0200)]
libmultipath: disassemble_map(): require non-NULL pathvec

We would fail in store_path() at the latest if pathvec was NULL.
And all callers set pathvec anyway.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: disassemble_map(): always search paths by dev_t
Martin Wilck [Thu, 4 Jun 2020 10:15:00 +0000 (12:15 +0200)]
libmultipath: disassemble_map(): always search paths by dev_t

There's no point in searching for the devname first. dev_t is the primary
device property for both device mapper and udev.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: update_pathvec_from_dm: handle pp->mpp mismatch
Martin Wilck [Tue, 7 Jul 2020 22:14:46 +0000 (00:14 +0200)]
libmultipath: update_pathvec_from_dm: handle pp->mpp mismatch

Treat this like a WWID mismatch.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: add update_pathvec_from_dm()
Martin Wilck [Tue, 7 Jul 2020 20:23:39 +0000 (22:23 +0200)]
libmultipath: add update_pathvec_from_dm()

It can happen in particular during boot or startup that we encounter
paths as map members which haven't been discovered or fully initialized
yet, and are thus not in the pathvec. These paths need special treatment
in various ways. Currently this is dealt with in disassemble_map(). That's
a layer violation, and the way it is done is suboptimal in various ways.

As a preparation to change that, this patch introduces a new function,
update_pathvec_from_dm(), that is supposed to deal with newly discovered
paths from disassemble_map(). It has to be called after disassemble_map()
has finished.

The logic of the new function is similar but not identical to what
disassemble_map() was doing before. Firstly, the function will simply
remove path devices that don't exist - there's no point in carrying these
around. Map reloads can't be called from this code for reasons of the
overall program logic. But it's prepared to signal to the caller that
a map reload is in order. Using this return value will be future work.

Second, pathinfo() is now called on new paths rather than just setting
pp->dev. The pathinfo flags can be passed as argument to make the function
flexible for different callers.

Finally, treatment of WWIDs is different now. There'll be only one attempt
at guessing the map WWID if it's not set yet. If a non-matching path WWID
is found, the path is removed, and a new uevent triggered (this is the
most important change wrt the dangerous previous code that would simply
overwrite the path WWID). If the path WWID is empty, it will still be
set from the map WWID, which I consider not perfect, but no worse
than what we did before.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: orphan_paths(): avoid BUG message
Martin Wilck [Tue, 11 Aug 2020 19:08:27 +0000 (21:08 +0200)]
libmultipath: orphan_paths(): avoid BUG message

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.

Fixes: c44d769 ("libmultipath: warn if freeing path that holds mpp->hwe")

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agomultipathd: deal with INIT_REMOVED during path addition
Martin Wilck [Sat, 6 Jun 2020 23:18:10 +0000 (01:18 +0200)]
multipathd: deal with INIT_REMOVED during path addition

With the introduction of INIT_REMOVED, we have to deal with the situation
when a path is re-added in this state. This enables us to detect the
situation where a path is added while still part of a map after a failed
removal, which we couldn't before. Dealing with this correctly requires
some additional logic. There's a good case (re-added path is still mapped
by a map with matching WWID) and a bad case (non-matching WWID).

The logic is very similar in uev_add_path() and cli_add_path().

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agomultipathd: ev_remove_path(): use INIT_REMOVED
Martin Wilck [Sat, 6 Jun 2020 23:00:13 +0000 (01:00 +0200)]
multipathd: ev_remove_path(): use INIT_REMOVED

Set paths belonging to a map to INIT_REMOVED state before attempting
to reload or flush the map. If the map association is successfully removed,
the path will be actually deleted, either via flush_map() -> orphan_paths(),
or in the update_multipath_strings()->sync_paths() code path.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: adopt_paths(): skip removed paths
Martin Wilck [Mon, 6 Jul 2020 22:20:39 +0000 (00:20 +0200)]
libmultipath: adopt_paths(): skip removed paths

If we don't do this, pathinfo() will fail on these paths, causing
adopt_paths() to fail.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: orphan_paths(): delete paths in INIT_REMOVED state
Martin Wilck [Sat, 6 Jun 2020 23:16:03 +0000 (01:16 +0200)]
libmultipath: orphan_paths(): delete paths in INIT_REMOVED state

A path in INIT_REMOVED state is only waiting for its last association
with a multipath map to be dropped. When orphan_paths() encounters
such a path, rather than orphaning it, free it.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: sync_paths(): handle INIT_REMOVED
Martin Wilck [Sat, 4 Jul 2020 20:47:12 +0000 (22:47 +0200)]
libmultipath: sync_paths(): handle INIT_REMOVED

sync_paths() is the function which is called after getting kernel
state with disassemble_map(). This is the place where we should
check if paths that can eventually be deleted.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: verify_paths(): don't delete paths from pathvec
Martin Wilck [Fri, 3 Jul 2020 11:33:22 +0000 (13:33 +0200)]
libmultipath: verify_paths(): don't delete paths from pathvec

If we encounter a non-existing device verify_paths(), just set
it to INIT_REMOVED state. Actual path deletion is postponed until
we don't see that path in the kernel map any more.

This allows us to get rid of the "pathvec" argument to this function.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: update_mpp_paths(): handle INIT_REMOVED
Martin Wilck [Sat, 6 Jun 2020 23:08:16 +0000 (01:08 +0200)]
libmultipath: update_mpp_paths(): handle INIT_REMOVED

Since the ancient commit b96dead ("[multipathd] remove the retry login in
uev_remove_path()"), update_mpp_paths() was used to check whether devices
found in disassembled maps were still in the pathvec, and to skip them
while re-assembling the new params string on reload. The reason was to
deal with failed reloads. With the introduction of INIT_REMOVED, we
need to skip paths in that state, too. Moreover, past reloads may have
succeeded in removing REMOVED paths from the map, so check if any
INIT_REMOVED paths can now be deleted for good. This is the right
place to do this check, because update_mpp_paths() is called after
obtaining the current kernel state, and before reloading.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agomultipath-tools: introduce INIT_REMOVED state
Martin Wilck [Sat, 6 Jun 2020 22:54:24 +0000 (00:54 +0200)]
multipath-tools: introduce INIT_REMOVED state

Introduce a new state for pp->initialized, INIT_REMOVED. This state
means that the path is about to be removed, either by a remove uevent
or by the operator. It will normally be a very short-lived state, because
the path will be deleted from pathvec quickly after setting this state.
Only if the path is member of a multipath map, and reloading the map
fails, this state will persist until a later map reload or flush eventually
cancels the membership in the map.

Paths in INIT_REMOVED state are treated as if they didn't exist.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: add uninitialize_path()
Martin Wilck [Mon, 22 Jun 2020 16:35:26 +0000 (18:35 +0200)]
libmultipath: add uninitialize_path()

This helper clears all fields of struct path (except pp->udev) that must be
re-ininitialized if the path ever is to be used again.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: protect use of pp->udev
Martin Wilck [Mon, 22 Jun 2020 15:29:10 +0000 (17:29 +0200)]
libmultipath: protect use of pp->udev

We could never be 100% certain that pp->udev was always set.
With the upcoming change, we can be even less certain. Always
check pp->udev before using it.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: get_refwwid(): skip strchop(), and constify dev parameter
Martin Wilck [Sun, 14 Jun 2020 22:05:28 +0000 (00:05 +0200)]
libmultipath: get_refwwid(): skip strchop(), and constify dev parameter

With the previous change, we can safely assume that strchop() has been
called already where appropriate (the only caller is multipath's
configure()). We can now use const char* for the "dev" parameter.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agomultipath: call strchop() on command line argument
Martin Wilck [Sun, 14 Jun 2020 21:54:10 +0000 (23:54 +0200)]
multipath: call strchop() on command line argument

It's useful to sanitize these right away. We can't do this for DEV_DEVMAP,
as aliases with trailing whitespace aren't strictly forbidden, but for
the other types we can.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: constify get_mpe_wwid()
Martin Wilck [Sun, 14 Jun 2020 22:04:09 +0000 (00:04 +0200)]
libmultipath: constify get_mpe_wwid()

As this returns a pointer to a struct member, the return value
should also be a const char*.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: get_refwwid(): use switch statement
Martin Wilck [Sun, 14 Jun 2020 14:47:33 +0000 (16:47 +0200)]
libmultipath: get_refwwid(): use switch statement

This code calls for a switch. Some additional compaction is possible by
observing that the code for DEV_DEVNODE, DEV_DEVT, and DEV_UEVENT is almost
the same, and factoring it out into a "common" section. Doing this with
a goto inside the switch statement is a bit unusual, but shows the intention
of the code more clearly than other variants I tried.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: get_refwwid(): use symbolic return values
Martin Wilck [Sun, 14 Jun 2020 14:42:38 +0000 (16:42 +0200)]
libmultipath: get_refwwid(): use symbolic return values

The return values of get_refwwid() are the same as those of pathinfo().
So use them.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: get_refwwid(): get rid of "check" label
Martin Wilck [Sun, 14 Jun 2020 20:51:24 +0000 (22:51 +0200)]
libmultipath: get_refwwid(): get rid of "check" label

This was necessary with with the interspersed pthread_cleanup_push()/pop()
statements, now we can write the code more cleanly.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: get_refwwid(): call get_multipath_config() only once
Martin Wilck [Sun, 14 Jun 2020 14:32:58 +0000 (16:32 +0200)]
libmultipath: get_refwwid(): call get_multipath_config() only once

rather than 7 times in a single function. In get_refwwid(), the code that
is not run under the RCU read lock is negligible, so we might as well
keep the lock.

The "invalid" variable becomes obsolete by this change.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: get_refwwid(): use find_path_by_devt()
Martin Wilck [Sat, 13 Jun 2020 19:48:07 +0000 (21:48 +0200)]
libmultipath: get_refwwid(): use find_path_by_devt()

If get_refwwid is called with a dev_t argument, there's no point in converting
it into a devname first. If the device doesn't exist, get_udev_device() will fail.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: path_discover(): explain pathinfo flags
Martin Wilck [Sat, 13 Jun 2020 19:52:45 +0000 (21:52 +0200)]
libmultipath: path_discover(): explain pathinfo flags

Add a comment explaining why we use different flags for "new" and
existing paths.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: path_discover(): use find_path_by_devt()
Martin Wilck [Sat, 13 Jun 2020 19:50:49 +0000 (21:50 +0200)]
libmultipath: path_discover(): use find_path_by_devt()

In path_discover(), it's actually expected that a the path to be discovered is
not already in pathvec. So, do search by devt in the first place rather than
searching twice.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: adopt_paths(): don't bail out on single path failure
Martin Wilck [Mon, 6 Jul 2020 22:54:06 +0000 (00:54 +0200)]
libmultipath: adopt_paths(): don't bail out on single path failure

If pathinfo fails for one path to be adopted, we currently
fail the entire function. This may cause ev_add_path() for a valid
path to fail because some other path is broken. Fix it by just
skipping paths that don't look healthy.

adopt_paths() may now succeed even if some paths couldn't be
added (e.g. because of pathinfo() failure). If this happens while we are
trying to add a specific path, we need to check after successful call to
adopt_paths() if that specific path had been actually added, and fail in the
caller otherwise.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: adopt_paths(): use find_path_by_devt()
Martin Wilck [Mon, 22 Jun 2020 20:23:44 +0000 (22:23 +0200)]
libmultipath: adopt_paths(): use find_path_by_devt()

pp->dev_t is the primary identifying property for both dm and udev.
Use it here, too.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: dm_addmap(): refuse creating map with empty WWID
Martin Wilck [Thu, 4 Jun 2020 09:40:14 +0000 (11:40 +0200)]
libmultipath: dm_addmap(): refuse creating map with empty WWID

We already avoid creating maps with empty WWID in coalesce_paths()
as well as in ev_add_path(). The only code path where it's difficult
to prove (although extremely unlikely) that we can't call
dm_addmap(ACT_CREATE) with an empty WWID is update_path_groups()->
reload_map(). To make the code easier to review and avoid ugly
corner cases, simply refuse to create maps with a zero-length
WWID.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: refuse reloading an existing map with different WWID
Martin Wilck [Wed, 3 Jun 2020 21:50:49 +0000 (23:50 +0200)]
libmultipath: refuse reloading an existing map with different WWID

If a map with given name (alias) already exists with a different WWID,
reloading it with a new WWID is harmful. The existing paths would first
be replaced by others with the new WWID. The WWIDs of the new paths wouldn't
match the map WWID; thus the next time the map is disassembled, the (correct)
path WWIDs would be overwritten by the different map WWID (with subsequent
patches from this series, they'd be orphaned instead, which is better but
still not ideal). When the map is reloaded later, paths with diffent WWIDs
may be mixed, causing data corruption.

Refuse reloading a map under such circumstances.

Note: this patch doesn't change multipath-tools behavior in the case
where valid map aliases are supposed to be swapped (typically if the bindings
file differs between initrd and root FS). In this case, select_action()
selects ACT_RENAME, which this patch doesn't affect. The user-visible behavior
in this case remains as-is: the map renames fail, and the aliases remain
unchanged, thus not matching the current bindings file, but art least
internally consistent.

To fully fix this use case, coalesce_paths() must cease setting up maps one
by one. Instead, we'd need to build up a full set of maps-to-be-set-up,
and create them in a single batch, figuring out a "rename strategy" beforehand
to avoid clashes between existing and to-be-created maps (for example,
a name swap A<->B would need to be carried out as B->tmp, A->B, tmp->A).
Implementing that is out of the scope of this patch series.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
8 months agolibmultipath: free_multipath(): remove mpp references
Martin Wilck [Wed, 3 Jun 2020 21:29:08 +0000 (23:29 +0200)]
libmultipath: free_multipath(): remove mpp references

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 Marzinski <bmarzins@redhat.com>