[dm-devel] [PATCH 1/7] multipathd: remove missing paths on startup

[Martin Wilck]

On Wed, 2021-10-20 at 14:15 -0500, Benjamin Marzinski wrote:
> If a path device was removed from the system while multipathd was not
> running, multipathd would not remove the path from the multipath
> table
> on start-up, or on a weak reconfigure. update_pathvec_from_dm() would
> return that a reload was necessary, but that information wasn't
> propigated back to where it could be used to reload the device.
> 
> Multipath devices now remember if they need to be reloaded, and if
> so,
> force_reload is set in select_action().  This means that even when
> configure is called with FORCE_RELOAD_WEAK, these devices will still
> be
> reloaded.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins at redhat.com>

Reviewed-by: Martin Wilck <mwilck at suse.com>

(I found the subject of this patch somewhat disconcerting, because it
describes what your patch set is supposed to prevent - mistakenly
deleting paths. But I understand you meant *completely* missing paths,
i.e. paths that aren't present in sysfs).

> ---
>  libmultipath/configure.c   | 2 ++
>  libmultipath/devmapper.c   | 2 ++
>  libmultipath/structs.h     | 1 +
>  libmultipath/structs_vec.c | 1 +
>  4 files changed, 6 insertions(+)
> 
> diff --git a/libmultipath/configure.c b/libmultipath/configure.c
> index eb8ec1bd..f1a890af 100644
> --- a/libmultipath/configure.c
> +++ b/libmultipath/configure.c
> @@ -715,6 +715,8 @@ void select_action (struct multipath *mpp, const
> struct _vector *curmp,
>  
>         cmpp = find_mp_by_wwid(curmp, mpp->wwid);
>         cmpp_by_name = find_mp_by_alias(curmp, mpp->alias);
> +       if (mpp->need_reload || (cmpp && cmpp->need_reload))
> +               force_reload = 1;
>  
>         if (!cmpp_by_name) {
>                 if (cmpp) {
> diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
> index c05dc201..3e1a7260 100644
> --- a/libmultipath/devmapper.c
> +++ b/libmultipath/devmapper.c
> @@ -522,6 +522,8 @@ freeout:
>  addout:
>         dm_task_destroy (dmt);
>  
> +       if (r)
> +               mpp->need_reload = false;
>         return r;
>  }
>  
> diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> index 399540e7..d0b266b7 100644
> --- a/libmultipath/structs.h
> +++ b/libmultipath/structs.h
> @@ -355,6 +355,7 @@ struct multipath {
>         int retain_hwhandler;
>         int deferred_remove;
>         bool in_recovery;
> +       bool need_reload;
>         int san_path_err_threshold;
>         int san_path_err_forget_rate;
>         int san_path_err_recovery_time;
> diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
> index 85d97ac1..e52db0c4 100644
> --- a/libmultipath/structs_vec.c
> +++ b/libmultipath/structs_vec.c
> @@ -237,6 +237,7 @@ bool update_pathvec_from_dm(vector pathvec,
> struct multipath *mpp,
>                 free_pathgroup(pgp, KEEP_PATHS);
>                 must_reload = true;
>         }
> +       mpp->need_reload = mpp->need_reload || must_reload;
>         return must_reload;
>  }
>