libmultipath: fix (max_)polling_interval setting logic
authorMartin Wilck <mwilck@suse.com>
Fri, 15 Nov 2019 14:41:52 +0000 (14:41 +0000)
committerChristophe Varoqui <christophe.varoqui@opensvc.com>
Mon, 2 Mar 2020 23:32:02 +0000 (00:32 +0100)
checkint should never be larger than max_checkint. The WATCHDOG_USEC
setting should be honored on "reconfigure", too, not only on startup.
Pathological values for checkint and integer overflows should be avoided.
The logic should work reasonably well if both polling_interval and
max_polling_interval, just one of them, or neither is set.

Signed-off-by: Martin Wilck <mwilck@suse.com>
libmultipath/config.c
libmultipath/config.h
libmultipath/defaults.h
multipathd/main.c

index 57f96fb..820fe26 100644 (file)
@@ -693,6 +693,27 @@ process_config_dir(struct config *conf, char *dir)
        pthread_cleanup_pop(1);
 }
 
+static void set_max_checkint_from_watchdog(struct config *conf)
+{
+#ifdef USE_SYSTEMD
+       char *envp = getenv("WATCHDOG_USEC");
+       unsigned long checkint;
+
+       if (envp && sscanf(envp, "%lu", &checkint) == 1) {
+               /* Value is in microseconds */
+               checkint /= 1000000;
+               if (checkint < 1 || checkint > UINT_MAX) {
+                       condlog(1, "invalid value for WatchdogSec: \"%s\"", envp);
+                       return;
+               }
+               if (conf->max_checkint == 0 || conf->max_checkint > checkint)
+                       conf->max_checkint = checkint;
+               condlog(3, "enabling watchdog, interval %ld", checkint);
+               conf->use_watchdog = true;
+       }
+#endif
+}
+
 struct config *
 load_config (char * file)
 {
@@ -713,7 +734,8 @@ load_config (char * file)
        conf->multipath_dir = set_default(DEFAULT_MULTIPATHDIR);
        conf->attribute_flags = 0;
        conf->reassign_maps = DEFAULT_REASSIGN_MAPS;
-       conf->checkint = DEFAULT_CHECKINT;
+       conf->checkint = CHECKINT_UNDEF;
+       conf->use_watchdog = false;
        conf->max_checkint = 0;
        conf->force_sync = DEFAULT_FORCE_SYNC;
        conf->partition_delim = (default_partition_delim != NULL ?
@@ -764,8 +786,20 @@ load_config (char * file)
        /*
         * fill the voids left in the config file
         */
-       if (conf->max_checkint == 0)
-               conf->max_checkint = MAX_CHECKINT(conf->checkint);
+       set_max_checkint_from_watchdog(conf);
+       if (conf->max_checkint == 0) {
+               if (conf->checkint == CHECKINT_UNDEF)
+                       conf->checkint = DEFAULT_CHECKINT;
+               conf->max_checkint = (conf->checkint < UINT_MAX / 4 ?
+                                     conf->checkint * 4 : UINT_MAX);
+       } else if (conf->checkint == CHECKINT_UNDEF)
+               conf->checkint = (conf->max_checkint >= 4 ?
+                                 conf->max_checkint / 4 : 1);
+       else if (conf->checkint > conf->max_checkint)
+               conf->checkint = conf->max_checkint;
+       condlog(3, "polling interval: %d, max: %d",
+               conf->checkint, conf->max_checkint);
+
        if (conf->blist_devnode == NULL) {
                conf->blist_devnode = vector_alloc();
 
index 74c0018..ceecff2 100644 (file)
@@ -140,6 +140,7 @@ struct config {
        int minio_rq;
        unsigned int checkint;
        unsigned int max_checkint;
+       bool use_watchdog;
        int pgfailback;
        int remove;
        int rr_weight;
index d703465..e5ee6af 100644 (file)
@@ -53,9 +53,8 @@
 /* Enable all foreign libraries by default */
 #define DEFAULT_ENABLE_FOREIGN ""
 
-#define CHECKINT_UNDEF         (~0U)
+#define CHECKINT_UNDEF         UINT_MAX
 #define DEFAULT_CHECKINT       5
-#define MAX_CHECKINT(a)                (a << 2)
 
 #define MAX_DEV_LOSS_TMO       UINT_MAX
 #define DEFAULT_PIDFILE                "/" RUN_DIR "/multipathd.pid"
index 85b4889..8baf9ab 100644 (file)
  */
 #include "checkers.h"
 
-#ifdef USE_SYSTEMD
-static int use_watchdog;
-#endif
-
 /*
  * libmultipath
  */
@@ -2284,6 +2280,7 @@ checkerloop (void *ap)
        struct timespec last_time;
        struct config *conf;
        int foreign_tick = 0;
+       bool use_watchdog;
 
        pthread_cleanup_push(rcu_unregister, NULL);
        rcu_register_thread();
@@ -2295,6 +2292,11 @@ checkerloop (void *ap)
        get_monotonic_time(&last_time);
        last_time.tv_sec -= 1;
 
+       /* use_watchdog is set from process environment and never changes */
+       conf = get_multipath_config();
+       use_watchdog = conf->use_watchdog;
+       put_multipath_config(conf);
+
        while (1) {
                struct timespec diff_time, start_time, end_time;
                int num_paths = 0, strict_timing, rc = 0;
@@ -2768,7 +2770,6 @@ child (__attribute__((unused)) void *param)
        struct multipath * mpp;
        int i;
 #ifdef USE_SYSTEMD
-       unsigned long checkint;
        int startup_done = 0;
 #endif
        int rc;
@@ -2845,21 +2846,6 @@ child (__attribute__((unused)) void *param)
        setscheduler();
        set_oom_adj();
 
-#ifdef USE_SYSTEMD
-       envp = getenv("WATCHDOG_USEC");
-       if (envp && sscanf(envp, "%lu", &checkint) == 1) {
-               /* Value is in microseconds */
-               conf->max_checkint = checkint / 1000000;
-               /* Rescale checkint */
-               if (conf->checkint > conf->max_checkint)
-                       conf->checkint = conf->max_checkint;
-               else
-                       conf->checkint = conf->max_checkint / 4;
-               condlog(3, "enabling watchdog, interval %d max %d",
-                       conf->checkint, conf->max_checkint);
-               use_watchdog = conf->checkint;
-       }
-#endif
        /*
         * Startup done, invalidate configuration
         */