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

[Benjamin Marzinski]

On Tue, Mar 22, 2022 at 3:35 AM Martin Wilck <mwilck at suse.com> wrote:
>
> On Mon, 2022-03-21 at 19:28 -0500, Benjamin Marzinski wrote:
> > On Fri, Mar 18, 2022 at 5:33 PM <mwilck at suse.com> wrote:
> > >
> > > From: Martin Wilck <mwilck at 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 at 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.

So all of the exported symbols from libmultipath are weak? Good to know.

Reviewed-by: Benjamin Marzinski <bmarzins at redhat.com>


> 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
>
>