[dm-devel] [PATCH v2 6/6] multipathd: cleanup logging for marginal paths

[Martin Wilck]

On Thu, 2021-01-14 at 20:20 -0600, Benjamin Marzinski wrote:
> io_err_stat logged at level 2 whenever it enqueued a path to check,
> which could happen multiple times while a path was marginal.  On the
> other hand if marginal_pathgroups wasn't set, multipathd didn't log
> when
> paths were set to marginal. Now io_err_stat only logs at level 2 when
> something unexpected happens, but multipathd will always log when a
> path switches its marginal state.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins at redhat.com>
> ---
>  libmultipath/io_err_stat.c |  7 +++----
>  multipathd/main.c          | 25 ++++++++++++++-----------
>  2 files changed, 17 insertions(+), 15 deletions(-)
> 
> diff --git a/libmultipath/io_err_stat.c b/libmultipath/io_err_stat.c
> index bf78a236..abdd0b4f 100644
> --- a/libmultipath/io_err_stat.c
> +++ b/libmultipath/io_err_stat.c
> @@ -252,7 +252,7 @@ static int enqueue_io_err_stat_by_path(struct
> path *path)
>         vector_set_slot(io_err_pathvec, p);
>         pthread_mutex_unlock(&io_err_pathvec_lock);
>  
> -       io_err_stat_log(2, "%s: enqueue path %s to check",
> +       io_err_stat_log(3, "%s: enqueue path %s to check",
>                         path->mpp->alias, path->dev);
>         return 0;
>  
> @@ -343,7 +343,7 @@ int need_io_err_check(struct path *pp)
>         if (uatomic_read(&io_err_thread_running) == 0)
>                 return 0;
>         if (count_active_paths(pp->mpp) <= 0) {
> -               io_err_stat_log(2, "%s: recover path early", pp-
> >dev);
> +               io_err_stat_log(2, "%s: no paths. recovering early",
> pp->dev);
>                 goto recover;
>         }
>         if (pp->io_err_pathfail_cnt != PATH_IO_ERR_WAITING_TO_CHECK)
> @@ -361,8 +361,7 @@ int need_io_err_check(struct path *pp)
>                  * Or else,  return 1 to set path state to PATH_SHAKY
>                  */
>                 if (r == 1) {
> -                       io_err_stat_log(3, "%s: enqueue fails, to
> recover",
> -                                       pp->dev);
> +                       io_err_stat_log(2, "%s: enqueue failed.
> recovering early", pp->dev);
>                         goto recover;
>                 } else
>                         pp->io_err_pathfail_cnt =
> PATH_IO_ERR_IN_CHECKING;
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 92c45d44..99a89a69 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -2132,8 +2132,8 @@ check_path (struct vectors * vecs, struct path
> * pp, unsigned int ticks)
>                 pathinfo(pp, conf, 0);
>                 pthread_cleanup_pop(1);
>                 return 1;
> -       } else if ((newstate != PATH_UP && newstate != PATH_GHOST) &&
> -                       (pp->state == PATH_DELAYED)) {
> +       } else if ((newstate != PATH_UP && newstate != PATH_GHOST &&
> +                   newstate != PATH_PENDING) && (pp->state ==
> PATH_DELAYED)) {

I think this is correct, but it needs to  be mentioned in the commit
message (or go into a separate patch).

Regards,
Martin


-- 
Dr. Martin Wilck <mwilck at suse.com>, Tel. +49 (0)911 74053 2107
SUSE Software Solutions Germany GmbH
HRB 36809, AG Nürnberg GF: Felix Imendörffer