[dm-devel] [PATCH] multipathd: fix missing persistent reseravtion for active path

[Martin Wilck]

Hello lixiaokeng,

thanks for your patch.

On Fri, 2021-09-10 at 20:31 +0800, lixiaokeng wrote:
> There are two paths(sucu as sda and adb) for one LUN. The two
> paths log in, but before the two uevents have been processed
> (for example there are many uevent), users use multipathd add
> path /dev/sda to cause mpatha and use mpathpersist -o -I to
> register prkey for mpatha. The add map uevent is after add path
> uevent, the the uevent(add sdb) will delay and missing persistent
> reseravtion check.
> 
> Here, we add persistent reseravtion check in ev_add_map if
> mpp->wait_for_udev > 1.
> 
> Signed-off-by: Lixiaokeng <lixiaokeng at huawei.com>
> ---
>  multipathd/main.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 3aff241d..ef456c34 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -706,6 +706,8 @@ ev_add_map (char * dev, const char * alias,
> struct vectors * vecs)
>         struct multipath * mpp;
>         int delayed_reconfig, reassign_maps;
>         struct config *conf;
> +       struct path *pp;
> +       int i;
> 
>         if (dm_is_mpath(alias) != 1) {
>                 condlog(4, "%s: not a multipath map", alias);
> @@ -721,6 +723,13 @@ ev_add_map (char * dev, const char * alias,
> struct vectors * vecs)
>                         if (update_map(mpp, vecs, 0))
>                                 /* setup multipathd removed the map
> */
>                                 return 1;

Should you make this conditional on mpp->prflag, perhaps?
After all, sda has already been added, sp prflag should should be set
if that was successful.

I think this should rather be added to update_map(). Ben?


> +
> +                       vector_foreach_slot(mpp->paths, pp, i) {
> +                               if ((pp->state == PATH_UP)  || (pp-
> >state == PATH_GHOST)) {
> +                                       /* persistent reseravtion
> check*/

Typo above.

> +                                       mpath_pr_event_handle(pp);
> +                               }
> +                       }
>                 }
>                 conf = get_multipath_config();
>                 delayed_reconfig = conf->delayed_reconfig;


Thanks,
Martin