* [PATCH] multipath-tools: Fix compiler warnings when built without systemd. @ 2020-05-16 17:55 Marius Bakke 2020-06-12 2:25 ` Xose Vazquez Perez 2020-06-16 21:53 ` Martin Wilck 0 siblings, 2 replies; 6+ messages in thread From: Marius Bakke @ 2020-05-16 17:55 UTC (permalink / raw) To: dm-devel --- libmultipath/config.c | 2 +- multipathd/main.c | 19 +++++++++++++------ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/libmultipath/config.c b/libmultipath/config.c index b4d87689..a28dc4f2 100644 --- a/libmultipath/config.c +++ b/libmultipath/config.c @@ -696,7 +696,7 @@ process_config_dir(struct config *conf, char *dir) pthread_cleanup_pop(1); } -static void set_max_checkint_from_watchdog(struct config *conf) +static void set_max_checkint_from_watchdog(__attribute__((unused)) struct config *conf) { #ifdef USE_SYSTEMD char *envp = getenv("WATCHDOG_USEC"); diff --git a/multipathd/main.c b/multipathd/main.c index 8baf9abe..8d3eace6 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -176,6 +176,7 @@ daemon_status(void) /* * I love you too, systemd ... */ +#ifdef USE_SYSTEMD static const char * sd_notify_status(enum daemon_status state) { @@ -195,7 +196,6 @@ sd_notify_status(enum daemon_status state) return NULL; } -#ifdef USE_SYSTEMD static void do_sd_notify(enum daemon_status old_state, enum daemon_status new_state) { @@ -247,7 +247,9 @@ enum daemon_status wait_for_state_change_if(enum daemon_status oldstate, static void __post_config_state(enum daemon_status state) { if (state != running_state && running_state != DAEMON_SHUTDOWN) { - enum daemon_status old_state = running_state; + /* save state for sd_notify */ + enum daemon_status + __attribute__((unused)) old_state = running_state; running_state = state; pthread_cond_broadcast(&config_cond); @@ -272,7 +274,9 @@ int set_config_state(enum daemon_status state) pthread_cleanup_push(config_cleanup, NULL); pthread_mutex_lock(&config_lock); if (running_state != state) { - enum daemon_status old_state = running_state; + /* save state for sd_notify */ + enum daemon_status + __attribute__((unused)) old_state = running_state; if (running_state == DAEMON_SHUTDOWN) rc = EINVAL; @@ -2280,7 +2284,6 @@ 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(); @@ -2292,11 +2295,15 @@ 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); +#ifdef USE_SYSTEMD + /* use_watchdog is set from process environment and never changes */ + bool use_watchdog; + use_watchdog = conf->use_watchdog; +#endif + while (1) { struct timespec diff_time, start_time, end_time; int num_paths = 0, strict_timing, rc = 0; -- 2.26.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] multipath-tools: Fix compiler warnings when built without systemd. 2020-05-16 17:55 [PATCH] multipath-tools: Fix compiler warnings when built without systemd Marius Bakke @ 2020-06-12 2:25 ` Xose Vazquez Perez 2020-06-16 21:53 ` Martin Wilck 1 sibling, 0 replies; 6+ messages in thread From: Xose Vazquez Perez @ 2020-06-12 2:25 UTC (permalink / raw) To: Marius Bakke, DM-DEVEL ML, Martin Wilck, Benjamin Marzinski, Christophe Varoqui On 5/16/20 7:55 PM, Marius Bakke wrote: You should send it, at least, to the project maintainer. > --- > libmultipath/config.c | 2 +- > multipathd/main.c | 19 +++++++++++++------ > 2 files changed, 14 insertions(+), 7 deletions(-) > > diff --git a/libmultipath/config.c b/libmultipath/config.c > index b4d87689..a28dc4f2 100644 > --- a/libmultipath/config.c > +++ b/libmultipath/config.c > @@ -696,7 +696,7 @@ process_config_dir(struct config *conf, char *dir) > pthread_cleanup_pop(1); > } > > -static void set_max_checkint_from_watchdog(struct config *conf) > +static void set_max_checkint_from_watchdog(__attribute__((unused)) struct config *conf) > { > #ifdef USE_SYSTEMD > char *envp = getenv("WATCHDOG_USEC"); > diff --git a/multipathd/main.c b/multipathd/main.c > index 8baf9abe..8d3eace6 100644 > --- a/multipathd/main.c > +++ b/multipathd/main.c > @@ -176,6 +176,7 @@ daemon_status(void) > /* > * I love you too, systemd ... > */ > +#ifdef USE_SYSTEMD > static const char * > sd_notify_status(enum daemon_status state) > { > @@ -195,7 +196,6 @@ sd_notify_status(enum daemon_status state) > return NULL; > } > > -#ifdef USE_SYSTEMD > static void do_sd_notify(enum daemon_status old_state, > enum daemon_status new_state) > { > @@ -247,7 +247,9 @@ enum daemon_status wait_for_state_change_if(enum daemon_status oldstate, > static void __post_config_state(enum daemon_status state) > { > if (state != running_state && running_state != DAEMON_SHUTDOWN) { > - enum daemon_status old_state = running_state; > + /* save state for sd_notify */ > + enum daemon_status > + __attribute__((unused)) old_state = running_state; > > running_state = state; > pthread_cond_broadcast(&config_cond); > @@ -272,7 +274,9 @@ int set_config_state(enum daemon_status state) > pthread_cleanup_push(config_cleanup, NULL); > pthread_mutex_lock(&config_lock); > if (running_state != state) { > - enum daemon_status old_state = running_state; > + /* save state for sd_notify */ > + enum daemon_status > + __attribute__((unused)) old_state = running_state; > > if (running_state == DAEMON_SHUTDOWN) > rc = EINVAL; > @@ -2280,7 +2284,6 @@ 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(); > @@ -2292,11 +2295,15 @@ 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); > > +#ifdef USE_SYSTEMD > + /* use_watchdog is set from process environment and never changes */ > + bool use_watchdog; > + use_watchdog = conf->use_watchdog; > +#endif > + > while (1) { > struct timespec diff_time, start_time, end_time; > int num_paths = 0, strict_timing, rc = 0; > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] multipath-tools: Fix compiler warnings when built without systemd. 2020-05-16 17:55 [PATCH] multipath-tools: Fix compiler warnings when built without systemd Marius Bakke 2020-06-12 2:25 ` Xose Vazquez Perez @ 2020-06-16 21:53 ` Martin Wilck 2020-06-16 23:11 ` [PATCH v2] multipath: " Marius Bakke 2020-06-16 23:17 ` [PATCH] multipath-tools: " Marius Bakke 1 sibling, 2 replies; 6+ messages in thread From: Martin Wilck @ 2020-06-16 21:53 UTC (permalink / raw) To: Marius Bakke, dm-devel; +Cc: Xose Vazquez Perez Hello Marius, On Sat, 2020-05-16 at 19:55 +0200, Marius Bakke wrote: > --- > libmultipath/config.c | 2 +- > multipathd/main.c | 19 +++++++++++++------ > 2 files changed, 14 insertions(+), 7 deletions(-) > thank you for the patch, and sorry for the late reply. Thanks to Xose, too, for making me aware of it. This patch needs some improvements, see remarks below. In general, please note that the development and in particular the testing of multipath-tools has been done almost exclusively on systems using systemd for several years now. The compilation issues you encountered may only be the tip of the iceberg. In particular, the udev rules shipped with multipath-tools rely on systemd for proper device setup. > diff --git a/libmultipath/config.c b/libmultipath/config.c > index b4d87689..a28dc4f2 100644 > --- a/libmultipath/config.c > +++ b/libmultipath/config.c > @@ -696,7 +696,7 @@ process_config_dir(struct config *conf, char > *dir) > pthread_cleanup_pop(1); > } > > -static void set_max_checkint_from_watchdog(struct config *conf) > +static void set_max_checkint_from_watchdog(__attribute__((unused)) > struct config *conf) Rather than adding the attribute, please put the whole function in an #if conditional, and the call in load_config(), too. We don't need this without systemd. > { > #ifdef USE_SYSTEMD > char *envp = getenv("WATCHDOG_USEC"); > diff --git a/multipathd/main.c b/multipathd/main.c > index 8baf9abe..8d3eace6 100644 > --- a/multipathd/main.c > +++ b/multipathd/main.c > @@ -176,6 +176,7 @@ daemon_status(void) > /* > * I love you too, systemd ... > */ > +#ifdef USE_SYSTEMD > static const char * > sd_notify_status(enum daemon_status state) > { > @@ -195,7 +196,6 @@ sd_notify_status(enum daemon_status state) > return NULL; > } > > -#ifdef USE_SYSTEMD > static void do_sd_notify(enum daemon_status old_state, > enum daemon_status new_state) > { > @@ -247,7 +247,9 @@ enum daemon_status wait_for_state_change_if(enum > daemon_status oldstate, > static void __post_config_state(enum daemon_status state) > { > if (state != running_state && running_state != DAEMON_SHUTDOWN) > { > - enum daemon_status old_state = running_state; > + /* save state for sd_notify */ > + enum daemon_status > + __attribute__((unused)) old_state = > running_state; again, please put the declaration of old_state in an #if conditional. > > running_state = state; > pthread_cond_broadcast(&config_cond); > @@ -272,7 +274,9 @@ int set_config_state(enum daemon_status state) > pthread_cleanup_push(config_cleanup, NULL); > pthread_mutex_lock(&config_lock); > if (running_state != state) { > - enum daemon_status old_state = running_state; > + /* save state for sd_notify */ > + enum daemon_status > + __attribute__((unused)) old_state = > running_state; likewise > > if (running_state == DAEMON_SHUTDOWN) > rc = EINVAL; > @@ -2280,7 +2284,6 @@ 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(); > @@ -2292,11 +2295,15 @@ 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); > > +#ifdef USE_SYSTEMD > + /* use_watchdog is set from process environment and never > changes */ > + bool use_watchdog; > + use_watchdog = conf->use_watchdog; > +#endif > + This is wrong. You have to access conf between get_multipath_config() and put_multipath_config(). I'd prefer to simply add conditionals around the declaration of use_watchdog, both in the function header and where it's set. Regards, Martin > while (1) { > struct timespec diff_time, start_time, end_time; > int num_paths = 0, strict_timing, rc = 0; ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] multipath: Fix compiler warnings when built without systemd. 2020-06-16 21:53 ` Martin Wilck @ 2020-06-16 23:11 ` Marius Bakke 2020-06-17 7:28 ` Martin Wilck 2020-06-16 23:17 ` [PATCH] multipath-tools: " Marius Bakke 1 sibling, 1 reply; 6+ messages in thread From: Marius Bakke @ 2020-06-16 23:11 UTC (permalink / raw) To: Martin Wilck, dm-devel; +Cc: Xose Vazquez Perez Add ifdef guards for code that is unused when systemd is not available. --- libmultipath/config.c | 6 ++++-- multipathd/main.c | 10 +++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/libmultipath/config.c b/libmultipath/config.c index b4d87689..658bec8b 100644 --- a/libmultipath/config.c +++ b/libmultipath/config.c @@ -696,9 +696,9 @@ process_config_dir(struct config *conf, char *dir) pthread_cleanup_pop(1); } +#ifdef USE_SYSTEMD static void set_max_checkint_from_watchdog(struct config *conf) { -#ifdef USE_SYSTEMD char *envp = getenv("WATCHDOG_USEC"); unsigned long checkint; @@ -714,8 +714,8 @@ static void set_max_checkint_from_watchdog(struct config *conf) condlog(3, "enabling watchdog, interval %ld", checkint); conf->use_watchdog = true; } -#endif } +#endif struct config * load_config (char * file) @@ -789,7 +789,9 @@ load_config (char * file) /* * fill the voids left in the config file */ +#ifdef USE_SYSTEMD set_max_checkint_from_watchdog(conf); +#endif if (conf->max_checkint == 0) { if (conf->checkint == CHECKINT_UNDEF) conf->checkint = DEFAULT_CHECKINT; diff --git a/multipathd/main.c b/multipathd/main.c index 8baf9abe..ac8ae4c2 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -176,6 +176,7 @@ daemon_status(void) /* * I love you too, systemd ... */ +#ifdef USE_SYSTEMD static const char * sd_notify_status(enum daemon_status state) { @@ -195,7 +196,6 @@ sd_notify_status(enum daemon_status state) return NULL; } -#ifdef USE_SYSTEMD static void do_sd_notify(enum daemon_status old_state, enum daemon_status new_state) { @@ -247,7 +247,9 @@ enum daemon_status wait_for_state_change_if(enum daemon_status oldstate, static void __post_config_state(enum daemon_status state) { if (state != running_state && running_state != DAEMON_SHUTDOWN) { +#ifdef USE_SYSTEMD enum daemon_status old_state = running_state; +#endif running_state = state; pthread_cond_broadcast(&config_cond); @@ -272,7 +274,9 @@ int set_config_state(enum daemon_status state) pthread_cleanup_push(config_cleanup, NULL); pthread_mutex_lock(&config_lock); if (running_state != state) { +#ifdef USE_SYSTEMD enum daemon_status old_state = running_state; +#endif if (running_state == DAEMON_SHUTDOWN) rc = EINVAL; @@ -2280,7 +2284,9 @@ checkerloop (void *ap) struct timespec last_time; struct config *conf; int foreign_tick = 0; +#ifdef USE_SYSTEMD bool use_watchdog; +#endif pthread_cleanup_push(rcu_unregister, NULL); rcu_register_thread(); @@ -2294,7 +2300,9 @@ checkerloop (void *ap) /* use_watchdog is set from process environment and never changes */ conf = get_multipath_config(); +#ifdef USE_SYSTEMD use_watchdog = conf->use_watchdog; +#endif put_multipath_config(conf); while (1) { -- 2.26.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] multipath: Fix compiler warnings when built without systemd. 2020-06-16 23:11 ` [PATCH v2] multipath: " Marius Bakke @ 2020-06-17 7:28 ` Martin Wilck 0 siblings, 0 replies; 6+ messages in thread From: Martin Wilck @ 2020-06-17 7:28 UTC (permalink / raw) To: Marius Bakke, dm-devel; +Cc: Xose Vazquez Perez On Wed, 2020-06-17 at 01:11 +0200, Marius Bakke wrote: > Add ifdef guards for code that is unused when systemd is not > available. > --- > libmultipath/config.c | 6 ++++-- > multipathd/main.c | 10 +++++++++- > 2 files changed, 13 insertions(+), 3 deletions(-) > Reviewed-by: Martin Wilck <mwilck@suse.com> Thanks! Martin ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] multipath-tools: Fix compiler warnings when built without systemd. 2020-06-16 21:53 ` Martin Wilck 2020-06-16 23:11 ` [PATCH v2] multipath: " Marius Bakke @ 2020-06-16 23:17 ` Marius Bakke 1 sibling, 0 replies; 6+ messages in thread From: Marius Bakke @ 2020-06-16 23:17 UTC (permalink / raw) To: Martin Wilck, dm-devel; +Cc: Xose Vazquez Perez Martin Wilck <mwilck@suse.com> writes: > Hello Marius, > > On Sat, 2020-05-16 at 19:55 +0200, Marius Bakke wrote: >> --- >> libmultipath/config.c | 2 +- >> multipathd/main.c | 19 +++++++++++++------ >> 2 files changed, 14 insertions(+), 7 deletions(-) >> > > thank you for the patch, and sorry for the late reply. > Thanks to Xose, too, for making me aware of it. > > This patch needs some improvements, see remarks below. > > In general, please note that the development and in particular the > testing of multipath-tools has been done almost exclusively on systems > using systemd for several years now. The compilation issues you > encountered may only be the tip of the iceberg. In particular, the udev > rules shipped with multipath-tools rely on systemd for proper device > setup. Thanks for reviewing. I'm mainly here for 'kpartx' anyway, but "good" to know that multipath-tools may need extra care outside of systemd. I'm hardly a C programmer, so thanks for bearing with me. Adding just plain old ifdef guards is much clearer than sprinkling __attribute__ around. V2 sent in a separate message. Thanks, Marius ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-06-17 7:28 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-05-16 17:55 [PATCH] multipath-tools: Fix compiler warnings when built without systemd Marius Bakke 2020-06-12 2:25 ` Xose Vazquez Perez 2020-06-16 21:53 ` Martin Wilck 2020-06-16 23:11 ` [PATCH v2] multipath: " Marius Bakke 2020-06-17 7:28 ` Martin Wilck 2020-06-16 23:17 ` [PATCH] multipath-tools: " Marius Bakke
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.