On Wed, Sep 04, 2024 at 02:51:19PM -0400, Benjamin Marzinski wrote:
> On Wed, Sep 04, 2024 at 05:38:52PM +0200, Martin Wilck wrote:
> > On Wed, 2024-08-28 at 18:17 -0400, Benjamin Marzinski wrote:
> > > Split out the code that updates a path's state and sets up the next
> > > check time into its own function, update_path().
> > >
> > > Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
> > > ---
> > > multipathd/main.c | 31 ++++++++++++++++++++++---------
> > > 1 file changed, 22 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/multipathd/main.c b/multipathd/main.c
> > > index 94d4e421..300f8247 100644
> > > --- a/multipathd/main.c
> > > +++ b/multipathd/main.c
> > > @@ -2390,6 +2390,7 @@ sync_mpp(struct vectors * vecs, struct
> > > multipath *mpp, unsigned int ticks)
> > > }
> > >
> > > enum check_path_return {
> > > + CHECK_PATH_STARTED,
> > > CHECK_PATH_CHECKED,
> > > CHECK_PATH_SKIPPED,
> > > CHECK_PATH_REMOVED,
> > > @@ -2629,13 +2630,10 @@ update_path_state (struct vectors * vecs,
> > > struct path * pp)
> > > }
> > >
> > > static int
> > > -check_path (struct vectors * vecs, struct path * pp, unsigned int
> > > ticks,
> > > - time_t start_secs)
> > > +check_path (struct path * pp, unsigned int ticks)
> >
> > check_path() used to be one of our core functions, and you now re-
> > introduce it with quite different semantics.
> >
> > Perhaps choose a new name?
>
> Sure. Although the new check_path() is just the beginning part of the
> old check_path(), where we actually run the checker, so it seems
> reasonable to me. But your objection is also reasonable. I was just
> getting sick of coming up with new function names by this point.
>
> -Ben
Do you have ideas for the name, because I can't think of anything that
makes more sense then check_path() in the code paths. Here's the code
paths
checkerloop (initialized paths)
-----------------------------------------
check_paths update_paths
check_path update_path
start_path_check update_path_state
start_checker check_path_state
checker_check get_state
checker_get_state
checkerloop (uninitialized paths)
-----------------------------------------
check_paths update_paths
check_uninitialized_path update_uninitialized_path
start_path_check check_path_state
start_checker get_state
checker_check checker_get_state
pathinfo
----------------
start_checker get_state
checker_check checker_get_state
The only function that stands out to me as misnamed is
"check_path_state", which I think I'm going to change to
"get_updated_state", since that's a better description of what it's
doing, and it avoids using "check" in the update code path.
Also, like I said before, if you're looking for the function that gets
run to see if a path is due for checking, and runs the path checker if
it is, that's check_path(), just like it used to be. It just no longer
updates the path and mpp state based on the checker result like it used
to. That's now done by update_path() and update_mpp_prio().
>
> >
> > Martin
>
