On Fri, 2025-12-19 at 23:23 -0500, Benjamin Marzinski wrote: > 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 <[email protected]> > > Signed-off-by: Martin Wilck <[email protected]> > > --- > > 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.
Sorry, I don't understand. The __attribute__((cleanup())) functions don't protect against thread cancellation. If we want to achieve what you describe, we must add a pthread_cleanup_push()/pop() pair around the update_multipath_table__() call. But would that buy us anything? update_multipath_table__() only manipulates multipathd's internal data structures, which, if a cancellation happens, will almost immediately all be freed anyway. What harm would be done if these paths weren't orphaned in such a case? Yes, the some paths might now link to a non-existing struct multipath. I'd see your point if we accessed pp->mpp in the path-freeing code path, but we don't do this any more. > 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(). Like I said above, that would have no effect in the case of cancellation. I can add pthread_cleanup_push()/pop() pair if you insist, but so far I don't see the benefit. Regards Martin
