Re: [PATCH 06/15] libmultipath: split get_state into two functions

[Benjamin Marzinski <bmarzins@redhat.com>]

On Wed, Sep 04, 2024 at 05:14:32PM +0200, Martin Wilck wrote:
> On Wed, 2024-08-28 at 18:17 -0400, Benjamin Marzinski wrote:
> > get_state() is now split into start_checker(), which runs the checker
> > but doesn't wait for async checkers, and get_state(), which returns
> > the
> > state from the checker, after waiting for async checkers if
> > necessary.
> > 
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > ---
> >  libmultipath/discovery.c | 22 ++++++++++++++++------
> >  libmultipath/discovery.h |  4 +++-
> >  libmultipath/propsel.c   |  1 +
> >  multipathd/main.c        |  4 +++-
> >  4 files changed, 23 insertions(+), 8 deletions(-)
> > 
> > diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> > index 5648be60..e0f46ff2 100644
> > --- a/libmultipath/discovery.c
> > +++ b/libmultipath/discovery.c
> > @@ -1974,30 +1974,29 @@ cciss_ioctl_pathinfo(struct path *pp)
> >  }
> >  
> >  int
> > -get_state (struct path * pp, struct config *conf, int daemon, int
> > oldstate)
> > +start_checker (struct path * pp, struct config *conf, int daemon,
> > int oldstate)
> >  {
> >  	struct checker * c = &pp->checker;
> > -	int state;
> >  
> >  	if (!checker_selected(c)) {
> >  		if (daemon) {
> >  			if (pathinfo(pp, conf, DI_SYSFS) !=
> > PATHINFO_OK) {
> >  				condlog(3, "%s: couldn't get sysfs
> > pathinfo",
> >  					pp->dev);
> > -				return PATH_UNCHECKED;
> > +				return -1;
> >  			}
> >  		}
> >  		select_detect_checker(conf, pp);
> >  		select_checker(conf, pp);
> >  		if (!checker_selected(c)) {
> >  			condlog(3, "%s: No checker selected", pp-
> > >dev);
> > -			return PATH_UNCHECKED;
> > +			return -1;
> >  		}
> >  		checker_set_fd(c, pp->fd);
> >  		if (checker_init(c, pp->mpp?&pp->mpp-
> > >mpcontext:NULL)) {
> >  			checker_clear(c);
> >  			condlog(3, "%s: checker init failed", pp-
> > >dev);
> > -			return PATH_UNCHECKED;
> > +			return -1;
> >  		}
> >  	}
> >  	if (pp->mpp && !c->mpcontext)
> > @@ -2008,6 +2007,15 @@ get_state (struct path * pp, struct config
> > *conf, int daemon, int oldstate)
> >  	else
> >  		checker_set_sync(c);
> >  	checker_check(c, oldstate);
> > +	return 0;
> > +}
> > +
> > +int
> > +get_state (struct path * pp)
> > +{
> > +	struct checker * c = &pp->checker;
> > +	int state;
> > +
> >  	state = checker_get_state(c);
> >  	condlog(3, "%s: %s state = %s", pp->dev,
> >  		checker_name(c), checker_state_name(state));
> > @@ -2455,7 +2463,9 @@ int pathinfo(struct path *pp, struct config
> > *conf, int mask)
> >  
> >  	if (mask & DI_CHECKER) {
> >  		if (path_state == PATH_UP) {
> > -			int newstate = get_state(pp, conf, 0,
> > path_state);
> > +			int newstate = PATH_UNCHECKED;
> > +			if (start_checker(pp, conf, 0, path_state)
> > == 0)
> > +				newstate = get_state(pp);
> >  			if (newstate != PATH_PENDING ||
> >  			    pp->state == PATH_UNCHECKED ||
> >  			    pp->state == PATH_WILD)
> > diff --git a/libmultipath/discovery.h b/libmultipath/discovery.h
> > index c93abf1c..f3e0c618 100644
> > --- a/libmultipath/discovery.h
> > +++ b/libmultipath/discovery.h
> > @@ -34,7 +34,9 @@ int path_discovery (vector pathvec, int flag);
> >  int path_get_tpgs(struct path *pp); /* This function never returns
> > TPGS_UNDEF */
> >  int do_tur (char *);
> >  int path_offline (struct path *);
> > -int get_state (struct path * pp, struct config * conf, int daemon,
> > int state);
> > +int start_checker(struct path * pp, struct config * conf, int
> > daemon,
> > +		  int state);
> > +int get_state(struct path * pp);
> >  int get_vpd_sgio (int fd, int pg, int vend_id, char * str, int
> > maxlen);
> >  int pathinfo (struct path * pp, struct config * conf, int mask);
> >  int alloc_path_with_pathinfo (struct config *conf, struct
> > udev_device *udevice,
> > diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
> > index a3fce203..ad771d35 100644
> > --- a/libmultipath/propsel.c
> > +++ b/libmultipath/propsel.c
> > @@ -700,6 +700,7 @@ out:
> >  	condlog(3, "%s: path_checker = %s %s", pp->dev,
> >  		checker_name(c), origin);
> >  	c->timeout = pp->checker_timeout;
> > +	c->path_state = PATH_UNCHECKED;
> 
> Should this be in 01/15 already?

Yeah. That makes sense.

-Ben

> 
> Martin