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

[Bart Van Assche]

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?

Bart.