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
1diff --git a/debian/changelog b/debian/changelog
2index ff94693..31fbb41 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,13 @@
6+pacemaker (2.0.3-3ubuntu4) focal; urgency=medium
7+
8+ * Fix clock_gettime() timing functions (LP: #1881762)
9+ + d/p/ubuntu-2.0.3-fixes/lp1881762-*:
10+ - 01-Executor-systemd-execution-time-fixes.patch
11+ - 02-Low-executor-record-correct-last-run-and-last-rc.patch
12+ - 03-Refactor-executor-functionize-getting-current.patch
13+
14+ -- Rafael David Tinoco <rafaeldtinoco@ubuntu.com> Thu, 08 Oct 2020 05:38:22 +0000
15+
16 pacemaker (2.0.3-3ubuntu3) focal; urgency=medium
17
18 * Post 2.0.3 release fixes backported to Ubuntu (LP: #1870235)
19diff --git a/debian/patches/series b/debian/patches/series
20index 0221c25..f539cbb 100644
21--- a/debian/patches/series
22+++ b/debian/patches/series
23@@ -18,3 +18,10 @@ ubuntu-2.0.3-fixes/lp1870235-28bfd00e9-Low-libcrmservice-handle-child-wait-error
24 ubuntu-2.0.3-fixes/lp1870235-dec326391-Log-libcrmcommon-correct-log-line-length.patch
25 ubuntu-2.0.3-fixes/lp1870235-c98987824-Fix-iso8601-Fix-crm_time_parse_offset.patch
26 ubuntu-2.0.3-fixes/lp1870235-426f06cc0-Fix-tools-Fix-curses_indented_printf.patch
27+#
28+# https://bugs.launchpad.net/bugs/1881762
29+#
30+ubuntu-2.0.3-fixes/lp1881762-01-Executor-systemd-execution-time-fixes.patch
31+ubuntu-2.0.3-fixes/lp1881762-02-Low-executor-record-correct-last-run-and-last-rc.patch
32+ubuntu-2.0.3-fixes/lp1881762-03-Refactor-executor-functionize-getting-current.patch
33+
34diff --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
35new file mode 100644
36index 0000000..4a1413b
37--- /dev/null
38+++ b/debian/patches/ubuntu-2.0.3-fixes/lp1881762-01-Executor-systemd-execution-time-fixes.patch
39@@ -0,0 +1,167 @@
40+Description: Executor: systemd execution time fixes
41+
42+ This patch is a backport of the following upstream patches:
43+
44+ 077229220 Fix: executor: handle systemd execution times under 1 second
45+ c9ce7ed50 Low: executor: correctly set first run time
46+ 08e3f7e44 Fix: executor: correctly convert nanoseconds to milliseconds
47+
48+ It fixes systemd resources execution times shorter than 1 second. They
49+ were consolidated because last 2 patches are minor fixes to the first
50+ one.
51+
52+ [Backport]
53+
54+ After SRU review it was decided that, instead of cherry-picking the 2
55+ upstream merges pointed by upstream maintainer (#1992 and #1997) we
56+ would only backport changes that affect clock_gettime() code base and
57+ execution path. This is per SRU guidelines, trying to minimize amount
58+ of changes to be reviewed and merged.
59+
60+Author: Rafael David Tinoco <rafaeldtinoco@ubuntu.com>
61+Origin: backport, https://github.com/ClusterLabs/pacemaker/commit/077229220
62+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1881762
63+Reviewed-by: Rafael David Tinoco <rafaeldtinoco@ubuntu.com>
64+Last-Update: 2020-10-07
65+---
66+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
67+---
68+ daemons/execd/execd_commands.c | 67 ++++++++++++++++++++++------------
69+ 1 file changed, 44 insertions(+), 23 deletions(-)
70+
71+diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c
72+index 9ded90c33..facdea629 100644
73+--- a/daemons/execd/execd_commands.c
74++++ b/daemons/execd/execd_commands.c
75+@@ -229,6 +229,10 @@ free_lrmd_cmd(lrmd_cmd_t * cmd)
76+ free(cmd);
77+ }
78+
79++#ifdef PCMK__TIME_USE_CGT
80++static inline bool time_is_set(struct timespec *);
81++#endif
82++
83+ static gboolean
84+ stonith_recurring_op_helper(gpointer data)
85+ {
86+@@ -250,7 +254,7 @@ stonith_recurring_op_helper(gpointer data)
87+ rsc->pending_ops = g_list_append(rsc->pending_ops, cmd);
88+ #ifdef PCMK__TIME_USE_CGT
89+ clock_gettime(CLOCK_MONOTONIC, &cmd->t_queue);
90+- if (cmd->t_first_queue.tv_sec == 0) {
91++ if (!time_is_set(&(cmd->t_first_queue))) {
92+ cmd->t_first_queue = cmd->t_queue;
93+ }
94+ #elif defined(HAVE_SYS_TIMEB_H)
95+@@ -382,7 +386,7 @@ schedule_lrmd_cmd(lrmd_rsc_t * rsc, lrmd_cmd_t * cmd)
96+ rsc->pending_ops = g_list_append(rsc->pending_ops, cmd);
97+ #ifdef PCMK__TIME_USE_CGT
98+ clock_gettime(CLOCK_MONOTONIC, &cmd->t_queue);
99+- if (cmd->t_first_queue.tv_sec == 0) {
100++ if (!time_is_set(&(cmd->t_first_queue))) {
101+ cmd->t_first_queue = cmd->t_queue;
102+ }
103+ #elif defined(HAVE_SYS_TIMEB_H)
104+@@ -449,7 +453,42 @@ send_client_notify(gpointer key, gpointer value, gpointer user_data)
105+ client->name, client->id, msg, rc);
106+ }
107+
108+-#if defined(PCMK__TIME_USE_CGT) || defined(HAVE_SYS_TIMEB_H)
109++#ifdef PCMK__TIME_USE_CGT /* clock_gettime() fixes only (LP: #1881762) */
110++
111++static inline bool
112++time_is_set(struct timespec *timespec)
113++{
114++ return (timespec != NULL) &&
115++ ((timespec->tv_sec != 0) || (timespec->tv_nsec != 0));
116++}
117++
118++static int
119++time_diff_ms(struct timespec *now, struct timespec *old)
120++{
121++ int diff_ms = 0;
122++
123++ if (time_is_set(old)) {
124++ struct timespec local_now = { 0, };
125++
126++ if (now == NULL) {
127++ clock_gettime(CLOCK_MONOTONIC, &local_now);
128++ now = &local_now;
129++ }
130++ diff_ms = (now->tv_sec - old->tv_sec) * 1000
131++ + (now->tv_nsec - old->tv_nsec) / 1000000;
132++ }
133++ return diff_ms;
134++}
135++
136++static void
137++cmd_original_times(lrmd_cmd_t * cmd)
138++{
139++ cmd->t_run = cmd->t_first_run;
140++ cmd->t_queue = cmd->t_first_queue;
141++}
142++
143++#elif defined(HAVE_SYS_TIMEB_H) /* ftime support kept (LP: #1881762) */
144++
145+ /*!
146+ * \internal
147+ * \brief Return difference between two times in milliseconds
148+@@ -463,38 +502,19 @@ send_client_notify(gpointer key, gpointer value, gpointer user_data)
149+ * 24 days or more.
150+ */
151+ static int
152+-#ifdef PCMK__TIME_USE_CGT
153+-time_diff_ms(struct timespec *now, struct timespec *old)
154+-{
155+- struct timespec local_now = { 0, };
156+-#else
157+ time_diff_ms(struct timeb *now, struct timeb *old)
158+ {
159+ struct timeb local_now = { 0, };
160+-#endif
161+
162+ if (now == NULL) {
163+-#ifdef PCMK__TIME_USE_CGT
164+- clock_gettime(CLOCK_MONOTONIC, &local_now);
165+-#else
166+ ftime(&local_now);
167+-#endif
168+ now = &local_now;
169+ }
170+ if ((old == NULL)
171+-#ifdef PCMK__TIME_USE_CGT
172+- || (old->tv_sec == 0)) {
173+-#else
174+ || (old->time == 0)) {
175+-#endif
176+ return 0;
177+ }
178+-#ifdef PCMK__TIME_USE_CGT
179+- return (now->tv_sec - old->tv_sec) * 1000
180+- + (now->tv_nsec - old->tv_nsec) / 1000;
181+-#else
182+ return (now->time - old->time) * 1000 + now->millitm - old->millitm;
183+-#endif
184+ }
185+
186+ /*!
187+@@ -523,6 +543,7 @@ cmd_original_times(lrmd_cmd_t * cmd)
188+ cmd->t_run = cmd->t_first_run;
189+ cmd->t_queue = cmd->t_first_queue;
190+ }
191++
192+ #endif
193+
194+ static void
195+@@ -1419,7 +1440,7 @@ lrmd_rsc_execute(lrmd_rsc_t * rsc)
196+ g_list_free_1(first);
197+
198+ #ifdef PCMK__TIME_USE_CGT
199+- if (cmd->t_first_run.tv_sec == 0) {
200++ if (!time_is_set(&(cmd->t_first_run))) {
201+ clock_gettime(CLOCK_MONOTONIC, &cmd->t_first_run);
202+ }
203+ clock_gettime(CLOCK_MONOTONIC, &cmd->t_run);
204+--
205+2.25.1
206+
207diff --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
208new file mode 100644
209index 0000000..d24bbfa
210--- /dev/null
211+++ b/debian/patches/ubuntu-2.0.3-fixes/lp1881762-02-Low-executor-record-correct-last-run-and-last-rc.patch
212@@ -0,0 +1,133 @@
213+Description: Low: executor: record correct last run and last rc change times
214+
215+ This is a regression introduced by 4b8b84cc, which replaced ftime()
216+ calls with calls to clock_gettime(CLOCK_MONOTONIC, ...).
217+
218+ However the last run and last rc change times were passed back to the
219+ client as epoch timestamps, which would end up in the CIB resource
220+ operation history (and crm_mon output).
221+
222+ The last rc change time was never used to calculate an interval, which
223+ means it never really needed to use ftime() to begin with -- a time_t
224+ set to time(NULL) would have been sufficient, which is what is now used.
225+
226+ For the last run, which is also used to calculate an interval (queue
227+ time and execution time), we now store both the monotonic timestamp (for
228+ interval calculation) and an equivalent epoch time_t (for return to the
229+ client).
230+
231+ [Backport]
232+
233+ After SRU review it was decided that, instead of cherry-picking the 2
234+ upstream merges pointed by upstream maintainer (#1992 and #1997) we
235+ would only backport changes that affect clock_gettime() code base and
236+ execution path. This is per SRU guidelines, trying to minimize amount
237+ of changes to be reviewed and merged.
238+
239+Author: Ken Gaillot <kgaillot@redhat.com>
240+Origin: backport, https://github.com/ClusterLabs/pacemaker/commit/9075ad9
241+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1881762
242+Reviewed-by: Rafael David Tinoco <rafaeldtinoco@ubuntu.com>
243+Last-Update: 2020-10-08
244+---
245+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
246+---
247+ daemons/execd/execd_commands.c | 50 +++++++++++++++++++++++++---------
248+ 1 file changed, 37 insertions(+), 13 deletions(-)
249+
250+diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c
251+index facdea629..e14be0276 100644
252+--- a/daemons/execd/execd_commands.c
253++++ b/daemons/execd/execd_commands.c
254+@@ -70,13 +70,33 @@ typedef struct lrmd_cmd_s {
255+ * command per operation, so they need info about the original and the most
256+ * recent.
257+ */
258+-#ifdef PCMK__TIME_USE_CGT
259+- struct timespec t_first_run; /* Timestamp of when op first ran */
260+- struct timespec t_run; /* Timestamp of when op most recently ran */
261+- struct timespec t_first_queue; /* Timestamp of when op first was queued */
262+- struct timespec t_queue; /* Timestamp of when op most recently was queued */
263+- struct timespec t_rcchange; /* Timestamp of last rc change */
264+-#elif defined(HAVE_SYS_TIMEB_H)
265++#ifdef PCMK__TIME_USE_CGT /* clock_gettime() fixes only (LP: #1881762) */
266++
267++ /* We can track operation queue time and run time, to be saved with the CIB
268++ * resource history (and displayed in cluster status). We need
269++ * high-resolution monotonic time for this purpose, so we use
270++ * clock_gettime(CLOCK_MONOTONIC, ...) (if available, otherwise this feature
271++ * is disabled).
272++ *
273++ * However, we also need epoch timestamps for recording the time the command
274++ * last ran and the time its return value last changed, for use in time
275++ * displays (as opposed to interval calculations). We keep time_t values for
276++ * this purpose.
277++ *
278++ * The last run time is used for both purposes, so we keep redundant
279++ * monotonic and epoch values for this. Technically the two could represent
280++ * different times, but since time_t has only second resolution and the
281++ * values are used for distinct purposes, that is not significant.
282++ */
283++
284++ struct timespec t_first_run; // When op first ran
285++ struct timespec t_run; // When op most recently ran
286++ struct timespec t_first_queue; // When op was first queued
287++ struct timespec t_queue; // When op was most recently queued
288++ time_t epoch_last_run; // Epoch timestamp of when op last ran
289++ time_t epoch_rcchange; // Epoch timestamp of when rc last changed
290++
291++#elif defined(HAVE_SYS_TIMEB_H) /* ftime support kept (LP: #1881762) */
292+ struct timeb t_first_run; /* Timestamp of when op first ran */
293+ struct timeb t_run; /* Timestamp of when op most recently ran */
294+ struct timeb t_first_queue; /* Timestamp of when op first was queued */
295+@@ -589,10 +609,10 @@ send_cmd_complete_notify(lrmd_cmd_t * cmd)
296+ crm_xml_add_int(notify, F_LRMD_RSC_DELETED, cmd->rsc_deleted);
297+
298+ #if defined(PCMK__TIME_USE_CGT) || defined(HAVE_SYS_TIMEB_H)
299+-#ifdef PCMK__TIME_USE_CGT
300+- crm_xml_add_ll(notify, F_LRMD_RSC_RUN_TIME, (long long) cmd->t_run.tv_sec);
301+- crm_xml_add_ll(notify, F_LRMD_RSC_RCCHANGE_TIME, (long long) cmd->t_rcchange.tv_sec);
302+-#else
303++#ifdef PCMK__TIME_USE_CGT /* clock_gettime() fixes only (LP: #1881762) */
304++ crm_xml_add_ll(notify, F_LRMD_RSC_RUN_TIME, (long long) cmd->epoch_last_run);
305++ crm_xml_add_ll(notify, F_LRMD_RSC_RCCHANGE_TIME, (long long) cmd->epoch_rcchange);
306++#else /* ftime support kept (LP: #1881762) */
307+ crm_xml_add_ll(notify, F_LRMD_RSC_RUN_TIME, (long long) cmd->t_run.time);
308+ crm_xml_add_ll(notify, F_LRMD_RSC_RCCHANGE_TIME, (long long) cmd->t_rcchange.time);
309+ #endif
310+@@ -669,6 +689,9 @@ cmd_reset(lrmd_cmd_t * cmd)
311+ #if defined(PCMK__TIME_USE_CGT) || defined(HAVE_SYS_TIMEB_H)
312+ memset(&cmd->t_run, 0, sizeof(cmd->t_run));
313+ memset(&cmd->t_queue, 0, sizeof(cmd->t_queue));
314++#endif
315++#if defined(PCMK__TIME_USE_CGT) /* clock_gettime() fixes only (LP: #1881762) */
316++ cmd->epoch_last_run = 0;
317+ #endif
318+ free(cmd->exit_reason);
319+ cmd->exit_reason = NULL;
320+@@ -910,7 +933,7 @@ action_complete(svc_action_t * action)
321+
322+ #ifdef PCMK__TIME_USE_CGT
323+ if (cmd->exec_rc != action->rc) {
324+- clock_gettime(CLOCK_MONOTONIC, &cmd->t_rcchange);
325++ cmd->epoch_rcchange = time(NULL);
326+ }
327+ #elif defined(HAVE_SYS_TIMEB_H)
328+ if (cmd->exec_rc != action->rc) {
329+@@ -1439,11 +1462,12 @@ lrmd_rsc_execute(lrmd_rsc_t * rsc)
330+ rsc->pending_ops = g_list_remove_link(rsc->pending_ops, first);
331+ g_list_free_1(first);
332+
333+-#ifdef PCMK__TIME_USE_CGT
334++#ifdef PCMK__TIME_USE_CGT /* clock_gettime() fixes only (LP: #1881762) */
335+ if (!time_is_set(&(cmd->t_first_run))) {
336+ clock_gettime(CLOCK_MONOTONIC, &cmd->t_first_run);
337+ }
338+ clock_gettime(CLOCK_MONOTONIC, &cmd->t_run);
339++ cmd->epoch_last_run = time(NULL);
340+ #elif defined(HAVE_SYS_TIMEB_H)
341+ if (cmd->t_first_run.time == 0) {
342+ ftime(&cmd->t_first_run);
343+--
344+2.25.1
345+
346diff --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
347new file mode 100644
348index 0000000..641acd3
349--- /dev/null
350+++ b/debian/patches/ubuntu-2.0.3-fixes/lp1881762-03-Refactor-executor-functionize-getting-current.patch
351@@ -0,0 +1,88 @@
352+Description: Refactor: executor: functionize getting current time in
353+ timespec
354+
355+ ... to reduce code duplication.
356+
357+ Also, previously, when executing a resource command, the run and first
358+ run times would be set with separate clock_gettime() calls, so they
359+ would slightly different; this avoids that by copying the run time to
360+ the first run time.
361+
362+Author: Ken Gaillot <kgaillot@redhat.com>
363+Origin: upstream, https://github.com/ClusterLabs/pacemaker/commit/71ae72d
364+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1881762
365+Reviewed-by: Rafael David Tinoco <rafaeldtinoco@ubuntu.com>
366+Last-Update: 2020-10-08
367+---
368+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
369+---
370+ daemons/execd/execd_commands.c | 25 +++++++++++++------------
371+ 1 file changed, 13 insertions(+), 12 deletions(-)
372+
373+diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c
374+index e14be0276..903f72a53 100644
375+--- a/daemons/execd/execd_commands.c
376++++ b/daemons/execd/execd_commands.c
377+@@ -251,6 +251,7 @@ free_lrmd_cmd(lrmd_cmd_t * cmd)
378+
379+ #ifdef PCMK__TIME_USE_CGT
380+ static inline bool time_is_set(struct timespec *);
381++static void get_current_time(struct timespec *, struct timespec *);
382+ #endif
383+
384+ static gboolean
385+@@ -273,10 +274,7 @@ stonith_recurring_op_helper(gpointer data)
386+ rsc->recurring_ops = g_list_remove(rsc->recurring_ops, cmd);
387+ rsc->pending_ops = g_list_append(rsc->pending_ops, cmd);
388+ #ifdef PCMK__TIME_USE_CGT
389+- clock_gettime(CLOCK_MONOTONIC, &cmd->t_queue);
390+- if (!time_is_set(&(cmd->t_first_queue))) {
391+- cmd->t_first_queue = cmd->t_queue;
392+- }
393++ get_current_time(&(cmd->t_queue), &(cmd->t_first_queue));
394+ #elif defined(HAVE_SYS_TIMEB_H)
395+ ftime(&cmd->t_queue);
396+ if (cmd->t_first_queue.time == 0) {
397+@@ -405,10 +403,7 @@ schedule_lrmd_cmd(lrmd_rsc_t * rsc, lrmd_cmd_t * cmd)
398+
399+ rsc->pending_ops = g_list_append(rsc->pending_ops, cmd);
400+ #ifdef PCMK__TIME_USE_CGT
401+- clock_gettime(CLOCK_MONOTONIC, &cmd->t_queue);
402+- if (!time_is_set(&(cmd->t_first_queue))) {
403+- cmd->t_first_queue = cmd->t_queue;
404+- }
405++ get_current_time(&(cmd->t_queue), &(cmd->t_first_queue));
406+ #elif defined(HAVE_SYS_TIMEB_H)
407+ ftime(&cmd->t_queue);
408+ if (cmd->t_first_queue.time == 0) {
409+@@ -507,6 +502,15 @@ cmd_original_times(lrmd_cmd_t * cmd)
410+ cmd->t_queue = cmd->t_first_queue;
411+ }
412+
413++static void
414++get_current_time(struct timespec *t_current, struct timespec *t_orig)
415++{
416++ clock_gettime(CLOCK_MONOTONIC, t_current);
417++ if ((t_orig != NULL) && !time_is_set(t_orig)) {
418++ *t_orig = *t_current;
419++ }
420++}
421++
422+ #elif defined(HAVE_SYS_TIMEB_H) /* ftime support kept (LP: #1881762) */
423+
424+ /*!
425+@@ -1463,10 +1467,7 @@ lrmd_rsc_execute(lrmd_rsc_t * rsc)
426+ g_list_free_1(first);
427+
428+ #ifdef PCMK__TIME_USE_CGT /* clock_gettime() fixes only (LP: #1881762) */
429+- if (!time_is_set(&(cmd->t_first_run))) {
430+- clock_gettime(CLOCK_MONOTONIC, &cmd->t_first_run);
431+- }
432+- clock_gettime(CLOCK_MONOTONIC, &cmd->t_run);
433++ get_current_time(&(cmd->t_run), &(cmd->t_first_run));
434+ cmd->epoch_last_run = time(NULL);
435+ #elif defined(HAVE_SYS_TIMEB_H)
436+ if (cmd->t_first_run.time == 0) {
437+--
438+2.25.1
439+
440diff --git a/debian/rules b/debian/rules
441index b6d3b94..77d4952 100755
442--- a/debian/rules
443+++ b/debian/rules
444@@ -21,8 +21,20 @@ ifeq ($(shell dpkg-vendor --query vendor)/$(DEB_HOST_ARCH), Ubuntu/i386)
445 -Npacemaker-resource-agents
446 endif
447
448-# ftime causes deprecation errors now, but the clock_gettime replacement is
449-# disabled by default in 2.0.3. Enable it.
450+# https://bugs.launchpad.net/bugs/1881762 (LP: #1881762)
451+#
452+# Sticking with ftime() causes deprecation errors. By mistake clock_gettime()
453+# code was not finished and made default in version 2.0.3 (git log: b5ff0e4).
454+# I could have removed ftime() support by picking upstream merges #1992 and
455+# #1997 entirely, but for the purpose of a SRU it didn't feel right (after
456+# discussions with SRU team).
457+#
458+# Commenting the line below will re-enable ftime() clock functions but,
459+# unfortunately, it will break nagios and systemd resources support (as they
460+# require clock_gettime). It will also cause compilation warning-as-errors
461+# for deprecated ftime() function (which can be work around by
462+# -Wno-error=deprecated-declarations if ever needed).
463+#
464 CPPFLAGS = -UPCMK_TIME_EMERGENCY_CGT
465
466 %:

Subscribers

People subscribed via source and target branches