[dm-devel] [PATCH 5/7] Move the declaration of mpath_mx_alloc_len to a header file

Benjamin Marzinski bmarzins at redhat.com
Thu May 18 21:00:51 UTC 2017


On Thu, May 18, 2017 at 08:36:10PM +0000, Bart Van Assche wrote:
> On Thu, 2017-05-18 at 15:06 -0500, Benjamin Marzinski wrote:
> > On Wed, May 17, 2017 at 08:43:07AM -0700, Bart Van Assche wrote:
> > > This allows the compiler to verify consistency of declaration and
> > > definition.
> > > Signed-off-by: Bart Van Assche <bart.vanassche at sandisk.com>
> > > ---
> > >  libmpathpersist/mpath_persist.h | 3 +++
> > >  mpathpersist/main.c             | 1 -
> > >  multipathd/main.c               | 2 --
> > >  3 files changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/libmpathpersist/mpath_persist.h b/libmpathpersist/mpath_persist.h
> > > index 79de5b5b..3faa8c9e 100644
> > > --- a/libmpathpersist/mpath_persist.h
> > > +++ b/libmpathpersist/mpath_persist.h
> > > @@ -81,6 +81,9 @@ extern "C" {
> > >  
> > >  
> > >  
> > > +extern unsigned int mpath_mx_alloc_len;
> > > +
> > > +
> > >  
> > >  struct prin_readdescr
> > >  {
> > > diff --git a/mpathpersist/main.c b/mpathpersist/main.c
> > > index 2e0aba3c..9de52b98 100644
> > > --- a/mpathpersist/main.c
> > > +++ b/mpathpersist/main.c
> > > @@ -40,7 +40,6 @@ void mpath_print_transport_id(struct prin_fulldescr *fdesc);
> > >  int construct_transportid(const char * inp, struct transportid transid[], int num_transportids);
> > >  
> > >  int logsink;
> > > -unsigned int mpath_mx_alloc_len;
> > >  struct config *multipath_conf;
> > >  
> > >  struct config *get_multipath_config(void)
> > > diff --git a/multipathd/main.c b/multipathd/main.c
> > > index b167cb4c..81c76cab 100644
> > > --- a/multipathd/main.c
> > > +++ b/multipathd/main.c
> > > @@ -104,8 +104,6 @@ struct mpath_event_param
> > >  	struct multipath *mpp;
> > >  };
> > >  
> > > -unsigned int mpath_mx_alloc_len;
> > > -
> > >  int logsink;
> > >  int verbosity;
> > >  int bindings_read_only; 
> >
> > I get why you did this, but I'm not totally happy with where the extern
> > declaration is.  libmpathpersist is actually a public library, unlike
> > libmultipath, and mpath_persist.h is that header for that library.  As
> > such, I'd rather not add things to it that users aren't supposed to mess
> > with.  So, is your intention to let users set the max alloc length. If
> > so, we should probably give them a function, or at the very least some
> > comments telling them what this variable does. If we only want the
> > mpathpersist program to be able to change this, then we probably
> > should probably put the declaration in a different header file, perhaps
> > mpath_pr_ioctl.c. Now that I'm looking at this, I see that this option
> > (-l) isn't even documented in the mpathpersist manpage. So right now
> > this seems likely a completely unused feature. I guess since it exists,
> > I'd lean towards actually documenting it and putting it in the
> > libmpathpersist API in a more useful way.
> 
> (changed top-posting into bottom-posting)
> 
> Hello Ben,
> 
> It was not my intention to make the mpath_mx_alloc_len variable accessible
> to users of the libmpathpersist library. From what I can see in
> libmpathpersist/Makefile the only public header file of the libmpathpersist
> library is mpath_persist.h. Would you agree to move that declaration into
> mpathpr.h?

Sure

-Ben

> 
> Bart.




More information about the dm-devel mailing list