On 08/24/2016 11:41 AM, Fabiano Fidêncio wrote:
On Wed, 2016-08-24 at 11:29 +0200, Pavel Březina wrote:

 From 09e5ad3448694d9609bd2a9e89e8a05ef82f66bc Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Mon, 22 Aug 2016 13:15:04 +0200
Subject: [PATCH] watchdog: cope with time shift

Resolves:
  https://fedorahosted.org/sssd/ticket/3154

May I ask you to be a bit more verbose in the commit message, please?

Sure.


---
  src/util/util_watchdog.c | 41
+++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)

diff --git a/src/util/util_watchdog.c b/src/util/util_watchdog.c
index
5032fddba1b94b3fc7e560162c392dfa57d699cf..178ff92d5c6b950d0086204012354
74499ba595d 100644
--- a/src/util/util_watchdog.c
+++ b/src/util/util_watchdog.c
@@ -29,8 +29,39 @@ struct watchdog_ctx {
      struct timeval interval;
      struct tevent_timer *te;
      volatile int ticks;
+
+    /* To detect time shift. */
+    struct tevent_context *ev;
+    int input_interval;
+    time_t timestamp;
  } watchdog_ctx;

+bool watchdog_detect_timeshift(void)

This function may be static, as it's not used anywhere out of this
file.

Yep.

Patch looks good to me apart from the minor comments.

Thank you, new patch is attached.


From 49e14a215794eaae3976d00fc7d676d48d6731b4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Mon, 22 Aug 2016 13:15:04 +0200
Subject: [PATCH] watchdog: cope with time shift

When a time is changed into the past during sssd runtime
(e.g. on boot during time correction), it is possible that
we never hit watchdog tevent timer since it is based on
system time.

This patch adds a past-time shift detection mechanism. If a time
shift is detected we restart watchdog.

Resolves:
https://fedorahosted.org/sssd/ticket/3154
---
 src/util/util_watchdog.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/src/util/util_watchdog.c b/src/util/util_watchdog.c
index 5032fddba1b94b3fc7e560162c392dfa57d699cf..1c27d73f13b3042ecb549a2184e1368e8339d199 100644
--- a/src/util/util_watchdog.c
+++ b/src/util/util_watchdog.c
@@ -29,8 +29,39 @@ struct watchdog_ctx {
     struct timeval interval;
     struct tevent_timer *te;
     volatile int ticks;
+
+    /* To detect time shift. */
+    struct tevent_context *ev;
+    int input_interval;
+    time_t timestamp;
 } watchdog_ctx;
 
+static bool watchdog_detect_timeshift(void)
+{
+    time_t prev_time;
+    time_t cur_time;
+    errno_t ret;
+
+    prev_time = watchdog_ctx.timestamp;
+    cur_time = watchdog_ctx.timestamp = time(NULL);
+    if (cur_time < prev_time) {
+        /* Time shift detected. We need to restart watchdog. */
+        DEBUG(SSSDBG_IMPORTANT_INFO, "Time shift detected, "
+              "restarting watchdog!\n");
+        teardown_watchdog();
+        ret = setup_watchdog(watchdog_ctx.ev, watchdog_ctx.input_interval);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_FATAL_FAILURE, "Unable to restart watchdog "
+                  "[%d]: %s\n", ret, sss_strerror(ret));
+            orderly_shutdown(1);
+        }
+
+        return true;
+    }
+
+    return false;
+}
+
 /* the watchdog is purposefully *not* handled by the tevent
  * signal handler as it is meant to check if the daemon is
  * still processing the event queue itself. A stuck process
@@ -38,6 +69,12 @@ struct watchdog_ctx {
  * signals either */
 static void watchdog_handler(int sig)
 {
+    /* Do not count ticks if time shift was detected
+     * since watchdog was restarted. */
+    if (watchdog_detect_timeshift()) {
+        return;
+    }
+
     /* if 3 ticks passed by kills itself */
 
     if (__sync_add_and_fetch(&watchdog_ctx.ticks, 1) > 3) {
@@ -101,6 +138,10 @@ int setup_watchdog(struct tevent_context *ev, int interval)
     watchdog_ctx.interval.tv_sec = interval;
     watchdog_ctx.interval.tv_usec = 0;
 
+    watchdog_ctx.ev = ev;
+    watchdog_ctx.input_interval = interval;
+    watchdog_ctx.timestamp = time(NULL);
+
     /* Start the timer */
     /* we give 1 second head start to the watchdog event */
     its.it_value.tv_sec = interval + 1;
-- 
2.1.0

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

Reply via email to