Merge ~rafaeldtinoco/ubuntu/+source/pacemaker:lp1881762-focal-backport into ubuntu/+source/pacemaker:ubuntu/focal-devel
- Git
- lp:~rafaeldtinoco/ubuntu/+source/pacemaker
- lp1881762-focal-backport
- Merge into ubuntu/focal-devel
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) |
||||
Related bugs: |
|
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.
Rafael David Tinoco (rafaeldtinoco) wrote : | # |
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?
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.
Rafael David Tinoco (rafaeldtinoco) wrote : | # |
Okay, I'm addressing your comments and pushing this again in here. Thanks for the careful review again.
Rafael David Tinoco (rafaeldtinoco) wrote : | # |
Please let me know if you're good so I can upload this. Thank you
Robie Basak (racb) wrote : | # |
Looks good. Thanks! If you upload, I can accept into the proposed pocket immediately.
Rafael David Tinoco (rafaeldtinoco) wrote : | # |
Awesome.. thanks a lot for all. Here is the output from the tests:
[rafaeldtinoco@
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-focal02 (stonith:
* fence-focal03 (stonith:
* Resource Group: webserver_vip:
* webserver (systemd:lighttpd): Stopped (disabled)
* virtual_ip (ocf::heartbeat
----
[rafaeldtinoco@
----
[rafaeldtinoco@
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-focal02 (stonith:
* fence-focal03 (stonith:
* Resource Group: webserver_vip:
* webserver (systemd:lighttpd): Started focal03
* virtual_ip (ocf::heartbeat
----
A 2s timeout works as well. I'm going ahead and uploading it then.
Rafael David Tinoco (rafaeldtinoco) wrote : | # |
[rafaeldtinoco@
upload/
[rafaeldtinoco@
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.
* [new tag] upload/
$ debdiff pacemaker_
changelog
patches/series
patches/
patches/
patches/
rules
$ dput ubuntu pacemaker_
Uploading to ubuntu (via ftp to upload.ubuntu.com):
Uploading pacemaker_
Uploading pacemaker_
Uploading pacemaker_
Uploading pacemaker_
Successfully uploaded packages.
Preview Diff
1 | diff --git a/debian/changelog b/debian/changelog |
2 | index 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) |
19 | diff --git a/debian/patches/series b/debian/patches/series |
20 | index 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 | + |
34 | 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 |
35 | new file mode 100644 |
36 | index 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 | + |
207 | 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 |
208 | new file mode 100644 |
209 | index 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 | + |
346 | 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 |
347 | new file mode 100644 |
348 | index 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 | + |
440 | diff --git a/debian/rules b/debian/rules |
441 | index 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 | %: |
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.