Re: [dm-devel] [PATCH v2 06/11] libmultipath: add callback for remove_map()

[Martin Wilck <mwilck@suse.com>]

On Mon, 2022-03-21 at 19:28 -0500, Benjamin Marzinski wrote:
> On Fri, Mar 18, 2022 at 5:33 PM <mwilck@suse.com> wrote:
> > 
> > From: Martin Wilck <mwilck@suse.com>
> > 
> > This callback is be used by multipathd to unblock pending
> > reconfigure requests if a map is removed that multipathd is
> > currently
> > waiting for.
> > 
> > Signed-off-by: Martin Wilck <mwilck@suse.com>
> > ---
> >  libmultipath/libmultipath.version | 3 ++-
> >  libmultipath/structs_vec.c        | 6 ++++++
> >  2 files changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libmultipath/libmultipath.version
> > b/libmultipath/libmultipath.version
> > index 216f0ee..8132df7 100644
> > --- a/libmultipath/libmultipath.version
> > +++ b/libmultipath/libmultipath.version
> > @@ -31,7 +31,7 @@
> >   *   The new version inherits the previous ones.
> >   */
> > 
> > -LIBMULTIPATH_14.0.0 {
> > +LIBMULTIPATH_14.1.0 {
> >  global:
> >         /* symbols referenced by multipath and multipathd */
> >         add_foreign;
> > @@ -164,6 +164,7 @@ global:
> >         remember_wwid;
> >         remove_map;
> >         remove_map_by_alias;
> > +       remove_map_callback;
> >         remove_maps;
> >         remove_wwid;
> >         replace_wwids;
> > diff --git a/libmultipath/structs_vec.c
> > b/libmultipath/structs_vec.c
> > index 6c23df8..a69f064 100644
> > --- a/libmultipath/structs_vec.c
> > +++ b/libmultipath/structs_vec.c
> > @@ -336,11 +336,17 @@ void set_path_removed(struct path *pp)
> >         pp->initialized = INIT_REMOVED;
> >  }
> > 
> > +void remove_map_callback(struct multipath *mpp
> > __attribute__((unused)))
> > +{
> > +}
> > +
> 
> Does this work? I thought that unless you specifically declared the
> symbol weak, the call in remove_map() would have already gotten
> resolved to point to the existing remove_map_callback() when the
> shared library was getting created.  Is it because the function is
> empty? Am I just misunderstanding something?

This works because I added the symbol to libmultipath.version,
assigning it "global" visibility. To be consistent, we could do the
same thing with get_multipath_config() et al., but I didn't want to
change that just now.

We (or actually, users and distro integrators) have to be somewhat
careful with adding linker flags. As discussed e.g. in
https://github.com/opensvc/multipath-tools/issues/26
flags like "-Bsymbolic-functions" would mess this up, because this flag
overrides the settings from our linker script. But declaring the symbol
"weak" wouldn't protect against -Bsymbolic mess-up, either.

I had a long discussion with our toolchain experts about this, which
lead to the conclusion above. I am pretty positive about it. 
Feel free to ask the RH experts, too ;-)

Regards
Martin


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel