* [PATCH v2 00/14] multipath marginal pathgroups
@ 2019-08-23 17:48 Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 01/14] libmultipath: make vector_foreach_slot_backwards work as expected Benjamin Marzinski
` (14 more replies)
0 siblings, 15 replies; 16+ messages in thread
From: Benjamin Marzinski @ 2019-08-23 17:48 UTC (permalink / raw)
To: Christophe Varoqui
Cc: device-mapper development, Muneendra Kumar, Martin Wilck
The first patch in this series is simply a resend of my previous patch
to fix vector_foreach_slot_backwards(). The rest of the patches are
related to adding support for an alternative way to deal with marginal
paths. Instead of delaying their reinstatement, which can cause problems
if multipath isn't set to queue IO when there are no usable paths, this
patchset adds a marginal_pathgroups option. If this is set, marginal
paths will be reinstated, but added to seperate marginal pathgroups.
They will remain there until the time when they would normally be
reinstated, at which point they will be returned to their regular
pathgroups. Marginal pathgroups will have a priority lower than all
regular pathgroups. This has the advantage of continuing to track
marginal paths, even if all the other paths go down temporarily, so that
multipath can switch back to the normal paths as soon as they come back
up. This code also allows users to manually move paths between marginal
and regular pathgroups. This is especially important for Broadcom's
Fiber Channel Transport Daemon, since it doesn't use the multipathd
marginal path detectors, and thus will not automatically reinstate
marginal paths when all other paths have failed.
https://www.mail-archive.com/dm-devel@redhat.com/msg12956.html
Changes in v2:
- patch 04/14 is a combination of the old 04/16, 06/16, and 07/16
patches, based on Martin's suggestion. Since all the code is the same
as the previous patches, I have kept Martin's Reviewed-by
- old patch 09/16 has been moved up to 05/14 based on Martin's
suggestion. There are no code changes
- patch 09/14 (old patch 11/16) moves path_group_prio_update()
earlier in select_path_group() to make sure the pgp->marginal check
uses a current value.
- patch 12/14 (old patch 14/16) changes how delay_checks is selected,
to ignore the delay_checks values if any san_path_err options are
set, based on Martin's suggestions. It also changes the man page
wording.
Benjamin Marzinski (14):
libmultipath: make vector_foreach_slot_backwards work as expected
libmultipath: add marginal paths and groups infrastructure
tests: add path grouping policy unit tests.
libmultipath: add wrapper function around pgpolicyfn
tests: update pgpolicy tests to work with group_paths()
libmultipath: fix double free in pgpolicyfn error paths
libmultipath: consolidate group_by_* functions
libmultipath: make pgpolicyfn take a paths vector
libmultipath: make group_paths handle marginal paths
tests: add tests for grouping marginal paths.
libmultipath: add marginal_pathgroups config option
libmutipath: deprecate delay_*_checks
multipathd: use marginal_pathgroups
multipath: update man pages
libmultipath/config.h | 1 +
libmultipath/configure.c | 23 +-
libmultipath/dict.c | 3 +
libmultipath/pgpolicies.c | 346 +++++-------
libmultipath/pgpolicies.h | 12 +-
libmultipath/print.c | 18 +
libmultipath/print.h | 6 +-
libmultipath/propsel.c | 89 +++-
libmultipath/propsel.h | 3 +-
libmultipath/structs.c | 16 +-
libmultipath/structs.h | 15 +-
libmultipath/switchgroup.c | 15 +-
libmultipath/vector.h | 2 +-
multipath/multipath.conf.5 | 74 ++-
multipathd/cli.c | 5 +
multipathd/cli.h | 4 +
multipathd/cli_handlers.c | 91 ++++
multipathd/cli_handlers.h | 3 +
multipathd/main.c | 90 ++--
multipathd/multipathd.8 | 19 +
tests/Makefile | 2 +-
tests/pgpolicy.c | 1036 ++++++++++++++++++++++++++++++++++++
22 files changed, 1523 insertions(+), 350 deletions(-)
create mode 100644 tests/pgpolicy.c
--
2.17.2
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 01/14] libmultipath: make vector_foreach_slot_backwards work as expected
2019-08-23 17:48 [PATCH v2 00/14] multipath marginal pathgroups Benjamin Marzinski
@ 2019-08-23 17:48 ` Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 02/14] libmultipath: add marginal paths and groups infrastructure Benjamin Marzinski
` (13 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Benjamin Marzinski @ 2019-08-23 17:48 UTC (permalink / raw)
To: Christophe Varoqui
Cc: device-mapper development, Muneendra Kumar, Martin Wilck
All of the code that uses vector_foreach_slot_backwards() treats "i" as
the index of the entry "p", but the way it was coded, that wasn't the
case. "i" was the number of the entry counting from 1, not 0.
Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/vector.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libmultipath/vector.h b/libmultipath/vector.h
index 41d2b896..344dffd5 100644
--- a/libmultipath/vector.h
+++ b/libmultipath/vector.h
@@ -40,7 +40,7 @@ typedef struct _vector *vector;
#define vector_foreach_slot_after(v,p,i) \
for (; (v) && i < VECTOR_SIZE(v) && ((p) = (v)->slot[i]); i++)
#define vector_foreach_slot_backwards(v,p,i) \
- for (i = VECTOR_SIZE(v); i > 0 && ((p) = (v)->slot[i-1]); i--)
+ for (i = VECTOR_SIZE(v) - 1; (int)i >= 0 && ((p) = (v)->slot[i]); i--)
#define identity(x) (x)
/*
--
2.17.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 02/14] libmultipath: add marginal paths and groups infrastructure
2019-08-23 17:48 [PATCH v2 00/14] multipath marginal pathgroups Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 01/14] libmultipath: make vector_foreach_slot_backwards work as expected Benjamin Marzinski
@ 2019-08-23 17:48 ` Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 03/14] tests: add path grouping policy unit tests Benjamin Marzinski
` (12 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Benjamin Marzinski @ 2019-08-23 17:48 UTC (permalink / raw)
To: Christophe Varoqui
Cc: device-mapper development, Muneendra Kumar, Martin Wilck
This commit adds a marginal variable ot the paths and pathgroups structs.
The marginal paths variable can be set by
multipathd path <path> setmarginal
and cleared by
multipathd path <path> unsetmarginal
All of the marginal paths on a multipath device can be cleared by
multipathd map <map> unsetmarginal
Currently, the marginal variable of a pathgroup will not change. This
will be added by a future commit. The marginal state of a path or
pathgroup is printable with the %M wildcard, and is displayed in the
json output.
Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/print.c | 18 ++++++++
libmultipath/print.h | 6 ++-
libmultipath/structs.h | 2 +
multipathd/cli.c | 5 +++
multipathd/cli.h | 4 ++
multipathd/cli_handlers.c | 91 +++++++++++++++++++++++++++++++++++++++
multipathd/cli_handlers.h | 3 ++
multipathd/main.c | 3 ++
8 files changed, 130 insertions(+), 2 deletions(-)
diff --git a/libmultipath/print.c b/libmultipath/print.c
index fc40b0f0..907469ad 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -502,6 +502,14 @@ snprint_pg_state (char * buff, size_t len, const struct pathgroup * pgp)
}
}
+static int
+snprint_pg_marginal (char * buff, size_t len, const struct pathgroup * pgp)
+{
+ if (pgp->marginal)
+ return snprintf(buff, len, "marginal");
+ return snprintf(buff, len, "normal");
+}
+
static int
snprint_path_size (char * buff, size_t len, const struct path * pp)
{
@@ -672,6 +680,14 @@ snprint_path_protocol(char * buff, size_t len, const struct path * pp)
}
}
+int
+snprint_path_marginal(char * buff, size_t len, const struct path * pp)
+{
+ if (pp->marginal)
+ return snprintf(buff, len, "marginal");
+ return snprintf(buff, len, "normal");
+}
+
struct multipath_data mpd[] = {
{'n', "name", 0, snprint_name},
{'w', "uuid", 0, snprint_multipath_uuid},
@@ -713,6 +729,7 @@ struct path_data pd[] = {
{'p', "pri", 0, snprint_pri},
{'S', "size", 0, snprint_path_size},
{'z', "serial", 0, snprint_path_serial},
+ {'M', "marginal_st", 0, snprint_path_marginal},
{'m', "multipath", 0, snprint_path_mpp},
{'N', "host WWNN", 0, snprint_host_wwnn},
{'n', "target WWNN", 0, snprint_tgt_wwnn},
@@ -729,6 +746,7 @@ struct pathgroup_data pgd[] = {
{'s', "selector", 0, snprint_pg_selector},
{'p', "pri", 0, snprint_pg_pri},
{'t', "dm_st", 0, snprint_pg_state},
+ {'M', "marginal_st", 0, snprint_pg_marginal},
{0, NULL, 0 , NULL}
};
diff --git a/libmultipath/print.h b/libmultipath/print.h
index e2fb865c..7e36ec63 100644
--- a/libmultipath/print.h
+++ b/libmultipath/print.h
@@ -50,7 +50,8 @@
#define PRINT_JSON_GROUP "{\n" \
" \"selector\" : \"%s\",\n" \
" \"pri\" : %p,\n" \
- " \"dm_st\" : \"%t\","
+ " \"dm_st\" : \"%t\",\n" \
+ " \"marginal_st\" : \"%M\","
#define PRINT_JSON_GROUP_NUM " \"group\" : %d,\n"
@@ -66,7 +67,8 @@
" \"target_wwnn\" : \"%n\",\n" \
" \"host_wwpn\" : \"%R\",\n" \
" \"target_wwpn\" : \"%r\",\n" \
- " \"host_adapter\" : \"%a\""
+ " \"host_adapter\" : \"%a\",\n" \
+ " \"marginal_st\" : \"%M\""
#define MAX_LINE_LEN 80
#define MAX_LINES 64
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index 7879d763..1a3d827b 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -289,6 +289,7 @@ struct path {
int io_err_pathfail_cnt;
int io_err_pathfail_starttime;
int find_multipaths_timeout;
+ int marginal;
/* configlet pointers */
vector hwe;
struct gen_path generic_path;
@@ -403,6 +404,7 @@ struct pathgroup {
int status;
int priority;
int enabled_paths;
+ int marginal;
vector paths;
struct multipath *mpp;
struct gen_pathgroup generic_pg;
diff --git a/multipathd/cli.c b/multipathd/cli.c
index 17795b61..800c0fbe 100644
--- a/multipathd/cli.c
+++ b/multipathd/cli.c
@@ -215,6 +215,8 @@ load_keys (void)
r += add_key(keys, "unsetprkey", UNSETPRKEY, 0);
r += add_key(keys, "key", KEY, 1);
r += add_key(keys, "local", LOCAL, 0);
+ r += add_key(keys, "setmarginal", SETMARGINAL, 0);
+ r += add_key(keys, "unsetmarginal", UNSETMARGINAL, 0);
if (r) {
@@ -589,6 +591,9 @@ cli_init (void) {
add_handler(UNSETPRKEY+MAP, NULL);
add_handler(FORCEQ+DAEMON, NULL);
add_handler(RESTOREQ+DAEMON, NULL);
+ add_handler(SETMARGINAL+PATH, NULL);
+ add_handler(UNSETMARGINAL+PATH, NULL);
+ add_handler(UNSETMARGINAL+MAP, NULL);
return 0;
}
diff --git a/multipathd/cli.h b/multipathd/cli.h
index 32dcffac..fdfb9aed 100644
--- a/multipathd/cli.h
+++ b/multipathd/cli.h
@@ -45,6 +45,8 @@ enum {
__UNSETPRKEY,
__KEY,
__LOCAL,
+ __SETMARGINAL,
+ __UNSETMARGINAL,
};
#define LIST (1 << __LIST)
@@ -89,6 +91,8 @@ enum {
#define UNSETPRKEY (1ULL << __UNSETPRKEY)
#define KEY (1ULL << __KEY)
#define LOCAL (1ULL << __LOCAL)
+#define SETMARGINAL (1ULL << __SETMARGINAL)
+#define UNSETMARGINAL (1ULL << __UNSETMARGINAL)
#define INITIAL_REPLY_LEN 1200
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 4c32d953..8a899049 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -1537,3 +1537,94 @@ cli_setprkey(void * v, char ** reply, int * len, void * data)
return ret;
}
+
+int cli_set_marginal(void * v, char ** reply, int * len, void * data)
+{
+ struct vectors * vecs = (struct vectors *)data;
+ char * param = get_keyparam(v, PATH);
+ struct path * pp;
+
+ param = convert_dev(param, 1);
+ pp = find_path_by_dev(vecs->pathvec, param);
+
+ if (!pp)
+ pp = find_path_by_devt(vecs->pathvec, param);
+
+ if (!pp || !pp->mpp || !pp->mpp->alias)
+ return 1;
+
+ condlog(2, "%s: set marginal path %s (operator)",
+ pp->mpp->alias, pp->dev_t);
+ if (pp->mpp->wait_for_udev) {
+ condlog(2, "%s: device not fully created, failing set marginal",
+ pp->mpp->alias);
+ return 1;
+ }
+ pp->marginal = 1;
+
+ return update_path_groups(pp->mpp, vecs, 0);
+}
+
+int cli_unset_marginal(void * v, char ** reply, int * len, void * data)
+{
+ struct vectors * vecs = (struct vectors *)data;
+ char * param = get_keyparam(v, PATH);
+ struct path * pp;
+
+ param = convert_dev(param, 1);
+ pp = find_path_by_dev(vecs->pathvec, param);
+
+ if (!pp)
+ pp = find_path_by_devt(vecs->pathvec, param);
+
+ if (!pp || !pp->mpp || !pp->mpp->alias)
+ return 1;
+
+ condlog(2, "%s: unset marginal path %s (operator)",
+ pp->mpp->alias, pp->dev_t);
+ if (pp->mpp->wait_for_udev) {
+ condlog(2, "%s: device not fully created, "
+ "failing unset marginal", pp->mpp->alias);
+ return 1;
+ }
+ pp->marginal = 0;
+
+ return update_path_groups(pp->mpp, vecs, 0);
+}
+
+int cli_unset_all_marginal(void * v, char ** reply, int * len, void * data)
+{
+ struct vectors * vecs = (struct vectors *)data;
+ char * mapname = get_keyparam(v, MAP);
+ struct multipath *mpp;
+ struct pathgroup * pgp;
+ struct path * pp;
+ unsigned int i, j;
+ int minor;
+
+ mapname = convert_dev(mapname, 0);
+ condlog(2, "%s: unset all marginal paths (operator)",
+ mapname);
+
+ if (sscanf(mapname, "dm-%d", &minor) == 1)
+ mpp = find_mp_by_minor(vecs->mpvec, minor);
+ else
+ mpp = find_mp_by_alias(vecs->mpvec, mapname);
+
+ if (!mpp) {
+ condlog(0, "%s: invalid map name. "
+ "cannot unset marginal paths", mapname);
+ return 1;
+ }
+ if (mpp->wait_for_udev) {
+ condlog(2, "%s: device not fully created, "
+ "failing unset all marginal", mpp->alias);
+ return 1;
+ }
+
+ vector_foreach_slot (mpp->pg, pgp, i)
+ vector_foreach_slot (pgp->paths, pp, j)
+ pp->marginal = 0;
+
+ return update_path_groups(mpp, vecs, 0);
+}
diff --git a/multipathd/cli_handlers.h b/multipathd/cli_handlers.h
index edbdf063..0f451064 100644
--- a/multipathd/cli_handlers.h
+++ b/multipathd/cli_handlers.h
@@ -49,3 +49,6 @@ int cli_unsetprstatus(void * v, char ** reply, int * len, void * data);
int cli_getprkey(void * v, char ** reply, int * len, void * data);
int cli_setprkey(void * v, char ** reply, int * len, void * data);
int cli_unsetprkey(void * v, char ** reply, int * len, void * data);
+int cli_set_marginal(void * v, char ** reply, int * len, void * data);
+int cli_unset_marginal(void * v, char ** reply, int * len, void * data);
+int cli_unset_all_marginal(void * v, char ** reply, int * len, void * data);
diff --git a/multipathd/main.c b/multipathd/main.c
index 7a5cd115..7db15736 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1609,6 +1609,9 @@ uxlsnrloop (void * ap)
set_handler_callback(GETPRKEY+MAP, cli_getprkey);
set_handler_callback(SETPRKEY+MAP+KEY, cli_setprkey);
set_handler_callback(UNSETPRKEY+MAP, cli_unsetprkey);
+ set_handler_callback(SETMARGINAL+PATH, cli_set_marginal);
+ set_handler_callback(UNSETMARGINAL+PATH, cli_unset_marginal);
+ set_handler_callback(UNSETMARGINAL+MAP, cli_unset_all_marginal);
umask(077);
uxsock_listen(&uxsock_trigger, ux_sock, ap);
--
2.17.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 03/14] tests: add path grouping policy unit tests.
2019-08-23 17:48 [PATCH v2 00/14] multipath marginal pathgroups Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 01/14] libmultipath: make vector_foreach_slot_backwards work as expected Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 02/14] libmultipath: add marginal paths and groups infrastructure Benjamin Marzinski
@ 2019-08-23 17:48 ` Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 04/14] libmultipath: add wrapper function around pgpolicyfn Benjamin Marzinski
` (11 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Benjamin Marzinski @ 2019-08-23 17:48 UTC (permalink / raw)
To: Christophe Varoqui
Cc: device-mapper development, Muneendra Kumar, Martin Wilck
In preparation for changing the path grouping code, add some unit tests
to verify that it works correctly. The only test that currently fails
(and so it being skipped) is using MULTIBUS when mp->paths is empty. All
the other path grouping policies free mp->paths, even if it is empty.
one_group() should as well. This will be fixed when the path grouping
code is updated.
Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
tests/Makefile | 2 +-
tests/pgpolicy.c | 708 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 709 insertions(+), 1 deletion(-)
create mode 100644 tests/pgpolicy.c
diff --git a/tests/Makefile b/tests/Makefile
index bf159b2d..a5cdf390 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -3,7 +3,7 @@ include ../Makefile.inc
CFLAGS += $(BIN_CFLAGS) -I$(multipathdir) -I$(mpathcmddir)
LIBDEPS += -L$(multipathdir) -lmultipath -lcmocka
-TESTS := uevent parser util dmevents hwtable blacklist unaligned vpd
+TESTS := uevent parser util dmevents hwtable blacklist unaligned vpd pgpolicy
.SILENT: $(TESTS:%=%.o)
.PRECIOUS: $(TESTS:%=%-test)
diff --git a/tests/pgpolicy.c b/tests/pgpolicy.c
new file mode 100644
index 00000000..fbb8589e
--- /dev/null
+++ b/tests/pgpolicy.c
@@ -0,0 +1,708 @@
+/*
+ * Copyright (c) 2018 Benjamin Marzinski, Redhat
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <https://www.gnu.org/licenses/>.
+ *
+ */
+
+#include <stdint.h>
+#include <stdbool.h>
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <stdlib.h>
+#include <cmocka.h>
+
+#include "globals.c"
+#include "pgpolicies.h"
+
+struct multipath mp8, mp4, mp1, mp0, mp_null;
+struct path p8[8], p4[4], p1[1];
+
+
+static void set_priority(struct path *pp, int *prio, int size)
+{
+ int i;
+
+ for (i = 0; i < size; i++) {
+ pp[i].priority = prio[i];
+ }
+}
+
+static void set_tgt_node_name(struct path *pp, char **tgt_node_name, int size)
+{
+ int i;
+
+ for (i = 0; i < size; i++) {
+ strcpy(pp[i].tgt_node_name, tgt_node_name[i]);
+ }
+}
+
+static void set_serial(struct path *pp, char **serial, int size)
+{
+ int i;
+
+ for (i = 0; i < size; i++) {
+ strcpy(pp[i].serial, serial[i]);
+ }
+}
+
+static int setup(void **state)
+{
+ int i;
+
+ for (i = 0; i < 8; i++) {
+ sprintf(p8[i].dev, "p8_%d", i);
+ sprintf(p8[i].dev_t, "8:%d", i);
+ p8[i].state = PATH_UP;
+ }
+ for (i = 0; i < 4; i++) {
+ sprintf(p4[i].dev, "p4_%d", i);
+ sprintf(p4[i].dev_t, "4:%d", i);
+ p4[i].state = PATH_UP;
+ }
+ sprintf(p1[0].dev, "p1_0");
+ sprintf(p1[0].dev_t, "4:0");
+ p1[0].state = PATH_UP;
+ return 0;
+}
+
+static int setupX(struct multipath *mp, struct path *pp, int size)
+{
+ int i;
+ int prio[8] = {10, 10, 10, 10, 10, 10, 10, 10};
+
+ mp->paths = vector_alloc();
+ if (!mp->paths)
+ return -1;
+ for (i = 0; i < size; i++) {
+ if (!vector_alloc_slot(mp->paths))
+ return -1;
+ vector_set_slot(mp->paths, &pp[i]);
+ }
+ set_priority(pp, prio, size);
+ return 0;
+}
+
+static int setup8(void **state)
+{
+ return setupX(&mp8, p8, 8);
+}
+
+static int setup4(void **state)
+{
+ return setupX(&mp4, p4, 4);
+}
+
+static int setup1(void **state)
+{
+ return setupX(&mp1, p1, 1);
+}
+
+static int setup0(void **state)
+{
+ return setupX(&mp0, NULL, 0);
+}
+
+static int setup_null(void **state)
+{
+ return 0;
+}
+
+static int teardownX(struct multipath *mp)
+{
+ free_pgvec(mp->pg, KEEP_PATHS);
+ mp->pg = NULL;
+ return 0;
+}
+
+static int teardown8(void **state)
+{
+ return teardownX(&mp8);
+}
+
+static int teardown4(void **state)
+{
+ return teardownX(&mp4);
+}
+
+static int teardown1(void **state)
+{
+ return teardownX(&mp1);
+}
+
+static int teardown0(void **state)
+{
+ return teardownX(&mp0);
+}
+
+static int teardown_null(void **state)
+{
+ return teardownX(&mp_null);
+}
+
+static void
+verify_pathgroups(struct multipath *mp, struct path *pp, int **groups,
+ int *group_size, int size)
+{
+ int i, j;
+ struct pathgroup *pgp;
+
+ assert_null(mp->paths);
+ assert_non_null(mp->pg);
+ assert_int_equal(VECTOR_SIZE(mp->pg), size);
+ for (i = 0; i < size; i++) {
+ pgp = VECTOR_SLOT(mp->pg, i);
+ assert_non_null(pgp);
+ assert_non_null(pgp->paths);
+ assert_int_equal(VECTOR_SIZE(pgp->paths), group_size[i]);
+ for (j = 0; j < group_size[i]; j++) {
+ int path_nr = groups[i][j];
+ struct path *pgp_path = VECTOR_SLOT(pgp->paths, j);
+ struct path *pp_path = &pp[path_nr];
+ /* Test names instead of pointers to get a more
+ * useful error message */
+ assert_string_equal(pgp_path->dev, pp_path->dev);
+ /* This test is just a backkup in case the
+ * something wenth wrong naming the paths */
+ assert_ptr_equal(pgp_path, pp_path);
+ }
+ }
+}
+
+static void test_one_group8(void **state)
+{
+ int paths[] = {0,1,2,3,4,5,6,7};
+ int *groups[] = {paths};
+ int group_size[] = {8};
+
+ assert_int_equal(one_group(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, 1);
+}
+
+static void test_one_group4(void **state)
+{
+ int paths[] = {0,1,2,3};
+ int *groups[] = {paths};
+ int group_size[] = {4};
+
+ assert_int_equal(one_group(&mp4), 0);
+ verify_pathgroups(&mp4, p4, groups, group_size, 1);
+}
+
+static void test_one_group1(void **state)
+{
+ int paths[] = {0};
+ int *groups[] = {paths};
+ int group_size[] = {1};
+
+ assert_int_equal(one_group(&mp1), 0);
+ verify_pathgroups(&mp1, p1, groups, group_size, 1);
+}
+
+static void test_one_group0(void **state)
+{
+ assert_int_equal(one_group(&mp0), 0);
+ skip(); /* BROKEN */
+ verify_pathgroups(&mp0, NULL, NULL, NULL, 0);
+}
+
+static void test_one_group_null(void **state)
+{
+ assert_int_equal(one_group(&mp_null), 0);
+ verify_pathgroups(&mp_null, NULL, NULL, NULL, 0);
+}
+
+static void test_one_path_per_group_same8(void **state)
+{
+ int paths[] = {0,1,2,3,4,5,6,7};
+ int *groups[] = {&paths[0], &paths[1], &paths[2], &paths[3],
+ &paths[4], &paths[5], &paths[6], &paths[7]};
+ int group_size[] = {1,1,1,1,1,1,1,1};
+
+ assert_int_equal(one_path_per_group(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, 8);
+}
+
+static void test_one_path_per_group_increasing8(void **state)
+{
+ int prio[] = {1,2,3,4,5,6,7,8};
+ int paths[] = {7,6,5,4,3,2,1,0};
+ int *groups[] = {&paths[0], &paths[1], &paths[2], &paths[3],
+ &paths[4], &paths[5], &paths[6], &paths[7]};
+ int group_size[] = {1,1,1,1,1,1,1,1};
+
+ set_priority(p8, prio, 8);
+ assert_int_equal(one_path_per_group(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, 8);
+}
+
+static void test_one_path_per_group_decreasing8(void **state)
+{
+ int prio[] = {8,7,6,5,4,3,2,1};
+ int paths[] = {0,1,2,3,4,5,6,7};
+ int *groups[] = {&paths[0], &paths[1], &paths[2], &paths[3],
+ &paths[4], &paths[5], &paths[6], &paths[7]};
+ int group_size[] = {1,1,1,1,1,1,1,1};
+
+ set_priority(p8, prio, 8);
+ assert_int_equal(one_path_per_group(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, 8);
+}
+
+static void test_one_path_per_group_mixed8(void **state)
+{
+ int prio[] = {7,1,3,3,5,2,8,2};
+ int paths[] = {6,0,4,2,3,5,7,1};
+ int *groups[] = {&paths[0], &paths[1], &paths[2], &paths[3],
+ &paths[4], &paths[5], &paths[6], &paths[7]};
+ int group_size[] = {1,1,1,1,1,1,1,1};
+
+ set_priority(p8, prio, 8);
+ assert_int_equal(one_path_per_group(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, 8);
+}
+
+static void test_one_path_per_group4(void **state)
+{
+ int paths[] = {0,1,2,3};
+ int *groups[] = {&paths[0], &paths[1], &paths[2], &paths[3]};
+ int group_size[] = {1,1,1,1};
+
+ assert_int_equal(one_path_per_group(&mp4), 0);
+ verify_pathgroups(&mp4, p4, groups, group_size, 4);
+}
+
+static void test_one_path_per_group1(void **state)
+{
+ int paths[] = {0};
+ int *groups[] = {paths};
+ int group_size[] = {1};
+
+ assert_int_equal(one_path_per_group(&mp1), 0);
+ verify_pathgroups(&mp1, p1, groups, group_size, 1);
+}
+
+static void test_one_path_per_group0(void **state)
+{
+ assert_int_equal(one_path_per_group(&mp0), 0);
+ verify_pathgroups(&mp0, NULL, NULL, NULL, 0);
+}
+
+static void test_one_path_per_group_null(void **state)
+{
+ assert_int_equal(one_path_per_group(&mp_null), 0);
+ verify_pathgroups(&mp_null, NULL, NULL, NULL, 0);
+}
+
+static void test_group_by_prio_same8(void **state)
+{
+ int paths[] = {0,1,2,3,4,5,6,7};
+ int *groups[] = {paths};
+ int group_size[] = {8};
+
+ assert_int_equal(group_by_prio(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, 1);
+}
+
+static void test_group_by_prio_increasing8(void **state)
+{
+ int prio[] = {1,2,3,4,5,6,7,8};
+ int paths[] = {7,6,5,4,3,2,1,0};
+ int *groups[] = {&paths[0], &paths[1], &paths[2], &paths[3],
+ &paths[4], &paths[5], &paths[6], &paths[7]};
+ int group_size[] = {1,1,1,1,1,1,1,1};
+
+ set_priority(p8, prio, 8);
+ assert_int_equal(group_by_prio(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, 8);
+}
+
+static void test_group_by_prio_decreasing8(void **state)
+{
+ int prio[] = {8,7,6,5,4,3,2,1};
+ int paths[] = {0,1,2,3,4,5,6,7};
+ int *groups[] = {&paths[0], &paths[1], &paths[2], &paths[3],
+ &paths[4], &paths[5], &paths[6], &paths[7]};
+ int group_size[] = {1,1,1,1,1,1,1,1};
+
+ set_priority(p8, prio, 8);
+ assert_int_equal(group_by_prio(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, 8);
+}
+
+static void test_group_by_prio_mixed8(void **state)
+{
+ int prio[] = {7,1,3,3,5,2,8,2};
+ int group0[] = {6};
+ int group1[] = {0};
+ int group2[] = {4};
+ int group3[] = {2,3};
+ int group4[] = {5,7};
+ int group5[] = {1};
+ int *groups[] = {group0, group1, group2, group3,
+ group4, group5};
+ int group_size[] = {1,1,1,2,2,1};
+
+ set_priority(p8, prio, 8);
+ assert_int_equal(group_by_prio(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, 6);
+}
+
+static void test_group_by_prio_2_groups8(void **state)
+{
+ int prio[] = {1,2,2,1,2,1,1,2};
+ int group0[] = {1,2,4,7};
+ int group1[] = {0,3,5,6};
+ int *groups[] = {group0, group1};
+ int group_size[] = {4,4};
+
+ set_priority(p8, prio, 8);
+ assert_int_equal(group_by_prio(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, 2);
+}
+
+static void test_group_by_prio_mixed4(void **state)
+{
+ int prio[] = {2,3,1,3};
+ int group0[] = {1,3};
+ int group1[] = {0};
+ int group2[] = {2};
+ int *groups[] = {group0, group1, group2};
+ int group_size[] = {2,1,1};
+
+ set_priority(p4, prio, 4);
+ assert_int_equal(group_by_prio(&mp4), 0);
+ verify_pathgroups(&mp4, p4, groups, group_size, 3);
+}
+
+static void test_group_by_prio_2_groups4(void **state)
+{
+ int prio[] = {2,1,1,2};
+ int group0[] = {0,3};
+ int group1[] = {1,2};
+ int *groups[] = {group0, group1};
+ int group_size[] = {2,2};
+
+ set_priority(p4, prio, 4);
+ assert_int_equal(group_by_prio(&mp4), 0);
+ verify_pathgroups(&mp4, p4, groups, group_size, 2);
+}
+
+static void test_group_by_prio1(void **state)
+{
+ int paths[] = {0};
+ int *groups[] = {paths};
+ int group_size[] = {1};
+
+ assert_int_equal(group_by_prio(&mp1), 0);
+ verify_pathgroups(&mp1, p1, groups, group_size, 1);
+}
+
+static void test_group_by_prio0(void **state)
+{
+ assert_int_equal(group_by_prio(&mp0), 0);
+ verify_pathgroups(&mp0, NULL, NULL, NULL, 0);
+}
+
+static void test_group_by_prio_null(void **state)
+{
+ assert_int_equal(group_by_prio(&mp_null), 0);
+ verify_pathgroups(&mp_null, NULL, NULL, NULL, 0);
+}
+
+static void test_group_by_node_name_same8(void **state)
+{
+ char *node_name[] = {"a","a","a","a","a","a","a","a"};
+ int paths[] = {0,1,2,3,4,5,6,7};
+ int *groups[] = {paths};
+ int group_size[] = {8};
+
+ set_tgt_node_name(p8, node_name, 8);
+ assert_int_equal(group_by_node_name(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, 1);
+}
+
+static void test_group_by_node_name_increasing8(void **state)
+{
+ char *node_name[] = {"a","b","c","d","e","f","g","h"};
+ int prio[] = {1,2,3,4,5,6,7,8};
+ int paths[] = {7,6,5,4,3,2,1,0};
+ int *groups[] = {&paths[0], &paths[1], &paths[2], &paths[3],
+ &paths[4], &paths[5], &paths[6], &paths[7]};
+ int group_size[] = {1,1,1,1,1,1,1,1};
+
+ set_priority(p8, prio, 8);
+ set_tgt_node_name(p8, node_name, 8);
+ assert_int_equal(group_by_node_name(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, 8);
+}
+
+static void test_group_by_node_name_3_groups8(void **state)
+{
+ char *node_name[] = {"a","b","a","c","b","c","c","a"};
+ int prio[] = {4,1,4,1,1,1,1,4};
+ int group0[] = {0,2,7};
+ int group1[] = {3,5,6};
+ int group2[] = {1,4};
+ int *groups[] = {group0, group1, group2};
+ int group_size[] = {3,3,2};
+
+ set_priority(p8, prio, 8);
+ set_tgt_node_name(p8, node_name, 8);
+ assert_int_equal(group_by_node_name(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, 3);
+}
+
+static void test_group_by_node_name_2_groups8(void **state)
+{
+ char *node_name[] = {"a", "a", "b", "a", "b", "b", "b", "a"};
+ int prio[] = {4,1,2,1,2,2,2,1};
+ int group0[] = {2,4,5,6};
+ int group1[] = {0,1,3,7};
+ int *groups[] = {group0, group1};
+ int group_size[] = {4,4};
+
+ set_priority(p8, prio, 8);
+ set_tgt_node_name(p8, node_name, 8);
+ assert_int_equal(group_by_node_name(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, 2);
+}
+
+static void test_group_by_node_name_3_groups4(void **state)
+{
+ char *node_name[] = {"a","b","c","a"};
+ int prio[] = {3,1,3,1};
+ int group0[] = {2};
+ int group1[] = {0,3};
+ int group2[] = {1};
+ int *groups[] = {group0, group1, group2};
+ int group_size[] = {1,2,1};
+
+ set_priority(p4, prio, 4);
+ set_tgt_node_name(p4, node_name, 4);
+ assert_int_equal(group_by_node_name(&mp4), 0);
+ verify_pathgroups(&mp4, p4, groups, group_size, 3);
+}
+
+static void test_group_by_node_name_2_groups4(void **state)
+{
+ char *node_name[] = {"a","b","b","a"};
+ int prio[] = {2,1,1,2};
+ int group0[] = {0,3};
+ int group1[] = {1,2};
+ int *groups[] = {group0, group1};
+ int group_size[] = {2,2};
+
+ set_priority(p4, prio, 4);
+ set_tgt_node_name(p4, node_name, 4);
+ assert_int_equal(group_by_node_name(&mp4), 0);
+ verify_pathgroups(&mp4, p4, groups, group_size, 2);
+}
+
+static void test_group_by_node_name1(void **state)
+{
+ char *node_name[] = {"a"};
+ int paths[] = {0};
+ int *groups[] = {paths};
+ int group_size[] = {1};
+
+ set_tgt_node_name(p1, node_name, 1);
+ assert_int_equal(group_by_node_name(&mp1), 0);
+ verify_pathgroups(&mp1, p1, groups, group_size, 1);
+}
+
+static void test_group_by_node_name0(void **state)
+{
+ assert_int_equal(group_by_node_name(&mp0), 0);
+ verify_pathgroups(&mp0, NULL, NULL, NULL, 0);
+}
+
+static void test_group_by_node_name_null(void **state)
+{
+ assert_int_equal(group_by_node_name(&mp_null), 0);
+ verify_pathgroups(&mp_null, NULL, NULL, NULL, 0);
+}
+
+static void test_group_by_serial_same8(void **state)
+{
+ char *serial[] = {"1","1","1","1","1","1","1","1"};
+ int paths[] = {0,1,2,3,4,5,6,7};
+ int *groups[] = {paths};
+ int group_size[] = {8};
+
+ set_serial(p8, serial, 8);
+ assert_int_equal(group_by_serial(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, 1);
+}
+
+static void test_group_by_serial_increasing8(void **state)
+{
+ char *serial[] = {"1","2","3","4","5","6","7","8"};
+ int prio[] = {1,2,3,4,5,6,7,8};
+ int paths[] = {7,6,5,4,3,2,1,0};
+ int *groups[] = {&paths[0], &paths[1], &paths[2], &paths[3],
+ &paths[4], &paths[5], &paths[6], &paths[7]};
+ int group_size[] = {1,1,1,1,1,1,1,1};
+
+ set_priority(p8, prio, 8);
+ set_serial(p8, serial, 8);
+ assert_int_equal(group_by_serial(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, 8);
+}
+
+static void test_group_by_serial_3_groups8(void **state)
+{
+ char *serial[] = {"1","2","1","3","2","3","2","1"};
+ int prio[] = {4,1,4,3,1,3,1,4};
+ int group0[] = {0,2,7};
+ int group1[] = {3,5};
+ int group2[] = {1,4,6};
+ int *groups[] = {group0, group1, group2};
+ int group_size[] = {3,2,3};
+
+ set_priority(p8, prio, 8);
+ set_serial(p8, serial, 8);
+ assert_int_equal(group_by_serial(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, 3);
+}
+
+static void test_group_by_serial_2_groups8(void **state)
+{
+ char *serial[] = {"1", "2", "1", "1", "2", "2", "1", "2"};
+ int prio[] = {3,2,2,1,2,2,1,2};
+ int group0[] = {1,4,5,7};
+ int group1[] = {0,2,3,6};
+ int *groups[] = {group0, group1};
+ int group_size[] = {4,4};
+
+ set_priority(p8, prio, 8);
+ set_serial(p8, serial, 8);
+ assert_int_equal(group_by_serial(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, 2);
+}
+
+static void test_group_by_serial_3_groups4(void **state)
+{
+ char *serial[] = {"1","2","3","2"};
+ int prio[] = {3,1,3,1};
+ int group0[] = {0};
+ int group1[] = {2};
+ int group2[] = {1,3};
+ int *groups[] = {group0, group1, group2};
+ int group_size[] = {1,1,2};
+
+ set_priority(p4, prio, 4);
+ set_serial(p4, serial, 4);
+ assert_int_equal(group_by_serial(&mp4), 0);
+ verify_pathgroups(&mp4, p4, groups, group_size, 3);
+}
+
+static void test_group_by_serial_2_groups4(void **state)
+{
+ char *serial[] = {"1","2","1","2"};
+ int prio[] = {3,1,3,1};
+ int group0[] = {0,2};
+ int group1[] = {1,3};
+ int *groups[] = {group0, group1};
+ int group_size[] = {2,2};
+
+ set_priority(p4, prio, 4);
+ set_serial(p4, serial, 4);
+ assert_int_equal(group_by_serial(&mp4), 0);
+ verify_pathgroups(&mp4, p4, groups, group_size, 2);
+}
+
+static void test_group_by_serial1(void **state)
+{
+ char *serial[1] = {"1"};
+ int paths[1] = {0};
+ int *groups[1] = {paths};
+ int group_size[1] = {1};
+
+ set_serial(p1, serial, 1);
+ assert_int_equal(group_by_serial(&mp1), 0);
+ verify_pathgroups(&mp1, p1, groups, group_size, 1);
+}
+
+static void test_group_by_serial0(void **state)
+{
+ assert_int_equal(group_by_serial(&mp0), 0);
+ verify_pathgroups(&mp0, NULL, NULL, NULL, 0);
+}
+
+static void test_group_by_serial_null(void **state)
+{
+ assert_int_equal(group_by_serial(&mp_null), 0);
+ verify_pathgroups(&mp_null, NULL, NULL, NULL, 0);
+}
+
+#define setup_test(name, nr) \
+cmocka_unit_test_setup_teardown(name ## nr, setup ## nr, teardown ## nr)
+
+int test_pgpolicies(void)
+{
+ const struct CMUnitTest tests[] = {
+ setup_test(test_one_group, 8),
+ setup_test(test_one_group, 4),
+ setup_test(test_one_group, 1),
+ setup_test(test_one_group, 0),
+ setup_test(test_one_group, _null),
+ setup_test(test_one_path_per_group_same, 8),
+ setup_test(test_one_path_per_group_increasing, 8),
+ setup_test(test_one_path_per_group_decreasing, 8),
+ setup_test(test_one_path_per_group_mixed, 8),
+ setup_test(test_one_path_per_group, 4),
+ setup_test(test_one_path_per_group, 1),
+ setup_test(test_one_path_per_group, 0),
+ setup_test(test_one_path_per_group, _null),
+ setup_test(test_group_by_prio_same, 8),
+ setup_test(test_group_by_prio_increasing, 8),
+ setup_test(test_group_by_prio_decreasing, 8),
+ setup_test(test_group_by_prio_mixed, 8),
+ setup_test(test_group_by_prio_2_groups, 8),
+ setup_test(test_group_by_prio_mixed, 4),
+ setup_test(test_group_by_prio_2_groups, 4),
+ setup_test(test_group_by_prio, 1),
+ setup_test(test_group_by_prio, 0),
+ setup_test(test_group_by_prio, _null),
+ setup_test(test_group_by_node_name_same, 8),
+ setup_test(test_group_by_node_name_increasing, 8),
+ setup_test(test_group_by_node_name_3_groups, 8),
+ setup_test(test_group_by_node_name_2_groups, 8),
+ setup_test(test_group_by_node_name_3_groups, 4),
+ setup_test(test_group_by_node_name_2_groups, 4),
+ setup_test(test_group_by_node_name, 1),
+ setup_test(test_group_by_node_name, 0),
+ setup_test(test_group_by_node_name, _null),
+ setup_test(test_group_by_serial_same, 8),
+ setup_test(test_group_by_serial_increasing, 8),
+ setup_test(test_group_by_serial_3_groups, 8),
+ setup_test(test_group_by_serial_2_groups, 8),
+ setup_test(test_group_by_serial_3_groups, 4),
+ setup_test(test_group_by_serial_2_groups, 4),
+ setup_test(test_group_by_serial, 1),
+ setup_test(test_group_by_serial, 0),
+ setup_test(test_group_by_serial, _null),
+ };
+ return cmocka_run_group_tests(tests, setup, NULL);
+}
+
+int main(void)
+{
+ int ret = 0;
+
+ ret += test_pgpolicies();
+ return ret;
+}
--
2.17.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 04/14] libmultipath: add wrapper function around pgpolicyfn
2019-08-23 17:48 [PATCH v2 00/14] multipath marginal pathgroups Benjamin Marzinski
` (2 preceding siblings ...)
2019-08-23 17:48 ` [PATCH v2 03/14] tests: add path grouping policy unit tests Benjamin Marzinski
@ 2019-08-23 17:48 ` Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 05/14] tests: update pgpolicy tests to work with group_paths() Benjamin Marzinski
` (10 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Benjamin Marzinski @ 2019-08-23 17:48 UTC (permalink / raw)
To: Christophe Varoqui
Cc: device-mapper development, Muneendra Kumar, Martin Wilck
group_paths() is a wrapper around the pgpolicy functions, that pulls out
the common code from the beginning and the end. For this to work,
one_group() needs to change how it sets up the pathgroups vector to work
like the other pgpolicy functions. This does means that the pathgroups
in group_by_prio are now needlessly sorted afterwards. That will be
dealt with in a later patch. Also, since store_pathgroup() is only
called by add_pathgroup(), it doesn't need to exist as a seperate
function.
Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/configure.c | 2 +-
libmultipath/pgpolicies.c | 83 ++++++++++++++-------------------------
libmultipath/pgpolicies.h | 2 +-
libmultipath/structs.c | 16 ++------
libmultipath/structs.h | 1 -
5 files changed, 35 insertions(+), 69 deletions(-)
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index b09ef529..3c309d64 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -387,7 +387,7 @@ int setup_map(struct multipath *mpp, char *params, int params_size,
vector_free(mpp->pg);
mpp->pg = NULL;
}
- if (mpp->pgpolicyfn && mpp->pgpolicyfn(mpp))
+ if (group_paths(mpp))
return 1;
/*
diff --git a/libmultipath/pgpolicies.c b/libmultipath/pgpolicies.c
index 660768a4..1b59485c 100644
--- a/libmultipath/pgpolicies.c
+++ b/libmultipath/pgpolicies.c
@@ -84,6 +84,26 @@ sort_pathgroups (struct multipath *mp) {
}
+int group_paths(struct multipath *mp)
+{
+ if (!mp->pg)
+ mp->pg = vector_alloc();
+ if (!mp->pg)
+ return 1;
+
+ if (VECTOR_SIZE(mp->paths) > 0 &&
+ (!mp->pgpolicyfn || mp->pgpolicyfn(mp))) {
+ vector_free(mp->pg);
+ mp->pg = NULL;
+ return 1;
+ }
+
+ sort_pathgroups(mp);
+ vector_free(mp->paths);
+ mp->paths = NULL;
+ return 0;
+}
+
/*
* One path group per unique tgt_node_name present in the path vector
*/
@@ -95,12 +115,6 @@ int group_by_node_name(struct multipath * mp)
struct pathgroup * pgp;
struct path * pp2;
- if (!mp->pg)
- mp->pg = vector_alloc();
-
- if (!mp->pg)
- return 1;
-
/* init the bitmap */
bitmap = (int *)MALLOC(VECTOR_SIZE(mp->paths) * sizeof (int));
@@ -146,9 +160,6 @@ int group_by_node_name(struct multipath * mp)
}
}
FREE(bitmap);
- sort_pathgroups(mp);
- free_pathvec(mp->paths, KEEP_PATHS);
- mp->paths = NULL;
return 0;
out2:
free_pathgroup(pgp, KEEP_PATHS);
@@ -171,12 +182,6 @@ int group_by_serial(struct multipath * mp)
struct pathgroup * pgp;
struct path * pp2;
- if (!mp->pg)
- mp->pg = vector_alloc();
-
- if (!mp->pg)
- return 1;
-
/* init the bitmap */
bitmap = (int *)MALLOC(VECTOR_SIZE(mp->paths) * sizeof (int));
@@ -221,9 +226,6 @@ int group_by_serial(struct multipath * mp)
}
}
FREE(bitmap);
- sort_pathgroups(mp);
- free_pathvec(mp->paths, KEEP_PATHS);
- mp->paths = NULL;
return 0;
out2:
free_pathgroup(pgp, KEEP_PATHS);
@@ -241,12 +243,6 @@ int one_path_per_group(struct multipath *mp)
struct path * pp;
struct pathgroup * pgp;
- if (!mp->pg)
- mp->pg = vector_alloc();
-
- if (!mp->pg)
- return 1;
-
for (i = 0; i < VECTOR_SIZE(mp->paths); i++) {
pp = VECTOR_SLOT(mp->paths, i);
pgp = alloc_pathgroup();
@@ -260,9 +256,6 @@ int one_path_per_group(struct multipath *mp)
if (store_path(pgp->paths, pp))
goto out1;
}
- sort_pathgroups(mp);
- free_pathvec(mp->paths, KEEP_PATHS);
- mp->paths = NULL;
return 0;
out1:
free_pathgroup(pgp, KEEP_PATHS);
@@ -274,32 +267,24 @@ out:
int one_group(struct multipath *mp) /* aka multibus */
{
+ int i;
+ struct path * pp;
struct pathgroup * pgp;
- if (VECTOR_SIZE(mp->paths) < 0)
- return 0;
+ pgp = alloc_pathgroup();
- if (!mp->pg)
- mp->pg = vector_alloc();
+ if (!pgp)
+ goto out;
- if (!mp->pg)
- return 1;
+ if (add_pathgroup(mp, pgp))
+ goto out1;
- if (VECTOR_SIZE(mp->paths) > 0) {
- pgp = alloc_pathgroup();
+ for (i = 0; i < VECTOR_SIZE(mp->paths); i++) {
+ pp = VECTOR_SLOT(mp->paths, i);
- if (!pgp)
+ if (store_path(pgp->paths, pp))
goto out;
-
- vector_free(pgp->paths);
-
- if (add_pathgroup(mp, pgp))
- goto out1;
-
- pgp->paths = mp->paths;
- mp->paths = NULL;
}
-
return 0;
out1:
free_pathgroup(pgp, KEEP_PATHS);
@@ -317,12 +302,6 @@ int group_by_prio(struct multipath *mp)
struct pathgroup * pgp;
vector pathvec = NULL;
- if (!mp->pg)
- mp->pg = vector_alloc();
-
- if (!mp->pg)
- return 1;
-
pathvec = vector_alloc();
if (!pathvec)
goto out;
@@ -387,8 +366,6 @@ int group_by_prio(struct multipath *mp)
}
}
free_pathvec(pathvec, KEEP_PATHS);
- free_pathvec(mp->paths, KEEP_PATHS);
- mp->paths = NULL;
return 0;
out2:
free_pathgroup(pgp, KEEP_PATHS);
diff --git a/libmultipath/pgpolicies.h b/libmultipath/pgpolicies.h
index c0eaa7f4..11834011 100644
--- a/libmultipath/pgpolicies.h
+++ b/libmultipath/pgpolicies.h
@@ -21,7 +21,7 @@ enum iopolicies {
int get_pgpolicy_id(char *);
int get_pgpolicy_name (char *, int, int);
-
+int group_paths(struct multipath *);
/*
* policies
*/
diff --git a/libmultipath/structs.c b/libmultipath/structs.c
index fee899bd..bf7fdd73 100644
--- a/libmultipath/structs.c
+++ b/libmultipath/structs.c
@@ -318,23 +318,13 @@ store_path (vector pathvec, struct path * pp)
return 0;
}
-int
-store_pathgroup (vector pgvec, struct pathgroup * pgp)
+int add_pathgroup(struct multipath *mpp, struct pathgroup *pgp)
{
- if (!vector_alloc_slot(pgvec))
+ if (!vector_alloc_slot(mpp->pg))
return 1;
- vector_set_slot(pgvec, pgp);
-
- return 0;
-}
-
-int add_pathgroup(struct multipath *mpp, struct pathgroup *pgp)
-{
- int ret = store_pathgroup(mpp->pg, pgp);
+ vector_set_slot(mpp->pg, pgp);
- if (ret)
- return ret;
pgp->mpp = mpp;
return 0;
}
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index 1a3d827b..893074eb 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -445,7 +445,6 @@ int store_adaptergroup(vector adapters, struct adapter_group *agp);
int store_hostgroup(vector hostgroupvec, struct host_group *hgp);
int store_path (vector pathvec, struct path * pp);
-int store_pathgroup (vector pgvec, struct pathgroup * pgp);
int add_pathgroup(struct multipath*, struct pathgroup *);
struct multipath * find_mp_by_alias (const struct _vector *mp, const char *alias);
--
2.17.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 05/14] tests: update pgpolicy tests to work with group_paths()
2019-08-23 17:48 [PATCH v2 00/14] multipath marginal pathgroups Benjamin Marzinski
` (3 preceding siblings ...)
2019-08-23 17:48 ` [PATCH v2 04/14] libmultipath: add wrapper function around pgpolicyfn Benjamin Marzinski
@ 2019-08-23 17:48 ` Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 06/14] libmultipath: fix double free in pgpolicyfn error paths Benjamin Marzinski
` (9 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Benjamin Marzinski @ 2019-08-23 17:48 UTC (permalink / raw)
To: Christophe Varoqui
Cc: device-mapper development, Muneendra Kumar, Martin Wilck
The pgpolicy unit tests now work again, using group_paths().
test_one_group0(), which was skipped with the old path grouping code
because it failed, is now working correctly.
Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
tests/pgpolicy.c | 125 +++++++++++++++++++++++++++++++----------------
1 file changed, 83 insertions(+), 42 deletions(-)
diff --git a/tests/pgpolicy.c b/tests/pgpolicy.c
index fbb8589e..04a77c4c 100644
--- a/tests/pgpolicy.c
+++ b/tests/pgpolicy.c
@@ -92,6 +92,7 @@ static int setupX(struct multipath *mp, struct path *pp, int size)
vector_set_slot(mp->paths, &pp[i]);
}
set_priority(pp, prio, size);
+ mp->pgpolicyfn = NULL;
return 0;
}
@@ -187,7 +188,8 @@ static void test_one_group8(void **state)
int *groups[] = {paths};
int group_size[] = {8};
- assert_int_equal(one_group(&mp8), 0);
+ mp8.pgpolicyfn = one_group;
+ assert_int_equal(group_paths(&mp8), 0);
verify_pathgroups(&mp8, p8, groups, group_size, 1);
}
@@ -197,7 +199,8 @@ static void test_one_group4(void **state)
int *groups[] = {paths};
int group_size[] = {4};
- assert_int_equal(one_group(&mp4), 0);
+ mp4.pgpolicyfn = one_group;
+ assert_int_equal(group_paths(&mp4), 0);
verify_pathgroups(&mp4, p4, groups, group_size, 1);
}
@@ -207,20 +210,22 @@ static void test_one_group1(void **state)
int *groups[] = {paths};
int group_size[] = {1};
- assert_int_equal(one_group(&mp1), 0);
+ mp1.pgpolicyfn = one_group;
+ assert_int_equal(group_paths(&mp1), 0);
verify_pathgroups(&mp1, p1, groups, group_size, 1);
}
static void test_one_group0(void **state)
{
- assert_int_equal(one_group(&mp0), 0);
- skip(); /* BROKEN */
+ mp0.pgpolicyfn = one_group;
+ assert_int_equal(group_paths(&mp0), 0);
verify_pathgroups(&mp0, NULL, NULL, NULL, 0);
}
static void test_one_group_null(void **state)
{
- assert_int_equal(one_group(&mp_null), 0);
+ mp_null.pgpolicyfn = one_group;
+ assert_int_equal(group_paths(&mp_null), 0);
verify_pathgroups(&mp_null, NULL, NULL, NULL, 0);
}
@@ -231,7 +236,8 @@ static void test_one_path_per_group_same8(void **state)
&paths[4], &paths[5], &paths[6], &paths[7]};
int group_size[] = {1,1,1,1,1,1,1,1};
- assert_int_equal(one_path_per_group(&mp8), 0);
+ mp8.pgpolicyfn = one_path_per_group;
+ assert_int_equal(group_paths(&mp8), 0);
verify_pathgroups(&mp8, p8, groups, group_size, 8);
}
@@ -244,7 +250,8 @@ static void test_one_path_per_group_increasing8(void **state)
int group_size[] = {1,1,1,1,1,1,1,1};
set_priority(p8, prio, 8);
- assert_int_equal(one_path_per_group(&mp8), 0);
+ mp8.pgpolicyfn = one_path_per_group;
+ assert_int_equal(group_paths(&mp8), 0);
verify_pathgroups(&mp8, p8, groups, group_size, 8);
}
@@ -257,7 +264,8 @@ static void test_one_path_per_group_decreasing8(void **state)
int group_size[] = {1,1,1,1,1,1,1,1};
set_priority(p8, prio, 8);
- assert_int_equal(one_path_per_group(&mp8), 0);
+ mp8.pgpolicyfn = one_path_per_group;
+ assert_int_equal(group_paths(&mp8), 0);
verify_pathgroups(&mp8, p8, groups, group_size, 8);
}
@@ -270,7 +278,8 @@ static void test_one_path_per_group_mixed8(void **state)
int group_size[] = {1,1,1,1,1,1,1,1};
set_priority(p8, prio, 8);
- assert_int_equal(one_path_per_group(&mp8), 0);
+ mp8.pgpolicyfn = one_path_per_group;
+ assert_int_equal(group_paths(&mp8), 0);
verify_pathgroups(&mp8, p8, groups, group_size, 8);
}
@@ -280,7 +289,8 @@ static void test_one_path_per_group4(void **state)
int *groups[] = {&paths[0], &paths[1], &paths[2], &paths[3]};
int group_size[] = {1,1,1,1};
- assert_int_equal(one_path_per_group(&mp4), 0);
+ mp4.pgpolicyfn = one_path_per_group;
+ assert_int_equal(group_paths(&mp4), 0);
verify_pathgroups(&mp4, p4, groups, group_size, 4);
}
@@ -290,19 +300,22 @@ static void test_one_path_per_group1(void **state)
int *groups[] = {paths};
int group_size[] = {1};
- assert_int_equal(one_path_per_group(&mp1), 0);
+ mp1.pgpolicyfn = one_path_per_group;
+ assert_int_equal(group_paths(&mp1), 0);
verify_pathgroups(&mp1, p1, groups, group_size, 1);
}
static void test_one_path_per_group0(void **state)
{
- assert_int_equal(one_path_per_group(&mp0), 0);
+ mp0.pgpolicyfn = one_path_per_group;
+ assert_int_equal(group_paths(&mp0), 0);
verify_pathgroups(&mp0, NULL, NULL, NULL, 0);
}
static void test_one_path_per_group_null(void **state)
{
- assert_int_equal(one_path_per_group(&mp_null), 0);
+ mp_null.pgpolicyfn = one_path_per_group;
+ assert_int_equal(group_paths(&mp_null), 0);
verify_pathgroups(&mp_null, NULL, NULL, NULL, 0);
}
@@ -312,7 +325,8 @@ static void test_group_by_prio_same8(void **state)
int *groups[] = {paths};
int group_size[] = {8};
- assert_int_equal(group_by_prio(&mp8), 0);
+ mp8.pgpolicyfn = group_by_prio;
+ assert_int_equal(group_paths(&mp8), 0);
verify_pathgroups(&mp8, p8, groups, group_size, 1);
}
@@ -325,7 +339,8 @@ static void test_group_by_prio_increasing8(void **state)
int group_size[] = {1,1,1,1,1,1,1,1};
set_priority(p8, prio, 8);
- assert_int_equal(group_by_prio(&mp8), 0);
+ mp8.pgpolicyfn = group_by_prio;
+ assert_int_equal(group_paths(&mp8), 0);
verify_pathgroups(&mp8, p8, groups, group_size, 8);
}
@@ -338,7 +353,8 @@ static void test_group_by_prio_decreasing8(void **state)
int group_size[] = {1,1,1,1,1,1,1,1};
set_priority(p8, prio, 8);
- assert_int_equal(group_by_prio(&mp8), 0);
+ mp8.pgpolicyfn = group_by_prio;
+ assert_int_equal(group_paths(&mp8), 0);
verify_pathgroups(&mp8, p8, groups, group_size, 8);
}
@@ -356,7 +372,8 @@ static void test_group_by_prio_mixed8(void **state)
int group_size[] = {1,1,1,2,2,1};
set_priority(p8, prio, 8);
- assert_int_equal(group_by_prio(&mp8), 0);
+ mp8.pgpolicyfn = group_by_prio;
+ assert_int_equal(group_paths(&mp8), 0);
verify_pathgroups(&mp8, p8, groups, group_size, 6);
}
@@ -369,7 +386,8 @@ static void test_group_by_prio_2_groups8(void **state)
int group_size[] = {4,4};
set_priority(p8, prio, 8);
- assert_int_equal(group_by_prio(&mp8), 0);
+ mp8.pgpolicyfn = group_by_prio;
+ assert_int_equal(group_paths(&mp8), 0);
verify_pathgroups(&mp8, p8, groups, group_size, 2);
}
@@ -383,7 +401,8 @@ static void test_group_by_prio_mixed4(void **state)
int group_size[] = {2,1,1};
set_priority(p4, prio, 4);
- assert_int_equal(group_by_prio(&mp4), 0);
+ mp4.pgpolicyfn = group_by_prio;
+ assert_int_equal(group_paths(&mp4), 0);
verify_pathgroups(&mp4, p4, groups, group_size, 3);
}
@@ -396,7 +415,8 @@ static void test_group_by_prio_2_groups4(void **state)
int group_size[] = {2,2};
set_priority(p4, prio, 4);
- assert_int_equal(group_by_prio(&mp4), 0);
+ mp4.pgpolicyfn = group_by_prio;
+ assert_int_equal(group_paths(&mp4), 0);
verify_pathgroups(&mp4, p4, groups, group_size, 2);
}
@@ -406,19 +426,22 @@ static void test_group_by_prio1(void **state)
int *groups[] = {paths};
int group_size[] = {1};
- assert_int_equal(group_by_prio(&mp1), 0);
+ mp1.pgpolicyfn = group_by_prio;
+ assert_int_equal(group_paths(&mp1), 0);
verify_pathgroups(&mp1, p1, groups, group_size, 1);
}
static void test_group_by_prio0(void **state)
{
- assert_int_equal(group_by_prio(&mp0), 0);
+ mp0.pgpolicyfn = group_by_prio;
+ assert_int_equal(group_paths(&mp0), 0);
verify_pathgroups(&mp0, NULL, NULL, NULL, 0);
}
static void test_group_by_prio_null(void **state)
{
- assert_int_equal(group_by_prio(&mp_null), 0);
+ mp_null.pgpolicyfn = group_by_prio;
+ assert_int_equal(group_paths(&mp_null), 0);
verify_pathgroups(&mp_null, NULL, NULL, NULL, 0);
}
@@ -430,7 +453,8 @@ static void test_group_by_node_name_same8(void **state)
int group_size[] = {8};
set_tgt_node_name(p8, node_name, 8);
- assert_int_equal(group_by_node_name(&mp8), 0);
+ mp8.pgpolicyfn = group_by_node_name;
+ assert_int_equal(group_paths(&mp8), 0);
verify_pathgroups(&mp8, p8, groups, group_size, 1);
}
@@ -445,7 +469,8 @@ static void test_group_by_node_name_increasing8(void **state)
set_priority(p8, prio, 8);
set_tgt_node_name(p8, node_name, 8);
- assert_int_equal(group_by_node_name(&mp8), 0);
+ mp8.pgpolicyfn = group_by_node_name;
+ assert_int_equal(group_paths(&mp8), 0);
verify_pathgroups(&mp8, p8, groups, group_size, 8);
}
@@ -461,7 +486,8 @@ static void test_group_by_node_name_3_groups8(void **state)
set_priority(p8, prio, 8);
set_tgt_node_name(p8, node_name, 8);
- assert_int_equal(group_by_node_name(&mp8), 0);
+ mp8.pgpolicyfn = group_by_node_name;
+ assert_int_equal(group_paths(&mp8), 0);
verify_pathgroups(&mp8, p8, groups, group_size, 3);
}
@@ -476,7 +502,8 @@ static void test_group_by_node_name_2_groups8(void **state)
set_priority(p8, prio, 8);
set_tgt_node_name(p8, node_name, 8);
- assert_int_equal(group_by_node_name(&mp8), 0);
+ mp8.pgpolicyfn = group_by_node_name;
+ assert_int_equal(group_paths(&mp8), 0);
verify_pathgroups(&mp8, p8, groups, group_size, 2);
}
@@ -492,7 +519,8 @@ static void test_group_by_node_name_3_groups4(void **state)
set_priority(p4, prio, 4);
set_tgt_node_name(p4, node_name, 4);
- assert_int_equal(group_by_node_name(&mp4), 0);
+ mp4.pgpolicyfn = group_by_node_name;
+ assert_int_equal(group_paths(&mp4), 0);
verify_pathgroups(&mp4, p4, groups, group_size, 3);
}
@@ -507,7 +535,8 @@ static void test_group_by_node_name_2_groups4(void **state)
set_priority(p4, prio, 4);
set_tgt_node_name(p4, node_name, 4);
- assert_int_equal(group_by_node_name(&mp4), 0);
+ mp4.pgpolicyfn = group_by_node_name;
+ assert_int_equal(group_paths(&mp4), 0);
verify_pathgroups(&mp4, p4, groups, group_size, 2);
}
@@ -519,19 +548,22 @@ static void test_group_by_node_name1(void **state)
int group_size[] = {1};
set_tgt_node_name(p1, node_name, 1);
- assert_int_equal(group_by_node_name(&mp1), 0);
+ mp1.pgpolicyfn = group_by_node_name;
+ assert_int_equal(group_paths(&mp1), 0);
verify_pathgroups(&mp1, p1, groups, group_size, 1);
}
static void test_group_by_node_name0(void **state)
{
- assert_int_equal(group_by_node_name(&mp0), 0);
+ mp0.pgpolicyfn = group_by_node_name;
+ assert_int_equal(group_paths(&mp0), 0);
verify_pathgroups(&mp0, NULL, NULL, NULL, 0);
}
static void test_group_by_node_name_null(void **state)
{
- assert_int_equal(group_by_node_name(&mp_null), 0);
+ mp_null.pgpolicyfn = group_by_node_name;
+ assert_int_equal(group_paths(&mp_null), 0);
verify_pathgroups(&mp_null, NULL, NULL, NULL, 0);
}
@@ -543,7 +575,8 @@ static void test_group_by_serial_same8(void **state)
int group_size[] = {8};
set_serial(p8, serial, 8);
- assert_int_equal(group_by_serial(&mp8), 0);
+ mp8.pgpolicyfn = group_by_serial;
+ assert_int_equal(group_paths(&mp8), 0);
verify_pathgroups(&mp8, p8, groups, group_size, 1);
}
@@ -558,7 +591,8 @@ static void test_group_by_serial_increasing8(void **state)
set_priority(p8, prio, 8);
set_serial(p8, serial, 8);
- assert_int_equal(group_by_serial(&mp8), 0);
+ mp8.pgpolicyfn = group_by_serial;
+ assert_int_equal(group_paths(&mp8), 0);
verify_pathgroups(&mp8, p8, groups, group_size, 8);
}
@@ -574,7 +608,8 @@ static void test_group_by_serial_3_groups8(void **state)
set_priority(p8, prio, 8);
set_serial(p8, serial, 8);
- assert_int_equal(group_by_serial(&mp8), 0);
+ mp8.pgpolicyfn = group_by_serial;
+ assert_int_equal(group_paths(&mp8), 0);
verify_pathgroups(&mp8, p8, groups, group_size, 3);
}
@@ -589,7 +624,8 @@ static void test_group_by_serial_2_groups8(void **state)
set_priority(p8, prio, 8);
set_serial(p8, serial, 8);
- assert_int_equal(group_by_serial(&mp8), 0);
+ mp8.pgpolicyfn = group_by_serial;
+ assert_int_equal(group_paths(&mp8), 0);
verify_pathgroups(&mp8, p8, groups, group_size, 2);
}
@@ -605,7 +641,8 @@ static void test_group_by_serial_3_groups4(void **state)
set_priority(p4, prio, 4);
set_serial(p4, serial, 4);
- assert_int_equal(group_by_serial(&mp4), 0);
+ mp4.pgpolicyfn = group_by_serial;
+ assert_int_equal(group_paths(&mp4), 0);
verify_pathgroups(&mp4, p4, groups, group_size, 3);
}
@@ -620,7 +657,8 @@ static void test_group_by_serial_2_groups4(void **state)
set_priority(p4, prio, 4);
set_serial(p4, serial, 4);
- assert_int_equal(group_by_serial(&mp4), 0);
+ mp4.pgpolicyfn = group_by_serial;
+ assert_int_equal(group_paths(&mp4), 0);
verify_pathgroups(&mp4, p4, groups, group_size, 2);
}
@@ -632,19 +670,22 @@ static void test_group_by_serial1(void **state)
int group_size[1] = {1};
set_serial(p1, serial, 1);
- assert_int_equal(group_by_serial(&mp1), 0);
+ mp1.pgpolicyfn = group_by_serial;
+ assert_int_equal(group_paths(&mp1), 0);
verify_pathgroups(&mp1, p1, groups, group_size, 1);
}
static void test_group_by_serial0(void **state)
{
- assert_int_equal(group_by_serial(&mp0), 0);
+ mp0.pgpolicyfn = group_by_serial;
+ assert_int_equal(group_paths(&mp0), 0);
verify_pathgroups(&mp0, NULL, NULL, NULL, 0);
}
static void test_group_by_serial_null(void **state)
{
- assert_int_equal(group_by_serial(&mp_null), 0);
+ mp_null.pgpolicyfn = group_by_serial;
+ assert_int_equal(group_paths(&mp_null), 0);
verify_pathgroups(&mp_null, NULL, NULL, NULL, 0);
}
--
2.17.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 06/14] libmultipath: fix double free in pgpolicyfn error paths
2019-08-23 17:48 [PATCH v2 00/14] multipath marginal pathgroups Benjamin Marzinski
` (4 preceding siblings ...)
2019-08-23 17:48 ` [PATCH v2 05/14] tests: update pgpolicy tests to work with group_paths() Benjamin Marzinski
@ 2019-08-23 17:48 ` Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 07/14] libmultipath: consolidate group_by_* functions Benjamin Marzinski
` (8 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Benjamin Marzinski @ 2019-08-23 17:48 UTC (permalink / raw)
To: Christophe Varoqui
Cc: device-mapper development, Muneendra Kumar, Martin Wilck
In the pgpolicy functions, if an error is encountered after
alloc_pathgroup() is called, but before the path group is added to a
multipath device with add_pathgroup(), the pathgroup needs to be cleaned
up by calling free_pathgroup(). However, after the pathgroup has been
added to the multipath device, calling free_pgvec() will clean it up. In
this case, if free_pathgroup() is called first, the recently added
pathgroup will be freed twice.
Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/pgpolicies.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/libmultipath/pgpolicies.c b/libmultipath/pgpolicies.c
index 1b59485c..1af42f52 100644
--- a/libmultipath/pgpolicies.c
+++ b/libmultipath/pgpolicies.c
@@ -139,7 +139,7 @@ int group_by_node_name(struct multipath * mp)
/* feed the first path */
if (store_path(pgp->paths, pp))
- goto out2;
+ goto out1;
bitmap[i] = 1;
@@ -153,7 +153,7 @@ int group_by_node_name(struct multipath * mp)
if (!strncmp(pp->tgt_node_name, pp2->tgt_node_name,
NODE_NAME_SIZE)) {
if (store_path(pgp->paths, pp2))
- goto out2;
+ goto out1;
bitmap[j] = 1;
}
@@ -206,7 +206,7 @@ int group_by_serial(struct multipath * mp)
/* feed the first path */
if (store_path(pgp->paths, pp))
- goto out2;
+ goto out1;
bitmap[i] = 1;
@@ -219,7 +219,7 @@ int group_by_serial(struct multipath * mp)
if (0 == strcmp(pp->serial, pp2->serial)) {
if (store_path(pgp->paths, pp2))
- goto out2;
+ goto out1;
bitmap[j] = 1;
}
@@ -254,7 +254,7 @@ int one_path_per_group(struct multipath *mp)
goto out1;
if (store_path(pgp->paths, pp))
- goto out1;
+ goto out;
}
return 0;
out1:
@@ -358,7 +358,7 @@ int group_by_prio(struct multipath *mp)
vector_foreach_slot(pathvec, pp, i) {
if (pp->priority == prio) {
if (store_path(pgp->paths, pp))
- goto out2;
+ goto out1;
vector_del_slot(pathvec, i);
i--;
--
2.17.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 07/14] libmultipath: consolidate group_by_* functions
2019-08-23 17:48 [PATCH v2 00/14] multipath marginal pathgroups Benjamin Marzinski
` (5 preceding siblings ...)
2019-08-23 17:48 ` [PATCH v2 06/14] libmultipath: fix double free in pgpolicyfn error paths Benjamin Marzinski
@ 2019-08-23 17:48 ` Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 08/14] libmultipath: make pgpolicyfn take a paths vector Benjamin Marzinski
` (7 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Benjamin Marzinski @ 2019-08-23 17:48 UTC (permalink / raw)
To: Christophe Varoqui
Cc: device-mapper development, Muneendra Kumar, Martin Wilck
group_by_node_name() and group_by_serial() are exactly the same except
for how the paths are compared. group_by_prio() is different but its
pathvec solves the same issue as the bitmap from the other two
functions, and since we are always running sort_pathgroups() after
calling pgpriorityfn, there is no need to sort the pathgroups in
group_by_prio(). This means that all three functions can be replaced
with one function, group_by_match() that takes a match function as an
argument.
Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/pgpolicies.c | 189 +++++++++-----------------------------
1 file changed, 41 insertions(+), 148 deletions(-)
diff --git a/libmultipath/pgpolicies.c b/libmultipath/pgpolicies.c
index 1af42f52..2e4db74c 100644
--- a/libmultipath/pgpolicies.c
+++ b/libmultipath/pgpolicies.c
@@ -4,6 +4,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <stdbool.h>
#include "checkers.h"
#include "util.h"
@@ -104,10 +105,29 @@ int group_paths(struct multipath *mp)
return 0;
}
-/*
- * One path group per unique tgt_node_name present in the path vector
- */
-int group_by_node_name(struct multipath * mp)
+typedef bool (path_match_fn)(struct path *pp1, struct path *pp2);
+
+bool
+node_names_match(struct path *pp1, struct path *pp2)
+{
+ return (strncmp(pp1->tgt_node_name, pp2->tgt_node_name,
+ NODE_NAME_SIZE) == 0);
+}
+
+bool
+serials_match(struct path *pp1, struct path *pp2)
+{
+ return (strncmp(pp1->serial, pp2->serial, SERIAL_SIZE) == 0);
+}
+
+bool
+prios_match(struct path *pp1, struct path *pp2)
+{
+ return (pp1->priority == pp2->priority);
+}
+
+int group_by_match(struct multipath * mp,
+ bool (*path_match_fn)(struct path *, struct path *))
{
int i, j;
int * bitmap;
@@ -150,8 +170,7 @@ int group_by_node_name(struct multipath * mp)
pp2 = VECTOR_SLOT(mp->paths, j);
- if (!strncmp(pp->tgt_node_name, pp2->tgt_node_name,
- NODE_NAME_SIZE)) {
+ if (path_match_fn(pp, pp2)) {
if (store_path(pgp->paths, pp2))
goto out1;
@@ -171,70 +190,28 @@ out:
return 1;
}
+/*
+ * One path group per unique tgt_node_name present in the path vector
+ */
+int group_by_node_name(struct multipath * mp)
+{
+ return group_by_match(mp, node_names_match);
+}
+
/*
* One path group per unique serial number present in the path vector
*/
int group_by_serial(struct multipath * mp)
{
- int i, j;
- int * bitmap;
- struct path * pp;
- struct pathgroup * pgp;
- struct path * pp2;
-
- /* init the bitmap */
- bitmap = (int *)MALLOC(VECTOR_SIZE(mp->paths) * sizeof (int));
-
- if (!bitmap)
- goto out;
-
- for (i = 0; i < VECTOR_SIZE(mp->paths); i++) {
-
- if (bitmap[i])
- continue;
-
- pp = VECTOR_SLOT(mp->paths, i);
-
- /* here, we really got a new pg */
- pgp = alloc_pathgroup();
-
- if (!pgp)
- goto out1;
-
- if (add_pathgroup(mp, pgp))
- goto out2;
-
- /* feed the first path */
- if (store_path(pgp->paths, pp))
- goto out1;
-
- bitmap[i] = 1;
-
- for (j = i + 1; j < VECTOR_SIZE(mp->paths); j++) {
-
- if (bitmap[j])
- continue;
-
- pp2 = VECTOR_SLOT(mp->paths, j);
-
- if (0 == strcmp(pp->serial, pp2->serial)) {
- if (store_path(pgp->paths, pp2))
- goto out1;
+ return group_by_match(mp, serials_match);
+}
- bitmap[j] = 1;
- }
- }
- }
- FREE(bitmap);
- return 0;
-out2:
- free_pathgroup(pgp, KEEP_PATHS);
-out1:
- FREE(bitmap);
-out:
- free_pgvec(mp->pg, KEEP_PATHS);
- mp->pg = NULL;
- return 1;
+/*
+ * One path group per priority present in the path vector
+ */
+int group_by_prio(struct multipath *mp)
+{
+ return group_by_match(mp, prios_match);
}
int one_path_per_group(struct multipath *mp)
@@ -293,87 +270,3 @@ out:
mp->pg = NULL;
return 1;
}
-
-int group_by_prio(struct multipath *mp)
-{
- int i;
- int prio;
- struct path * pp;
- struct pathgroup * pgp;
- vector pathvec = NULL;
-
- pathvec = vector_alloc();
- if (!pathvec)
- goto out;
-
- vector_foreach_slot(mp->paths, pp, i) {
- if (!vector_alloc_slot(pathvec))
- goto out1;
- vector_set_slot(pathvec, pp);
- }
-
- while (VECTOR_SIZE(pathvec) > 0) {
- pp = VECTOR_SLOT(pathvec, 0);
- prio = pp->priority;
-
- /*
- * Find the position to insert the new path group. All groups
- * are ordered by the priority value (higher value first).
- */
- vector_foreach_slot(mp->pg, pgp, i) {
- pp = VECTOR_SLOT(pgp->paths, 0);
-
- if (prio > pp->priority)
- break;
- }
-
- /*
- * Initialize the new path group.
- */
- pgp = alloc_pathgroup();
-
- if (!pgp)
- goto out1;
-
- if (store_path(pgp->paths, VECTOR_SLOT(pathvec, 0)))
- goto out2;
-
- vector_del_slot(pathvec, 0);
-
- /*
- * Store the new path group into the vector.
- */
- if (i < VECTOR_SIZE(mp->pg)) {
- if (!vector_insert_slot(mp->pg, i, pgp))
- goto out2;
- pgp->mpp = mp;
- } else {
- if (add_pathgroup(mp, pgp))
- goto out2;
- }
-
- /*
- * add the other paths with the same prio
- */
- vector_foreach_slot(pathvec, pp, i) {
- if (pp->priority == prio) {
- if (store_path(pgp->paths, pp))
- goto out1;
-
- vector_del_slot(pathvec, i);
- i--;
- }
- }
- }
- free_pathvec(pathvec, KEEP_PATHS);
- return 0;
-out2:
- free_pathgroup(pgp, KEEP_PATHS);
-out1:
- free_pathvec(pathvec, KEEP_PATHS);
-out:
- free_pgvec(mp->pg, KEEP_PATHS);
- mp->pg = NULL;
- return 1;
-
-}
--
2.17.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 08/14] libmultipath: make pgpolicyfn take a paths vector
2019-08-23 17:48 [PATCH v2 00/14] multipath marginal pathgroups Benjamin Marzinski
` (6 preceding siblings ...)
2019-08-23 17:48 ` [PATCH v2 07/14] libmultipath: consolidate group_by_* functions Benjamin Marzinski
@ 2019-08-23 17:48 ` Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 09/14] libmultipath: make group_paths handle marginal paths Benjamin Marzinski
` (6 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Benjamin Marzinski @ 2019-08-23 17:48 UTC (permalink / raw)
To: Christophe Varoqui
Cc: device-mapper development, Muneendra Kumar, Martin Wilck
To enable future changes, mp->pgpolicyfn() now takes a vector of
paths instead of always using mp->paths.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/pgpolicies.c | 38 +++++++++++++++++++-------------------
libmultipath/pgpolicies.h | 10 +++++-----
libmultipath/structs.h | 2 +-
3 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/libmultipath/pgpolicies.c b/libmultipath/pgpolicies.c
index 2e4db74c..2e7c0502 100644
--- a/libmultipath/pgpolicies.c
+++ b/libmultipath/pgpolicies.c
@@ -93,7 +93,7 @@ int group_paths(struct multipath *mp)
return 1;
if (VECTOR_SIZE(mp->paths) > 0 &&
- (!mp->pgpolicyfn || mp->pgpolicyfn(mp))) {
+ (!mp->pgpolicyfn || mp->pgpolicyfn(mp, mp->paths))) {
vector_free(mp->pg);
mp->pg = NULL;
return 1;
@@ -126,7 +126,7 @@ prios_match(struct path *pp1, struct path *pp2)
return (pp1->priority == pp2->priority);
}
-int group_by_match(struct multipath * mp,
+int group_by_match(struct multipath * mp, vector paths,
bool (*path_match_fn)(struct path *, struct path *))
{
int i, j;
@@ -136,17 +136,17 @@ int group_by_match(struct multipath * mp,
struct path * pp2;
/* init the bitmap */
- bitmap = (int *)MALLOC(VECTOR_SIZE(mp->paths) * sizeof (int));
+ bitmap = (int *)MALLOC(VECTOR_SIZE(paths) * sizeof (int));
if (!bitmap)
goto out;
- for (i = 0; i < VECTOR_SIZE(mp->paths); i++) {
+ for (i = 0; i < VECTOR_SIZE(paths); i++) {
if (bitmap[i])
continue;
- pp = VECTOR_SLOT(mp->paths, i);
+ pp = VECTOR_SLOT(paths, i);
/* here, we really got a new pg */
pgp = alloc_pathgroup();
@@ -163,12 +163,12 @@ int group_by_match(struct multipath * mp,
bitmap[i] = 1;
- for (j = i + 1; j < VECTOR_SIZE(mp->paths); j++) {
+ for (j = i + 1; j < VECTOR_SIZE(paths); j++) {
if (bitmap[j])
continue;
- pp2 = VECTOR_SLOT(mp->paths, j);
+ pp2 = VECTOR_SLOT(paths, j);
if (path_match_fn(pp, pp2)) {
if (store_path(pgp->paths, pp2))
@@ -193,35 +193,35 @@ out:
/*
* One path group per unique tgt_node_name present in the path vector
*/
-int group_by_node_name(struct multipath * mp)
+int group_by_node_name(struct multipath * mp, vector paths)
{
- return group_by_match(mp, node_names_match);
+ return group_by_match(mp, paths, node_names_match);
}
/*
* One path group per unique serial number present in the path vector
*/
-int group_by_serial(struct multipath * mp)
+int group_by_serial(struct multipath * mp, vector paths)
{
- return group_by_match(mp, serials_match);
+ return group_by_match(mp, paths, serials_match);
}
/*
* One path group per priority present in the path vector
*/
-int group_by_prio(struct multipath *mp)
+int group_by_prio(struct multipath *mp, vector paths)
{
- return group_by_match(mp, prios_match);
+ return group_by_match(mp, paths, prios_match);
}
-int one_path_per_group(struct multipath *mp)
+int one_path_per_group(struct multipath *mp, vector paths)
{
int i;
struct path * pp;
struct pathgroup * pgp;
- for (i = 0; i < VECTOR_SIZE(mp->paths); i++) {
- pp = VECTOR_SLOT(mp->paths, i);
+ for (i = 0; i < VECTOR_SIZE(paths); i++) {
+ pp = VECTOR_SLOT(paths, i);
pgp = alloc_pathgroup();
if (!pgp)
@@ -242,7 +242,7 @@ out:
return 1;
}
-int one_group(struct multipath *mp) /* aka multibus */
+int one_group(struct multipath *mp, vector paths) /* aka multibus */
{
int i;
struct path * pp;
@@ -256,8 +256,8 @@ int one_group(struct multipath *mp) /* aka multibus */
if (add_pathgroup(mp, pgp))
goto out1;
- for (i = 0; i < VECTOR_SIZE(mp->paths); i++) {
- pp = VECTOR_SLOT(mp->paths, i);
+ for (i = 0; i < VECTOR_SIZE(paths); i++) {
+ pp = VECTOR_SLOT(paths, i);
if (store_path(pgp->paths, pp))
goto out;
diff --git a/libmultipath/pgpolicies.h b/libmultipath/pgpolicies.h
index 11834011..7532d75f 100644
--- a/libmultipath/pgpolicies.h
+++ b/libmultipath/pgpolicies.h
@@ -25,10 +25,10 @@ int group_paths(struct multipath *);
/*
* policies
*/
-int one_path_per_group(struct multipath *);
-int one_group(struct multipath *);
-int group_by_serial(struct multipath *);
-int group_by_prio(struct multipath *);
-int group_by_node_name(struct multipath *);
+int one_path_per_group(struct multipath *, vector);
+int one_group(struct multipath *, vector);
+int group_by_serial(struct multipath *, vector);
+int group_by_prio(struct multipath *, vector);
+int group_by_node_name(struct multipath *, vector);
#endif
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index 893074eb..a8b9d325 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -295,7 +295,7 @@ struct path {
struct gen_path generic_path;
};
-typedef int (pgpolicyfn) (struct multipath *);
+typedef int (pgpolicyfn) (struct multipath *, vector);
struct multipath {
char wwid[WWID_SIZE];
--
2.17.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 09/14] libmultipath: make group_paths handle marginal paths
2019-08-23 17:48 [PATCH v2 00/14] multipath marginal pathgroups Benjamin Marzinski
` (7 preceding siblings ...)
2019-08-23 17:48 ` [PATCH v2 08/14] libmultipath: make pgpolicyfn take a paths vector Benjamin Marzinski
@ 2019-08-23 17:48 ` Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 10/14] tests: add tests for grouping " Benjamin Marzinski
` (5 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Benjamin Marzinski @ 2019-08-23 17:48 UTC (permalink / raw)
To: Christophe Varoqui
Cc: device-mapper development, Muneendra Kumar, Martin Wilck
group_paths() will now create seperate path groups for marginal and
normal paths, and place all of the marginal path groups after the normal
ones, in order by priority.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/pgpolicies.c | 83 +++++++++++++++++++++++++++++++++-----
libmultipath/switchgroup.c | 15 ++++++-
2 files changed, 88 insertions(+), 10 deletions(-)
diff --git a/libmultipath/pgpolicies.c b/libmultipath/pgpolicies.c
index 2e7c0502..6fb2d28a 100644
--- a/libmultipath/pgpolicies.c
+++ b/libmultipath/pgpolicies.c
@@ -72,9 +72,11 @@ sort_pathgroups (struct multipath *mp) {
pgp2 = VECTOR_SLOT(mp->pg, j);
if (!pgp2)
continue;
- if (pgp2->priority > pgp1->priority ||
- (pgp2->priority == pgp1->priority &&
- pgp2->enabled_paths >= pgp1->enabled_paths)) {
+ if (pgp2->marginal < pgp1->marginal ||
+ (pgp2->marginal == pgp1->marginal &&
+ (pgp2->priority > pgp1->priority ||
+ (pgp2->priority == pgp1->priority &&
+ pgp2->enabled_paths >= pgp1->enabled_paths)))) {
vector_move_up(mp->pg, i, j + 1);
break;
}
@@ -84,25 +86,88 @@ sort_pathgroups (struct multipath *mp) {
}
}
+static int
+split_marginal_paths(vector paths, vector *normal_p, vector *marginal_p)
+{
+ int i;
+ int has_marginal = 0;
+ int has_normal = 0;
+ struct path *pp;
+ vector normal = NULL;
+ vector marginal = NULL;
+
+ *normal_p = *marginal_p = NULL;
+ vector_foreach_slot(paths, pp, i) {
+ if (pp->marginal)
+ has_marginal = 1;
+ else
+ has_normal = 1;
+ }
+
+ if (!has_marginal || !has_normal)
+ return -1;
+
+ normal = vector_alloc();
+ marginal = vector_alloc();
+ if (!normal || !marginal)
+ goto fail;
+
+ vector_foreach_slot(paths, pp, i) {
+ if (pp->marginal) {
+ if (store_path(marginal, pp))
+ goto fail;
+ }
+ else {
+ if (store_path(normal, pp))
+ goto fail;
+ }
+ }
+ *normal_p = normal;
+ *marginal_p = marginal;
+ return 0;
+fail:
+ vector_free(normal);
+ vector_free(marginal);
+ return -1;
+}
int group_paths(struct multipath *mp)
{
+ vector normal, marginal;
+
if (!mp->pg)
mp->pg = vector_alloc();
if (!mp->pg)
return 1;
- if (VECTOR_SIZE(mp->paths) > 0 &&
- (!mp->pgpolicyfn || mp->pgpolicyfn(mp, mp->paths))) {
- vector_free(mp->pg);
- mp->pg = NULL;
- return 1;
+ if (VECTOR_SIZE(mp->paths) == 0)
+ goto out;
+ if (!mp->pgpolicyfn)
+ goto fail;
+
+ if (split_marginal_paths(mp->paths, &normal, &marginal) != 0) {
+ if (mp->pgpolicyfn(mp, mp->paths) != 0)
+ goto fail;
+ } else {
+ if (mp->pgpolicyfn(mp, normal) != 0)
+ goto fail_marginal;
+ if (mp->pgpolicyfn(mp, marginal) != 0)
+ goto fail_marginal;
+ vector_free(normal);
+ vector_free(marginal);
}
-
sort_pathgroups(mp);
+out:
vector_free(mp->paths);
mp->paths = NULL;
return 0;
+fail_marginal:
+ vector_free(normal);
+ vector_free(marginal);
+fail:
+ vector_free(mp->pg);
+ mp->pg = NULL;
+ return 1;
}
typedef bool (path_match_fn)(struct path *pp1, struct path *pp2);
diff --git a/libmultipath/switchgroup.c b/libmultipath/switchgroup.c
index 9632ce2d..6fdfcfa7 100644
--- a/libmultipath/switchgroup.c
+++ b/libmultipath/switchgroup.c
@@ -11,6 +11,7 @@ void path_group_prio_update(struct pathgroup *pgp)
{
int i;
int priority = 0;
+ int marginal = 0;
struct path * pp;
pgp->enabled_paths = 0;
@@ -19,6 +20,8 @@ void path_group_prio_update(struct pathgroup *pgp)
return;
}
vector_foreach_slot (pgp->paths, pp, i) {
+ if (pp->marginal)
+ marginal++;
if (pp->state == PATH_UP ||
pp->state == PATH_GHOST) {
priority += pp->priority;
@@ -29,11 +32,14 @@ void path_group_prio_update(struct pathgroup *pgp)
pgp->priority = priority / pgp->enabled_paths;
else
pgp->priority = 0;
+ if (marginal && marginal == i)
+ pgp->marginal = 1;
}
int select_path_group(struct multipath *mpp)
{
int i;
+ int normal_pgp = 0;
int max_priority = 0;
int bestpg = 1;
int max_enabled_paths = 1;
@@ -47,8 +53,15 @@ int select_path_group(struct multipath *mpp)
continue;
path_group_prio_update(pgp);
+ if (pgp->marginal && normal_pgp)
+ continue;
if (pgp->enabled_paths) {
- if (pgp->priority > max_priority) {
+ if (!pgp->marginal && !normal_pgp) {
+ normal_pgp = 1;
+ max_priority = pgp->priority;
+ max_enabled_paths = pgp->enabled_paths;
+ bestpg = i + 1;
+ } else if (pgp->priority > max_priority) {
max_priority = pgp->priority;
max_enabled_paths = pgp->enabled_paths;
bestpg = i + 1;
--
2.17.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 10/14] tests: add tests for grouping marginal paths.
2019-08-23 17:48 [PATCH v2 00/14] multipath marginal pathgroups Benjamin Marzinski
` (8 preceding siblings ...)
2019-08-23 17:48 ` [PATCH v2 09/14] libmultipath: make group_paths handle marginal paths Benjamin Marzinski
@ 2019-08-23 17:48 ` Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 11/14] libmultipath: add marginal_pathgroups config option Benjamin Marzinski
` (4 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Benjamin Marzinski @ 2019-08-23 17:48 UTC (permalink / raw)
To: Christophe Varoqui
Cc: device-mapper development, Muneendra Kumar, Martin Wilck
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
tests/pgpolicy.c | 337 +++++++++++++++++++++++++++++++++++++++++------
1 file changed, 295 insertions(+), 42 deletions(-)
diff --git a/tests/pgpolicy.c b/tests/pgpolicy.c
index 04a77c4c..ab09f91c 100644
--- a/tests/pgpolicy.c
+++ b/tests/pgpolicy.c
@@ -40,6 +40,15 @@ static void set_priority(struct path *pp, int *prio, int size)
}
}
+static void set_marginal(struct path *pp, int *marginal, int size)
+{
+ int i;
+
+ for (i = 0; i < size; i++) {
+ pp[i].marginal = marginal[i];
+ }
+}
+
static void set_tgt_node_name(struct path *pp, char **tgt_node_name, int size)
{
int i;
@@ -82,6 +91,7 @@ static int setupX(struct multipath *mp, struct path *pp, int size)
{
int i;
int prio[8] = {10, 10, 10, 10, 10, 10, 10, 10};
+ int marginal[8] = {0, 0, 0, 0, 0, 0, 0, 0};
mp->paths = vector_alloc();
if (!mp->paths)
@@ -92,6 +102,7 @@ static int setupX(struct multipath *mp, struct path *pp, int size)
vector_set_slot(mp->paths, &pp[i]);
}
set_priority(pp, prio, size);
+ set_marginal(pp, marginal, size);
mp->pgpolicyfn = NULL;
return 0;
}
@@ -155,7 +166,7 @@ static int teardown_null(void **state)
static void
verify_pathgroups(struct multipath *mp, struct path *pp, int **groups,
- int *group_size, int size)
+ int *group_size, int *marginal, int size)
{
int i, j;
struct pathgroup *pgp;
@@ -168,6 +179,10 @@ verify_pathgroups(struct multipath *mp, struct path *pp, int **groups,
assert_non_null(pgp);
assert_non_null(pgp->paths);
assert_int_equal(VECTOR_SIZE(pgp->paths), group_size[i]);
+ if (marginal)
+ assert_int_equal(pgp->marginal, marginal[i]);
+ else
+ assert_int_equal(pgp->marginal, 0);
for (j = 0; j < group_size[i]; j++) {
int path_nr = groups[i][j];
struct path *pgp_path = VECTOR_SLOT(pgp->paths, j);
@@ -190,7 +205,7 @@ static void test_one_group8(void **state)
mp8.pgpolicyfn = one_group;
assert_int_equal(group_paths(&mp8), 0);
- verify_pathgroups(&mp8, p8, groups, group_size, 1);
+ verify_pathgroups(&mp8, p8, groups, group_size, NULL, 1);
}
static void test_one_group4(void **state)
@@ -201,7 +216,7 @@ static void test_one_group4(void **state)
mp4.pgpolicyfn = one_group;
assert_int_equal(group_paths(&mp4), 0);
- verify_pathgroups(&mp4, p4, groups, group_size, 1);
+ verify_pathgroups(&mp4, p4, groups, group_size, NULL, 1);
}
static void test_one_group1(void **state)
@@ -212,21 +227,65 @@ static void test_one_group1(void **state)
mp1.pgpolicyfn = one_group;
assert_int_equal(group_paths(&mp1), 0);
- verify_pathgroups(&mp1, p1, groups, group_size, 1);
+ verify_pathgroups(&mp1, p1, groups, group_size, NULL, 1);
}
static void test_one_group0(void **state)
{
mp0.pgpolicyfn = one_group;
assert_int_equal(group_paths(&mp0), 0);
- verify_pathgroups(&mp0, NULL, NULL, NULL, 0);
+ verify_pathgroups(&mp0, NULL, NULL, NULL, NULL, 0);
}
static void test_one_group_null(void **state)
{
mp_null.pgpolicyfn = one_group;
assert_int_equal(group_paths(&mp_null), 0);
- verify_pathgroups(&mp_null, NULL, NULL, NULL, 0);
+ verify_pathgroups(&mp_null, NULL, NULL, NULL, NULL, 0);
+}
+
+static void test_one_group_all_marginal8(void **state)
+{
+ int paths[] = {0,1,2,3,4,5,6,7};
+ int marginal[] = {1,1,1,1,1,1,1,1};
+ int *groups[] = {paths};
+ int group_size[] = {8};
+ int group_marginal[] = {1};
+
+ set_marginal(p8, marginal, 8);
+ mp8.pgpolicyfn = one_group;
+ assert_int_equal(group_paths(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 1);
+}
+
+static void test_one_group_half_marginal8(void **state)
+{
+ int marginal[] = {1,0,1,0,1,1,0,0};
+ int group0[] = {1,3,6,7};
+ int group1[] = {0,2,4,5};
+ int *groups[] = {group0, group1};
+ int group_size[] = {4,4};
+ int group_marginal[] = {0,1};
+
+ set_marginal(p8, marginal, 8);
+ mp8.pgpolicyfn = one_group;
+ assert_int_equal(group_paths(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 2);
+}
+
+static void test_one_group_one_marginal8(void **state)
+{
+ int marginal[] = {0,0,0,0,0,1,0,0};
+ int group0[] = {0,1,2,3,4,6,7};
+ int group1[] = {5};
+ int *groups[] = {group0, group1};
+ int group_size[] = {7,1};
+ int group_marginal[] = {0,1};
+
+ set_marginal(p8, marginal, 8);
+ mp8.pgpolicyfn = one_group;
+ assert_int_equal(group_paths(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 2);
}
static void test_one_path_per_group_same8(void **state)
@@ -238,7 +297,7 @@ static void test_one_path_per_group_same8(void **state)
mp8.pgpolicyfn = one_path_per_group;
assert_int_equal(group_paths(&mp8), 0);
- verify_pathgroups(&mp8, p8, groups, group_size, 8);
+ verify_pathgroups(&mp8, p8, groups, group_size, NULL, 8);
}
static void test_one_path_per_group_increasing8(void **state)
@@ -252,7 +311,7 @@ static void test_one_path_per_group_increasing8(void **state)
set_priority(p8, prio, 8);
mp8.pgpolicyfn = one_path_per_group;
assert_int_equal(group_paths(&mp8), 0);
- verify_pathgroups(&mp8, p8, groups, group_size, 8);
+ verify_pathgroups(&mp8, p8, groups, group_size, NULL, 8);
}
static void test_one_path_per_group_decreasing8(void **state)
@@ -266,7 +325,7 @@ static void test_one_path_per_group_decreasing8(void **state)
set_priority(p8, prio, 8);
mp8.pgpolicyfn = one_path_per_group;
assert_int_equal(group_paths(&mp8), 0);
- verify_pathgroups(&mp8, p8, groups, group_size, 8);
+ verify_pathgroups(&mp8, p8, groups, group_size, NULL, 8);
}
static void test_one_path_per_group_mixed8(void **state)
@@ -280,7 +339,7 @@ static void test_one_path_per_group_mixed8(void **state)
set_priority(p8, prio, 8);
mp8.pgpolicyfn = one_path_per_group;
assert_int_equal(group_paths(&mp8), 0);
- verify_pathgroups(&mp8, p8, groups, group_size, 8);
+ verify_pathgroups(&mp8, p8, groups, group_size, NULL, 8);
}
static void test_one_path_per_group4(void **state)
@@ -291,7 +350,7 @@ static void test_one_path_per_group4(void **state)
mp4.pgpolicyfn = one_path_per_group;
assert_int_equal(group_paths(&mp4), 0);
- verify_pathgroups(&mp4, p4, groups, group_size, 4);
+ verify_pathgroups(&mp4, p4, groups, group_size, NULL, 4);
}
static void test_one_path_per_group1(void **state)
@@ -302,21 +361,55 @@ static void test_one_path_per_group1(void **state)
mp1.pgpolicyfn = one_path_per_group;
assert_int_equal(group_paths(&mp1), 0);
- verify_pathgroups(&mp1, p1, groups, group_size, 1);
+ verify_pathgroups(&mp1, p1, groups, group_size, NULL, 1);
}
static void test_one_path_per_group0(void **state)
{
mp0.pgpolicyfn = one_path_per_group;
assert_int_equal(group_paths(&mp0), 0);
- verify_pathgroups(&mp0, NULL, NULL, NULL, 0);
+ verify_pathgroups(&mp0, NULL, NULL, NULL, NULL, 0);
}
static void test_one_path_per_group_null(void **state)
{
mp_null.pgpolicyfn = one_path_per_group;
assert_int_equal(group_paths(&mp_null), 0);
- verify_pathgroups(&mp_null, NULL, NULL, NULL, 0);
+ verify_pathgroups(&mp_null, NULL, NULL, NULL, NULL, 0);
+}
+
+static void test_one_path_per_group_mixed_all_marginal8(void **state)
+{
+ int prio[] = {7,1,3,3,5,2,8,2};
+ int marginal[] = {1,1,1,1,1,1,1,1};
+ int paths[] = {6,0,4,2,3,5,7,1};
+ int *groups[] = {&paths[0], &paths[1], &paths[2], &paths[3],
+ &paths[4], &paths[5], &paths[6], &paths[7]};
+ int group_size[] = {1,1,1,1,1,1,1,1};
+ int group_marginal[] = {1,1,1,1,1,1,1,1};
+
+ set_priority(p8, prio, 8);
+ set_marginal(p8, marginal, 8);
+ mp8.pgpolicyfn = one_path_per_group;
+ assert_int_equal(group_paths(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 8);
+}
+
+static void test_one_path_per_group_mixed_half_marginal8(void **state)
+{
+ int prio[] = {7,1,3,3,5,2,8,2};
+ int marginal[] = {0,1,1,0,0,0,1,1};
+ int paths[] = {0,4,3,5,6,2,7,1};
+ int *groups[] = {&paths[0], &paths[1], &paths[2], &paths[3],
+ &paths[4], &paths[5], &paths[6], &paths[7]};
+ int group_size[] = {1,1,1,1,1,1,1,1};
+ int group_marginal[] = {0,0,0,0,1,1,1,1};
+
+ set_priority(p8, prio, 8);
+ set_marginal(p8, marginal, 8);
+ mp8.pgpolicyfn = one_path_per_group;
+ assert_int_equal(group_paths(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 8);
}
static void test_group_by_prio_same8(void **state)
@@ -327,7 +420,7 @@ static void test_group_by_prio_same8(void **state)
mp8.pgpolicyfn = group_by_prio;
assert_int_equal(group_paths(&mp8), 0);
- verify_pathgroups(&mp8, p8, groups, group_size, 1);
+ verify_pathgroups(&mp8, p8, groups, group_size, NULL, 1);
}
static void test_group_by_prio_increasing8(void **state)
@@ -341,7 +434,7 @@ static void test_group_by_prio_increasing8(void **state)
set_priority(p8, prio, 8);
mp8.pgpolicyfn = group_by_prio;
assert_int_equal(group_paths(&mp8), 0);
- verify_pathgroups(&mp8, p8, groups, group_size, 8);
+ verify_pathgroups(&mp8, p8, groups, group_size, NULL, 8);
}
static void test_group_by_prio_decreasing8(void **state)
@@ -355,7 +448,7 @@ static void test_group_by_prio_decreasing8(void **state)
set_priority(p8, prio, 8);
mp8.pgpolicyfn = group_by_prio;
assert_int_equal(group_paths(&mp8), 0);
- verify_pathgroups(&mp8, p8, groups, group_size, 8);
+ verify_pathgroups(&mp8, p8, groups, group_size, NULL, 8);
}
static void test_group_by_prio_mixed8(void **state)
@@ -374,7 +467,7 @@ static void test_group_by_prio_mixed8(void **state)
set_priority(p8, prio, 8);
mp8.pgpolicyfn = group_by_prio;
assert_int_equal(group_paths(&mp8), 0);
- verify_pathgroups(&mp8, p8, groups, group_size, 6);
+ verify_pathgroups(&mp8, p8, groups, group_size, NULL, 6);
}
static void test_group_by_prio_2_groups8(void **state)
@@ -388,7 +481,7 @@ static void test_group_by_prio_2_groups8(void **state)
set_priority(p8, prio, 8);
mp8.pgpolicyfn = group_by_prio;
assert_int_equal(group_paths(&mp8), 0);
- verify_pathgroups(&mp8, p8, groups, group_size, 2);
+ verify_pathgroups(&mp8, p8, groups, group_size, NULL, 2);
}
static void test_group_by_prio_mixed4(void **state)
@@ -403,7 +496,7 @@ static void test_group_by_prio_mixed4(void **state)
set_priority(p4, prio, 4);
mp4.pgpolicyfn = group_by_prio;
assert_int_equal(group_paths(&mp4), 0);
- verify_pathgroups(&mp4, p4, groups, group_size, 3);
+ verify_pathgroups(&mp4, p4, groups, group_size, NULL, 3);
}
static void test_group_by_prio_2_groups4(void **state)
@@ -417,7 +510,7 @@ static void test_group_by_prio_2_groups4(void **state)
set_priority(p4, prio, 4);
mp4.pgpolicyfn = group_by_prio;
assert_int_equal(group_paths(&mp4), 0);
- verify_pathgroups(&mp4, p4, groups, group_size, 2);
+ verify_pathgroups(&mp4, p4, groups, group_size, NULL, 2);
}
static void test_group_by_prio1(void **state)
@@ -428,21 +521,89 @@ static void test_group_by_prio1(void **state)
mp1.pgpolicyfn = group_by_prio;
assert_int_equal(group_paths(&mp1), 0);
- verify_pathgroups(&mp1, p1, groups, group_size, 1);
+ verify_pathgroups(&mp1, p1, groups, group_size, NULL, 1);
}
static void test_group_by_prio0(void **state)
{
mp0.pgpolicyfn = group_by_prio;
assert_int_equal(group_paths(&mp0), 0);
- verify_pathgroups(&mp0, NULL, NULL, NULL, 0);
+ verify_pathgroups(&mp0, NULL, NULL, NULL, NULL, 0);
}
static void test_group_by_prio_null(void **state)
{
mp_null.pgpolicyfn = group_by_prio;
assert_int_equal(group_paths(&mp_null), 0);
- verify_pathgroups(&mp_null, NULL, NULL, NULL, 0);
+ verify_pathgroups(&mp_null, NULL, NULL, NULL, NULL, 0);
+}
+
+static void test_group_by_prio_mixed_all_marginal8(void **state)
+{
+ int prio[] = {7,1,3,3,5,2,8,2};
+ int marginal[] = {1,1,1,1,1,1,1,1};
+ int group0[] = {6};
+ int group1[] = {0};
+ int group2[] = {4};
+ int group3[] = {2,3};
+ int group4[] = {5,7};
+ int group5[] = {1};
+ int *groups[] = {group0, group1, group2, group3,
+ group4, group5};
+ int group_size[] = {1,1,1,2,2,1};
+ int group_marginal[] = {1,1,1,1,1,1};
+
+ set_priority(p8, prio, 8);
+ set_marginal(p8, marginal, 8);
+ mp8.pgpolicyfn = group_by_prio;
+ assert_int_equal(group_paths(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 6);
+}
+
+static void test_group_by_prio_mixed_half_marginal8(void **state)
+{
+ int prio[] = {7,1,3,3,5,2,8,2};
+ int marginal[] = {0,0,0,1,0,1,1,1};
+ int group0[] = {0};
+ int group1[] = {4};
+ int group2[] = {2};
+ int group3[] = {1};
+ int group4[] = {6};
+ int group5[] = {3};
+ int group6[] = {5,7};
+ int *groups[] = {group0, group1, group2, group3,
+ group4, group5, group6};
+ int group_size[] = {1,1,1,1,1,1,2};
+ int group_marginal[] = {0,0,0,0,1,1,1};
+
+ set_priority(p8, prio, 8);
+ set_marginal(p8, marginal, 8);
+ mp8.pgpolicyfn = group_by_prio;
+ assert_int_equal(group_paths(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 7);
+}
+
+static void test_group_by_prio_mixed_one_marginal8(void **state)
+{
+ int prio[] = {7,1,3,3,5,2,8,2};
+ int marginal[] = {0,0,0,0,0,1,0,0};
+ int group0[] = {6};
+ int group1[] = {0};
+ int group2[] = {4};
+ int group3[] = {2,3};
+ int group4[] = {7};
+ int group5[] = {1};
+ int group6[] = {5};
+ int *groups[] = {group0, group1, group2, group3,
+ group4, group5, group6};
+ int group_size[] = {1,1,1,2,1,1,1};
+ int group_marginal[] = {0,0,0,0,0,0,1};
+
+ set_priority(p8, prio, 8);
+ set_marginal(p8, marginal, 8);
+ mp8.pgpolicyfn = group_by_prio;
+ assert_int_equal(group_paths(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 7);
}
static void test_group_by_node_name_same8(void **state)
@@ -455,7 +616,7 @@ static void test_group_by_node_name_same8(void **state)
set_tgt_node_name(p8, node_name, 8);
mp8.pgpolicyfn = group_by_node_name;
assert_int_equal(group_paths(&mp8), 0);
- verify_pathgroups(&mp8, p8, groups, group_size, 1);
+ verify_pathgroups(&mp8, p8, groups, group_size, NULL, 1);
}
static void test_group_by_node_name_increasing8(void **state)
@@ -471,7 +632,7 @@ static void test_group_by_node_name_increasing8(void **state)
set_tgt_node_name(p8, node_name, 8);
mp8.pgpolicyfn = group_by_node_name;
assert_int_equal(group_paths(&mp8), 0);
- verify_pathgroups(&mp8, p8, groups, group_size, 8);
+ verify_pathgroups(&mp8, p8, groups, group_size, NULL, 8);
}
static void test_group_by_node_name_3_groups8(void **state)
@@ -488,7 +649,7 @@ static void test_group_by_node_name_3_groups8(void **state)
set_tgt_node_name(p8, node_name, 8);
mp8.pgpolicyfn = group_by_node_name;
assert_int_equal(group_paths(&mp8), 0);
- verify_pathgroups(&mp8, p8, groups, group_size, 3);
+ verify_pathgroups(&mp8, p8, groups, group_size, NULL, 3);
}
static void test_group_by_node_name_2_groups8(void **state)
@@ -504,7 +665,7 @@ static void test_group_by_node_name_2_groups8(void **state)
set_tgt_node_name(p8, node_name, 8);
mp8.pgpolicyfn = group_by_node_name;
assert_int_equal(group_paths(&mp8), 0);
- verify_pathgroups(&mp8, p8, groups, group_size, 2);
+ verify_pathgroups(&mp8, p8, groups, group_size, NULL, 2);
}
static void test_group_by_node_name_3_groups4(void **state)
@@ -521,7 +682,7 @@ static void test_group_by_node_name_3_groups4(void **state)
set_tgt_node_name(p4, node_name, 4);
mp4.pgpolicyfn = group_by_node_name;
assert_int_equal(group_paths(&mp4), 0);
- verify_pathgroups(&mp4, p4, groups, group_size, 3);
+ verify_pathgroups(&mp4, p4, groups, group_size, NULL, 3);
}
static void test_group_by_node_name_2_groups4(void **state)
@@ -537,7 +698,7 @@ static void test_group_by_node_name_2_groups4(void **state)
set_tgt_node_name(p4, node_name, 4);
mp4.pgpolicyfn = group_by_node_name;
assert_int_equal(group_paths(&mp4), 0);
- verify_pathgroups(&mp4, p4, groups, group_size, 2);
+ verify_pathgroups(&mp4, p4, groups, group_size, NULL, 2);
}
static void test_group_by_node_name1(void **state)
@@ -550,21 +711,61 @@ static void test_group_by_node_name1(void **state)
set_tgt_node_name(p1, node_name, 1);
mp1.pgpolicyfn = group_by_node_name;
assert_int_equal(group_paths(&mp1), 0);
- verify_pathgroups(&mp1, p1, groups, group_size, 1);
+ verify_pathgroups(&mp1, p1, groups, group_size, NULL, 1);
}
static void test_group_by_node_name0(void **state)
{
mp0.pgpolicyfn = group_by_node_name;
assert_int_equal(group_paths(&mp0), 0);
- verify_pathgroups(&mp0, NULL, NULL, NULL, 0);
+ verify_pathgroups(&mp0, NULL, NULL, NULL, NULL, 0);
}
static void test_group_by_node_name_null(void **state)
{
mp_null.pgpolicyfn = group_by_node_name;
assert_int_equal(group_paths(&mp_null), 0);
- verify_pathgroups(&mp_null, NULL, NULL, NULL, 0);
+ verify_pathgroups(&mp_null, NULL, NULL, NULL, NULL, 0);
+}
+
+static void test_group_by_node_name_2_groups_all_marginal8(void **state)
+{
+ char *node_name[] = {"a", "a", "b", "a", "b", "b", "b", "a"};
+ int prio[] = {4,1,2,1,2,2,2,1};
+ int marginal[] = {1,1,1,1,1,1,1,1};
+ int group0[] = {2,4,5,6};
+ int group1[] = {0,1,3,7};
+ int *groups[] = {group0, group1};
+ int group_size[] = {4,4};
+ int group_marginal[] = {1,1};
+
+ set_priority(p8, prio, 8);
+ set_marginal(p8, marginal, 8);
+ set_tgt_node_name(p8, node_name, 8);
+ mp8.pgpolicyfn = group_by_node_name;
+ assert_int_equal(group_paths(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 2);
+}
+
+static void test_group_by_node_name_2_groups_half_marginal8(void **state)
+{
+ char *node_name[] = {"a", "a", "b", "a", "b", "b", "b", "a"};
+ int prio[] = {4,1,2,1,2,2,2,1};
+ int marginal[] = {1,0,1,1,0,1,0,0};
+ int group0[] = {4,6};
+ int group1[] = {1,7};
+ int group2[] = {0,3};
+ int group3[] = {2,5};
+ int *groups[] = {group0, group1, group2, group3};
+ int group_size[] = {2,2,2,2};
+ int group_marginal[] = {0,0,1,1};
+
+ set_priority(p8, prio, 8);
+ set_marginal(p8, marginal, 8);
+ set_tgt_node_name(p8, node_name, 8);
+ mp8.pgpolicyfn = group_by_node_name;
+ assert_int_equal(group_paths(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 4);
}
static void test_group_by_serial_same8(void **state)
@@ -577,7 +778,7 @@ static void test_group_by_serial_same8(void **state)
set_serial(p8, serial, 8);
mp8.pgpolicyfn = group_by_serial;
assert_int_equal(group_paths(&mp8), 0);
- verify_pathgroups(&mp8, p8, groups, group_size, 1);
+ verify_pathgroups(&mp8, p8, groups, group_size, NULL, 1);
}
static void test_group_by_serial_increasing8(void **state)
@@ -593,7 +794,7 @@ static void test_group_by_serial_increasing8(void **state)
set_serial(p8, serial, 8);
mp8.pgpolicyfn = group_by_serial;
assert_int_equal(group_paths(&mp8), 0);
- verify_pathgroups(&mp8, p8, groups, group_size, 8);
+ verify_pathgroups(&mp8, p8, groups, group_size, NULL, 8);
}
static void test_group_by_serial_3_groups8(void **state)
@@ -610,7 +811,7 @@ static void test_group_by_serial_3_groups8(void **state)
set_serial(p8, serial, 8);
mp8.pgpolicyfn = group_by_serial;
assert_int_equal(group_paths(&mp8), 0);
- verify_pathgroups(&mp8, p8, groups, group_size, 3);
+ verify_pathgroups(&mp8, p8, groups, group_size, NULL, 3);
}
static void test_group_by_serial_2_groups8(void **state)
@@ -626,7 +827,7 @@ static void test_group_by_serial_2_groups8(void **state)
set_serial(p8, serial, 8);
mp8.pgpolicyfn = group_by_serial;
assert_int_equal(group_paths(&mp8), 0);
- verify_pathgroups(&mp8, p8, groups, group_size, 2);
+ verify_pathgroups(&mp8, p8, groups, group_size, NULL, 2);
}
static void test_group_by_serial_3_groups4(void **state)
@@ -643,7 +844,7 @@ static void test_group_by_serial_3_groups4(void **state)
set_serial(p4, serial, 4);
mp4.pgpolicyfn = group_by_serial;
assert_int_equal(group_paths(&mp4), 0);
- verify_pathgroups(&mp4, p4, groups, group_size, 3);
+ verify_pathgroups(&mp4, p4, groups, group_size, NULL, 3);
}
static void test_group_by_serial_2_groups4(void **state)
@@ -659,7 +860,7 @@ static void test_group_by_serial_2_groups4(void **state)
set_serial(p4, serial, 4);
mp4.pgpolicyfn = group_by_serial;
assert_int_equal(group_paths(&mp4), 0);
- verify_pathgroups(&mp4, p4, groups, group_size, 2);
+ verify_pathgroups(&mp4, p4, groups, group_size, NULL, 2);
}
static void test_group_by_serial1(void **state)
@@ -672,21 +873,61 @@ static void test_group_by_serial1(void **state)
set_serial(p1, serial, 1);
mp1.pgpolicyfn = group_by_serial;
assert_int_equal(group_paths(&mp1), 0);
- verify_pathgroups(&mp1, p1, groups, group_size, 1);
+ verify_pathgroups(&mp1, p1, groups, group_size, NULL, 1);
}
static void test_group_by_serial0(void **state)
{
mp0.pgpolicyfn = group_by_serial;
assert_int_equal(group_paths(&mp0), 0);
- verify_pathgroups(&mp0, NULL, NULL, NULL, 0);
+ verify_pathgroups(&mp0, NULL, NULL, NULL, NULL, 0);
}
static void test_group_by_serial_null(void **state)
{
mp_null.pgpolicyfn = group_by_serial;
assert_int_equal(group_paths(&mp_null), 0);
- verify_pathgroups(&mp_null, NULL, NULL, NULL, 0);
+ verify_pathgroups(&mp_null, NULL, NULL, NULL, NULL, 0);
+}
+
+static void test_group_by_serial_2_groups8_all_marginal8(void **state)
+{
+ char *serial[] = {"1", "2", "1", "1", "2", "2", "1", "2"};
+ int marginal[] = {1,1,1,1,1,1,1,1};
+ int prio[] = {3,2,2,1,2,2,1,2};
+ int group0[] = {1,4,5,7};
+ int group1[] = {0,2,3,6};
+ int *groups[] = {group0, group1};
+ int group_size[] = {4,4};
+ int group_marginal[] = {1,1};
+
+ set_priority(p8, prio, 8);
+ set_serial(p8, serial, 8);
+ set_marginal(p8, marginal, 8);
+ mp8.pgpolicyfn = group_by_serial;
+ assert_int_equal(group_paths(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 2);
+}
+
+static void test_group_by_serial_2_groups8_half_marginal8(void **state)
+{
+ char *serial[] = {"1", "2", "1", "1", "2", "2", "1", "2"};
+ int marginal[] = {0,0,1,1,1,1,0,0};
+ int prio[] = {3,2,2,1,2,2,1,2};
+ int group0[] = {0,6};
+ int group1[] = {1,7};
+ int group2[] = {4,5};
+ int group3[] = {2,3};
+ int *groups[] = {group0, group1, group2, group3};
+ int group_size[] = {2,2,2,2};
+ int group_marginal[] = {0,0,1,1};
+
+ set_priority(p8, prio, 8);
+ set_serial(p8, serial, 8);
+ set_marginal(p8, marginal, 8);
+ mp8.pgpolicyfn = group_by_serial;
+ assert_int_equal(group_paths(&mp8), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 4);
}
#define setup_test(name, nr) \
@@ -700,6 +941,9 @@ int test_pgpolicies(void)
setup_test(test_one_group, 1),
setup_test(test_one_group, 0),
setup_test(test_one_group, _null),
+ setup_test(test_one_group_all_marginal, 8),
+ setup_test(test_one_group_half_marginal, 8),
+ setup_test(test_one_group_one_marginal, 8),
setup_test(test_one_path_per_group_same, 8),
setup_test(test_one_path_per_group_increasing, 8),
setup_test(test_one_path_per_group_decreasing, 8),
@@ -708,6 +952,8 @@ int test_pgpolicies(void)
setup_test(test_one_path_per_group, 1),
setup_test(test_one_path_per_group, 0),
setup_test(test_one_path_per_group, _null),
+ setup_test(test_one_path_per_group_mixed_all_marginal, 8),
+ setup_test(test_one_path_per_group_mixed_half_marginal, 8),
setup_test(test_group_by_prio_same, 8),
setup_test(test_group_by_prio_increasing, 8),
setup_test(test_group_by_prio_decreasing, 8),
@@ -718,6 +964,9 @@ int test_pgpolicies(void)
setup_test(test_group_by_prio, 1),
setup_test(test_group_by_prio, 0),
setup_test(test_group_by_prio, _null),
+ setup_test(test_group_by_prio_mixed_all_marginal, 8),
+ setup_test(test_group_by_prio_mixed_half_marginal, 8),
+ setup_test(test_group_by_prio_mixed_one_marginal, 8),
setup_test(test_group_by_node_name_same, 8),
setup_test(test_group_by_node_name_increasing, 8),
setup_test(test_group_by_node_name_3_groups, 8),
@@ -727,6 +976,8 @@ int test_pgpolicies(void)
setup_test(test_group_by_node_name, 1),
setup_test(test_group_by_node_name, 0),
setup_test(test_group_by_node_name, _null),
+ setup_test(test_group_by_node_name_2_groups_all_marginal, 8),
+ setup_test(test_group_by_node_name_2_groups_half_marginal, 8),
setup_test(test_group_by_serial_same, 8),
setup_test(test_group_by_serial_increasing, 8),
setup_test(test_group_by_serial_3_groups, 8),
@@ -736,6 +987,8 @@ int test_pgpolicies(void)
setup_test(test_group_by_serial, 1),
setup_test(test_group_by_serial, 0),
setup_test(test_group_by_serial, _null),
+ setup_test(test_group_by_serial_2_groups8_all_marginal, 8),
+ setup_test(test_group_by_serial_2_groups8_half_marginal, 8),
};
return cmocka_run_group_tests(tests, setup, NULL);
}
--
2.17.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 11/14] libmultipath: add marginal_pathgroups config option
2019-08-23 17:48 [PATCH v2 00/14] multipath marginal pathgroups Benjamin Marzinski
` (9 preceding siblings ...)
2019-08-23 17:48 ` [PATCH v2 10/14] tests: add tests for grouping " Benjamin Marzinski
@ 2019-08-23 17:48 ` Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 12/14] libmutipath: deprecate delay_*_checks Benjamin Marzinski
` (3 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Benjamin Marzinski @ 2019-08-23 17:48 UTC (permalink / raw)
To: Christophe Varoqui
Cc: device-mapper development, Muneendra Kumar, Martin Wilck
group_paths now gets passed this to determine whether to enable
marginal pathgroups. The unit tests have also been updated.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/config.h | 1 +
libmultipath/configure.c | 5 +-
libmultipath/dict.c | 3 +
libmultipath/pgpolicies.c | 5 +-
libmultipath/pgpolicies.h | 2 +-
tests/pgpolicy.c | 140 +++++++++++++++++++++++---------------
6 files changed, 98 insertions(+), 58 deletions(-)
diff --git a/libmultipath/config.h b/libmultipath/config.h
index ff2b4e86..0b978970 100644
--- a/libmultipath/config.h
+++ b/libmultipath/config.h
@@ -186,6 +186,7 @@ struct config {
int max_sectors_kb;
int ghost_delay;
int find_multipaths_timeout;
+ int marginal_pathgroups;
unsigned int version[3];
char * multipath_dir;
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 3c309d64..3238d485 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -297,7 +297,7 @@ int setup_map(struct multipath *mpp, char *params, int params_size,
{
struct pathgroup * pgp;
struct config *conf;
- int i, n_paths;
+ int i, n_paths, marginal_pathgroups;
/*
* don't bother if devmap size is unknown
@@ -357,6 +357,7 @@ int setup_map(struct multipath *mpp, char *params, int params_size,
select_flush_on_last_del(conf, mpp);
sysfs_set_scsi_tmo(mpp, conf->checkint);
+ marginal_pathgroups = conf->marginal_pathgroups;
pthread_cleanup_pop(1);
if (marginal_path_check_enabled(mpp)) {
@@ -387,7 +388,7 @@ int setup_map(struct multipath *mpp, char *params, int params_size,
vector_free(mpp->pg);
mpp->pg = NULL;
}
- if (group_paths(mpp))
+ if (group_paths(mpp, marginal_pathgroups))
return 1;
/*
diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index c6eba0f6..b5feb884 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -1339,6 +1339,8 @@ declare_ovr_snprint(all_tg_pt, print_yes_no_undef)
declare_hw_handler(all_tg_pt, set_yes_no_undef)
declare_hw_snprint(all_tg_pt, print_yes_no_undef)
+declare_def_handler(marginal_pathgroups, set_yes_no)
+declare_def_snprint(marginal_pathgroups, print_yes_no)
static int
def_uxsock_timeout_handler(struct config *conf, vector strvec)
@@ -1710,6 +1712,7 @@ init_keywords(vector keywords)
install_keyword("find_multipaths_timeout",
&def_find_multipaths_timeout_handler,
&snprint_def_find_multipaths_timeout);
+ install_keyword("marginal_pathgroups", &def_marginal_pathgroups_handler, &snprint_def_marginal_pathgroups);
__deprecated install_keyword("default_selector", &def_selector_handler, NULL);
__deprecated install_keyword("default_path_grouping_policy", &def_pgpolicy_handler, NULL);
__deprecated install_keyword("default_uid_attribute", &def_uid_attribute_handler, NULL);
diff --git a/libmultipath/pgpolicies.c b/libmultipath/pgpolicies.c
index 6fb2d28a..8f7c6b13 100644
--- a/libmultipath/pgpolicies.c
+++ b/libmultipath/pgpolicies.c
@@ -131,7 +131,7 @@ fail:
return -1;
}
-int group_paths(struct multipath *mp)
+int group_paths(struct multipath *mp, int marginal_pathgroups)
{
vector normal, marginal;
@@ -145,7 +145,8 @@ int group_paths(struct multipath *mp)
if (!mp->pgpolicyfn)
goto fail;
- if (split_marginal_paths(mp->paths, &normal, &marginal) != 0) {
+ if (!marginal_pathgroups ||
+ split_marginal_paths(mp->paths, &normal, &marginal) != 0) {
if (mp->pgpolicyfn(mp, mp->paths) != 0)
goto fail;
} else {
diff --git a/libmultipath/pgpolicies.h b/libmultipath/pgpolicies.h
index 7532d75f..15927610 100644
--- a/libmultipath/pgpolicies.h
+++ b/libmultipath/pgpolicies.h
@@ -21,7 +21,7 @@ enum iopolicies {
int get_pgpolicy_id(char *);
int get_pgpolicy_name (char *, int, int);
-int group_paths(struct multipath *);
+int group_paths(struct multipath *, int);
/*
* policies
*/
diff --git a/tests/pgpolicy.c b/tests/pgpolicy.c
index ab09f91c..3f61b123 100644
--- a/tests/pgpolicy.c
+++ b/tests/pgpolicy.c
@@ -204,7 +204,7 @@ static void test_one_group8(void **state)
int group_size[] = {8};
mp8.pgpolicyfn = one_group;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 0), 0);
verify_pathgroups(&mp8, p8, groups, group_size, NULL, 1);
}
@@ -215,7 +215,7 @@ static void test_one_group4(void **state)
int group_size[] = {4};
mp4.pgpolicyfn = one_group;
- assert_int_equal(group_paths(&mp4), 0);
+ assert_int_equal(group_paths(&mp4, 0), 0);
verify_pathgroups(&mp4, p4, groups, group_size, NULL, 1);
}
@@ -226,21 +226,21 @@ static void test_one_group1(void **state)
int group_size[] = {1};
mp1.pgpolicyfn = one_group;
- assert_int_equal(group_paths(&mp1), 0);
+ assert_int_equal(group_paths(&mp1, 0), 0);
verify_pathgroups(&mp1, p1, groups, group_size, NULL, 1);
}
static void test_one_group0(void **state)
{
mp0.pgpolicyfn = one_group;
- assert_int_equal(group_paths(&mp0), 0);
+ assert_int_equal(group_paths(&mp0, 0), 0);
verify_pathgroups(&mp0, NULL, NULL, NULL, NULL, 0);
}
static void test_one_group_null(void **state)
{
mp_null.pgpolicyfn = one_group;
- assert_int_equal(group_paths(&mp_null), 0);
+ assert_int_equal(group_paths(&mp_null, 0), 0);
verify_pathgroups(&mp_null, NULL, NULL, NULL, NULL, 0);
}
@@ -254,7 +254,7 @@ static void test_one_group_all_marginal8(void **state)
set_marginal(p8, marginal, 8);
mp8.pgpolicyfn = one_group;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 1), 0);
verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 1);
}
@@ -269,10 +269,23 @@ static void test_one_group_half_marginal8(void **state)
set_marginal(p8, marginal, 8);
mp8.pgpolicyfn = one_group;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 1), 0);
verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 2);
}
+static void test_one_group_ignore_marginal8(void **state)
+{
+ int marginal[] = {1,0,1,0,1,1,0,0};
+ int paths[] = {0,1,2,3,4,5,6,7};
+ int *groups[] = {paths};
+ int group_size[] = {8};
+
+ set_marginal(p8, marginal, 8);
+ mp8.pgpolicyfn = one_group;
+ assert_int_equal(group_paths(&mp8, 0), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, NULL, 1);
+}
+
static void test_one_group_one_marginal8(void **state)
{
int marginal[] = {0,0,0,0,0,1,0,0};
@@ -284,7 +297,7 @@ static void test_one_group_one_marginal8(void **state)
set_marginal(p8, marginal, 8);
mp8.pgpolicyfn = one_group;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 1), 0);
verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 2);
}
@@ -296,7 +309,7 @@ static void test_one_path_per_group_same8(void **state)
int group_size[] = {1,1,1,1,1,1,1,1};
mp8.pgpolicyfn = one_path_per_group;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 0), 0);
verify_pathgroups(&mp8, p8, groups, group_size, NULL, 8);
}
@@ -310,7 +323,7 @@ static void test_one_path_per_group_increasing8(void **state)
set_priority(p8, prio, 8);
mp8.pgpolicyfn = one_path_per_group;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 0), 0);
verify_pathgroups(&mp8, p8, groups, group_size, NULL, 8);
}
@@ -324,7 +337,7 @@ static void test_one_path_per_group_decreasing8(void **state)
set_priority(p8, prio, 8);
mp8.pgpolicyfn = one_path_per_group;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 0), 0);
verify_pathgroups(&mp8, p8, groups, group_size, NULL, 8);
}
@@ -338,7 +351,7 @@ static void test_one_path_per_group_mixed8(void **state)
set_priority(p8, prio, 8);
mp8.pgpolicyfn = one_path_per_group;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 0), 0);
verify_pathgroups(&mp8, p8, groups, group_size, NULL, 8);
}
@@ -349,7 +362,7 @@ static void test_one_path_per_group4(void **state)
int group_size[] = {1,1,1,1};
mp4.pgpolicyfn = one_path_per_group;
- assert_int_equal(group_paths(&mp4), 0);
+ assert_int_equal(group_paths(&mp4, 0), 0);
verify_pathgroups(&mp4, p4, groups, group_size, NULL, 4);
}
@@ -360,21 +373,21 @@ static void test_one_path_per_group1(void **state)
int group_size[] = {1};
mp1.pgpolicyfn = one_path_per_group;
- assert_int_equal(group_paths(&mp1), 0);
+ assert_int_equal(group_paths(&mp1, 0), 0);
verify_pathgroups(&mp1, p1, groups, group_size, NULL, 1);
}
static void test_one_path_per_group0(void **state)
{
mp0.pgpolicyfn = one_path_per_group;
- assert_int_equal(group_paths(&mp0), 0);
+ assert_int_equal(group_paths(&mp0, 0), 0);
verify_pathgroups(&mp0, NULL, NULL, NULL, NULL, 0);
}
static void test_one_path_per_group_null(void **state)
{
mp_null.pgpolicyfn = one_path_per_group;
- assert_int_equal(group_paths(&mp_null), 0);
+ assert_int_equal(group_paths(&mp_null, 0), 0);
verify_pathgroups(&mp_null, NULL, NULL, NULL, NULL, 0);
}
@@ -391,7 +404,7 @@ static void test_one_path_per_group_mixed_all_marginal8(void **state)
set_priority(p8, prio, 8);
set_marginal(p8, marginal, 8);
mp8.pgpolicyfn = one_path_per_group;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 1), 0);
verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 8);
}
@@ -408,7 +421,7 @@ static void test_one_path_per_group_mixed_half_marginal8(void **state)
set_priority(p8, prio, 8);
set_marginal(p8, marginal, 8);
mp8.pgpolicyfn = one_path_per_group;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 1), 0);
verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 8);
}
@@ -419,7 +432,7 @@ static void test_group_by_prio_same8(void **state)
int group_size[] = {8};
mp8.pgpolicyfn = group_by_prio;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 0), 0);
verify_pathgroups(&mp8, p8, groups, group_size, NULL, 1);
}
@@ -433,7 +446,7 @@ static void test_group_by_prio_increasing8(void **state)
set_priority(p8, prio, 8);
mp8.pgpolicyfn = group_by_prio;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 0), 0);
verify_pathgroups(&mp8, p8, groups, group_size, NULL, 8);
}
@@ -447,7 +460,7 @@ static void test_group_by_prio_decreasing8(void **state)
set_priority(p8, prio, 8);
mp8.pgpolicyfn = group_by_prio;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 0), 0);
verify_pathgroups(&mp8, p8, groups, group_size, NULL, 8);
}
@@ -466,7 +479,26 @@ static void test_group_by_prio_mixed8(void **state)
set_priority(p8, prio, 8);
mp8.pgpolicyfn = group_by_prio;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 0), 0);
+ verify_pathgroups(&mp8, p8, groups, group_size, NULL, 6);
+}
+
+static void test_group_by_prio_mixed_no_marginal8(void **state)
+{
+ int prio[] = {7,1,3,3,5,2,8,2};
+ int group0[] = {6};
+ int group1[] = {0};
+ int group2[] = {4};
+ int group3[] = {2,3};
+ int group4[] = {5,7};
+ int group5[] = {1};
+ int *groups[] = {group0, group1, group2, group3,
+ group4, group5};
+ int group_size[] = {1,1,1,2,2,1};
+
+ set_priority(p8, prio, 8);
+ mp8.pgpolicyfn = group_by_prio;
+ assert_int_equal(group_paths(&mp8, 1), 0);
verify_pathgroups(&mp8, p8, groups, group_size, NULL, 6);
}
@@ -480,7 +512,7 @@ static void test_group_by_prio_2_groups8(void **state)
set_priority(p8, prio, 8);
mp8.pgpolicyfn = group_by_prio;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 0), 0);
verify_pathgroups(&mp8, p8, groups, group_size, NULL, 2);
}
@@ -495,7 +527,7 @@ static void test_group_by_prio_mixed4(void **state)
set_priority(p4, prio, 4);
mp4.pgpolicyfn = group_by_prio;
- assert_int_equal(group_paths(&mp4), 0);
+ assert_int_equal(group_paths(&mp4, 0), 0);
verify_pathgroups(&mp4, p4, groups, group_size, NULL, 3);
}
@@ -509,7 +541,7 @@ static void test_group_by_prio_2_groups4(void **state)
set_priority(p4, prio, 4);
mp4.pgpolicyfn = group_by_prio;
- assert_int_equal(group_paths(&mp4), 0);
+ assert_int_equal(group_paths(&mp4, 0), 0);
verify_pathgroups(&mp4, p4, groups, group_size, NULL, 2);
}
@@ -520,21 +552,21 @@ static void test_group_by_prio1(void **state)
int group_size[] = {1};
mp1.pgpolicyfn = group_by_prio;
- assert_int_equal(group_paths(&mp1), 0);
+ assert_int_equal(group_paths(&mp1, 0), 0);
verify_pathgroups(&mp1, p1, groups, group_size, NULL, 1);
}
static void test_group_by_prio0(void **state)
{
mp0.pgpolicyfn = group_by_prio;
- assert_int_equal(group_paths(&mp0), 0);
+ assert_int_equal(group_paths(&mp0, 0), 0);
verify_pathgroups(&mp0, NULL, NULL, NULL, NULL, 0);
}
static void test_group_by_prio_null(void **state)
{
mp_null.pgpolicyfn = group_by_prio;
- assert_int_equal(group_paths(&mp_null), 0);
+ assert_int_equal(group_paths(&mp_null, 0), 0);
verify_pathgroups(&mp_null, NULL, NULL, NULL, NULL, 0);
}
@@ -556,7 +588,7 @@ static void test_group_by_prio_mixed_all_marginal8(void **state)
set_priority(p8, prio, 8);
set_marginal(p8, marginal, 8);
mp8.pgpolicyfn = group_by_prio;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 1), 0);
verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 6);
}
@@ -579,7 +611,7 @@ static void test_group_by_prio_mixed_half_marginal8(void **state)
set_priority(p8, prio, 8);
set_marginal(p8, marginal, 8);
mp8.pgpolicyfn = group_by_prio;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 1), 0);
verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 7);
}
@@ -602,7 +634,7 @@ static void test_group_by_prio_mixed_one_marginal8(void **state)
set_priority(p8, prio, 8);
set_marginal(p8, marginal, 8);
mp8.pgpolicyfn = group_by_prio;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 1), 0);
verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 7);
}
@@ -615,7 +647,7 @@ static void test_group_by_node_name_same8(void **state)
set_tgt_node_name(p8, node_name, 8);
mp8.pgpolicyfn = group_by_node_name;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 0), 0);
verify_pathgroups(&mp8, p8, groups, group_size, NULL, 1);
}
@@ -631,7 +663,7 @@ static void test_group_by_node_name_increasing8(void **state)
set_priority(p8, prio, 8);
set_tgt_node_name(p8, node_name, 8);
mp8.pgpolicyfn = group_by_node_name;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 0), 0);
verify_pathgroups(&mp8, p8, groups, group_size, NULL, 8);
}
@@ -648,7 +680,7 @@ static void test_group_by_node_name_3_groups8(void **state)
set_priority(p8, prio, 8);
set_tgt_node_name(p8, node_name, 8);
mp8.pgpolicyfn = group_by_node_name;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 0), 0);
verify_pathgroups(&mp8, p8, groups, group_size, NULL, 3);
}
@@ -664,7 +696,7 @@ static void test_group_by_node_name_2_groups8(void **state)
set_priority(p8, prio, 8);
set_tgt_node_name(p8, node_name, 8);
mp8.pgpolicyfn = group_by_node_name;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 0), 0);
verify_pathgroups(&mp8, p8, groups, group_size, NULL, 2);
}
@@ -681,7 +713,7 @@ static void test_group_by_node_name_3_groups4(void **state)
set_priority(p4, prio, 4);
set_tgt_node_name(p4, node_name, 4);
mp4.pgpolicyfn = group_by_node_name;
- assert_int_equal(group_paths(&mp4), 0);
+ assert_int_equal(group_paths(&mp4, 0), 0);
verify_pathgroups(&mp4, p4, groups, group_size, NULL, 3);
}
@@ -697,7 +729,7 @@ static void test_group_by_node_name_2_groups4(void **state)
set_priority(p4, prio, 4);
set_tgt_node_name(p4, node_name, 4);
mp4.pgpolicyfn = group_by_node_name;
- assert_int_equal(group_paths(&mp4), 0);
+ assert_int_equal(group_paths(&mp4, 0), 0);
verify_pathgroups(&mp4, p4, groups, group_size, NULL, 2);
}
@@ -710,21 +742,21 @@ static void test_group_by_node_name1(void **state)
set_tgt_node_name(p1, node_name, 1);
mp1.pgpolicyfn = group_by_node_name;
- assert_int_equal(group_paths(&mp1), 0);
+ assert_int_equal(group_paths(&mp1,0), 0);
verify_pathgroups(&mp1, p1, groups, group_size, NULL, 1);
}
static void test_group_by_node_name0(void **state)
{
mp0.pgpolicyfn = group_by_node_name;
- assert_int_equal(group_paths(&mp0), 0);
+ assert_int_equal(group_paths(&mp0, 0), 0);
verify_pathgroups(&mp0, NULL, NULL, NULL, NULL, 0);
}
static void test_group_by_node_name_null(void **state)
{
mp_null.pgpolicyfn = group_by_node_name;
- assert_int_equal(group_paths(&mp_null), 0);
+ assert_int_equal(group_paths(&mp_null, 0), 0);
verify_pathgroups(&mp_null, NULL, NULL, NULL, NULL, 0);
}
@@ -743,7 +775,7 @@ static void test_group_by_node_name_2_groups_all_marginal8(void **state)
set_marginal(p8, marginal, 8);
set_tgt_node_name(p8, node_name, 8);
mp8.pgpolicyfn = group_by_node_name;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 1), 0);
verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 2);
}
@@ -764,7 +796,7 @@ static void test_group_by_node_name_2_groups_half_marginal8(void **state)
set_marginal(p8, marginal, 8);
set_tgt_node_name(p8, node_name, 8);
mp8.pgpolicyfn = group_by_node_name;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 1), 0);
verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 4);
}
@@ -777,7 +809,7 @@ static void test_group_by_serial_same8(void **state)
set_serial(p8, serial, 8);
mp8.pgpolicyfn = group_by_serial;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 0), 0);
verify_pathgroups(&mp8, p8, groups, group_size, NULL, 1);
}
@@ -793,7 +825,7 @@ static void test_group_by_serial_increasing8(void **state)
set_priority(p8, prio, 8);
set_serial(p8, serial, 8);
mp8.pgpolicyfn = group_by_serial;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 0), 0);
verify_pathgroups(&mp8, p8, groups, group_size, NULL, 8);
}
@@ -810,7 +842,7 @@ static void test_group_by_serial_3_groups8(void **state)
set_priority(p8, prio, 8);
set_serial(p8, serial, 8);
mp8.pgpolicyfn = group_by_serial;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 0), 0);
verify_pathgroups(&mp8, p8, groups, group_size, NULL, 3);
}
@@ -826,7 +858,7 @@ static void test_group_by_serial_2_groups8(void **state)
set_priority(p8, prio, 8);
set_serial(p8, serial, 8);
mp8.pgpolicyfn = group_by_serial;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 0), 0);
verify_pathgroups(&mp8, p8, groups, group_size, NULL, 2);
}
@@ -843,7 +875,7 @@ static void test_group_by_serial_3_groups4(void **state)
set_priority(p4, prio, 4);
set_serial(p4, serial, 4);
mp4.pgpolicyfn = group_by_serial;
- assert_int_equal(group_paths(&mp4), 0);
+ assert_int_equal(group_paths(&mp4, 0), 0);
verify_pathgroups(&mp4, p4, groups, group_size, NULL, 3);
}
@@ -859,7 +891,7 @@ static void test_group_by_serial_2_groups4(void **state)
set_priority(p4, prio, 4);
set_serial(p4, serial, 4);
mp4.pgpolicyfn = group_by_serial;
- assert_int_equal(group_paths(&mp4), 0);
+ assert_int_equal(group_paths(&mp4, 0), 0);
verify_pathgroups(&mp4, p4, groups, group_size, NULL, 2);
}
@@ -872,21 +904,21 @@ static void test_group_by_serial1(void **state)
set_serial(p1, serial, 1);
mp1.pgpolicyfn = group_by_serial;
- assert_int_equal(group_paths(&mp1), 0);
+ assert_int_equal(group_paths(&mp1, 0), 0);
verify_pathgroups(&mp1, p1, groups, group_size, NULL, 1);
}
static void test_group_by_serial0(void **state)
{
mp0.pgpolicyfn = group_by_serial;
- assert_int_equal(group_paths(&mp0), 0);
+ assert_int_equal(group_paths(&mp0, 0), 0);
verify_pathgroups(&mp0, NULL, NULL, NULL, NULL, 0);
}
static void test_group_by_serial_null(void **state)
{
mp_null.pgpolicyfn = group_by_serial;
- assert_int_equal(group_paths(&mp_null), 0);
+ assert_int_equal(group_paths(&mp_null, 0), 0);
verify_pathgroups(&mp_null, NULL, NULL, NULL, NULL, 0);
}
@@ -905,7 +937,7 @@ static void test_group_by_serial_2_groups8_all_marginal8(void **state)
set_serial(p8, serial, 8);
set_marginal(p8, marginal, 8);
mp8.pgpolicyfn = group_by_serial;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 1), 0);
verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 2);
}
@@ -926,7 +958,7 @@ static void test_group_by_serial_2_groups8_half_marginal8(void **state)
set_serial(p8, serial, 8);
set_marginal(p8, marginal, 8);
mp8.pgpolicyfn = group_by_serial;
- assert_int_equal(group_paths(&mp8), 0);
+ assert_int_equal(group_paths(&mp8, 1), 0);
verify_pathgroups(&mp8, p8, groups, group_size, group_marginal, 4);
}
@@ -943,6 +975,7 @@ int test_pgpolicies(void)
setup_test(test_one_group, _null),
setup_test(test_one_group_all_marginal, 8),
setup_test(test_one_group_half_marginal, 8),
+ setup_test(test_one_group_ignore_marginal, 8),
setup_test(test_one_group_one_marginal, 8),
setup_test(test_one_path_per_group_same, 8),
setup_test(test_one_path_per_group_increasing, 8),
@@ -958,6 +991,7 @@ int test_pgpolicies(void)
setup_test(test_group_by_prio_increasing, 8),
setup_test(test_group_by_prio_decreasing, 8),
setup_test(test_group_by_prio_mixed, 8),
+ setup_test(test_group_by_prio_mixed_no_marginal, 8),
setup_test(test_group_by_prio_2_groups, 8),
setup_test(test_group_by_prio_mixed, 4),
setup_test(test_group_by_prio_2_groups, 4),
--
2.17.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 12/14] libmutipath: deprecate delay_*_checks
2019-08-23 17:48 [PATCH v2 00/14] multipath marginal pathgroups Benjamin Marzinski
` (10 preceding siblings ...)
2019-08-23 17:48 ` [PATCH v2 11/14] libmultipath: add marginal_pathgroups config option Benjamin Marzinski
@ 2019-08-23 17:48 ` Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 13/14] multipathd: use marginal_pathgroups Benjamin Marzinski
` (2 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: Benjamin Marzinski @ 2019-08-23 17:48 UTC (permalink / raw)
To: Christophe Varoqui
Cc: device-mapper development, Muneendra Kumar, Martin Wilck
The delay_checks shaky paths detection method works the same way as the
san_path_err method, but not as well, with less configurability, and
with the code spread all over check_path(). The only real difference is
that marks the path as marginal for a certain number of path checks
instead of for a specific time. This patch deprecates the delay_checks
method and maps it to the the san_path_err method.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/configure.c | 18 +-------
libmultipath/propsel.c | 89 ++++++++++++++++++++++++++++----------
libmultipath/propsel.h | 3 +-
libmultipath/structs.h | 10 -----
multipath/multipath.conf.5 | 40 +++++++++--------
multipathd/main.c | 34 ++-------------
6 files changed, 96 insertions(+), 98 deletions(-)
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 3238d485..9897cc37 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -342,8 +342,6 @@ int setup_map(struct multipath *mpp, char *params, int params_size,
select_dev_loss(conf, mpp);
select_reservation_key(conf, mpp);
select_deferred_remove(conf, mpp);
- select_delay_watch_checks(conf, mpp);
- select_delay_wait_checks(conf, mpp);
select_marginal_path_err_sample_time(conf, mpp);
select_marginal_path_err_rate_threshold(conf, mpp);
select_marginal_path_err_recheck_gap_time(conf, mpp);
@@ -351,6 +349,7 @@ int setup_map(struct multipath *mpp, char *params, int params_size,
select_san_path_err_threshold(conf, mpp);
select_san_path_err_forget_rate(conf, mpp);
select_san_path_err_recovery_time(conf, mpp);
+ select_delay_checks(conf, mpp);
select_skip_kpartx(conf, mpp);
select_max_sectors_kb(conf, mpp);
select_ghost_delay(conf, mpp);
@@ -360,21 +359,8 @@ int setup_map(struct multipath *mpp, char *params, int params_size,
marginal_pathgroups = conf->marginal_pathgroups;
pthread_cleanup_pop(1);
- if (marginal_path_check_enabled(mpp)) {
- if (delay_check_enabled(mpp)) {
- condlog(1, "%s: WARNING: both marginal_path and delay_checks error detection selected",
- mpp->alias);
- condlog(0, "%s: unexpected behavior may occur!",
- mpp->alias);
- }
+ if (marginal_path_check_enabled(mpp))
start_io_err_stat_thread(vecs);
- }
- if (san_path_check_enabled(mpp) && delay_check_enabled(mpp)) {
- condlog(1, "%s: WARNING: both san_path_err and delay_checks error detection selected",
- mpp->alias);
- condlog(0, "%s: unexpected behavior may occur!",
- mpp->alias);
- }
n_paths = VECTOR_SIZE(mpp->paths);
/*
diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index 6af2513d..27e8d68a 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -85,6 +85,10 @@ static const char autodetect_origin[] =
"(setting: storage device autodetected)";
static const char marginal_path_origin[] =
"(setting: implied by marginal_path check)";
+static const char delay_watch_origin[] =
+ "(setting: implied by delay_watch_checks)";
+static const char delay_wait_origin[] =
+ "(setting: implied by delay_wait_checks)";
#define do_default(dest, value) \
do { \
@@ -877,39 +881,80 @@ out:
return 0;
}
-int select_delay_watch_checks(struct config *conf, struct multipath *mp)
+static inline int san_path_check_options_set(const struct multipath *mp)
{
- const char *origin;
+ return mp->san_path_err_threshold > 0 ||
+ mp->san_path_err_forget_rate > 0 ||
+ mp->san_path_err_recovery_time > 0;
+}
+
+static int
+use_delay_watch_checks(struct config *conf, struct multipath *mp)
+{
+ int value = NU_UNDEF;
+ const char *origin = default_origin;
char buff[12];
- mp_set_mpe(delay_watch_checks);
- mp_set_ovr(delay_watch_checks);
- mp_set_hwe(delay_watch_checks);
- mp_set_conf(delay_watch_checks);
- mp_set_default(delay_watch_checks, DEFAULT_DELAY_CHECKS);
+ do_set(delay_watch_checks, mp->mpe, value, multipaths_origin);
+ do_set(delay_watch_checks, conf->overrides, value, overrides_origin);
+ do_set_from_hwe(delay_watch_checks, mp, value, hwe_origin);
+ do_set(delay_watch_checks, conf, value, conf_origin);
out:
- if (print_off_int_undef(buff, 12, mp->delay_watch_checks) != 0)
- condlog(3, "%s: delay_watch_checks = %s %s",
- mp->alias, buff, origin);
- return 0;
+ if (print_off_int_undef(buff, 12, value) != 0)
+ condlog(3, "%s: delay_watch_checks = %s %s", mp->alias, buff,
+ origin);
+ return value;
}
-int select_delay_wait_checks(struct config *conf, struct multipath *mp)
+static int
+use_delay_wait_checks(struct config *conf, struct multipath *mp)
{
- const char *origin;
+ int value = NU_UNDEF;
+ const char *origin = default_origin;
char buff[12];
- mp_set_mpe(delay_wait_checks);
- mp_set_ovr(delay_wait_checks);
- mp_set_hwe(delay_wait_checks);
- mp_set_conf(delay_wait_checks);
- mp_set_default(delay_wait_checks, DEFAULT_DELAY_CHECKS);
+ do_set(delay_wait_checks, mp->mpe, value, multipaths_origin);
+ do_set(delay_wait_checks, conf->overrides, value, overrides_origin);
+ do_set_from_hwe(delay_wait_checks, mp, value, hwe_origin);
+ do_set(delay_wait_checks, conf, value, conf_origin);
out:
- if (print_off_int_undef(buff, 12, mp->delay_wait_checks) != 0)
- condlog(3, "%s: delay_wait_checks = %s %s",
- mp->alias, buff, origin);
- return 0;
+ if (print_off_int_undef(buff, 12, value) != 0)
+ condlog(3, "%s: delay_wait_checks = %s %s", mp->alias, buff,
+ origin);
+ return value;
+}
+
+int select_delay_checks(struct config *conf, struct multipath *mp)
+{
+ int watch_checks, wait_checks;
+ char buff[12];
+ watch_checks = use_delay_watch_checks(conf, mp);
+ wait_checks = use_delay_wait_checks(conf, mp);
+ if (watch_checks <= 0 && wait_checks <= 0)
+ return 0;
+ if (san_path_check_options_set(mp)) {
+ condlog(3, "%s: both marginal_path and delay_checks error detection options selected", mp->alias);
+ condlog(3, "%s: ignoring delay_checks options", mp->alias);
+ return 0;
+ }
+ mp->san_path_err_threshold = 1;
+ condlog(3, "%s: san_path_err_threshold = 1 %s", mp->alias,
+ (watch_checks > 0)? delay_watch_origin : delay_wait_origin);
+ if (watch_checks > 0) {
+ mp->san_path_err_forget_rate = watch_checks;
+ print_off_int_undef(buff, 12, mp->san_path_err_forget_rate);
+ condlog(3, "%s: san_path_err_forget_rate = %s %s", mp->alias,
+ buff, delay_watch_origin);
+ }
+ if (wait_checks > 0) {
+ mp->san_path_err_recovery_time = wait_checks *
+ conf->max_checkint;
+ print_off_int_undef(buff, 12, mp->san_path_err_recovery_time);
+ condlog(3, "%s: san_path_err_recovery_time = %s %s", mp->alias,
+ buff, delay_wait_origin);
+ }
+ return 0;
}
static int san_path_deprecated_warned;
diff --git a/libmultipath/propsel.h b/libmultipath/propsel.h
index b352c16a..ddfd6262 100644
--- a/libmultipath/propsel.h
+++ b/libmultipath/propsel.h
@@ -22,8 +22,7 @@ int select_retain_hwhandler (struct config *conf, struct multipath * mp);
int select_detect_prio(struct config *conf, struct path * pp);
int select_detect_checker(struct config *conf, struct path * pp);
int select_deferred_remove(struct config *conf, struct multipath *mp);
-int select_delay_watch_checks (struct config *conf, struct multipath * mp);
-int select_delay_wait_checks (struct config *conf, struct multipath * mp);
+int select_delay_checks(struct config *conf, struct multipath * mp);
int select_skip_kpartx (struct config *conf, struct multipath * mp);
int select_max_sectors_kb (struct config *conf, struct multipath * mp);
int select_san_path_err_forget_rate(struct config *conf, struct multipath *mp);
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index a8b9d325..a3adf906 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -268,8 +268,6 @@ struct path {
int pgindex;
int detect_prio;
int detect_checker;
- int watch_checks;
- int wait_checks;
int tpgs;
char * uid_attribute;
char * getuid;
@@ -321,8 +319,6 @@ struct multipath {
int fast_io_fail;
int retain_hwhandler;
int deferred_remove;
- int delay_watch_checks;
- int delay_wait_checks;
int san_path_err_threshold;
int san_path_err_forget_rate;
int san_path_err_recovery_time;
@@ -393,12 +389,6 @@ static inline int san_path_check_enabled(const struct multipath *mpp)
mpp->san_path_err_recovery_time > 0;
}
-static inline int delay_check_enabled(const struct multipath *mpp)
-{
- return mpp->delay_watch_checks != NU_NO ||
- mpp->delay_wait_checks != NU_NO;
-}
-
struct pathgroup {
long id;
int status;
diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index f7d21b4c..08297a41 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -1013,10 +1013,12 @@ The default is: \fBno\fR
.
.TP
.B delay_watch_checks
-If set to a value greater than 0, multipathd will watch paths that have
-recently become valid for this many checks. If they fail again while they are
-being watched, when they next become valid, they will not be used until they
-have stayed up for \fIdelay_wait_checks\fR checks. See "Shaky paths detection" below.
+This option is \fBdeprecated\fR, and mapped to \fIsan_path_err_forget_rate\fR.
+If this is set to a value greater than 0 and no \fIsan_path_err\fR options
+are set, \fIsan_path_err_forget_rate\fR will be set to the value of
+\fIdelay_watch_checks\fR and \fIsan_path_err_threshold\fR will be set to 1.
+See the \fIsan_path_err_forget_rate\fR and \fIsan_path_err_threshold\fR
+options, and "Shaky paths detection" below for more information.
.RS
.TP
The default is: \fBno\fR
@@ -1025,10 +1027,14 @@ The default is: \fBno\fR
.
.TP
.B delay_wait_checks
-If set to a value greater than 0, when a device that has recently come back
-online fails again within \fIdelay_watch_checks\fR checks, the next time it
-comes back online, it will marked and delayed, and not used until it has passed
-\fIdelay_wait_checks\fR checks. See "Shaky paths detection" below.
+This option is \fBdeprecated\fR, and mapped to \fIsan_path_err_recovery_time\fR.
+If this is set to a value greater than 0 and no \fIsan_path_err\fR options
+are set, \fIsan_path_err_recovery_time\fR will be set to the value of
+\fIdelay_wait_checks\fR times \fImax_polling_interval\fR. This will give
+approximately the same wait time as delay_wait_checks previously did.
+Also, \fIsan_path_err_threshold\fR will be set to 1. See the
+\fIsan_path_err_recovery_time\fR and \fIsan_path_err_threshold\fR
+options, and "Shaky paths detection" below for more information.
.RS
.TP
The default is: \fBno\fR
@@ -1689,13 +1695,10 @@ if the healthy state appears to be stable. The logic of determining
differs between the three methods.
.TP 8
.B \(dqdelay_checks\(dq failure tracking
-If a path fails again within a
-\fIdelay_watch_checks\fR interval after a failure, don't
-reinstate it until it passes a \fIdelay_wait_checks\fR interval
-in always good status.
-The intervals are measured in \(dqticks\(dq, i.e. the
-time between path checks by multipathd, which is variable and controlled by the
-\fIpolling_interval\fR and \fImax_polling_interval\fR parameters.
+This method is \fBdeprecated\fR and mapped to the \(dqsan_path_err\(dq method.
+See the \fIdelay_watch_checks\fR and \fIdelay_wait_checks\fR options above
+for more information.
+
.TP
.B \(dqmarginal_path\(dq failure tracking
If a second failure event (good->bad transition) occurs within
@@ -1712,12 +1715,13 @@ in seconds.
.B \(dqsan_path_err\(dq failure tracking
multipathd counts path failures for each path. Once the number of failures
exceeds the value given by \fIsan_path_err_threshold\fR, the path is not
-reinstated for \fIsan_path_err_recovery_time\fR ticks. While counting
+reinstated for \fIsan_path_err_recovery_time\fR seconds. While counting
failures, multipathd \(dqforgets\(dq one past failure every
\(dqsan_path_err_forget_rate\(dq ticks; thus if errors don't occur more
often then once in the forget rate interval, the failure count doesn't
-increase and the threshold is never reached. As for the \fIdelay_xy\fR method,
-intervals are measured in \(dqticks\(dq.
+increase and the threshold is never reached. Ticks are the time between
+path checks by multipathd, which is variable and controlled by the
+\fIpolling_interval\fR and \fImax_polling_interval\fR parameters.
.
.RS 8
.LP
diff --git a/multipathd/main.c b/multipathd/main.c
index 7db15736..dca2214c 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -2122,16 +2122,6 @@ check_path (struct vectors * vecs, struct path * pp, int ticks)
return 1;
}
- if ((newstate == PATH_UP || newstate == PATH_GHOST) &&
- pp->wait_checks > 0) {
- if (pp->mpp->nr_active > 0) {
- pp->state = PATH_DELAYED;
- pp->wait_checks--;
- return 1;
- } else
- pp->wait_checks = 0;
- }
-
/*
* don't reinstate failed path, if its in stand-by
* and if target supports only implicit tpgs mode.
@@ -2162,19 +2152,10 @@ check_path (struct vectors * vecs, struct path * pp, int ticks)
* proactively fail path in the DM
*/
if (oldstate == PATH_UP ||
- oldstate == PATH_GHOST) {
+ oldstate == PATH_GHOST)
fail_path(pp, 1);
- if (pp->mpp->delay_wait_checks > 0 &&
- pp->watch_checks > 0) {
- pp->wait_checks = pp->mpp->delay_wait_checks;
- pp->watch_checks = 0;
- }
- } else {
+ else
fail_path(pp, 0);
- if (pp->wait_checks > 0)
- pp->wait_checks =
- pp->mpp->delay_wait_checks;
- }
/*
* cancel scheduled failback
@@ -2200,15 +2181,10 @@ check_path (struct vectors * vecs, struct path * pp, int ticks)
* reinstate this path
*/
if (oldstate != PATH_UP &&
- oldstate != PATH_GHOST) {
- if (pp->mpp->delay_watch_checks > 0)
- pp->watch_checks = pp->mpp->delay_watch_checks;
+ oldstate != PATH_GHOST)
add_active = 1;
- } else {
- if (pp->watch_checks > 0)
- pp->watch_checks--;
+ else
add_active = 0;
- }
if (!disable_reinstate && reinstate_path(pp, add_active)) {
condlog(3, "%s: reload map", pp->dev);
ev_add_path(pp, vecs, 1);
@@ -2253,8 +2229,6 @@ check_path (struct vectors * vecs, struct path * pp, int ticks)
condlog(4, "%s: delay next check %is",
pp->dev_t, pp->checkint);
}
- if (pp->watch_checks > 0)
- pp->watch_checks--;
pp->tick = pp->checkint;
}
}
--
2.17.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 13/14] multipathd: use marginal_pathgroups
2019-08-23 17:48 [PATCH v2 00/14] multipath marginal pathgroups Benjamin Marzinski
` (11 preceding siblings ...)
2019-08-23 17:48 ` [PATCH v2 12/14] libmutipath: deprecate delay_*_checks Benjamin Marzinski
@ 2019-08-23 17:48 ` Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 14/14] multipath: update man pages Benjamin Marzinski
2019-08-30 7:09 ` [PATCH v2 00/14] multipath marginal pathgroups Martin Wilck
14 siblings, 0 replies; 16+ messages in thread
From: Benjamin Marzinski @ 2019-08-23 17:48 UTC (permalink / raw)
To: Christophe Varoqui
Cc: device-mapper development, Muneendra Kumar, Martin Wilck
This commit makes the marginal_pathgroups option work with the
existing methods for determining marginal paths. It also merges the
code for the marginal_path and sand_path_err methods. This has the
side effect of making the marginal_path code set a marginal path's state
to "delayed" instead of "shaky" like it previously did.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
multipathd/main.c | 53 +++++++++++++++++++++++++++++++++--------------
1 file changed, 38 insertions(+), 15 deletions(-)
diff --git a/multipathd/main.c b/multipathd/main.c
index dca2214c..04b2b56a 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1960,6 +1960,18 @@ reinstate_path:
return 0;
}
+static int
+should_skip_path(struct path *pp){
+ if (marginal_path_check_enabled(pp->mpp)) {
+ if (pp->io_err_disable_reinstate && need_io_err_check(pp))
+ return 1;
+ } else if (san_path_check_enabled(pp->mpp)) {
+ if (check_path_reinstate_state(pp))
+ return 1;
+ }
+ return 0;
+}
+
/*
* Returns '1' if the path has been checked, '-1' if it was blacklisted
* and '0' otherwise
@@ -1975,6 +1987,7 @@ check_path (struct vectors * vecs, struct path * pp, int ticks)
int oldchkrstate = pp->chkrstate;
int retrigger_tries, checkint, max_checkint, verbosity;
struct config *conf;
+ int marginal_pathgroups, marginal_changed = 0;
int ret;
if ((pp->initialized == INIT_OK ||
@@ -1991,6 +2004,7 @@ check_path (struct vectors * vecs, struct path * pp, int ticks)
checkint = conf->checkint;
max_checkint = conf->max_checkint;
verbosity = conf->verbosity;
+ marginal_pathgroups = conf->marginal_pathgroups;
put_multipath_config(conf);
if (pp->checkint == CHECKINT_UNDEF) {
@@ -2106,20 +2120,27 @@ check_path (struct vectors * vecs, struct path * pp, int ticks)
set_no_path_retry(pp->mpp);
if ((newstate == PATH_UP || newstate == PATH_GHOST) &&
- check_path_reinstate_state(pp)) {
- pp->state = PATH_DELAYED;
- return 1;
- }
-
- if ((newstate == PATH_UP || newstate == PATH_GHOST) &&
- pp->io_err_disable_reinstate && need_io_err_check(pp)) {
- pp->state = PATH_SHAKY;
- /*
- * to reschedule as soon as possible,so that this path can
- * be recoverd in time
- */
- pp->tick = 1;
- return 1;
+ (san_path_check_enabled(pp->mpp) ||
+ marginal_path_check_enabled(pp->mpp))) {
+ int was_marginal = pp->marginal;
+ if (should_skip_path(pp)) {
+ if (!marginal_pathgroups) {
+ if (marginal_path_check_enabled(pp->mpp))
+ /* to reschedule as soon as possible,
+ * so that this path can be recovered
+ * in time */
+ pp->tick = 1;
+ pp->state = PATH_DELAYED;
+ return 1;
+ }
+ if (!was_marginal) {
+ pp->marginal = 1;
+ marginal_changed = 1;
+ }
+ } else if (marginal_pathgroups && was_marginal) {
+ pp->marginal = 0;
+ marginal_changed = 1;
+ }
}
/*
@@ -2258,7 +2279,9 @@ check_path (struct vectors * vecs, struct path * pp, int ticks)
*/
condlog(4, "path prio refresh");
- if (update_prio(pp, new_path_up) &&
+ if (marginal_changed)
+ update_path_groups(pp->mpp, vecs, 1);
+ else if (update_prio(pp, new_path_up) &&
(pp->mpp->pgpolicyfn == (pgpolicyfn *)group_by_prio) &&
pp->mpp->pgfailback == -FAILBACK_IMMEDIATE)
update_path_groups(pp->mpp, vecs, !new_path_up);
--
2.17.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 14/14] multipath: update man pages
2019-08-23 17:48 [PATCH v2 00/14] multipath marginal pathgroups Benjamin Marzinski
` (12 preceding siblings ...)
2019-08-23 17:48 ` [PATCH v2 13/14] multipathd: use marginal_pathgroups Benjamin Marzinski
@ 2019-08-23 17:48 ` Benjamin Marzinski
2019-08-30 7:09 ` [PATCH v2 00/14] multipath marginal pathgroups Martin Wilck
14 siblings, 0 replies; 16+ messages in thread
From: Benjamin Marzinski @ 2019-08-23 17:48 UTC (permalink / raw)
To: Christophe Varoqui
Cc: device-mapper development, Muneendra Kumar, Martin Wilck
Add documentation for the marginal_pathgroups option and the
(un)setmarginal commands.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
multipath/multipath.conf.5 | 34 ++++++++++++++++++++++++++++++----
multipathd/multipathd.8 | 19 +++++++++++++++++++
2 files changed, 49 insertions(+), 4 deletions(-)
diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index 08297a41..ac8eadd0 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -1042,6 +1042,28 @@ The default is: \fBno\fR
.
.
.TP
+.B marginal_pathgroups
+If set to \fIno\fR, the \fIdelay_*_checks\fR, \fImarginal_path_*\fR, and
+\fIsan_path_err_*\fR options will keep marginal, or \(dqshaky\(dq, paths from
+being reinstated until they have been monitored for some time. This can cause
+situations where all non-marginal paths are down, and no paths are usable
+until multipathd detects this and reinstates a marginal path. If the multipath
+device is not configured to queue IO in this case, it can cause IO errors to
+occur, even though there are marginal paths available. However, if this
+option is set to \fIyes\fR, when one of the marginal path detecting methods
+determines that a path is marginal, it will be reinstated and placed in a
+seperate pathgroup that will only be used after all the non-marginal pathgroups
+have been tried first. This prevents the possibility of IO errors occuring
+while marginal paths are still usable. After the path has been monitored
+for the configured time, and is declared healthy, it will be returned to its
+normal pathgroup. See "Shaky paths detection" below for more information.
+.RS
+.TP
+The default is: \fBno\fR
+.RE
+.
+.
+.TP
.B find_multipaths
This option controls whether multipath and multipathd try to create multipath
maps over non-blacklisted devices they encounter. This matters a) when a device is
@@ -1689,10 +1711,14 @@ events. \fImultipathd\fR supports three different methods for detecting this
situation and dealing with it. All methods share the same basic mode of
operation: If a path is found to be \(dqshaky\(dq or \(dqflipping\(dq,
and appears to be in healthy status, it is not reinstated (put back to use)
-immediately. Instead, it is watched for some time, and only reinstated
-if the healthy state appears to be stable. The logic of determining
-\(dqshaky\(dq condition, as well as the logic when to reinstate,
-differs between the three methods.
+immediately. Instead, it is placed in the \(dqdelayed\(dq state and watched
+for some time, and only reinstated if the healthy state appears to be stable.
+If the \fImarginal_pathgroups\fR option is set, the path will reinstated
+immediately, but placed in a special pathgroup for marginal paths. Marginal
+pathgroups will not be used until all other pathgroups have been tried. At the
+time when the path would normally be reinstated, it will be returned to its
+normal pathgroup. The logic of determining \(dqshaky\(dq condition, as well as
+the logic when to reinstate, differs between the three methods.
.TP 8
.B \(dqdelay_checks\(dq failure tracking
This method is \fBdeprecated\fR and mapped to the \(dqsan_path_err\(dq method.
diff --git a/multipathd/multipathd.8 b/multipathd/multipathd.8
index edac7a92..048a838d 100644
--- a/multipathd/multipathd.8
+++ b/multipathd/multipathd.8
@@ -277,6 +277,25 @@ Remove the persistent reservation key associated with $map from the
\fIreservation_key\fR is set to \fBfile\fR in \fI/etc/multipath.conf\fR.
.
.TP
+.B path $path setmarginal
+move $path to a marginal pathgroup. The path will remain in the marginal
+path group until \fIunsetmarginal\fR is called. This command will only
+work if \fImarginal_pathgroups\fR is enabled and there is no Shaky paths
+detection method configured (see the multipath.conf man page for details).
+.
+.TP
+.B path $path unsetmarginal
+return marginal path $path to its normal pathgroup. This command will only
+work if \fImarginal_pathgroups\fR is enabled and there is no Shaky paths
+detection method configured (see the multipath.conf man page for details).
+.
+.TP
+.B map $map unsetmarginal
+return all marginal paths in $map to their normal pathgroups. This command
+will only work if \fImarginal_pathgroups\fR is enabled and there is no Shaky
+paths detection method configured (see the multipath.conf man page for details).
+.
+.TP
.B quit|exit
End interactive session.
.
--
2.17.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 00/14] multipath marginal pathgroups
2019-08-23 17:48 [PATCH v2 00/14] multipath marginal pathgroups Benjamin Marzinski
` (13 preceding siblings ...)
2019-08-23 17:48 ` [PATCH v2 14/14] multipath: update man pages Benjamin Marzinski
@ 2019-08-30 7:09 ` Martin Wilck
14 siblings, 0 replies; 16+ messages in thread
From: Martin Wilck @ 2019-08-30 7:09 UTC (permalink / raw)
To: bmarzins, christophe.varoqui; +Cc: dm-devel, mkumar
On Fri, 2019-08-23 at 12:48 -0500, Benjamin Marzinski wrote:
> The first patch in this series is simply a resend of my previous
> patch
> to fix vector_foreach_slot_backwards(). The rest of the patches are
> related to adding support for an alternative way to deal with
> marginal
> paths. Instead of delaying their reinstatement, which can cause
> problems
> if multipath isn't set to queue IO when there are no usable paths,
> this
> patchset adds a marginal_pathgroups option. If this is set, marginal
> paths will be reinstated, but added to seperate marginal pathgroups.
> They will remain there until the time when they would normally be
> reinstated, at which point they will be returned to their regular
> pathgroups. Marginal pathgroups will have a priority lower than all
> regular pathgroups. This has the advantage of continuing to track
> marginal paths, even if all the other paths go down temporarily, so
> that
> multipath can switch back to the normal paths as soon as they come
> back
> up. This code also allows users to manually move paths between
> marginal
> and regular pathgroups. This is especially important for Broadcom's
> Fiber Channel Transport Daemon, since it doesn't use the multipathd
> marginal path detectors, and thus will not automatically reinstate
> marginal paths when all other paths have failed.
>
> https://www.mail-archive.com/dm-devel@redhat.com/msg12956.html
>
> Changes in v2:
> - patch 04/14 is a combination of the old 04/16, 06/16, and 07/16
> patches, based on Martin's suggestion. Since all the code is the
> same
> as the previous patches, I have kept Martin's Reviewed-by
>
> - old patch 09/16 has been moved up to 05/14 based on Martin's
> suggestion. There are no code changes
>
> - patch 09/14 (old patch 11/16) moves path_group_prio_update()
> earlier in select_path_group() to make sure the pgp->marginal check
> uses a current value.
>
> - patch 12/14 (old patch 14/16) changes how delay_checks is selected,
> to ignore the delay_checks values if any san_path_err options are
> set, based on Martin's suggestions. It also changes the man page
> wording.
>
> Benjamin Marzinski (14):
> libmultipath: make vector_foreach_slot_backwards work as expected
> libmultipath: add marginal paths and groups infrastructure
> tests: add path grouping policy unit tests.
> libmultipath: add wrapper function around pgpolicyfn
> tests: update pgpolicy tests to work with group_paths()
> libmultipath: fix double free in pgpolicyfn error paths
> libmultipath: consolidate group_by_* functions
> libmultipath: make pgpolicyfn take a paths vector
> libmultipath: make group_paths handle marginal paths
> tests: add tests for grouping marginal paths.
> libmultipath: add marginal_pathgroups config option
> libmutipath: deprecate delay_*_checks
> multipathd: use marginal_pathgroups
> multipath: update man pages
>
> libmultipath/config.h | 1 +
> libmultipath/configure.c | 23 +-
> libmultipath/dict.c | 3 +
> libmultipath/pgpolicies.c | 346 +++++-------
> libmultipath/pgpolicies.h | 12 +-
> libmultipath/print.c | 18 +
> libmultipath/print.h | 6 +-
> libmultipath/propsel.c | 89 +++-
> libmultipath/propsel.h | 3 +-
> libmultipath/structs.c | 16 +-
> libmultipath/structs.h | 15 +-
> libmultipath/switchgroup.c | 15 +-
> libmultipath/vector.h | 2 +-
> multipath/multipath.conf.5 | 74 ++-
> multipathd/cli.c | 5 +
> multipathd/cli.h | 4 +
> multipathd/cli_handlers.c | 91 ++++
> multipathd/cli_handlers.h | 3 +
> multipathd/main.c | 90 ++--
> multipathd/multipathd.8 | 19 +
> tests/Makefile | 2 +-
> tests/pgpolicy.c | 1036
> ++++++++++++++++++++++++++++++++++++
> 22 files changed, 1523 insertions(+), 350 deletions(-)
> create mode 100644 tests/pgpolicy.c
>
For the entire set:
Reviewed-by: Martin Wilck <mwilck@suse.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-08-30 7:09 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-23 17:48 [PATCH v2 00/14] multipath marginal pathgroups Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 01/14] libmultipath: make vector_foreach_slot_backwards work as expected Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 02/14] libmultipath: add marginal paths and groups infrastructure Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 03/14] tests: add path grouping policy unit tests Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 04/14] libmultipath: add wrapper function around pgpolicyfn Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 05/14] tests: update pgpolicy tests to work with group_paths() Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 06/14] libmultipath: fix double free in pgpolicyfn error paths Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 07/14] libmultipath: consolidate group_by_* functions Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 08/14] libmultipath: make pgpolicyfn take a paths vector Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 09/14] libmultipath: make group_paths handle marginal paths Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 10/14] tests: add tests for grouping " Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 11/14] libmultipath: add marginal_pathgroups config option Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 12/14] libmutipath: deprecate delay_*_checks Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 13/14] multipathd: use marginal_pathgroups Benjamin Marzinski
2019-08-23 17:48 ` [PATCH v2 14/14] multipath: update man pages Benjamin Marzinski
2019-08-30 7:09 ` [PATCH v2 00/14] multipath marginal pathgroups Martin Wilck
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.