[dm-devel] [PATCH 1/3] multipath: add libmpathvalid library

[Martin Wilck]

On Thu, 2020-09-17 at 18:52 -0500, Benjamin Marzinski wrote:
> On Wed, Sep 16, 2020 at 02:18:49PM +0000, Martin Wilck wrote:
> > On Tue, 2020-09-15 at 16:45 -0500, Benjamin Marzinski wrote:
> > > 
> > > +struct config;
> > > +extern struct config *mpathvalid_conf;
> > 
> > What do you need this for?
> 
> This isn't really necessary. Library users are expected to define
> methods like
> 
> struct config *get_multipath_config(void)
> {
>         return mpathvalid_conf;
> }
> 
> void put_multipath_config(__attribute__((unused))void *conf)
> {
>         /* Noop */
> }
> 
> It would be possible to have mpathvalid_init() return
> 
> struct config *
> 
> and mpathvalid_exit() take that pointer. That would allow the caller
> to
> name their config pointer whatever they liked.  However the only
> thing
> that the caller can effect is the verbosity, so there's never really
> any
> point to have more than one config pointer.
> 
> If you have objections, I can switch this to make the library use a
> user provided config pointer, instead of declaring one itself.

No, I'd rather not. I was mainly wondering why you export the pointer.
I believe that libmultipath's way to require the application to define
certain symbols is suboptimal, and I'd like to get rid of it (see my
other patch set). I can see that (as far as libmultipath is concerned)
this comes too late for sid, but I wouldn't want to repeat that design
decision again.

So, I guess exporting the pointer is just fine. Applications can still
define a symbol by the same name, overriding libmpathvalid's. It might
get problematic if sid will support runtime reconfiguration, like
multipathd. In that case it'd be better if you let the application
override get_multpath_config().

Martin

> 
> -Ben
> 
>  
> > This looks good to me. It would be even better with a few unit
> > tests...
> > 
> > Anyway, I've been working on a patch set to spare callers to define
> > the symbols {get,put}_multipath_config, udev, and logsink (we've
> > been
> > discussing that in the past). I'm going to submit it soon. I think
> > you
> > might be interested in that for this new library.
> > 
> > Regards
> > Martin

-- 
Dr. Martin Wilck <mwilck at suse.com>, Tel. +49 (0)911 74053 2107
SUSE  Software Solutions Germany GmbH
HRB 36809, AG Nürnberg GF: Felix
Imendörffer