Control: tag -1 - confirmed Jonathan Wiltshire <j...@debian.org> writes:
> On Wed, Jul 05, 2023 at 07:14:09PM +0200, Ferenc Wágner wrote: > >> Shortly after the release of bookworm we got a report that Pacemaker >> regressed in certain migration scenarios when compared to the bullseye >> version. Upstream identified the cause (a bug already fixed in 2.1.6), >> and after backporting the fix the submitter acknowledged that they can't >> reproduce the bug anymore with the proposed packages. >> https://bugs.clusterlabs.org/show_bug.cgi?id=5521 >> Pacemaker package bug opened after discussion on the mailing list: >> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1040165 > > Please go ahead, and bear in mind the upload window closes next weekend. Thanks, Jonathan! Does it mean that I have to upload before 15th of July? On the other hand, meanwhile upstream notified me that to fully fix this bug I need to backport one more patch, which in turn required including a third one. So the debdiff grew a little, please reconfirm the upload: $ debdiff pacemaker_2.1.5-1.dsc pacemaker_2.1.5-1+deb12u1.dsc dpkg-source: warning: extracting unsigned source package (/home/wferi/ha/pacemaker/pacemaker_2.1.5-1.dsc) diff -Nru pacemaker-2.1.5/debian/changelog pacemaker-2.1.5/debian/changelog --- pacemaker-2.1.5/debian/changelog 2023-01-22 16:38:34.000000000 +0100 +++ pacemaker-2.1.5/debian/changelog 2023-07-09 23:10:45.000000000 +0200 @@ -1,3 +1,17 @@ +pacemaker (2.1.5-1+deb12u1) bookworm; urgency=medium + + * [0c22be8] New patches fixing migration regression. + Backport of https://github.com/ClusterLabs/pacemaker/pull/3020/ to + Pacemaker 2.1.5 (without the CTS changes, which we don't ship): + 5754a2af9 Refactor: scheduler: improve xpath efficiency when unpacking + 3f6f524f1 Low: scheduler: unknown_on_node() should ignore pending actions + ad9fd9548 Fix: scheduler: handle cleaned migrate_from history correctly + The starting refactor is required by the other two patches, but the + third patch still needed backporting. + Thanks to Ken Gaillot (Closes: #1040165) + + -- Ferenc Wágner <wf...@debian.org> Sun, 09 Jul 2023 23:10:45 +0200 + pacemaker (2.1.5-1) unstable; urgency=medium * [5792d59] Work around lazy loading of GitHub release pages in watch file diff -Nru pacemaker-2.1.5/debian/gbp.conf pacemaker-2.1.5/debian/gbp.conf --- pacemaker-2.1.5/debian/gbp.conf 2023-01-22 13:10:39.000000000 +0100 +++ pacemaker-2.1.5/debian/gbp.conf 2023-07-09 22:33:06.000000000 +0200 @@ -1,5 +1,5 @@ [DEFAULT] -debian-branch = debian/master +debian-branch = debian/bookworm upstream-branch = upstream/latest [import-orig] diff -Nru pacemaker-2.1.5/debian/patches/Fix-scheduler-handle-cleaned-migrate_from-history-correct.patch pacemaker-2.1.5/debian/patches/Fix-scheduler-handle-cleaned-migrate_from-history-correct.patch --- pacemaker-2.1.5/debian/patches/Fix-scheduler-handle-cleaned-migrate_from-history-correct.patch 1970-01-01 01:00:00.000000000 +0100 +++ pacemaker-2.1.5/debian/patches/Fix-scheduler-handle-cleaned-migrate_from-history-correct.patch 2023-07-09 23:07:30.000000000 +0200 @@ -0,0 +1,30 @@ +From: Ken Gaillot <kgail...@redhat.com> +Date: Wed, 1 Feb 2023 17:12:13 -0600 +Subject: Fix: scheduler: handle cleaned migrate_from history correctly + +Fixes T623 +--- + lib/pengine/unpack.c | 10 ++++++++++ + 1 file changed, 10 insertions(+) + +diff --git a/lib/pengine/unpack.c b/lib/pengine/unpack.c +index e9fcae1..99a2dc4 100644 +--- a/lib/pengine/unpack.c ++++ b/lib/pengine/unpack.c +@@ -2937,6 +2937,16 @@ unpack_migrate_to_success(pe_resource_t *rsc, pe_node_t *node, xmlNode *xml_op, + } + + } else { // Pending, or complete but erased ++ ++ /* If there is no history at all for the resource on an online target, then ++ * it was likely cleaned. Just return, and we'll schedule a probe. Once we ++ * have the probe result, it will be reflected in target_newer_state. ++ */ ++ if ((target_node != NULL) && target_node->details->online ++ && unknown_on_node(rsc, target)) { ++ return; ++ } ++ + /* If the resource has newer state on the target, this migrate_to no + * longer matters for the target. + */ diff -Nru pacemaker-2.1.5/debian/patches/Low-scheduler-unknown_on_node-should-ignore-pending-actio.patch pacemaker-2.1.5/debian/patches/Low-scheduler-unknown_on_node-should-ignore-pending-actio.patch --- pacemaker-2.1.5/debian/patches/Low-scheduler-unknown_on_node-should-ignore-pending-actio.patch 1970-01-01 01:00:00.000000000 +0100 +++ pacemaker-2.1.5/debian/patches/Low-scheduler-unknown_on_node-should-ignore-pending-actio.patch 2023-07-09 23:07:30.000000000 +0200 @@ -0,0 +1,80 @@ +From: Ken Gaillot <kgail...@redhat.com> +Date: Thu, 2 Feb 2023 10:25:53 -0600 +Subject: Low: scheduler: unknown_on_node() should ignore pending actions + +Previously, unknown_on_node() looked for any lrm_rsc_op at all to decide +whether a resource is known on a node. However if the only action is pending, +the resource is not yet known. + +Also drop a redundant argument and add a doxygen block. (The rsc argument is +not const due to a getDocPtr() call in the chain, as well as libxml2 calls that +are likely const in practice but aren't marked as such.) +--- + lib/pengine/unpack.c | 37 +++++++++++++++++++++++++------------ + 1 file changed, 25 insertions(+), 12 deletions(-) + +diff --git a/lib/pengine/unpack.c b/lib/pengine/unpack.c +index 41cb980..e9fcae1 100644 +--- a/lib/pengine/unpack.c ++++ b/lib/pengine/unpack.c +@@ -2654,19 +2654,32 @@ find_lrm_resource(const char *rsc_id, const char *node_name, + return xml; + } + ++/*! ++ * \internal ++ * \brief Check whether a resource has no completed action history on a node ++ * ++ * \param[in,out] rsc Resource to check ++ * \param[in] node_name Node to check ++ * ++ * \return true if \p rsc_id is unknown on \p node_name, otherwise false ++ */ + static bool +-unknown_on_node(const char *rsc_id, const char *node_name, +- pe_working_set_t *data_set) ++unknown_on_node(pe_resource_t *rsc, const char *node_name) + { +- xmlNode *lrm_resource = NULL; +- +- lrm_resource = find_lrm_resource(rsc_id, node_name, data_set); ++ bool result = false; ++ xmlXPathObjectPtr search; ++ GString *xpath = g_string_sized_new(256); + +- /* If the resource has no lrm_rsc_op history on the node, that means its +- * state is unknown there. +- */ +- return (lrm_resource == NULL +- || first_named_child(lrm_resource, XML_LRM_TAG_RSC_OP) == NULL); ++ pcmk__g_strcat(xpath, ++ XPATH_NODE_STATE "[@" XML_ATTR_UNAME "='", node_name, "']" ++ SUB_XPATH_LRM_RESOURCE "[@" XML_ATTR_ID "='", rsc->id, "']" ++ SUB_XPATH_LRM_RSC_OP "[@" XML_LRM_ATTR_RC "!='193']", ++ NULL); ++ search = xpath_search(rsc->cluster->input, (const char *) xpath->str); ++ result = (numXpathResults(search) == 0); ++ freeXpathObject(search); ++ g_string_free(xpath, TRUE); ++ return result; + } + + /*! +@@ -2979,7 +2992,7 @@ unpack_migrate_to_failure(pe_resource_t *rsc, pe_node_t *node, xmlNode *xml_op, + * Don't just consider it running there. We will get back here anyway in + * case the probe detects it's running there. + */ +- !unknown_on_node(rsc->id, target, data_set) ++ !unknown_on_node(rsc, target) + /* If the resource has newer state on the target after the migration + * events, this migrate_to no longer matters for the target. + */ +@@ -3031,7 +3044,7 @@ unpack_migrate_from_failure(pe_resource_t *rsc, pe_node_t *node, + * Don't just consider it running there. We will get back here anyway in + * case the probe detects it's running there. + */ +- !unknown_on_node(rsc->id, source, data_set) ++ !unknown_on_node(rsc, source) + /* If the resource has newer state on the source after the migration + * events, this migrate_from no longer matters for the source. + */ diff -Nru pacemaker-2.1.5/debian/patches/Refactor-scheduler-improve-xpath-efficiency-when-unpackin.patch pacemaker-2.1.5/debian/patches/Refactor-scheduler-improve-xpath-efficiency-when-unpackin.patch --- pacemaker-2.1.5/debian/patches/Refactor-scheduler-improve-xpath-efficiency-when-unpackin.patch 1970-01-01 01:00:00.000000000 +0100 +++ pacemaker-2.1.5/debian/patches/Refactor-scheduler-improve-xpath-efficiency-when-unpackin.patch 2023-07-09 23:07:30.000000000 +0200 @@ -0,0 +1,55 @@ +From: Ken Gaillot <kgail...@redhat.com> +Date: Thu, 2 Feb 2023 09:42:01 -0600 +Subject: Refactor: scheduler: improve xpath efficiency when unpacking + +Using "//" means that every child must be searched recursively. If we know the +exact path, we should explicitly specify it. +--- + lib/pengine/unpack.c | 20 ++++++++++++-------- + 1 file changed, 12 insertions(+), 8 deletions(-) + +diff --git a/lib/pengine/unpack.c b/lib/pengine/unpack.c +index 5fcba3b..41cb980 100644 +--- a/lib/pengine/unpack.c ++++ b/lib/pengine/unpack.c +@@ -2577,6 +2577,13 @@ set_node_score(gpointer key, gpointer value, gpointer user_data) + node->weight = *score; + } + ++#define XPATH_NODE_STATE "/" XML_TAG_CIB "/" XML_CIB_TAG_STATUS \ ++ "/" XML_CIB_TAG_STATE ++#define SUB_XPATH_LRM_RESOURCE "/" XML_CIB_TAG_LRM \ ++ "/" XML_LRM_TAG_RESOURCES \ ++ "/" XML_LRM_TAG_RESOURCE ++#define SUB_XPATH_LRM_RSC_OP "/" XML_LRM_TAG_RSC_OP ++ + static xmlNode * + find_lrm_op(const char *resource, const char *op, const char *node, const char *source, + int target_rc, pe_working_set_t *data_set) +@@ -2589,10 +2596,9 @@ find_lrm_op(const char *resource, const char *op, const char *node, const char * + + xpath = g_string_sized_new(256); + pcmk__g_strcat(xpath, +- "//" XML_CIB_TAG_STATE "[@" XML_ATTR_UNAME "='", node, "']" +- "//" XML_LRM_TAG_RESOURCE +- "[@" XML_ATTR_ID "='", resource, "']" +- "/" XML_LRM_TAG_RSC_OP "[@" XML_LRM_ATTR_TASK "='", op, "'", ++ XPATH_NODE_STATE "[@" XML_ATTR_UNAME "='", node, "']" ++ SUB_XPATH_LRM_RESOURCE "[@" XML_ATTR_ID "='", resource, "']" ++ SUB_XPATH_LRM_RSC_OP "[@" XML_LRM_ATTR_TASK "='", op, "'", + NULL); + + /* Need to check against transition_magic too? */ +@@ -2637,10 +2643,8 @@ find_lrm_resource(const char *rsc_id, const char *node_name, + + xpath = g_string_sized_new(256); + pcmk__g_strcat(xpath, +- "//" XML_CIB_TAG_STATE +- "[@" XML_ATTR_UNAME "='", node_name, "']" +- "//" XML_LRM_TAG_RESOURCE +- "[@" XML_ATTR_ID "='", rsc_id, "']", ++ XPATH_NODE_STATE "[@" XML_ATTR_UNAME "='", node_name, "']" ++ SUB_XPATH_LRM_RESOURCE "[@" XML_ATTR_ID "='", rsc_id, "']", + NULL); + + xml = get_xpath_object((const char *) xpath->str, data_set->input, diff -Nru pacemaker-2.1.5/debian/patches/series pacemaker-2.1.5/debian/patches/series --- pacemaker-2.1.5/debian/patches/series 2023-01-22 13:31:42.000000000 +0100 +++ pacemaker-2.1.5/debian/patches/series 2023-07-09 23:07:30.000000000 +0200 @@ -5,3 +5,6 @@ Shipping-the-CTS-is-not-useful.patch Always-run-Inkscape-under-the-C.UTF-8-locale.patch Fix-typos-resouce-resource.patch +Refactor-scheduler-improve-xpath-efficiency-when-unpackin.patch +Low-scheduler-unknown_on_node-should-ignore-pending-actio.patch +Fix-scheduler-handle-cleaned-migrate_from-history-correct.patch diff -Nru pacemaker-2.1.5/debian/salsa-ci.yml pacemaker-2.1.5/debian/salsa-ci.yml --- pacemaker-2.1.5/debian/salsa-ci.yml 2023-01-22 13:10:39.000000000 +0100 +++ pacemaker-2.1.5/debian/salsa-ci.yml 2023-07-09 22:33:06.000000000 +0200 @@ -5,6 +5,7 @@ variables: SALSA_CI_REPROTEST_ENABLE_DIFFOSCOPE: 1 + RELEASE: bookworm autopkgtest: extends: .test-autopkgtest -- Thanks, Feri.
