Re: [PATCH v2 14/26] multipathd: add_map_without_path(): orphan paths instead of freeing them

[Benjamin Marzinski <bmarzins@redhat.com>]

On Fri, Dec 19, 2025 at 03:40:56PM +0100, Martin Wilck wrote:
> If add_map_without_path() fails to set up the multipath data structures,
> don't free the paths associated with the map as
> cleanup_multipath_and_paths() would; just orphan the paths.
> update_pathvec_from_dm() will handle the paths and see if they
> need to be removed.
> 
> Suggested-by: Benjamin Marzinski <bmarzins@suse.com>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  multipathd/main.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/multipathd/main.c b/multipathd/main.c
> index c03546b..7b522e8 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -771,8 +771,8 @@ fail:
>  
>  static int add_map_without_path (struct vectors *vecs, const char *alias)
>  {
> -	struct multipath __attribute__((cleanup(cleanup_multipath_and_paths)))
> -		*mpp = alloc_multipath();
> +	struct multipath __attribute__((cleanup(cleanup_multipath))) *mpp =
> +		alloc_multipath();
>  	char __attribute__((cleanup(cleanup_charp))) *params = NULL;
>  	char __attribute__((cleanup(cleanup_charp))) *status = NULL;
>  	struct config *conf;
> @@ -802,11 +802,13 @@ static int add_map_without_path (struct vectors *vecs, const char *alias)
>  	mpp->mpe = find_mpe(conf->mptable, mpp->wwid);
>  	put_multipath_config(conf);
>  
> -	if ((rc = update_multipath_table__(mpp, vecs->pathvec, 0, params, status)) != DMP_OK)
> +	if (update_multipath_table__(mpp, vecs->pathvec, 0, params, status) != DMP_OK ||
> +	    !vector_alloc_slot(vecs->mpvec)) {
> +		orphan_paths(vecs->pathvec, mpp, "failed to add map");
>  		return DMP_ERR;

I'm not sure we should just call orphan_paths() after we call
update_multipath_table__(). If we hit a cancellation point in the middle
of update_multipath_table__(), we want to fully orphan any paths that
got added to the device before we cancelled.  Perhaps the easiest way to
solve this is to make cleanup_multipath_and_paths() loop through all the
path in mpp->pg and orphan them before calling free_multipath(). We
shouldn't need to search through the entire pathvec here. Since this
multipath device was just created, the only paths that could have
pp->mpp set to this device will be in mpp->pg.

-Ben

> +	}
>  
> -	if (!vector_alloc_slot(vecs->mpvec))
> -		return DMP_ERR;
> +	/* Make sure mpp is not cleaned up on return */
>  	vector_set_slot(vecs->mpvec, steal_ptr(mpp));
>  
>  	/*
> -- 
> 2.52.0