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@xxxxxxxxxxx>
> > ---
> > 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.
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
