diff --git a/0067-kpartx-hold-device-open-until-partitions-have-been-c.patch b/0067-kpartx-hold-device-open-until-partitions-have-been-c.patch new file mode 100644 index 0000000..3a1e2c1 --- /dev/null +++ b/0067-kpartx-hold-device-open-until-partitions-have-been-c.patch @@ -0,0 +1,47 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Wed, 19 Oct 2022 12:57:10 -0500 +Subject: [PATCH] kpartx: hold device open until partitions have been created + +kpartx was closing the whole device after it read the partition +information off it. This allowed a race, where the device could be +removed and another one created with the same major:minor, after kpartx +read the partition information but before it created the partition +devices. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + kpartx/kpartx.c | 11 +++-------- + 1 file changed, 3 insertions(+), 8 deletions(-) + +diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c +index 7bc64543..e79fdd4a 100644 +--- a/kpartx/kpartx.c ++++ b/kpartx/kpartx.c +@@ -443,12 +443,7 @@ main(int argc, char **argv){ + if (n >= 0) + printf("%s: %d slices\n", ptp->type, n); + #endif +- +- if (n > 0) { +- close(fd); +- fd = -1; +- } +- else ++ if (n <= 0) + continue; + + switch(what) { +@@ -668,9 +663,9 @@ main(int argc, char **argv){ + if (n > 0) + break; + } ++ if (fd != -1) ++ close(fd); + if (what == LIST && loopcreated) { +- if (fd != -1) +- close(fd); + if (del_loop(device)) { + if (verbose) + fprintf(stderr, "can't del loop : %s\n", diff --git a/0068-libmultipath-cleanup-remove_feature.patch b/0068-libmultipath-cleanup-remove_feature.patch new file mode 100644 index 0000000..a83735e --- /dev/null +++ b/0068-libmultipath-cleanup-remove_feature.patch @@ -0,0 +1,153 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 7 Oct 2022 12:35:37 -0500 +Subject: [PATCH] libmultipath: cleanup remove_feature + +remove_feature() didn't correctly handle feature strings that used +whitespace other than spaces, which the kernel allows. It also didn't +check if the feature string to be removed was part of a larger feature +token. Finally, it did a lot of unnecessary work. By failing if the +feature string to be removed contains leading or trailing whitespace, +the function can be significanly simplified. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmultipath/structs.c | 82 +++++++++++++++--------------------------- + 1 file changed, 29 insertions(+), 53 deletions(-) + +diff --git a/libmultipath/structs.c b/libmultipath/structs.c +index acd4cbeb..83906ffe 100644 +--- a/libmultipath/structs.c ++++ b/libmultipath/structs.c +@@ -6,6 +6,7 @@ + #include + #include + #include ++#include + + #include "checkers.h" + #include "memory.h" +@@ -669,7 +670,7 @@ int add_feature(char **f, const char *n) + + int remove_feature(char **f, const char *o) + { +- int c = 0, d, l; ++ int c = 0, d; + char *e, *p, *n; + const char *q; + +@@ -680,33 +681,35 @@ int remove_feature(char **f, const char *o) + if (!o || *o == '\0') + return 0; + +- /* Check if not present */ +- if (!strstr(*f, o)) ++ d = strlen(o); ++ if (isspace(*o) || isspace(*(o + d - 1))) { ++ condlog(0, "internal error: feature \"%s\" has leading or trailing spaces", o); ++ return 1; ++ } ++ ++ /* Check if present and not part of a larger feature token*/ ++ p = *f + 1; /* the size must be at the start of the features string */ ++ while ((p = strstr(p, o)) != NULL) { ++ if (isspace(*(p - 1)) && ++ (isspace(*(p + d)) || *(p + d) == '\0')) ++ break; ++ p += d; ++ } ++ if (!p) + return 0; + + /* Get feature count */ + c = strtoul(*f, &e, 10); +- if (*f == e) +- /* parse error */ ++ if (*f == e || !isspace(*e)) { ++ condlog(0, "parse error in feature string \"%s\"", *f); + return 1; +- +- /* Normalize features */ +- while (*o == ' ') { +- o++; + } +- /* Just spaces, return */ +- if (*o == '\0') +- return 0; +- q = o + strlen(o); +- while (*q == ' ') +- q--; +- d = (int)(q - o); + + /* Update feature count */ + c--; + q = o; +- while (q[0] != '\0') { +- if (q[0] == ' ' && q[1] != ' ' && q[1] != '\0') ++ while (*q != '\0') { ++ if (isspace(*q) && !isspace(*(q + 1)) && *(q + 1) != '\0') + c--; + q++; + } +@@ -720,15 +723,8 @@ int remove_feature(char **f, const char *o) + goto out; + } + +- /* Search feature to be removed */ +- e = strstr(*f, o); +- if (!e) +- /* Not found, return */ +- return 0; +- + /* Update feature count space */ +- l = strlen(*f) - d; +- n = MALLOC(l + 1); ++ n = MALLOC(strlen(*f) - d + 1); + if (!n) + return 1; + +@@ -738,36 +734,16 @@ int remove_feature(char **f, const char *o) + * Copy existing features up to the feature + * about to be removed + */ +- p = strchr(*f, ' '); +- if (!p) { +- /* Internal error, feature string inconsistent */ +- FREE(n); +- return 1; +- } +- while (*p == ' ') +- p++; +- p--; +- if (e != p) { +- do { +- e--; +- d++; +- } while (*e == ' '); +- e++; d--; +- strncat(n, p, (size_t)(e - p)); +- p += (size_t)(e - p); +- } ++ strncat(n, e, (size_t)(p - e)); + /* Skip feature to be removed */ + p += d; +- + /* Copy remaining features */ +- if (strlen(p)) { +- while (*p == ' ') +- p++; +- if (strlen(p)) { +- p--; +- strcat(n, p); +- } +- } ++ while (isspace(*p)) ++ p++; ++ if (*p != '\0') ++ strcat(n, p); ++ else ++ strchop(n); + + out: + FREE(*f); diff --git a/0069-libmultipath-cleanup-add_feature.patch b/0069-libmultipath-cleanup-add_feature.patch new file mode 100644 index 0000000..04f6a51 --- /dev/null +++ b/0069-libmultipath-cleanup-add_feature.patch @@ -0,0 +1,107 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 7 Oct 2022 12:35:38 -0500 +Subject: [PATCH] libmultipath: cleanup add_feature + +add_feature() didn't correctly handle feature strings that used +whitespace other than spaces, which the kernel allows. It also didn't +allow adding features with multiple tokens. When it looked to see if the +feature string to be added already existed, it didn't check if the match +was part of a larger token. Finally, it did unnecessary work. By using +asprintf() to create the string, the function can be signifcantly +simplified. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmultipath/structs.c | 49 +++++++++++++++++++++--------------------- + 1 file changed, 24 insertions(+), 25 deletions(-) + +diff --git a/libmultipath/structs.c b/libmultipath/structs.c +index 83906ffe..5dfa86a8 100644 +--- a/libmultipath/structs.c ++++ b/libmultipath/structs.c +@@ -608,23 +608,33 @@ int add_feature(char **f, const char *n) + { + int c = 0, d, l; + char *e, *t; ++ const char *p; + + if (!f) + return 1; + + /* Nothing to do */ +- if (!n || *n == '0') ++ if (!n || *n == '\0') + return 0; + +- if (strchr(n, ' ') != NULL) { +- condlog(0, "internal error: feature \"%s\" contains spaces", n); ++ l = strlen(n); ++ if (isspace(*n) || isspace(*(n + l - 1))) { ++ condlog(0, "internal error: feature \"%s\" has leading or trailing spaces", n); + return 1; + } + ++ p = n; ++ d = 1; ++ while (*p != '\0') { ++ if (isspace(*p) && !isspace(*(p + 1)) && *(p + 1) != '\0') ++ d++; ++ p++; ++ } ++ + /* default feature is null */ + if(!*f) + { +- l = asprintf(&t, "1 %s", n); ++ l = asprintf(&t, "%0d %s", d, n); + if(l == -1) + return 1; + +@@ -633,35 +643,24 @@ int add_feature(char **f, const char *n) + } + + /* Check if feature is already present */ +- if (strstr(*f, n)) +- return 0; ++ e = *f; ++ while ((e = strstr(e, n)) != NULL) { ++ if (isspace(*(e - 1)) && ++ (isspace(*(e + l)) || *(e + l) == '\0')) ++ return 0; ++ e += l; ++ } + + /* Get feature count */ + c = strtoul(*f, &e, 10); +- if (*f == e || (*e != ' ' && *e != '\0')) { ++ if (*f == e || (!isspace(*e) && *e != '\0')) { + condlog(0, "parse error in feature string \"%s\"", *f); + return 1; + } +- +- /* Add 1 digit and 1 space */ +- l = strlen(e) + strlen(n) + 2; +- +- c++; +- /* Check if we need more digits for feature count */ +- for (d = c; d >= 10; d /= 10) +- l++; +- +- t = MALLOC(l + 1); +- if (!t) ++ c += d; ++ if (asprintf(&t, "%0d%s %s", c, e, n) < 0) + return 1; + +- /* e: old feature string with leading space, or "" */ +- if (*e == ' ') +- while (*(e + 1) == ' ') +- e++; +- +- snprintf(t, l + 1, "%0d%s %s", c, e, n); +- + FREE(*f); + *f = t; + diff --git a/0070-multipath-tests-tests-for-adding-and-removing-featur.patch b/0070-multipath-tests-tests-for-adding-and-removing-featur.patch new file mode 100644 index 0000000..7d18bf2 --- /dev/null +++ b/0070-multipath-tests-tests-for-adding-and-removing-featur.patch @@ -0,0 +1,351 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 7 Oct 2022 12:35:39 -0500 +Subject: [PATCH] multipath tests: tests for adding and removing features + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + tests/Makefile | 2 +- + tests/features.c | 319 +++++++++++++++++++++++++++++++++++++++++++++++ + 2 files changed, 320 insertions(+), 1 deletion(-) + create mode 100644 tests/features.c + +diff --git a/tests/Makefile b/tests/Makefile +index 8cbc4b73..972a5e04 100644 +--- a/tests/Makefile ++++ b/tests/Makefile +@@ -13,7 +13,7 @@ CFLAGS += $(BIN_CFLAGS) -I$(multipathdir) -I$(mpathcmddir) \ + LIBDEPS += -L. -L$(mpathcmddir) -lmultipath -lmpathcmd -lcmocka + + TESTS := uevent parser util dmevents hwtable blacklist unaligned vpd pgpolicy \ +- alias directio valid devt mpathvalid strbuf ++ alias directio valid devt mpathvalid strbuf features + HELPERS := test-lib.o test-log.o + + .SILENT: $(TESTS:%=%.o) +diff --git a/tests/features.c b/tests/features.c +new file mode 100644 +index 00000000..4d8f0860 +--- /dev/null ++++ b/tests/features.c +@@ -0,0 +1,319 @@ ++#include ++#include ++#include ++#include ++ ++#include "structs.h" ++#include "globals.c" ++ ++static void test_af_null_features_ptr(void **state) ++{ ++ assert_int_equal(add_feature(NULL, "test"), 1); ++} ++ ++static void af_helper(const char *features_start, const char *addition, ++ const char *features_end, int result) ++{ ++ char *f = NULL, *orig = NULL; ++ ++ if (features_start) { ++ f = strdup(features_start); ++ assert_non_null(f); ++ orig = f; ++ } ++ assert_int_equal(add_feature(&f, addition), result); ++ if (result != 0 || features_end == NULL) ++ assert_ptr_equal(orig, f); ++ else ++ assert_string_equal(f, features_end); ++ free(f); ++} ++ ++static void test_af_null_addition1(void **state) ++{ ++ af_helper("0", NULL, NULL, 0); ++} ++ ++static void test_af_null_addition2(void **state) ++{ ++ af_helper("1 queue_if_no_path", NULL, NULL, 0); ++} ++ ++static void test_af_empty_addition(void **state) ++{ ++ af_helper("2 pg_init_retries 5", "", NULL, 0); ++} ++ ++static void test_af_invalid_addition1(void **state) ++{ ++ af_helper("2 pg_init_retries 5", " ", NULL, 1); ++} ++ ++static void test_af_invalid_addition2(void **state) ++{ ++ af_helper("2 pg_init_retries 5", "\tbad", NULL, 1); ++} ++ ++static void test_af_invalid_addition3(void **state) ++{ ++ af_helper("2 pg_init_retries 5", "bad ", NULL, 1); ++} ++ ++static void test_af_invalid_addition4(void **state) ++{ ++ af_helper("2 pg_init_retries 5", " bad ", NULL, 1); ++} ++ ++static void test_af_null_features1(void **state) ++{ ++ af_helper(NULL, "test", "1 test", 0); ++} ++ ++static void test_af_null_features2(void **state) ++{ ++ af_helper(NULL, "test\t more", "2 test\t more", 0); ++} ++ ++static void test_af_null_features3(void **state) ++{ ++ af_helper(NULL, "test\neven\tmore", "3 test\neven\tmore", 0); ++} ++ ++static void test_af_already_exists1(void **state) ++{ ++ af_helper("4 this is a test", "test", NULL, 0); ++} ++ ++static void test_af_already_exists2(void **state) ++{ ++ af_helper("5 contest testy intestine test retest", "test", NULL, 0); ++} ++ ++static void test_af_almost_exists(void **state) ++{ ++ af_helper("3 contest testy intestine", "test", ++ "4 contest testy intestine test", 0); ++} ++ ++static void test_af_bad_features1(void **state) ++{ ++ af_helper("bad", "test", NULL, 1); ++} ++ ++static void test_af_bad_features2(void **state) ++{ ++ af_helper("1bad", "test", NULL, 1); ++} ++ ++static void test_af_add1(void **state) ++{ ++ af_helper("0", "test", "1 test", 0); ++} ++ ++static void test_af_add2(void **state) ++{ ++ af_helper("0", "this is a test", "4 this is a test", 0); ++} ++ ++static void test_af_add3(void **state) ++{ ++ af_helper("1 features", "more values", "3 features more values", 0); ++} ++ ++static void test_af_add4(void **state) ++{ ++ af_helper("2 one\ttwo", "three\t four", "4 one\ttwo three\t four", 0); ++} ++ ++static int test_add_features(void) ++{ ++ const struct CMUnitTest tests[] = { ++ cmocka_unit_test(test_af_null_features_ptr), ++ cmocka_unit_test(test_af_null_addition1), ++ cmocka_unit_test(test_af_null_addition2), ++ cmocka_unit_test(test_af_empty_addition), ++ cmocka_unit_test(test_af_invalid_addition1), ++ cmocka_unit_test(test_af_invalid_addition2), ++ cmocka_unit_test(test_af_invalid_addition3), ++ cmocka_unit_test(test_af_invalid_addition4), ++ cmocka_unit_test(test_af_null_features1), ++ cmocka_unit_test(test_af_null_features2), ++ cmocka_unit_test(test_af_null_features3), ++ cmocka_unit_test(test_af_already_exists1), ++ cmocka_unit_test(test_af_already_exists2), ++ cmocka_unit_test(test_af_almost_exists), ++ cmocka_unit_test(test_af_bad_features1), ++ cmocka_unit_test(test_af_bad_features2), ++ cmocka_unit_test(test_af_add1), ++ cmocka_unit_test(test_af_add2), ++ cmocka_unit_test(test_af_add3), ++ cmocka_unit_test(test_af_add4), ++ }; ++ return cmocka_run_group_tests(tests, NULL, NULL); ++} ++ ++static void test_rf_null_features_ptr(void **state) ++{ ++ assert_int_equal(remove_feature(NULL, "test"), 1); ++} ++ ++static void test_rf_null_features(void **state) ++{ ++ char *f = NULL; ++ assert_int_equal(remove_feature(&f, "test"), 1); ++} ++ ++static void rf_helper(const char *features_start, const char *removal, ++ const char *features_end, int result) ++{ ++ char *f = strdup(features_start); ++ char *orig = f; ++ ++ assert_non_null(f); ++ assert_int_equal(remove_feature(&f, removal), result); ++ if (result != 0 || features_end == NULL) ++ assert_ptr_equal(orig, f); ++ else ++ assert_string_equal(f, features_end); ++ free(f); ++} ++ ++static void test_rf_null_removal(void **state) ++{ ++ rf_helper("1 feature", NULL, NULL, 0); ++} ++ ++static void test_rf_empty_removal(void **state) ++{ ++ rf_helper("1 feature", "", NULL, 0); ++} ++ ++static void test_rf_invalid_removal1(void **state) ++{ ++ rf_helper("1 feature", " ", NULL, 1); ++} ++ ++static void test_rf_invalid_removal2(void **state) ++{ ++ rf_helper("1 feature", " bad", NULL, 1); ++} ++ ++static void test_rf_invalid_removal3(void **state) ++{ ++ rf_helper("1 feature", "bad\n", NULL, 1); ++} ++ ++static void test_rf_invalid_removal4(void **state) ++{ ++ rf_helper("1 feature", "\tbad \n", NULL, 1); ++} ++ ++static void test_rf_bad_features1(void **state) ++{ ++ rf_helper("invalid feature test string", "test", NULL, 1); ++} ++ ++static void test_rf_bad_features2(void **state) ++{ ++ rf_helper("2no space test", "test", NULL, 1); ++} ++ ++static void test_rf_missing_removal1(void **state) ++{ ++ rf_helper("0", "test", NULL, 0); ++} ++ ++static void test_rf_missing_removal2(void **state) ++{ ++ rf_helper("1 detest", "test", NULL, 0); ++} ++ ++static void test_rf_missing_removal3(void **state) ++{ ++ rf_helper("4 testing one two three", "test", NULL, 0); ++} ++ ++static void test_rf_missing_removal4(void **state) ++{ ++ rf_helper("1 contestant", "test", NULL, 0); ++} ++ ++static void test_rf_missing_removal5(void **state) ++{ ++ rf_helper("3 testament protest detestable", "test", NULL, 0); ++} ++ ++static void test_rf_remove_all_features1(void **state) ++{ ++ rf_helper("1 test", "test", "0", 0); ++} ++ ++static void test_rf_remove_all_features2(void **state) ++{ ++ rf_helper("2 another\t test", "another\t test", "0", 0); ++} ++ ++static void test_rf_remove1(void **state) ++{ ++ rf_helper("2 feature1 feature2", "feature2", "1 feature1", 0); ++} ++ ++static void test_rf_remove2(void **state) ++{ ++ rf_helper("2 feature1 feature2", "feature1", "1 feature2", 0); ++} ++ ++static void test_rf_remove3(void **state) ++{ ++ rf_helper("3 test1 test\ttest2", "test", "2 test1 test2", 0); ++} ++ ++static void test_rf_remove4(void **state) ++{ ++ rf_helper("4 this\t is a test", "is a", "2 this\t test", 0); ++} ++ ++static void test_rf_remove5(void **state) ++{ ++ rf_helper("3 one more test", "more test", "1 one", 0); ++} ++ ++static int test_remove_features(void) ++{ ++ const struct CMUnitTest tests[] = { ++ cmocka_unit_test(test_rf_null_features_ptr), ++ cmocka_unit_test(test_rf_null_features), ++ cmocka_unit_test(test_rf_null_removal), ++ cmocka_unit_test(test_rf_empty_removal), ++ cmocka_unit_test(test_rf_invalid_removal1), ++ cmocka_unit_test(test_rf_invalid_removal2), ++ cmocka_unit_test(test_rf_invalid_removal3), ++ cmocka_unit_test(test_rf_invalid_removal4), ++ cmocka_unit_test(test_rf_bad_features1), ++ cmocka_unit_test(test_rf_bad_features2), ++ cmocka_unit_test(test_rf_missing_removal1), ++ cmocka_unit_test(test_rf_missing_removal2), ++ cmocka_unit_test(test_rf_missing_removal3), ++ cmocka_unit_test(test_rf_missing_removal4), ++ cmocka_unit_test(test_rf_missing_removal5), ++ cmocka_unit_test(test_rf_remove_all_features1), ++ cmocka_unit_test(test_rf_remove_all_features2), ++ cmocka_unit_test(test_rf_remove1), ++ cmocka_unit_test(test_rf_remove2), ++ cmocka_unit_test(test_rf_remove3), ++ cmocka_unit_test(test_rf_remove4), ++ cmocka_unit_test(test_rf_remove5), ++ }; ++ return cmocka_run_group_tests(tests, NULL, NULL); ++} ++ ++int main(void) ++{ ++ int ret = 0; ++ ++ init_test_verbosity(-1); ++ ret += test_add_features(); ++ ret += test_remove_features(); ++ ++ return ret; ++} diff --git a/0071-libmultipath-fix-queue_mode-feature-handling.patch b/0071-libmultipath-fix-queue_mode-feature-handling.patch new file mode 100644 index 0000000..8937abe --- /dev/null +++ b/0071-libmultipath-fix-queue_mode-feature-handling.patch @@ -0,0 +1,177 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 7 Oct 2022 12:35:40 -0500 +Subject: [PATCH] libmultipath: fix queue_mode feature handling + +device-mapper is not able to change the queue_mode on a table reload. +Make sure that when multipath sets up the map, both on regular reloads +and reconfigures, it keeps the queue_mode the same. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmultipath/configure.c | 4 +++ + libmultipath/dmparser.c | 2 ++ + libmultipath/propsel.c | 55 ++++++++++++++++++++++++++++++++++++++ + libmultipath/structs.h | 7 +++++ + multipath/multipath.conf.5 | 7 +++-- + 5 files changed, 73 insertions(+), 2 deletions(-) + +diff --git a/libmultipath/configure.c b/libmultipath/configure.c +index 70049f47..cc778a22 100644 +--- a/libmultipath/configure.c ++++ b/libmultipath/configure.c +@@ -1118,6 +1118,7 @@ int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid, + struct config *conf = NULL; + int allow_queueing; + struct bitfield *size_mismatch_seen; ++ struct multipath * cmpp; + + /* ignore refwwid if it's empty */ + if (refwwid && !strlen(refwwid)) +@@ -1227,6 +1228,9 @@ int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid, + } + verify_paths(mpp); + ++ cmpp = find_mp_by_wwid(curmp, mpp->wwid); ++ if (cmpp) ++ mpp->queue_mode = cmpp->queue_mode; + if (setup_map(mpp, ¶ms, vecs)) { + remove_map(mpp, vecs->pathvec, NULL); + continue; +diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c +index bc311421..16377c54 100644 +--- a/libmultipath/dmparser.c ++++ b/libmultipath/dmparser.c +@@ -152,6 +152,8 @@ int disassemble_map(const struct _vector *pathvec, + + FREE(word); + } ++ mpp->queue_mode = strstr(mpp->features, "queue_mode bio") ? ++ QUEUE_MODE_BIO : QUEUE_MODE_RQ; + + /* + * hwhandler +diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c +index 2b47f5f8..9dea6f92 100644 +--- a/libmultipath/propsel.c ++++ b/libmultipath/propsel.c +@@ -27,6 +27,7 @@ + #include "strbuf.h" + #include + #include ++#include + + pgpolicyfn *pgpolicies[] = { + NULL, +@@ -414,6 +415,59 @@ void reconcile_features_with_options(const char *id, char **features, int* no_pa + } + } + ++static void reconcile_features_with_queue_mode(struct multipath *mp) ++{ ++ char *space = NULL, *val = NULL, *mode_str = NULL, *feat; ++ int features_mode = QUEUE_MODE_UNDEF; ++ ++ if (!mp->features) ++ return; ++ ++ pthread_cleanup_push(cleanup_free_ptr, &space); ++ pthread_cleanup_push(cleanup_free_ptr, &val); ++ pthread_cleanup_push(cleanup_free_ptr, &mode_str); ++ ++ if (!(feat = strstr(mp->features, "queue_mode")) || ++ feat == mp->features || !isspace(*(feat - 1)) || ++ sscanf(feat, "queue_mode%m[ \f\n\r\t\v]%ms", &space, &val) != 2) ++ goto sync_mode; ++ if (asprintf(&mode_str, "queue_mode%s%s", space, val) < 0) { ++ condlog(1, "failed to allocate space for queue_mode feature string"); ++ mode_str = NULL; /* value undefined on failure */ ++ goto exit; ++ } ++ ++ if (!strcmp(val, "rq") || !strcmp(val, "mq")) ++ features_mode = QUEUE_MODE_RQ; ++ else if (!strcmp(val, "bio")) ++ features_mode = QUEUE_MODE_BIO; ++ if (features_mode == QUEUE_MODE_UNDEF) { ++ condlog(2, "%s: ignoring invalid feature '%s'", ++ mp->alias, mode_str); ++ goto sync_mode; ++ } ++ ++ if (mp->queue_mode == QUEUE_MODE_UNDEF) ++ mp->queue_mode = features_mode; ++ if (mp->queue_mode == features_mode) ++ goto exit; ++ ++ condlog(2, ++ "%s: ignoring feature '%s' because queue_mode is set to '%s'", ++ mp->alias, mode_str, ++ (mp->queue_mode == QUEUE_MODE_RQ)? "rq" : "bio"); ++ ++sync_mode: ++ if (mode_str) ++ remove_feature(&mp->features, mode_str); ++ if (mp->queue_mode == QUEUE_MODE_BIO) ++ add_feature(&mp->features, "queue_mode bio"); ++exit: ++ pthread_cleanup_pop(1); ++ pthread_cleanup_pop(1); ++ pthread_cleanup_pop(1); ++} ++ + int select_features(struct config *conf, struct multipath *mp) + { + const char *origin; +@@ -429,6 +483,7 @@ out: + reconcile_features_with_options(mp->alias, &mp->features, + &mp->no_path_retry, + &mp->retain_hwhandler); ++ reconcile_features_with_queue_mode(mp); + condlog(3, "%s: features = \"%s\" %s", mp->alias, mp->features, origin); + return 0; + } +diff --git a/libmultipath/structs.h b/libmultipath/structs.h +index 8a07d470..b4f75de0 100644 +--- a/libmultipath/structs.h ++++ b/libmultipath/structs.h +@@ -170,6 +170,12 @@ enum max_sectors_kb_states { + MAX_SECTORS_KB_MIN = 4, /* can't be smaller than page size */ + }; + ++enum queue_mode_states { ++ QUEUE_MODE_UNDEF = 0, ++ QUEUE_MODE_BIO, ++ QUEUE_MODE_RQ, ++}; ++ + enum scsi_protocol { + SCSI_PROTOCOL_FCP = 0, /* Fibre Channel */ + SCSI_PROTOCOL_SPI = 1, /* parallel SCSI */ +@@ -387,6 +393,7 @@ struct multipath { + int needs_paths_uevent; + int ghost_delay; + int ghost_delay_tick; ++ int queue_mode; + uid_t uid; + gid_t gid; + mode_t mode; +diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5 +index f7de5140..e1a787d4 100644 +--- a/multipath/multipath.conf.5 ++++ b/multipath/multipath.conf.5 +@@ -468,8 +468,11 @@ precedence. See KNOWN ISSUES. + can be \fIbio\fR, \fIrq\fR or \fImq\fR, which corresponds to + bio-based, request-based, and block-multiqueue (blk-mq) request-based, + respectively. +-The default depends on the kernel parameter \fBdm_mod.use_blk_mq\fR. It is +-\fImq\fR if the latter is set, and \fIrq\fR otherwise. ++Before kernel 4.20 The default depends on the kernel parameter ++\fBdm_mod.use_blk_mq\fR. It is \fImq\fR if the latter is set, and \fIrq\fR ++otherwise. Since kernel 4.20, \fIrq\fR and \fImq\fR both correspond to ++block-multiqueue. Once a multipath device has been created, its queue_mode ++cannot be changed. + .TP + The default is: \fB\fR + .RE diff --git a/0072-multipath-tests-tests-for-reconcile_features_with_qu.patch b/0072-multipath-tests-tests-for-reconcile_features_with_qu.patch new file mode 100644 index 0000000..6fdbd80 --- /dev/null +++ b/0072-multipath-tests-tests-for-reconcile_features_with_qu.patch @@ -0,0 +1,291 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 7 Oct 2022 12:35:41 -0500 +Subject: [PATCH] multipath tests: tests for reconcile_features_with_queue_mode + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + tests/Makefile | 2 + + tests/features.c | 232 ++++++++++++++++++++++++++++++++++++++++++++++- + 2 files changed, 233 insertions(+), 1 deletion(-) + +diff --git a/tests/Makefile b/tests/Makefile +index 972a5e04..27e3ffa9 100644 +--- a/tests/Makefile ++++ b/tests/Makefile +@@ -33,6 +33,7 @@ ifneq ($(DIO_TEST_DEV),) + directio-test_FLAGS := -DDIO_TEST_DEV=\"$(DIO_TEST_DEV)\" + endif + mpathvalid-test_FLAGS := -I$(mpathvaliddir) ++features-test_FLAGS := -I$(multipathdir)/nvme + + # test-specific linker flags + # XYZ-test_TESTDEPS: test libraries containing __wrap_xyz functions +@@ -64,6 +65,7 @@ ifneq ($(DIO_TEST_DEV),) + directio-test_LIBDEPS := -laio + endif + strbuf-test_OBJDEPS := ../libmultipath/strbuf.o ++features-test_LIBDEPS := -ludev -lpthread + + %.o: %.c + $(CC) $(CFLAGS) $($*-test_FLAGS) -c -o $@ $< +diff --git a/tests/features.c b/tests/features.c +index 4d8f0860..31f978fd 100644 +--- a/tests/features.c ++++ b/tests/features.c +@@ -1,9 +1,10 @@ ++#define _GNU_SOURCE + #include + #include + #include + #include + +-#include "structs.h" ++#include "../libmultipath/propsel.c" + #include "globals.c" + + static void test_af_null_features_ptr(void **state) +@@ -307,6 +308,234 @@ static int test_remove_features(void) + return cmocka_run_group_tests(tests, NULL, NULL); + } + ++static void test_cf_null_features(void **state) ++{ ++ struct multipath mp = { ++ .alias = "test", ++ }; ++ reconcile_features_with_queue_mode(&mp); ++ assert_null(mp.features); ++} ++ ++static void cf_helper(const char *features_start, const char *features_end, ++ int queue_mode_start, int queue_mode_end) ++{ ++ struct multipath mp = { ++ .alias = "test", ++ .features = strdup(features_start), ++ .queue_mode = queue_mode_start, ++ }; ++ char *orig = mp.features; ++ ++ assert_non_null(orig); ++ reconcile_features_with_queue_mode(&mp); ++ if (!features_end) ++ assert_ptr_equal(orig, mp.features); ++ else ++ assert_string_equal(mp.features, features_end); ++ free(mp.features); ++ assert_int_equal(mp.queue_mode, queue_mode_end); ++} ++ ++static void test_cf_unset_unset1(void **state) ++{ ++ cf_helper("0", NULL, QUEUE_MODE_UNDEF, QUEUE_MODE_UNDEF); ++} ++ ++static void test_cf_unset_unset2(void **state) ++{ ++ cf_helper("1 queue_mode", NULL, QUEUE_MODE_UNDEF, QUEUE_MODE_UNDEF); ++} ++ ++static void test_cf_unset_unset3(void **state) ++{ ++ cf_helper("queue_mode", NULL, QUEUE_MODE_UNDEF, QUEUE_MODE_UNDEF); ++} ++ ++static void test_cf_unset_unset4(void **state) ++{ ++ cf_helper("2 queue_model bio", NULL, QUEUE_MODE_UNDEF, ++ QUEUE_MODE_UNDEF); ++} ++ ++static void test_cf_unset_unset5(void **state) ++{ ++ cf_helper("1 queue_if_no_path", NULL, QUEUE_MODE_UNDEF, ++ QUEUE_MODE_UNDEF); ++} ++ ++static void test_cf_invalid_unset1(void **state) ++{ ++ cf_helper("2 queue_mode biop", "0", QUEUE_MODE_UNDEF, QUEUE_MODE_UNDEF); ++} ++ ++static void test_cf_invalid_unset2(void **state) ++{ ++ cf_helper("3 queue_mode rqs queue_if_no_path", "1 queue_if_no_path", ++ QUEUE_MODE_UNDEF, QUEUE_MODE_UNDEF); ++} ++ ++static void test_cf_rq_unset1(void **state) ++{ ++ cf_helper("2 queue_mode rq", NULL, QUEUE_MODE_UNDEF, QUEUE_MODE_RQ); ++} ++ ++static void test_cf_rq_unset2(void **state) ++{ ++ cf_helper("2 queue_mode mq", NULL, QUEUE_MODE_UNDEF, QUEUE_MODE_RQ); ++} ++ ++static void test_cf_bio_unset(void **state) ++{ ++ cf_helper("2 queue_mode bio", NULL, QUEUE_MODE_UNDEF, QUEUE_MODE_BIO); ++} ++ ++static void test_cf_unset_bio1(void **state) ++{ ++ cf_helper("1 queue_if_no_path", "3 queue_if_no_path queue_mode bio", ++ QUEUE_MODE_BIO, QUEUE_MODE_BIO); ++} ++ ++static void test_cf_unset_bio2(void **state) ++{ ++ cf_helper("0", "2 queue_mode bio", QUEUE_MODE_BIO, QUEUE_MODE_BIO); ++} ++ ++static void test_cf_unset_bio3(void **state) ++{ ++ cf_helper("2 pg_init_retries 50", "4 pg_init_retries 50 queue_mode bio", ++ QUEUE_MODE_BIO, QUEUE_MODE_BIO); ++} ++ ++static void test_cf_invalid_bio1(void **state) ++{ ++ cf_helper("2 queue_mode bad", "2 queue_mode bio", ++ QUEUE_MODE_BIO, QUEUE_MODE_BIO); ++} ++ ++static void test_cf_invalid_bio2(void **state) ++{ ++ cf_helper("3 queue_if_no_path queue_mode\tbad", "3 queue_if_no_path queue_mode bio", ++ QUEUE_MODE_BIO, QUEUE_MODE_BIO); ++} ++ ++static void test_cf_bio_bio1(void **state) ++{ ++ cf_helper("2 queue_mode bio", NULL, QUEUE_MODE_BIO, QUEUE_MODE_BIO); ++} ++ ++static void test_cf_bio_bio2(void **state) ++{ ++ cf_helper("3 queue_if_no_path queue_mode bio", NULL, QUEUE_MODE_BIO, QUEUE_MODE_BIO); ++} ++ ++static void test_cf_bio_bio3(void **state) ++{ ++ cf_helper("3 queue_mode\nbio queue_if_no_path", NULL, QUEUE_MODE_BIO, QUEUE_MODE_BIO); ++} ++ ++static void test_cf_bio_rq1(void **state) ++{ ++ cf_helper("2\nqueue_mode\tbio", "0", QUEUE_MODE_RQ, QUEUE_MODE_RQ); ++} ++ ++static void test_cf_bio_rq2(void **state) ++{ ++ cf_helper("3 queue_if_no_path\nqueue_mode bio", "1 queue_if_no_path", ++ QUEUE_MODE_RQ, QUEUE_MODE_RQ); ++} ++ ++static void test_cf_bio_rq3(void **state) ++{ ++ cf_helper("4 queue_mode bio pg_init_retries 20", "2 pg_init_retries 20", ++ QUEUE_MODE_RQ, QUEUE_MODE_RQ); ++} ++ ++static void test_cf_unset_rq1(void **state) ++{ ++ cf_helper("0", NULL, QUEUE_MODE_RQ, QUEUE_MODE_RQ); ++} ++ ++static void test_cf_unset_rq2(void **state) ++{ ++ cf_helper("2 pg_init_retries 15", NULL, QUEUE_MODE_RQ, QUEUE_MODE_RQ); ++} ++ ++static void test_cf_invalid_rq1(void **state) ++{ ++ cf_helper("2 queue_mode bionic", "0", QUEUE_MODE_RQ, QUEUE_MODE_RQ); ++} ++ ++static void test_cf_invalid_rq2(void **state) ++{ ++ cf_helper("3 queue_mode b\nqueue_if_no_path", "1 queue_if_no_path", ++ QUEUE_MODE_RQ, QUEUE_MODE_RQ); ++} ++ ++static void test_cf_rq_rq1(void **state) ++{ ++ cf_helper("2 queue_mode rq", NULL, QUEUE_MODE_RQ, QUEUE_MODE_RQ); ++} ++ ++static void test_cf_rq_rq2(void **state) ++{ ++ cf_helper("3 queue_mode\t \trq\nqueue_if_no_path", NULL, QUEUE_MODE_RQ, QUEUE_MODE_RQ); ++} ++ ++static void test_cf_rq_bio1(void **state) ++{ ++ cf_helper("2 queue_mode rq", "2 queue_mode bio", QUEUE_MODE_BIO, ++ QUEUE_MODE_BIO); ++} ++ ++static void test_cf_rq_bio2(void **state) ++{ ++ cf_helper("3 queue_if_no_path\nqueue_mode rq", "3 queue_if_no_path queue_mode bio", QUEUE_MODE_BIO, QUEUE_MODE_BIO); ++} ++ ++static void test_cf_rq_bio3(void **state) ++{ ++ cf_helper("3 queue_mode rq\nqueue_if_no_path", "3 queue_if_no_path queue_mode bio", QUEUE_MODE_BIO, QUEUE_MODE_BIO); ++} ++ ++static int test_reconcile_features(void) ++{ ++ const struct CMUnitTest tests[] = { ++ cmocka_unit_test(test_cf_null_features), ++ cmocka_unit_test(test_cf_unset_unset1), ++ cmocka_unit_test(test_cf_unset_unset2), ++ cmocka_unit_test(test_cf_unset_unset3), ++ cmocka_unit_test(test_cf_unset_unset4), ++ cmocka_unit_test(test_cf_unset_unset5), ++ cmocka_unit_test(test_cf_invalid_unset1), ++ cmocka_unit_test(test_cf_invalid_unset2), ++ cmocka_unit_test(test_cf_rq_unset1), ++ cmocka_unit_test(test_cf_rq_unset2), ++ cmocka_unit_test(test_cf_bio_unset), ++ cmocka_unit_test(test_cf_unset_bio1), ++ cmocka_unit_test(test_cf_unset_bio2), ++ cmocka_unit_test(test_cf_unset_bio3), ++ cmocka_unit_test(test_cf_invalid_bio1), ++ cmocka_unit_test(test_cf_invalid_bio2), ++ cmocka_unit_test(test_cf_bio_bio1), ++ cmocka_unit_test(test_cf_bio_bio2), ++ cmocka_unit_test(test_cf_bio_bio3), ++ cmocka_unit_test(test_cf_bio_rq1), ++ cmocka_unit_test(test_cf_bio_rq2), ++ cmocka_unit_test(test_cf_bio_rq3), ++ cmocka_unit_test(test_cf_unset_rq1), ++ cmocka_unit_test(test_cf_unset_rq2), ++ cmocka_unit_test(test_cf_invalid_rq1), ++ cmocka_unit_test(test_cf_invalid_rq2), ++ cmocka_unit_test(test_cf_rq_rq1), ++ cmocka_unit_test(test_cf_rq_rq2), ++ cmocka_unit_test(test_cf_rq_bio1), ++ cmocka_unit_test(test_cf_rq_bio2), ++ cmocka_unit_test(test_cf_rq_bio3), ++ }; ++ return cmocka_run_group_tests(tests, NULL, NULL); ++} ++ + int main(void) + { + int ret = 0; +@@ -314,6 +543,7 @@ int main(void) + init_test_verbosity(-1); + ret += test_add_features(); + ret += test_remove_features(); ++ ret += test_reconcile_features(); + + return ret; + } diff --git a/0073-libmultipath-prepare-proto_id-for-use-by-non-scsi-de.patch b/0073-libmultipath-prepare-proto_id-for-use-by-non-scsi-de.patch new file mode 100644 index 0000000..a14544a --- /dev/null +++ b/0073-libmultipath-prepare-proto_id-for-use-by-non-scsi-de.patch @@ -0,0 +1,143 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 7 Oct 2022 12:35:42 -0500 +Subject: [PATCH] libmultipath: prepare proto_id for use by non-scsi devivces + +Make sure that when we are checking for a scsi protocol, we are first +checking that we are working with a scsi path. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmultipath/configure.c | 9 +++++---- + libmultipath/discovery.c | 13 ++++++++----- + libmultipath/print.c | 6 ++++-- + libmultipath/structs.c | 2 +- + libmultipath/structs.h | 4 +++- + multipathd/fpin_handlers.c | 2 +- + 6 files changed, 22 insertions(+), 14 deletions(-) + +diff --git a/libmultipath/configure.c b/libmultipath/configure.c +index cc778a22..c6803b40 100644 +--- a/libmultipath/configure.c ++++ b/libmultipath/configure.c +@@ -223,10 +223,11 @@ int rr_optimize_path_order(struct pathgroup *pgp) + + total_paths = VECTOR_SIZE(pgp->paths); + vector_foreach_slot(pgp->paths, pp, i) { +- if (pp->sg_id.proto_id != SCSI_PROTOCOL_FCP && +- pp->sg_id.proto_id != SCSI_PROTOCOL_SAS && +- pp->sg_id.proto_id != SCSI_PROTOCOL_ISCSI && +- pp->sg_id.proto_id != SCSI_PROTOCOL_SRP) { ++ if (pp->bus != SYSFS_BUS_SCSI || ++ (pp->sg_id.proto_id != SCSI_PROTOCOL_FCP && ++ pp->sg_id.proto_id != SCSI_PROTOCOL_SAS && ++ pp->sg_id.proto_id != SCSI_PROTOCOL_ISCSI && ++ pp->sg_id.proto_id != SCSI_PROTOCOL_SRP)) { + /* return success as default path order + * is maintained in path group + */ +diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c +index bcda8b09..7f2eb409 100644 +--- a/libmultipath/discovery.c ++++ b/libmultipath/discovery.c +@@ -481,10 +481,11 @@ int sysfs_get_host_adapter_name(const struct path *pp, char *adapter_name) + + proto_id = pp->sg_id.proto_id; + +- if (proto_id != SCSI_PROTOCOL_FCP && +- proto_id != SCSI_PROTOCOL_SAS && +- proto_id != SCSI_PROTOCOL_ISCSI && +- proto_id != SCSI_PROTOCOL_SRP) { ++ if (pp->bus != SYSFS_BUS_SCSI || ++ (proto_id != SCSI_PROTOCOL_FCP && ++ proto_id != SCSI_PROTOCOL_SAS && ++ proto_id != SCSI_PROTOCOL_ISCSI && ++ proto_id != SCSI_PROTOCOL_SRP)) { + return 1; + } + /* iscsi doesn't have adapter info in sysfs +@@ -1754,8 +1755,10 @@ sysfs_pathinfo(struct path *pp, const struct _vector *hwtable) + pp->bus = SYSFS_BUS_CCISS; + if (!strncmp(pp->dev,"dasd", 4)) + pp->bus = SYSFS_BUS_CCW; +- if (!strncmp(pp->dev,"sd", 2)) ++ if (!strncmp(pp->dev,"sd", 2)) { + pp->bus = SYSFS_BUS_SCSI; ++ pp->sg_id.proto_id = SCSI_PROTOCOL_UNSPEC; ++ } + if (!strncmp(pp->dev,"nvme", 4)) + pp->bus = SYSFS_BUS_NVME; + +diff --git a/libmultipath/print.c b/libmultipath/print.c +index 46e3d32e..082e4e30 100644 +--- a/libmultipath/print.c ++++ b/libmultipath/print.c +@@ -592,7 +592,8 @@ snprint_host_attr (struct strbuf *buff, const struct path * pp, char *attr) + const char *value = NULL; + int ret; + +- if (pp->sg_id.proto_id != SCSI_PROTOCOL_FCP) ++ if (pp->bus != SYSFS_BUS_SCSI || ++ pp->sg_id.proto_id != SCSI_PROTOCOL_FCP) + return append_strbuf_str(buff, "[undef]"); + sprintf(host_id, "host%d", pp->sg_id.host_no); + host_dev = udev_device_new_from_subsystem_sysname(udev, "fc_host", +@@ -631,7 +632,8 @@ snprint_tgt_wwpn (struct strbuf *buff, const struct path * pp) + const char *value = NULL; + int ret; + +- if (pp->sg_id.proto_id != SCSI_PROTOCOL_FCP) ++ if (pp->bus != SYSFS_BUS_SCSI || ++ pp->sg_id.proto_id != SCSI_PROTOCOL_FCP) + return append_strbuf_str(buff, "[undef]"); + sprintf(rport_id, "rport-%d:%d-%d", + pp->sg_id.host_no, pp->sg_id.channel, pp->sg_id.transport_id); +diff --git a/libmultipath/structs.c b/libmultipath/structs.c +index 5dfa86a8..be81a83c 100644 +--- a/libmultipath/structs.c ++++ b/libmultipath/structs.c +@@ -116,7 +116,7 @@ alloc_path (void) + pp->sg_id.channel = -1; + pp->sg_id.scsi_id = -1; + pp->sg_id.lun = SCSI_INVALID_LUN; +- pp->sg_id.proto_id = SCSI_PROTOCOL_UNSPEC; ++ pp->sg_id.proto_id = PROTOCOL_UNSET; + pp->fd = -1; + pp->tpgs = TPGS_UNDEF; + pp->priority = PRIO_UNDEF; +diff --git a/libmultipath/structs.h b/libmultipath/structs.h +index b4f75de0..2525af17 100644 +--- a/libmultipath/structs.h ++++ b/libmultipath/structs.h +@@ -176,6 +176,8 @@ enum queue_mode_states { + QUEUE_MODE_RQ, + }; + ++#define PROTOCOL_UNSET -1 ++ + enum scsi_protocol { + SCSI_PROTOCOL_FCP = 0, /* Fibre Channel */ + SCSI_PROTOCOL_SPI = 1, /* parallel SCSI */ +@@ -284,7 +286,7 @@ struct sg_id { + uint64_t lun; + short h_cmd_per_lun; + short d_queue_depth; +- enum scsi_protocol proto_id; ++ int proto_id; + int transport_id; + }; + +diff --git a/multipathd/fpin_handlers.c b/multipathd/fpin_handlers.c +index aaf5655d..571796e7 100644 +--- a/multipathd/fpin_handlers.c ++++ b/multipathd/fpin_handlers.c +@@ -219,7 +219,7 @@ static int fpin_chk_wwn_setpath_marginal(uint16_t host_num, struct vectors *ve + + vector_foreach_slot(vecs->pathvec, pp, k) { + /* Checks the host number and also for the SCSI FCP */ +- if (pp->sg_id.proto_id != SCSI_PROTOCOL_FCP || host_num != pp->sg_id.host_no) ++ if (pp->bus != SYSFS_BUS_SCSI || pp->sg_id.proto_id != SCSI_PROTOCOL_FCP || host_num != pp->sg_id.host_no) + continue; + sprintf(rport_id, "rport-%d:%d-%d", + pp->sg_id.host_no, pp->sg_id.channel, pp->sg_id.transport_id); diff --git a/0074-libmultipath-get-nvme-path-transport-protocol.patch b/0074-libmultipath-get-nvme-path-transport-protocol.patch new file mode 100644 index 0000000..128fa12 --- /dev/null +++ b/0074-libmultipath-get-nvme-path-transport-protocol.patch @@ -0,0 +1,194 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 7 Oct 2022 12:35:43 -0500 +Subject: [PATCH] libmultipath: get nvme path transport protocol + +Read the transport protocol from /sys/block/nvmeXnY/device/transport. +Update protocol_name[] and bus_protocol_id() to store the nvme protocol +names after the scsi protocol names. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmultipath/discovery.c | 18 ++++++++++++++++-- + libmultipath/structs.c | 22 +++++++++++++++++----- + libmultipath/structs.h | 33 +++++++++++++++++++++------------ + multipath/multipath.conf.5 | 10 +++++++--- + 4 files changed, 61 insertions(+), 22 deletions(-) + +diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c +index 7f2eb409..f593a7bf 100644 +--- a/libmultipath/discovery.c ++++ b/libmultipath/discovery.c +@@ -1483,6 +1483,7 @@ nvme_sysfs_pathinfo (struct path *pp, const struct _vector *hwtable) + struct udev_device *parent; + const char *attr_path = NULL; + const char *attr; ++ int i; + + if (pp->udev) + attr_path = udev_device_get_sysname(pp->udev); +@@ -1505,6 +1506,18 @@ nvme_sysfs_pathinfo (struct path *pp, const struct _vector *hwtable) + attr = udev_device_get_sysattr_value(parent, "cntlid"); + pp->sg_id.channel = attr ? atoi(attr) : 0; + ++ attr = udev_device_get_sysattr_value(parent, "transport"); ++ if (attr) { ++ for (i = 0; i < NVME_PROTOCOL_UNSPEC; i++){ ++ if (protocol_name[SYSFS_BUS_NVME + i] && ++ !strcmp(attr, ++ protocol_name[SYSFS_BUS_NVME + i] + 5)) { ++ pp->sg_id.proto_id = i; ++ break; ++ } ++ } ++ } ++ + snprintf(pp->vendor_id, SCSI_VENDOR_SIZE, "NVME"); + snprintf(pp->product_id, PATH_PRODUCT_SIZE, "%s", + udev_device_get_sysattr_value(parent, "model")); +@@ -1759,9 +1772,10 @@ sysfs_pathinfo(struct path *pp, const struct _vector *hwtable) + pp->bus = SYSFS_BUS_SCSI; + pp->sg_id.proto_id = SCSI_PROTOCOL_UNSPEC; + } +- if (!strncmp(pp->dev,"nvme", 4)) ++ if (!strncmp(pp->dev,"nvme", 4)) { + pp->bus = SYSFS_BUS_NVME; +- ++ pp->sg_id.proto_id = NVME_PROTOCOL_UNSPEC; ++ } + switch (pp->bus) { + case SYSFS_BUS_SCSI: + return scsi_sysfs_pathinfo(pp, hwtable); +diff --git a/libmultipath/structs.c b/libmultipath/structs.c +index be81a83c..a2e56890 100644 +--- a/libmultipath/structs.c ++++ b/libmultipath/structs.c +@@ -25,7 +25,6 @@ const char * const protocol_name[LAST_BUS_PROTOCOL_ID + 1] = { + [SYSFS_BUS_UNDEF] = "undef", + [SYSFS_BUS_CCW] = "ccw", + [SYSFS_BUS_CCISS] = "cciss", +- [SYSFS_BUS_NVME] = "nvme", + [SYSFS_BUS_SCSI + SCSI_PROTOCOL_FCP] = "scsi:fcp", + [SYSFS_BUS_SCSI + SCSI_PROTOCOL_SPI] = "scsi:spi", + [SYSFS_BUS_SCSI + SCSI_PROTOCOL_SSA] = "scsi:ssa", +@@ -37,6 +36,13 @@ const char * const protocol_name[LAST_BUS_PROTOCOL_ID + 1] = { + [SYSFS_BUS_SCSI + SCSI_PROTOCOL_ATA] = "scsi:ata", + [SYSFS_BUS_SCSI + SCSI_PROTOCOL_USB] = "scsi:usb", + [SYSFS_BUS_SCSI + SCSI_PROTOCOL_UNSPEC] = "scsi:unspec", ++ [SYSFS_BUS_NVME + NVME_PROTOCOL_PCIE] = "nvme:pcie", ++ [SYSFS_BUS_NVME + NVME_PROTOCOL_RDMA] = "nvme:rdma", ++ [SYSFS_BUS_NVME + NVME_PROTOCOL_FC] = "nvme:fc", ++ [SYSFS_BUS_NVME + NVME_PROTOCOL_TCP] = "nvme:tcp", ++ [SYSFS_BUS_NVME + NVME_PROTOCOL_LOOP] = "nvme:loop", ++ [SYSFS_BUS_NVME + NVME_PROTOCOL_APPLE_NVME] = "nvme:apple-nvme", ++ [SYSFS_BUS_NVME + NVME_PROTOCOL_UNSPEC] = "nvme:unspec", + }; + + struct adapter_group * +@@ -752,11 +758,17 @@ out: + } + + unsigned int bus_protocol_id(const struct path *pp) { +- if (!pp || pp->bus < 0 || pp->bus > SYSFS_BUS_SCSI) ++ if (!pp || pp->bus < 0 || pp->bus > SYSFS_BUS_NVME) + return SYSFS_BUS_UNDEF; +- if (pp->bus != SYSFS_BUS_SCSI) ++ if (pp->bus != SYSFS_BUS_SCSI && pp->bus != SYSFS_BUS_NVME) + return pp->bus; +- if ((int)pp->sg_id.proto_id < 0 || pp->sg_id.proto_id > SCSI_PROTOCOL_UNSPEC) ++ if (pp->sg_id.proto_id < 0) + return SYSFS_BUS_UNDEF; +- return SYSFS_BUS_SCSI + pp->sg_id.proto_id; ++ if (pp->bus == SYSFS_BUS_SCSI && ++ pp->sg_id.proto_id > SCSI_PROTOCOL_UNSPEC) ++ return SYSFS_BUS_UNDEF; ++ if (pp->bus == SYSFS_BUS_NVME && ++ pp->sg_id.proto_id > NVME_PROTOCOL_UNSPEC) ++ return SYSFS_BUS_UNDEF; ++ return pp->bus + pp->sg_id.proto_id; + } +diff --git a/libmultipath/structs.h b/libmultipath/structs.h +index 2525af17..0867b91d 100644 +--- a/libmultipath/structs.h ++++ b/libmultipath/structs.h +@@ -56,15 +56,6 @@ enum failback_mode { + FAILBACK_FOLLOWOVER + }; + +-/* SYSFS_BUS_SCSI should be last, see bus_protocol_id() */ +-enum sysfs_buses { +- SYSFS_BUS_UNDEF, +- SYSFS_BUS_CCW, +- SYSFS_BUS_CCISS, +- SYSFS_BUS_NVME, +- SYSFS_BUS_SCSI, +-}; +- + enum pathstates { + PSTATE_UNDEF, + PSTATE_FAILED, +@@ -190,14 +181,32 @@ enum scsi_protocol { + SCSI_PROTOCOL_ATA = 8, + SCSI_PROTOCOL_USB = 9, /* USB Attached SCSI (UAS), and others */ + SCSI_PROTOCOL_UNSPEC = 0xa, /* No specific protocol */ ++ SCSI_PROTOCOL_END = 0xb, /* offset of the next sysfs_buses entry */ ++}; ++ ++/* values from /sys/class/nvme/nvmeX */ ++enum nvme_protocol { ++ NVME_PROTOCOL_PCIE = 0, ++ NVME_PROTOCOL_RDMA = 1, ++ NVME_PROTOCOL_FC = 2, ++ NVME_PROTOCOL_TCP = 3, ++ NVME_PROTOCOL_LOOP = 4, ++ NVME_PROTOCOL_APPLE_NVME = 5, ++ NVME_PROTOCOL_UNSPEC = 6, /* unknown protocol */ ++}; ++ ++enum sysfs_buses { ++ SYSFS_BUS_UNDEF, ++ SYSFS_BUS_CCW, ++ SYSFS_BUS_CCISS, ++ SYSFS_BUS_SCSI, ++ SYSFS_BUS_NVME = SYSFS_BUS_SCSI + SCSI_PROTOCOL_END, + }; + + /* + * Linear ordering of bus/protocol +- * This assumes that SYSFS_BUS_SCSI is last in enum sysfs_buses +- * SCSI is the only bus type for which we distinguish protocols. + */ +-#define LAST_BUS_PROTOCOL_ID (SYSFS_BUS_SCSI + SCSI_PROTOCOL_UNSPEC) ++#define LAST_BUS_PROTOCOL_ID (SYSFS_BUS_NVME + NVME_PROTOCOL_UNSPEC) + unsigned int bus_protocol_id(const struct path *pp); + extern const char * const protocol_name[]; + +diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5 +index e1a787d4..7af53588 100644 +--- a/multipath/multipath.conf.5 ++++ b/multipath/multipath.conf.5 +@@ -1395,7 +1395,9 @@ Regular expression for the protocol of a device to be excluded/included. + The protocol strings that multipath recognizes are \fIscsi:fcp\fR, + \fIscsi:spi\fR, \fIscsi:ssa\fR, \fIscsi:sbp\fR, \fIscsi:srp\fR, + \fIscsi:iscsi\fR, \fIscsi:sas\fR, \fIscsi:adt\fR, \fIscsi:ata\fR, +-\fIscsi:unspec\fR, \fIccw\fR, \fIcciss\fR, \fInvme\fR, and \fIundef\fR. ++\fIscsi:unspec\fR, \fInvme:pcie\fR, \fInvme:rdma\fR, \fInvme:fc\fR, ++\fInvme:tcp\fR, \fInvme:loop\fR, \fInvme:apple-nvme\fR, \fInvme:unspec\fR, ++\fIccw\fR, \fIcciss\fR, and \fIundef\fR. + The protocol that a path is using can be viewed by running + \fBmultipathd show paths format "%d %P"\fR + .RE +@@ -1783,8 +1785,10 @@ The protocol subsection recognizes the following mandatory attribute: + The protocol string of the path device. The possible values are \fIscsi:fcp\fR, + \fIscsi:spi\fR, \fIscsi:ssa\fR, \fIscsi:sbp\fR, \fIscsi:srp\fR, + \fIscsi:iscsi\fR, \fIscsi:sas\fR, \fIscsi:adt\fR, \fIscsi:ata\fR, +-\fIscsi:unspec\fR, \fIccw\fR, \fIcciss\fR, \fInvme\fR, and \fIundef\fR. This is +-\fBnot\fR a regular expression. the path device protcol string must match ++\fIscsi:unspec\fR, \fInvme:pcie\fR, \fInvme:rdma\fR, \fInvme:fc\fR, ++\fInvme:tcp\fR, \fInvme:loop\fR, \fInvme:apple-nvme\fR, \fInvme:unspec\fR, ++\fIccw\fR, \fIcciss\fR, and \fIundef\fR. This is ++\fBnot\fR a regular expression. the path device protocol string must match + exactly. The protocol that a path is using can be viewed by running + \fBmultipathd show paths format "%d %P"\fR + .LP diff --git a/0075-libmultipath-enforce-queue_mode-bio-for-nmve-tcp-pat.patch b/0075-libmultipath-enforce-queue_mode-bio-for-nmve-tcp-pat.patch new file mode 100644 index 0000000..c95d2d4 --- /dev/null +++ b/0075-libmultipath-enforce-queue_mode-bio-for-nmve-tcp-pat.patch @@ -0,0 +1,111 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 7 Oct 2022 12:35:44 -0500 +Subject: [PATCH] libmultipath: enforce queue_mode bio for nmve:tcp paths + +nvme:tcp devices set BLK_MQ_F_BLOCKING (they are the only block devices +which multipath supports that do so), meaning that block_mq expects that +they can block at certain points while servicing a request. However, +due to the way device-mapper sets up its queue, it is not able to set +BLK_MQ_F_BLOCKING when it includes paths that set this flag. Patches +were written to address this issue but they were rejected upstream + +https://lore.kernel.org/linux-block/YcH%2FE4JNag0QYYAa@infradead.org/T/#t + +The proposed solution was to have multipath use the bio queue_mode for +multipath devices that include nvme:tcp paths. + +Multipath devices now automatically add the "queue_mode bio" feature if +they include nvme:tcp paths. If a multipath devices was created with +"queue_mode rq", it will disallow the addition of nvme:tcp paths. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmultipath/configure.c | 17 ++++++++++++++++- + libmultipath/structs_vec.c | 7 +++++++ + multipath/multipath.conf.5 | 4 +++- + 3 files changed, 26 insertions(+), 2 deletions(-) + +diff --git a/libmultipath/configure.c b/libmultipath/configure.c +index c6803b40..193bf27d 100644 +--- a/libmultipath/configure.c ++++ b/libmultipath/configure.c +@@ -296,6 +296,7 @@ static int wait_for_pending_paths(struct multipath *mpp, + int setup_map(struct multipath *mpp, char **params, struct vectors *vecs) + { + struct pathgroup * pgp; ++ struct path *pp; + struct config *conf; + int i, n_paths, marginal_pathgroups; + char *save_attr; +@@ -311,6 +312,14 @@ int setup_map(struct multipath *mpp, char **params, struct vectors *vecs) + if (mpp->disable_queueing && VECTOR_SIZE(mpp->paths) != 0) + mpp->disable_queueing = 0; + ++ /* Force QUEUE_MODE_BIO for maps with nvme:tcp paths */ ++ vector_foreach_slot(mpp->paths, pp, i) { ++ if (pp->bus == SYSFS_BUS_NVME && ++ pp->sg_id.proto_id == NVME_PROTOCOL_TCP) { ++ mpp->queue_mode = QUEUE_MODE_BIO; ++ break; ++ } ++ } + /* + * If this map was created with add_map_without_path(), + * mpp->hwe might not be set yet. +@@ -1191,6 +1200,13 @@ int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid, + continue; + } + ++ cmpp = find_mp_by_wwid(curmp, pp1->wwid); ++ if (cmpp && cmpp->queue_mode == QUEUE_MODE_RQ && ++ pp1->bus == SYSFS_BUS_NVME && pp1->sg_id.proto_id == ++ NVME_PROTOCOL_TCP) { ++ orphan_path(pp1, "nvme:tcp path not allowed with request queue_mode multipath device"); ++ continue; ++ } + /* + * at this point, we know we really got a new mp + */ +@@ -1229,7 +1245,6 @@ int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid, + } + verify_paths(mpp); + +- cmpp = find_mp_by_wwid(curmp, mpp->wwid); + if (cmpp) + mpp->queue_mode = cmpp->queue_mode; + if (setup_map(mpp, ¶ms, vecs)) { +diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c +index 85d97ac1..4a32b405 100644 +--- a/libmultipath/structs_vec.c ++++ b/libmultipath/structs_vec.c +@@ -262,6 +262,13 @@ int adopt_paths(vector pathvec, struct multipath *mpp) + } + if (pp->initialized == INIT_REMOVED) + continue; ++ if (mpp->queue_mode == QUEUE_MODE_RQ && ++ pp->bus == SYSFS_BUS_NVME && ++ pp->sg_id.proto_id == NVME_PROTOCOL_TCP) { ++ condlog(2, "%s: mulitpath device %s created with request queue_mode. Unable to add nvme:tcp paths", ++ pp->dev, mpp->alias); ++ continue; ++ } + if (!mpp->paths && !(mpp->paths = vector_alloc())) + goto err; + +diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5 +index 7af53588..01904feb 100644 +--- a/multipath/multipath.conf.5 ++++ b/multipath/multipath.conf.5 +@@ -472,7 +472,9 @@ Before kernel 4.20 The default depends on the kernel parameter + \fBdm_mod.use_blk_mq\fR. It is \fImq\fR if the latter is set, and \fIrq\fR + otherwise. Since kernel 4.20, \fIrq\fR and \fImq\fR both correspond to + block-multiqueue. Once a multipath device has been created, its queue_mode +-cannot be changed. ++cannot be changed. \fInvme:tcp\fR paths are only supported in multipath ++devices with queue_mode set to \fIbio\fR. multipath will automatically ++set this when creating a device with \fInvme:tcp\fR paths. + .TP + The default is: \fB\fR + .RE diff --git a/device-mapper-multipath.spec b/device-mapper-multipath.spec index 97dcaaf..53386c6 100644 --- a/device-mapper-multipath.spec +++ b/device-mapper-multipath.spec @@ -1,6 +1,6 @@ Name: device-mapper-multipath Version: 0.8.7 -Release: 14%{?dist} +Release: 15%{?dist} Summary: Tools to manage multipath devices using device-mapper License: GPLv2 URL: http://christophe.varoqui.free.fr/ @@ -76,6 +76,15 @@ Patch0063: 0063-libmultipath-return-success-if-we-raced-to-remove-a-.patch Patch0064: 0064-multipathd-Handle-losing-all-path-in-update_map.patch Patch0065: 0065-multipathd-ignore-duplicated-multipathd-command-keys.patch Patch0066: 0066-multipath-tools-use-run-instead-of-dev-shm.patch +Patch0067: 0067-kpartx-hold-device-open-until-partitions-have-been-c.patch +Patch0068: 0068-libmultipath-cleanup-remove_feature.patch +Patch0069: 0069-libmultipath-cleanup-add_feature.patch +Patch0070: 0070-multipath-tests-tests-for-adding-and-removing-featur.patch +Patch0071: 0071-libmultipath-fix-queue_mode-feature-handling.patch +Patch0072: 0072-multipath-tests-tests-for-reconcile_features_with_qu.patch +Patch0073: 0073-libmultipath-prepare-proto_id-for-use-by-non-scsi-de.patch +Patch0074: 0074-libmultipath-get-nvme-path-transport-protocol.patch +Patch0075: 0075-libmultipath-enforce-queue_mode-bio-for-nmve-tcp-pat.patch # runtime @@ -278,6 +287,20 @@ fi %{_pkgconfdir}/libdmmp.pc %changelog +* Thu Nov 10 2022 Benjamin Marzinski - 0.8.7-15 +- Add 0067-kpartx-hold-device-open-until-partitions-have-been-c.patch + * Fixes bz #2141860 +- Add 0068-libmultipath-cleanup-remove_feature.patch +- Add 0069-libmultipath-cleanup-add_feature.patch +- Add 0070-multipath-tests-tests-for-adding-and-removing-featur.patch +- Add 0071-libmultipath-fix-queue_mode-feature-handling.patch +- Add 0072-multipath-tests-tests-for-reconcile_features_with_qu.patch +- Add 0073-libmultipath-prepare-proto_id-for-use-by-non-scsi-de.patch +- Add 0074-libmultipath-get-nvme-path-transport-protocol.patch +- Add 0075-libmultipath-enforce-queue_mode-bio-for-nmve-tcp-pat.patch + * Fixes bz #2033080 +- Resolves: bz #2033080, #2141860 + * Thu Oct 13 2022 Benjamin Marzinski - 0.8.7-14 - Add 0065-multipathd-ignore-duplicated-multipathd-command-keys.patch * Fixes bz #2133999