Merge ~rafaeldtinoco/ubuntu/+source/pacemaker:lp1881762-focal-backport into ubuntu/+source/pacemaker:ubuntu/focal-devel

Proposed by Rafael David Tinoco
Status: Merged
Approved by: Rafael David Tinoco
Approved revision: de754cffed29f56ac41cc8f7431771e9803becab
Merged at revision: de754cffed29f56ac41cc8f7431771e9803becab
Proposed branch: ~rafaeldtinoco/ubuntu/+source/pacemaker:lp1881762-focal-backport
Merge into: ubuntu/+source/pacemaker:ubuntu/focal-devel
Diff against target: 466 lines (+419/-2)
6 files modified
debian/changelog (+10/-0)
debian/patches/series (+7/-0)
debian/patches/ubuntu-2.0.3-fixes/lp1881762-01-Executor-systemd-execution-time-fixes.patch (+167/-0)
debian/patches/ubuntu-2.0.3-fixes/lp1881762-02-Low-executor-record-correct-last-run-and-last-rc.patch (+133/-0)
debian/patches/ubuntu-2.0.3-fixes/lp1881762-03-Refactor-executor-functionize-getting-current.patch (+88/-0)
debian/rules (+14/-2)
Reviewer Review Type Date Requested Status
Rafael David Tinoco (community) Approve
Robie Basak sru Approve
Canonical Server Pending
Review via email: mp+391960@code.launchpad.net

Description of the change

I would like @racb's review (SRU wise) before uploading as we've been discussing about this particular SRU.

To post a comment you must log in.
Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

I'll still finish testing over built packages, but for now everything seems alright. Will upload once the testcase is checked and also all the regression/functional tests.

Revision history for this message
Robie Basak (racb) wrote :

Looks good, thanks! This was much easier to review so I'm much more confident in checking that it looks correct and won't regress anything, and I think this approach meets the SRU "minimal fix" policy much better. Minor comments inline.

How well does the test case in the bug cover these three specific things being fixed? Can we make sure to cover all three, such that if one of the fixes doesn't work, we'll find that out at SRU verification time?

I think the Regression Potential section in the bug needs updating now with this different approach?

review: Needs Fixing
Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

> Looks good, thanks! This was much easier to review so I'm much more confident
> in checking that it looks correct and won't regress anything, and I think this
> approach meets the SRU "minimal fix" policy much better. Minor comments
> inline.

I'll get into those next comment.

> How well does the test case in the bug cover these three specific things being
> fixed? Can we make sure to cover all three, such that if one of the fixes
> doesn't work, we'll find that out at SRU verification time?

1st patch: covered very well, original issue actually
2nd patch: its covered as a side effect in same test case (as 1st one would not work without it)
3rd patch: I don't see a clear way to test this without mocking things, but I see it as a very simple change that does not necessarily need to be tested aside (and it's related to 1st and 2nd).

> I think the Regression Potential section in the bug needs updating now with
> this different approach?

Updated with better description of potential regressions.

Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

Okay, I'm addressing your comments and pushing this again in here. Thanks for the careful review again.

Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

Please let me know if you're good so I can upload this. Thank you

Revision history for this message
Robie Basak (racb) wrote :

Looks good. Thanks! If you upload, I can accept into the proposed pocket immediately.

review: Approve (sru)
Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

Awesome.. thanks a lot for all. Here is the output from the tests:

[rafaeldtinoco@focal01 ~]$ crm status
Cluster Summary:
  * Stack: corosync
  * Current DC: focal01 (version 2.0.3-4b1f869f0f) - partition with quorum
  * Last updated: Thu Oct 8 17:32:20 2020
  * Last change: Thu Oct 8 17:32:18 2020 by root via cibadmin on focal01
  * 3 nodes configured
  * 5 resource instances configured (2 DISABLED)

Node List:
  * Online: [ focal01 focal02 focal03 ]

Full List of Resources:
  * fence-focal01 (stonith:fence_virsh): Started focal02
  * fence-focal02 (stonith:fence_virsh): Started focal01
  * fence-focal03 (stonith:fence_virsh): Started focal02
  * Resource Group: webserver_vip:
    * webserver (systemd:lighttpd): Stopped (disabled)
    * virtual_ip (ocf::heartbeat:IPaddr2): Stopped (disabled)

----

[rafaeldtinoco@focal01 ~]$ crm resource start webserver_vip

----

[rafaeldtinoco@focal01 ~]$ crm status
Cluster Summary:
  * Stack: corosync
  * Current DC: focal01 (version 2.0.3-4b1f869f0f) - partition with quorum
  * Last updated: Thu Oct 8 17:32:14 2020
  * Last change: Thu Oct 8 17:32:11 2020 by root via cibadmin on focal01
  * 3 nodes configured
  * 5 resource instances configured

Node List:
  * Online: [ focal01 focal02 focal03 ]

Full List of Resources:
  * fence-focal01 (stonith:fence_virsh): Started focal02
  * fence-focal02 (stonith:fence_virsh): Started focal01
  * fence-focal03 (stonith:fence_virsh): Started focal02
  * Resource Group: webserver_vip:
    * webserver (systemd:lighttpd): Started focal03
    * virtual_ip (ocf::heartbeat:IPaddr2): Started focal03

----

A 2s timeout works as well. I'm going ahead and uploading it then.

Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

[rafaeldtinoco@hafocal pacemaker]$ git describe
upload/2.0.3-3ubuntu4

[rafaeldtinoco@hafocal pacemaker]$ git push pkg upload/2.0.3-3ubuntu4
Enumerating objects: 25, done.
Counting objects: 100% (25/25), done.
Delta compression using up to 8 threads
Compressing objects: 100% (18/18), done.
Writing objects: 100% (18/18), 7.71 KiB | 3.86 MiB/s, done.
Total 18 (delta 11), reused 0 (delta 0)
To ssh://git.launchpad.net/ubuntu/+source/pacemaker
 * [new tag] upload/2.0.3-3ubuntu4 -> upload/2.0.3-3ubuntu4

$ debdiff pacemaker_2.0.3-3ubuntu3.dsc pacemaker_2.0.3-3ubuntu4.dsc | diffstat -l
changelog
patches/series
patches/ubuntu-2.0.3-fixes/lp1881762-01-Executor-systemd-execution-time-fixes.patch
patches/ubuntu-2.0.3-fixes/lp1881762-02-Low-executor-record-correct-last-run-and-last-rc.patch
patches/ubuntu-2.0.3-fixes/lp1881762-03-Refactor-executor-functionize-getting-current.patch
rules

$ dput ubuntu pacemaker_2.0.3-3ubuntu4_source.changes
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading pacemaker_2.0.3-3ubuntu4.dsc: done.
  Uploading pacemaker_2.0.3-3ubuntu4.debian.tar.xz: done.
  Uploading pacemaker_2.0.3-3ubuntu4_source.buildinfo: done.
  Uploading pacemaker_2.0.3-3ubuntu4_source.changes: done.
Successfully uploaded packages.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/debian/changelog b/debian/changelog
index ff94693..31fbb41 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,13 @@
1pacemaker (2.0.3-3ubuntu4) focal; urgency=medium
2
3 * Fix clock_gettime() timing functions (LP: #1881762)
4 + d/p/ubuntu-2.0.3-fixes/lp1881762-*:
5 - 01-Executor-systemd-execution-time-fixes.patch
6 - 02-Low-executor-record-correct-last-run-and-last-rc.patch
7 - 03-Refactor-executor-functionize-getting-current.patch
8
9 -- Rafael David Tinoco <rafaeldtinoco@ubuntu.com> Thu, 08 Oct 2020 05:38:22 +0000
10
1pacemaker (2.0.3-3ubuntu3) focal; urgency=medium11pacemaker (2.0.3-3ubuntu3) focal; urgency=medium
212
3 * Post 2.0.3 release fixes backported to Ubuntu (LP: #1870235)13 * Post 2.0.3 release fixes backported to Ubuntu (LP: #1870235)
diff --git a/debian/patches/series b/debian/patches/series
index 0221c25..f539cbb 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -18,3 +18,10 @@ ubuntu-2.0.3-fixes/lp1870235-28bfd00e9-Low-libcrmservice-handle-child-wait-error
18ubuntu-2.0.3-fixes/lp1870235-dec326391-Log-libcrmcommon-correct-log-line-length.patch18ubuntu-2.0.3-fixes/lp1870235-dec326391-Log-libcrmcommon-correct-log-line-length.patch
19ubuntu-2.0.3-fixes/lp1870235-c98987824-Fix-iso8601-Fix-crm_time_parse_offset.patch19ubuntu-2.0.3-fixes/lp1870235-c98987824-Fix-iso8601-Fix-crm_time_parse_offset.patch
20ubuntu-2.0.3-fixes/lp1870235-426f06cc0-Fix-tools-Fix-curses_indented_printf.patch20ubuntu-2.0.3-fixes/lp1870235-426f06cc0-Fix-tools-Fix-curses_indented_printf.patch
21#
22# https://bugs.launchpad.net/bugs/1881762
23#
24ubuntu-2.0.3-fixes/lp1881762-01-Executor-systemd-execution-time-fixes.patch
25ubuntu-2.0.3-fixes/lp1881762-02-Low-executor-record-correct-last-run-and-last-rc.patch
26ubuntu-2.0.3-fixes/lp1881762-03-Refactor-executor-functionize-getting-current.patch
27
diff --git a/debian/patches/ubuntu-2.0.3-fixes/lp1881762-01-Executor-systemd-execution-time-fixes.patch b/debian/patches/ubuntu-2.0.3-fixes/lp1881762-01-Executor-systemd-execution-time-fixes.patch
21new file mode 10064428new file mode 100644
index 0000000..4a1413b
--- /dev/null
+++ b/debian/patches/ubuntu-2.0.3-fixes/lp1881762-01-Executor-systemd-execution-time-fixes.patch
@@ -0,0 +1,167 @@
1Description: Executor: systemd execution time fixes
2
3 This patch is a backport of the following upstream patches:
4
5 077229220 Fix: executor: handle systemd execution times under 1 second
6 c9ce7ed50 Low: executor: correctly set first run time
7 08e3f7e44 Fix: executor: correctly convert nanoseconds to milliseconds
8
9 It fixes systemd resources execution times shorter than 1 second. They
10 were consolidated because last 2 patches are minor fixes to the first
11 one.
12
13 [Backport]
14
15 After SRU review it was decided that, instead of cherry-picking the 2
16 upstream merges pointed by upstream maintainer (#1992 and #1997) we
17 would only backport changes that affect clock_gettime() code base and
18 execution path. This is per SRU guidelines, trying to minimize amount
19 of changes to be reviewed and merged.
20
21Author: Rafael David Tinoco <rafaeldtinoco@ubuntu.com>
22Origin: backport, https://github.com/ClusterLabs/pacemaker/commit/077229220
23Bug-Ubuntu: https://bugs.launchpad.net/bugs/1881762
24Reviewed-by: Rafael David Tinoco <rafaeldtinoco@ubuntu.com>
25Last-Update: 2020-10-07
26---
27This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
28---
29 daemons/execd/execd_commands.c | 67 ++++++++++++++++++++++------------
30 1 file changed, 44 insertions(+), 23 deletions(-)
31
32diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c
33index 9ded90c33..facdea629 100644
34--- a/daemons/execd/execd_commands.c
35+++ b/daemons/execd/execd_commands.c
36@@ -229,6 +229,10 @@ free_lrmd_cmd(lrmd_cmd_t * cmd)
37 free(cmd);
38 }
39
40+#ifdef PCMK__TIME_USE_CGT
41+static inline bool time_is_set(struct timespec *);
42+#endif
43+
44 static gboolean
45 stonith_recurring_op_helper(gpointer data)
46 {
47@@ -250,7 +254,7 @@ stonith_recurring_op_helper(gpointer data)
48 rsc->pending_ops = g_list_append(rsc->pending_ops, cmd);
49 #ifdef PCMK__TIME_USE_CGT
50 clock_gettime(CLOCK_MONOTONIC, &cmd->t_queue);
51- if (cmd->t_first_queue.tv_sec == 0) {
52+ if (!time_is_set(&(cmd->t_first_queue))) {
53 cmd->t_first_queue = cmd->t_queue;
54 }
55 #elif defined(HAVE_SYS_TIMEB_H)
56@@ -382,7 +386,7 @@ schedule_lrmd_cmd(lrmd_rsc_t * rsc, lrmd_cmd_t * cmd)
57 rsc->pending_ops = g_list_append(rsc->pending_ops, cmd);
58 #ifdef PCMK__TIME_USE_CGT
59 clock_gettime(CLOCK_MONOTONIC, &cmd->t_queue);
60- if (cmd->t_first_queue.tv_sec == 0) {
61+ if (!time_is_set(&(cmd->t_first_queue))) {
62 cmd->t_first_queue = cmd->t_queue;
63 }
64 #elif defined(HAVE_SYS_TIMEB_H)
65@@ -449,7 +453,42 @@ send_client_notify(gpointer key, gpointer value, gpointer user_data)
66 client->name, client->id, msg, rc);
67 }
68
69-#if defined(PCMK__TIME_USE_CGT) || defined(HAVE_SYS_TIMEB_H)
70+#ifdef PCMK__TIME_USE_CGT /* clock_gettime() fixes only (LP: #1881762) */
71+
72+static inline bool
73+time_is_set(struct timespec *timespec)
74+{
75+ return (timespec != NULL) &&
76+ ((timespec->tv_sec != 0) || (timespec->tv_nsec != 0));
77+}
78+
79+static int
80+time_diff_ms(struct timespec *now, struct timespec *old)
81+{
82+ int diff_ms = 0;
83+
84+ if (time_is_set(old)) {
85+ struct timespec local_now = { 0, };
86+
87+ if (now == NULL) {
88+ clock_gettime(CLOCK_MONOTONIC, &local_now);
89+ now = &local_now;
90+ }
91+ diff_ms = (now->tv_sec - old->tv_sec) * 1000
92+ + (now->tv_nsec - old->tv_nsec) / 1000000;
93+ }
94+ return diff_ms;
95+}
96+
97+static void
98+cmd_original_times(lrmd_cmd_t * cmd)
99+{
100+ cmd->t_run = cmd->t_first_run;
101+ cmd->t_queue = cmd->t_first_queue;
102+}
103+
104+#elif defined(HAVE_SYS_TIMEB_H) /* ftime support kept (LP: #1881762) */
105+
106 /*!
107 * \internal
108 * \brief Return difference between two times in milliseconds
109@@ -463,38 +502,19 @@ send_client_notify(gpointer key, gpointer value, gpointer user_data)
110 * 24 days or more.
111 */
112 static int
113-#ifdef PCMK__TIME_USE_CGT
114-time_diff_ms(struct timespec *now, struct timespec *old)
115-{
116- struct timespec local_now = { 0, };
117-#else
118 time_diff_ms(struct timeb *now, struct timeb *old)
119 {
120 struct timeb local_now = { 0, };
121-#endif
122
123 if (now == NULL) {
124-#ifdef PCMK__TIME_USE_CGT
125- clock_gettime(CLOCK_MONOTONIC, &local_now);
126-#else
127 ftime(&local_now);
128-#endif
129 now = &local_now;
130 }
131 if ((old == NULL)
132-#ifdef PCMK__TIME_USE_CGT
133- || (old->tv_sec == 0)) {
134-#else
135 || (old->time == 0)) {
136-#endif
137 return 0;
138 }
139-#ifdef PCMK__TIME_USE_CGT
140- return (now->tv_sec - old->tv_sec) * 1000
141- + (now->tv_nsec - old->tv_nsec) / 1000;
142-#else
143 return (now->time - old->time) * 1000 + now->millitm - old->millitm;
144-#endif
145 }
146
147 /*!
148@@ -523,6 +543,7 @@ cmd_original_times(lrmd_cmd_t * cmd)
149 cmd->t_run = cmd->t_first_run;
150 cmd->t_queue = cmd->t_first_queue;
151 }
152+
153 #endif
154
155 static void
156@@ -1419,7 +1440,7 @@ lrmd_rsc_execute(lrmd_rsc_t * rsc)
157 g_list_free_1(first);
158
159 #ifdef PCMK__TIME_USE_CGT
160- if (cmd->t_first_run.tv_sec == 0) {
161+ if (!time_is_set(&(cmd->t_first_run))) {
162 clock_gettime(CLOCK_MONOTONIC, &cmd->t_first_run);
163 }
164 clock_gettime(CLOCK_MONOTONIC, &cmd->t_run);
165--
1662.25.1
167
diff --git a/debian/patches/ubuntu-2.0.3-fixes/lp1881762-02-Low-executor-record-correct-last-run-and-last-rc.patch b/debian/patches/ubuntu-2.0.3-fixes/lp1881762-02-Low-executor-record-correct-last-run-and-last-rc.patch
0new file mode 100644168new file mode 100644
index 0000000..d24bbfa
--- /dev/null
+++ b/debian/patches/ubuntu-2.0.3-fixes/lp1881762-02-Low-executor-record-correct-last-run-and-last-rc.patch
@@ -0,0 +1,133 @@
1Description: Low: executor: record correct last run and last rc change times
2
3 This is a regression introduced by 4b8b84cc, which replaced ftime()
4 calls with calls to clock_gettime(CLOCK_MONOTONIC, ...).
5
6 However the last run and last rc change times were passed back to the
7 client as epoch timestamps, which would end up in the CIB resource
8 operation history (and crm_mon output).
9
10 The last rc change time was never used to calculate an interval, which
11 means it never really needed to use ftime() to begin with -- a time_t
12 set to time(NULL) would have been sufficient, which is what is now used.
13
14 For the last run, which is also used to calculate an interval (queue
15 time and execution time), we now store both the monotonic timestamp (for
16 interval calculation) and an equivalent epoch time_t (for return to the
17 client).
18
19 [Backport]
20
21 After SRU review it was decided that, instead of cherry-picking the 2
22 upstream merges pointed by upstream maintainer (#1992 and #1997) we
23 would only backport changes that affect clock_gettime() code base and
24 execution path. This is per SRU guidelines, trying to minimize amount
25 of changes to be reviewed and merged.
26
27Author: Ken Gaillot <kgaillot@redhat.com>
28Origin: backport, https://github.com/ClusterLabs/pacemaker/commit/9075ad9
29Bug-Ubuntu: https://bugs.launchpad.net/bugs/1881762
30Reviewed-by: Rafael David Tinoco <rafaeldtinoco@ubuntu.com>
31Last-Update: 2020-10-08
32---
33This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
34---
35 daemons/execd/execd_commands.c | 50 +++++++++++++++++++++++++---------
36 1 file changed, 37 insertions(+), 13 deletions(-)
37
38diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c
39index facdea629..e14be0276 100644
40--- a/daemons/execd/execd_commands.c
41+++ b/daemons/execd/execd_commands.c
42@@ -70,13 +70,33 @@ typedef struct lrmd_cmd_s {
43 * command per operation, so they need info about the original and the most
44 * recent.
45 */
46-#ifdef PCMK__TIME_USE_CGT
47- struct timespec t_first_run; /* Timestamp of when op first ran */
48- struct timespec t_run; /* Timestamp of when op most recently ran */
49- struct timespec t_first_queue; /* Timestamp of when op first was queued */
50- struct timespec t_queue; /* Timestamp of when op most recently was queued */
51- struct timespec t_rcchange; /* Timestamp of last rc change */
52-#elif defined(HAVE_SYS_TIMEB_H)
53+#ifdef PCMK__TIME_USE_CGT /* clock_gettime() fixes only (LP: #1881762) */
54+
55+ /* We can track operation queue time and run time, to be saved with the CIB
56+ * resource history (and displayed in cluster status). We need
57+ * high-resolution monotonic time for this purpose, so we use
58+ * clock_gettime(CLOCK_MONOTONIC, ...) (if available, otherwise this feature
59+ * is disabled).
60+ *
61+ * However, we also need epoch timestamps for recording the time the command
62+ * last ran and the time its return value last changed, for use in time
63+ * displays (as opposed to interval calculations). We keep time_t values for
64+ * this purpose.
65+ *
66+ * The last run time is used for both purposes, so we keep redundant
67+ * monotonic and epoch values for this. Technically the two could represent
68+ * different times, but since time_t has only second resolution and the
69+ * values are used for distinct purposes, that is not significant.
70+ */
71+
72+ struct timespec t_first_run; // When op first ran
73+ struct timespec t_run; // When op most recently ran
74+ struct timespec t_first_queue; // When op was first queued
75+ struct timespec t_queue; // When op was most recently queued
76+ time_t epoch_last_run; // Epoch timestamp of when op last ran
77+ time_t epoch_rcchange; // Epoch timestamp of when rc last changed
78+
79+#elif defined(HAVE_SYS_TIMEB_H) /* ftime support kept (LP: #1881762) */
80 struct timeb t_first_run; /* Timestamp of when op first ran */
81 struct timeb t_run; /* Timestamp of when op most recently ran */
82 struct timeb t_first_queue; /* Timestamp of when op first was queued */
83@@ -589,10 +609,10 @@ send_cmd_complete_notify(lrmd_cmd_t * cmd)
84 crm_xml_add_int(notify, F_LRMD_RSC_DELETED, cmd->rsc_deleted);
85
86 #if defined(PCMK__TIME_USE_CGT) || defined(HAVE_SYS_TIMEB_H)
87-#ifdef PCMK__TIME_USE_CGT
88- crm_xml_add_ll(notify, F_LRMD_RSC_RUN_TIME, (long long) cmd->t_run.tv_sec);
89- crm_xml_add_ll(notify, F_LRMD_RSC_RCCHANGE_TIME, (long long) cmd->t_rcchange.tv_sec);
90-#else
91+#ifdef PCMK__TIME_USE_CGT /* clock_gettime() fixes only (LP: #1881762) */
92+ crm_xml_add_ll(notify, F_LRMD_RSC_RUN_TIME, (long long) cmd->epoch_last_run);
93+ crm_xml_add_ll(notify, F_LRMD_RSC_RCCHANGE_TIME, (long long) cmd->epoch_rcchange);
94+#else /* ftime support kept (LP: #1881762) */
95 crm_xml_add_ll(notify, F_LRMD_RSC_RUN_TIME, (long long) cmd->t_run.time);
96 crm_xml_add_ll(notify, F_LRMD_RSC_RCCHANGE_TIME, (long long) cmd->t_rcchange.time);
97 #endif
98@@ -669,6 +689,9 @@ cmd_reset(lrmd_cmd_t * cmd)
99 #if defined(PCMK__TIME_USE_CGT) || defined(HAVE_SYS_TIMEB_H)
100 memset(&cmd->t_run, 0, sizeof(cmd->t_run));
101 memset(&cmd->t_queue, 0, sizeof(cmd->t_queue));
102+#endif
103+#if defined(PCMK__TIME_USE_CGT) /* clock_gettime() fixes only (LP: #1881762) */
104+ cmd->epoch_last_run = 0;
105 #endif
106 free(cmd->exit_reason);
107 cmd->exit_reason = NULL;
108@@ -910,7 +933,7 @@ action_complete(svc_action_t * action)
109
110 #ifdef PCMK__TIME_USE_CGT
111 if (cmd->exec_rc != action->rc) {
112- clock_gettime(CLOCK_MONOTONIC, &cmd->t_rcchange);
113+ cmd->epoch_rcchange = time(NULL);
114 }
115 #elif defined(HAVE_SYS_TIMEB_H)
116 if (cmd->exec_rc != action->rc) {
117@@ -1439,11 +1462,12 @@ lrmd_rsc_execute(lrmd_rsc_t * rsc)
118 rsc->pending_ops = g_list_remove_link(rsc->pending_ops, first);
119 g_list_free_1(first);
120
121-#ifdef PCMK__TIME_USE_CGT
122+#ifdef PCMK__TIME_USE_CGT /* clock_gettime() fixes only (LP: #1881762) */
123 if (!time_is_set(&(cmd->t_first_run))) {
124 clock_gettime(CLOCK_MONOTONIC, &cmd->t_first_run);
125 }
126 clock_gettime(CLOCK_MONOTONIC, &cmd->t_run);
127+ cmd->epoch_last_run = time(NULL);
128 #elif defined(HAVE_SYS_TIMEB_H)
129 if (cmd->t_first_run.time == 0) {
130 ftime(&cmd->t_first_run);
131--
1322.25.1
133
diff --git a/debian/patches/ubuntu-2.0.3-fixes/lp1881762-03-Refactor-executor-functionize-getting-current.patch b/debian/patches/ubuntu-2.0.3-fixes/lp1881762-03-Refactor-executor-functionize-getting-current.patch
0new file mode 100644134new file mode 100644
index 0000000..641acd3
--- /dev/null
+++ b/debian/patches/ubuntu-2.0.3-fixes/lp1881762-03-Refactor-executor-functionize-getting-current.patch
@@ -0,0 +1,88 @@
1Description: Refactor: executor: functionize getting current time in
2 timespec
3
4 ... to reduce code duplication.
5
6 Also, previously, when executing a resource command, the run and first
7 run times would be set with separate clock_gettime() calls, so they
8 would slightly different; this avoids that by copying the run time to
9 the first run time.
10
11Author: Ken Gaillot <kgaillot@redhat.com>
12Origin: upstream, https://github.com/ClusterLabs/pacemaker/commit/71ae72d
13Bug-Ubuntu: https://bugs.launchpad.net/bugs/1881762
14Reviewed-by: Rafael David Tinoco <rafaeldtinoco@ubuntu.com>
15Last-Update: 2020-10-08
16---
17This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
18---
19 daemons/execd/execd_commands.c | 25 +++++++++++++------------
20 1 file changed, 13 insertions(+), 12 deletions(-)
21
22diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c
23index e14be0276..903f72a53 100644
24--- a/daemons/execd/execd_commands.c
25+++ b/daemons/execd/execd_commands.c
26@@ -251,6 +251,7 @@ free_lrmd_cmd(lrmd_cmd_t * cmd)
27
28 #ifdef PCMK__TIME_USE_CGT
29 static inline bool time_is_set(struct timespec *);
30+static void get_current_time(struct timespec *, struct timespec *);
31 #endif
32
33 static gboolean
34@@ -273,10 +274,7 @@ stonith_recurring_op_helper(gpointer data)
35 rsc->recurring_ops = g_list_remove(rsc->recurring_ops, cmd);
36 rsc->pending_ops = g_list_append(rsc->pending_ops, cmd);
37 #ifdef PCMK__TIME_USE_CGT
38- clock_gettime(CLOCK_MONOTONIC, &cmd->t_queue);
39- if (!time_is_set(&(cmd->t_first_queue))) {
40- cmd->t_first_queue = cmd->t_queue;
41- }
42+ get_current_time(&(cmd->t_queue), &(cmd->t_first_queue));
43 #elif defined(HAVE_SYS_TIMEB_H)
44 ftime(&cmd->t_queue);
45 if (cmd->t_first_queue.time == 0) {
46@@ -405,10 +403,7 @@ schedule_lrmd_cmd(lrmd_rsc_t * rsc, lrmd_cmd_t * cmd)
47
48 rsc->pending_ops = g_list_append(rsc->pending_ops, cmd);
49 #ifdef PCMK__TIME_USE_CGT
50- clock_gettime(CLOCK_MONOTONIC, &cmd->t_queue);
51- if (!time_is_set(&(cmd->t_first_queue))) {
52- cmd->t_first_queue = cmd->t_queue;
53- }
54+ get_current_time(&(cmd->t_queue), &(cmd->t_first_queue));
55 #elif defined(HAVE_SYS_TIMEB_H)
56 ftime(&cmd->t_queue);
57 if (cmd->t_first_queue.time == 0) {
58@@ -507,6 +502,15 @@ cmd_original_times(lrmd_cmd_t * cmd)
59 cmd->t_queue = cmd->t_first_queue;
60 }
61
62+static void
63+get_current_time(struct timespec *t_current, struct timespec *t_orig)
64+{
65+ clock_gettime(CLOCK_MONOTONIC, t_current);
66+ if ((t_orig != NULL) && !time_is_set(t_orig)) {
67+ *t_orig = *t_current;
68+ }
69+}
70+
71 #elif defined(HAVE_SYS_TIMEB_H) /* ftime support kept (LP: #1881762) */
72
73 /*!
74@@ -1463,10 +1467,7 @@ lrmd_rsc_execute(lrmd_rsc_t * rsc)
75 g_list_free_1(first);
76
77 #ifdef PCMK__TIME_USE_CGT /* clock_gettime() fixes only (LP: #1881762) */
78- if (!time_is_set(&(cmd->t_first_run))) {
79- clock_gettime(CLOCK_MONOTONIC, &cmd->t_first_run);
80- }
81- clock_gettime(CLOCK_MONOTONIC, &cmd->t_run);
82+ get_current_time(&(cmd->t_run), &(cmd->t_first_run));
83 cmd->epoch_last_run = time(NULL);
84 #elif defined(HAVE_SYS_TIMEB_H)
85 if (cmd->t_first_run.time == 0) {
86--
872.25.1
88
diff --git a/debian/rules b/debian/rules
index b6d3b94..77d4952 100755
--- a/debian/rules
+++ b/debian/rules
@@ -21,8 +21,20 @@ ifeq ($(shell dpkg-vendor --query vendor)/$(DEB_HOST_ARCH), Ubuntu/i386)
21 -Npacemaker-resource-agents21 -Npacemaker-resource-agents
22endif22endif
2323
24# ftime causes deprecation errors now, but the clock_gettime replacement is24# https://bugs.launchpad.net/bugs/1881762 (LP: #1881762)
25# disabled by default in 2.0.3. Enable it.25#
26# Sticking with ftime() causes deprecation errors. By mistake clock_gettime()
27# code was not finished and made default in version 2.0.3 (git log: b5ff0e4).
28# I could have removed ftime() support by picking upstream merges #1992 and
29# #1997 entirely, but for the purpose of a SRU it didn't feel right (after
30# discussions with SRU team).
31#
32# Commenting the line below will re-enable ftime() clock functions but,
33# unfortunately, it will break nagios and systemd resources support (as they
34# require clock_gettime). It will also cause compilation warning-as-errors
35# for deprecated ftime() function (which can be work around by
36# -Wno-error=deprecated-declarations if ever needed).
37#
26CPPFLAGS = -UPCMK_TIME_EMERGENCY_CGT38CPPFLAGS = -UPCMK_TIME_EMERGENCY_CGT
2739
28%:40%:

Subscribers

People subscribed via source and target branches