[dm-devel] [PATCH 07/19] multipathd: set_config_state(): avoid code duplication

[Benjamin Marzinski]

On Wed, Sep 16, 2020 at 05:37:06PM +0200, mwilck at suse.com wrote:
> From: Martin Wilck <mwilck at suse.com>
> 
> Use __post_config_state() and __wait_for_state_change(). This
> way __post_config_state() is the only place where running_state
> is ever changed, and we avoid code duplication.
> 

It's only tangentially related to this patch, but it's possible for
set_conf_state() to timeout, and we'd don't always retry it. That's
fine, be we don't always check for failure and notify the user that the
reconfigure isn't happening, and we probably should. But the patch
itself is fine.

Reviewed-by: Benjamin Marzinski <bmarzins at redhat.com> 

> Signed-off-by: Martin Wilck <mwilck at suse.com>
> ---
>  multipathd/main.c | 23 +++++------------------
>  1 file changed, 5 insertions(+), 18 deletions(-)
> 
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 49809e0..a5c4031 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -292,27 +292,14 @@ 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;
> -		else if (running_state != DAEMON_IDLE) {
> -			struct timespec ts;
> -
> -			get_monotonic_time(&ts);
> -			ts.tv_sec += 1;
> -			rc = pthread_cond_timedwait(&config_cond,
> -						    &config_lock, &ts);
> -		}
> -		if (!rc && (running_state != DAEMON_SHUTDOWN)) {
> -			running_state = state;
> -			pthread_cond_broadcast(&config_cond);
> -#ifdef USE_SYSTEMD
> -			do_sd_notify(old_state, state);
> -#endif
> -		}
> +		else
> +			rc = __wait_for_state_change(
> +				running_state != DAEMON_IDLE, 1000);
> +		if (!rc)
> +			__post_config_state(state);
>  	}
>  	pthread_cleanup_pop(1);
>  	return rc;
> -- 
> 2.28.0