diff --git a/SOURCES/bz1679792-1-votequorum-set-wfa-status-only-on-startup.patch b/SOURCES/bz1679792-1-votequorum-set-wfa-status-only-on-startup.patch new file mode 100644 index 0000000..c21ba76 --- /dev/null +++ b/SOURCES/bz1679792-1-votequorum-set-wfa-status-only-on-startup.patch @@ -0,0 +1,68 @@ +From 6894792d76b1e8932bc822bb040933ae17e1a0c7 Mon Sep 17 00:00:00 2001 +From: Jan Friesse +Date: Tue, 10 Mar 2020 17:49:27 +0100 +Subject: [PATCH] votequorum: set wfa status only on startup + +Previously reload of configuration with enabled wait_for_all result in +set of wait_for_all_status which set cluster_is_quorate to 0 but didn't +inform the quorum service so votequorum and quorum information may get +out of sync. + +Example is 1 node cluster, which is extended to 3 nodes. Quorum service +reports cluster as a quorate (incorrect) and votequorum as not-quorate +(correct). Similar behavior happens when extending cluster in general, +but some configurations are less incorrect (3->4). + +Discussed solution was to inform quorum service but that would mean +every reload would cause loss of quorum until all nodes would be seen +again. + +Such behaviour is consistent but seems to be a bit too strict. + +Proposed solution sets wait_for_all_status only on startup and +doesn't touch it during reload. + +This solution fulfills requirement of "cluster will be quorate for +the first time only after all nodes have been visible at least +once at the same time." because node clears wait_for_all_status only +after it sees all other nodes or joins cluster which is quorate. It also +solves problem with extending cluster, because when cluster becomes +unquorate (1->3) wait_for_all_status is set. + +Added assert is only for ensure that I haven't missed any case when +quorate cluster may become unquorate. + +Signed-off-by: Jan Friesse +Reviewed-by: Christine Caulfield +(cherry picked from commit ca320beac25f82c0c555799e647a47975a333c28) +--- + exec/votequorum.c | 6 ++++-- + 1 file changed, 4 insertions(+), 2 deletions(-) + +diff --git a/exec/votequorum.c b/exec/votequorum.c +index 1cbbe37..8b96199 100644 +--- a/exec/votequorum.c ++++ b/exec/votequorum.c +@@ -1016,7 +1016,7 @@ static void are_we_quorate(unsigned int total_votes) + "Waiting for all cluster members. " + "Current votes: %d expected_votes: %d", + total_votes, us->expected_votes); +- cluster_is_quorate = 0; ++ assert(!cluster_is_quorate); + return; + } + update_wait_for_all_status(0); +@@ -1548,7 +1548,9 @@ static char *votequorum_readconfig(int runtime) + update_ev_barrier(us->expected_votes); + update_two_node(); + if (wait_for_all) { +- update_wait_for_all_status(1); ++ if (!runtime) { ++ update_wait_for_all_status(1); ++ } + } else if (wait_for_all_autoset && wait_for_all_status) { + /* + * Reset wait for all status for consistency when wfa is auto-unset by 2node. +-- +1.8.3.1 + diff --git a/SOURCES/bz1780134-1-votequorum-Ignore-the-icmap_get_-return-value.patch b/SOURCES/bz1780134-1-votequorum-Ignore-the-icmap_get_-return-value.patch new file mode 100644 index 0000000..abac158 --- /dev/null +++ b/SOURCES/bz1780134-1-votequorum-Ignore-the-icmap_get_-return-value.patch @@ -0,0 +1,74 @@ +From 8ad3c6bbb4556332c5a6b7fecdab73310c045b24 Mon Sep 17 00:00:00 2001 +From: Jan Friesse +Date: Mon, 25 Nov 2019 18:21:52 +0100 +Subject: [PATCH] votequorum: Ignore the icmap_get_* return value + +Express intention to ignore icmap_get_* return +value and rely on default behavior of not changing the output +parameter on error. + +Signed-off-by: Jan Friesse +(cherry picked from commit cddd62f972bca276c934e58f08da84071cec1ddb) +--- + exec/votequorum.c | 22 +++++++++++----------- + 1 file changed, 11 insertions(+), 11 deletions(-) + +diff --git a/exec/votequorum.c b/exec/votequorum.c +index 49bcdfa..e3cb7eb 100644 +--- a/exec/votequorum.c ++++ b/exec/votequorum.c +@@ -1272,10 +1272,10 @@ static char *votequorum_readconfig(int runtime) + /* + * gather basic data here + */ +- icmap_get_uint32("quorum.expected_votes", &expected_votes); ++ (void)icmap_get_uint32("quorum.expected_votes", &expected_votes); + have_nodelist = votequorum_read_nodelist_configuration(&node_votes, &node_count, &node_expected_votes); + have_qdevice = votequorum_qdevice_is_configured(&qdevice_votes); +- icmap_get_uint8("quorum.two_node", &two_node); ++ (void)icmap_get_uint8("quorum.two_node", &two_node); + + /* + * do config verification and enablement +@@ -1320,13 +1320,13 @@ static char *votequorum_readconfig(int runtime) + wait_for_all = 1; + } + +- icmap_get_uint8("quorum.allow_downscale", &allow_downscale); +- icmap_get_uint8("quorum.wait_for_all", &wait_for_all); +- icmap_get_uint8("quorum.last_man_standing", &last_man_standing); +- icmap_get_uint32("quorum.last_man_standing_window", &last_man_standing_window); +- icmap_get_uint8("quorum.expected_votes_tracking", &ev_tracking); +- icmap_get_uint8("quorum.auto_tie_breaker", &atb); +- icmap_get_string("quorum.auto_tie_breaker_node", &atb_string); ++ (void)icmap_get_uint8("quorum.allow_downscale", &allow_downscale); ++ (void)icmap_get_uint8("quorum.wait_for_all", &wait_for_all); ++ (void)icmap_get_uint8("quorum.last_man_standing", &last_man_standing); ++ (void)icmap_get_uint32("quorum.last_man_standing_window", &last_man_standing_window); ++ (void)icmap_get_uint8("quorum.expected_votes_tracking", &ev_tracking); ++ (void)icmap_get_uint8("quorum.auto_tie_breaker", &atb); ++ (void)icmap_get_string("quorum.auto_tie_breaker_node", &atb_string); + + /* auto_tie_breaker defaults to LOWEST */ + if (atb) { +@@ -1518,7 +1518,7 @@ static char *votequorum_readconfig(int runtime) + us->expected_votes = node_expected_votes; + } else { + us->votes = 1; +- icmap_get_uint32("quorum.votes", &us->votes); ++ (void)icmap_get_uint32("quorum.votes", &us->votes); + } + + if (expected_votes) { +@@ -1569,7 +1569,7 @@ static void votequorum_refresh_config( + return ; + } + +- icmap_get_uint8("quorum.cancel_wait_for_all", &cancel_wfa); ++ (void)icmap_get_uint8("quorum.cancel_wait_for_all", &cancel_wfa); + if (strcmp(key_name, "quorum.cancel_wait_for_all") == 0 && + cancel_wfa >= 1) { + icmap_set_uint8("quorum.cancel_wait_for_all", 0); +-- +1.8.3.1 + diff --git a/SOURCES/bz1780134-2-votequorum-Reflect-runtime-change-of-2Node-to-WFA.patch b/SOURCES/bz1780134-2-votequorum-Reflect-runtime-change-of-2Node-to-WFA.patch new file mode 100644 index 0000000..a377c83 --- /dev/null +++ b/SOURCES/bz1780134-2-votequorum-Reflect-runtime-change-of-2Node-to-WFA.patch @@ -0,0 +1,81 @@ +From bfbed8c320b0c0c5d3db48630f3de77e5fd62b75 Mon Sep 17 00:00:00 2001 +From: Jan Friesse +Date: Thu, 16 Jan 2020 15:43:59 +0100 +Subject: [PATCH] votequorum: Reflect runtime change of 2Node to WFA + +When 2Node mode is set, WFA is also set unless WFA is configured +explicitly. This behavior was not reflected on runtime change, so +restarted corosync behavior was different (WFA not set). Also when +cluster is reduced from 3 nodes to 2 nodes during runtime, WFA was not +set, what may result in two quorate partitions. + +Solution is to set WFA depending on 2Node when WFA +is not explicitly configured. + +Signed-off-by: Jan Friesse +Reviewed-by: Christine Caulfield +(cherry picked from commit 8ce65bf951bc1e5b2d64b60ea027fbdc551d4fc8) +--- + exec/votequorum.c | 24 +++++++++++++++++++----- + 1 file changed, 19 insertions(+), 5 deletions(-) + +diff --git a/exec/votequorum.c b/exec/votequorum.c +index 2fb5db9..d87b6fd 100644 +--- a/exec/votequorum.c ++++ b/exec/votequorum.c +@@ -80,6 +80,7 @@ static uint8_t two_node = 0; + + static uint8_t wait_for_all = 0; + static uint8_t wait_for_all_status = 0; ++static uint8_t wait_for_all_autoset = 0; /* Wait for all is not set explicitly and follows two_node */ + + static enum {ATB_NONE, ATB_LOWEST, ATB_HIGHEST, ATB_LIST} auto_tie_breaker = ATB_NONE, initial_auto_tie_breaker = ATB_NONE; + static int lowest_node_id = -1; +@@ -1316,12 +1317,10 @@ static char *votequorum_readconfig(int runtime) + * Enable special features + */ + if (!runtime) { +- if (two_node) { +- wait_for_all = 1; +- } +- + (void)icmap_get_uint8("quorum.allow_downscale", &allow_downscale); +- (void)icmap_get_uint8("quorum.wait_for_all", &wait_for_all); ++ if (icmap_get_uint8("quorum.wait_for_all", &wait_for_all) != CS_OK) { ++ wait_for_all_autoset = 1; ++ } + (void)icmap_get_uint8("quorum.last_man_standing", &last_man_standing); + (void)icmap_get_uint32("quorum.last_man_standing_window", &last_man_standing_window); + (void)icmap_get_uint8("quorum.expected_votes_tracking", &ev_tracking); +@@ -1362,6 +1361,15 @@ static char *votequorum_readconfig(int runtime) + + } + ++ /* ++ * Changing of wait_for_all during runtime is not supported, but changing of two_node is ++ * and two_node may set wfa if not configured explicitly. It is safe to unset it ++ * (or set it back) when two_node changes. ++ */ ++ if (wait_for_all_autoset) { ++ wait_for_all = two_node; ++ } ++ + /* two_node and auto_tie_breaker are not compatible as two_node uses + * a fence race to decide quorum whereas ATB decides based on node id + */ +@@ -1541,6 +1549,12 @@ static char *votequorum_readconfig(int runtime) + update_two_node(); + if (wait_for_all) { + update_wait_for_all_status(1); ++ } else if (wait_for_all_autoset && wait_for_all_status) { ++ /* ++ * Reset wait for all status for consistency when wfa is auto-unset by 2node. ++ * wait_for_all_status would be ignored by are_we_quorate anyway. ++ */ ++ update_wait_for_all_status(0); + } + + out: +-- +1.8.3.1 + diff --git a/SPECS/corosync.spec b/SPECS/corosync.spec index 05ee841..30cab5f 100644 --- a/SPECS/corosync.spec +++ b/SPECS/corosync.spec @@ -29,7 +29,7 @@ Name: corosync Summary: The Corosync Cluster Engine and Application Programming Interfaces Version: 2.4.5 -Release: 4%{?gitver}%{?dist} +Release: 5%{?gitver}%{?dist} License: BSD Group: System Environment/Base URL: http://corosync.github.io/corosync/ @@ -43,6 +43,9 @@ Source1: https://github.com/jfriesse/spausedd/releases/download/%{spausedd_versi %endif Patch0: bz1656492-1-totem-Increase-ring_id-seq-after-load.patch +Patch1: bz1780134-1-votequorum-Ignore-the-icmap_get_-return-value.patch +Patch2: bz1780134-2-votequorum-Reflect-runtime-change-of-2Node-to-WFA.patch +Patch3: bz1679792-1-votequorum-set-wfa-status-only-on-startup.patch %if 0%{?rhel} ExclusiveArch: i686 x86_64 s390x ppc64le aarch64 @@ -111,6 +114,9 @@ BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) %endif %patch0 -p1 -b .bz1656492-1 +%patch1 -p1 -b .bz1780134-1 +%patch2 -p1 -b .bz1780134-2 +%patch3 -p1 -b .bz1679792-1 %build %if %{with runautogen} @@ -619,6 +625,17 @@ fi %endif %changelog +* Tue Mar 24 2020 Jan Friesse 2.4.5-5 +- Resolves: rhbz#1679792 +- Resolves: rhbz#1780134 + +- votequorum: Ignore the icmap_get_* return value (rhbz#1780134) +- merge upstream commit 8ad3c6bbb4556332c5a6b7fecdab73310c045b24 (rhbz#1780134) +- votequorum: Reflect runtime change of 2Node to WFA (rhbz#1780134) +- merge upstream commit bfbed8c320b0c0c5d3db48630f3de77e5fd62b75 (rhbz#1780134) +- votequorum: set wfa status only on startup (rhbz#1679792) +- merge upstream commit 6894792d76b1e8932bc822bb040933ae17e1a0c7 (rhbz#1679792) + * Wed Aug 07 2019 Jan Friesse - 2.4.5-4 - Related: rhbz#1737884