On Fri, Nov 15, 2019 at 02:41:52PM +0000, Martin Wilck wrote: > From: Martin Wilck <[email protected]> > > 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. > Reviewed-by: Benjamin Marzinski <[email protected]> > Signed-off-by: Martin Wilck <[email protected]> > --- > libmultipath/config.c | 40 +++++++++++++++++++++++++++++++++++++--- > libmultipath/config.h | 1 + > libmultipath/defaults.h | 3 +-- > multipathd/main.c | 26 ++++++-------------------- > 4 files changed, 45 insertions(+), 25 deletions(-) > > diff --git a/libmultipath/config.c b/libmultipath/config.c > index 49723add..0bf7bb40 100644 > --- a/libmultipath/config.c > +++ b/libmultipath/config.c > @@ -683,6 +683,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) > { > @@ -703,7 +724,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 ? > @@ -754,8 +776,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(); > > diff --git a/libmultipath/config.h b/libmultipath/config.h > index 2ab7b42c..a078947c 100644 > --- a/libmultipath/config.h > +++ b/libmultipath/config.h > @@ -139,6 +139,7 @@ struct config { > int minio_rq; > unsigned int checkint; > unsigned int max_checkint; > + bool use_watchdog; > int pgfailback; > int remove; > int rr_weight; > diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h > index d7034655..e5ee6afe 100644 > --- a/libmultipath/defaults.h > +++ b/libmultipath/defaults.h > @@ -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" > diff --git a/multipathd/main.c b/multipathd/main.c > index c0423602..95426d3d 100644 > --- a/multipathd/main.c > +++ b/multipathd/main.c > @@ -33,10 +33,6 @@ > */ > #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; > @@ -2766,7 +2768,6 @@ child (__attribute__((unused)) void *param) > struct multipath * mpp; > int i; > #ifdef USE_SYSTEMD > - unsigned long checkint; > int startup_done = 0; > #endif > int rc; > @@ -2843,21 +2844,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 > */ > -- > 2.24.0
-- dm-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/dm-devel
