Re: [dm-devel] [PATCH 07/35] multipathd: improve delayed reconfigure

[Martin Wilck <mwilck@suse.com>]

On Wed, 2021-09-15 at 18:00 -0500, Benjamin Marzinski wrote:
> On Fri, Sep 10, 2021 at 01:40:52PM +0200, mwilck@suse.com wrote:
> > From: Martin Wilck <mwilck@suse.com>
> > 
> > When a reconfigure operation is requested, either by the admin
> > or by some condition multipathd encounters, the current code
> > attempts to set DAEMON_CONFIGURE state and gives up after a second
> > if it doesn't succeed. Apart from shutdown, this happens only
> > if multipathd is either already reconfiguring, or busy in the
> > path checker loop.
> > 
> > This patch modifies the logic as follows: rather than waiting,
> > we set a flag that requests a reconfigure operation asap, i.e.
> > when the current operation is finished and the status switched
> > to DAEMON_IDLE. In this case, multipathd will not switch to IDLE
> > but start another reconfigure cycle.
> > 
> > This assumes that if a reconfigure is requested while one is
> > already
> > running, the admin has made some (additional) changes and wants
> > multipathd to pull them in. As we can't be sure that the currently
> > running reconfigure has seen the configuration changes, we need
> > to start over again.
> > 
> > A positive side effect is less waiting in clients and multipathd.
> > 
> > After this change, the only caller of set_config_state() is
> > checkerloop(). Waking up every second just to see that
> > DAEMON_RUNNING
> > couldn't be set makes no sense. Therefore set_config_state() is
> > changed to wait "forever", or until shutdown is requested. Unless
> > multipathd completely hangs, the wait will terminate sooner or
> > later.
> > 
> > Signed-off-by: Martin Wilck <mwilck@suse.com>
> > ---
> >  multipathd/cli_handlers.c | 10 +----
> >  multipathd/main.c         | 92 +++++++++++++++++++++++++++++------
> > ----
> >  multipathd/main.h         |  3 +-
> >  3 files changed, 71 insertions(+), 34 deletions(-)
> > 
> > diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
> > index 6d3a0ae..44f76ee 100644
> > 

> >                 condlog(2, "reset log (signal)");
> > diff --git a/multipathd/main.h b/multipathd/main.h
> > index bc1f938..23ce919 100644
> > --- a/multipathd/main.h
> > +++ b/multipathd/main.h
> > @@ -37,6 +37,7 @@ void exit_daemon(void);
> >  const char * daemon_status(void);
> >  enum daemon_status wait_for_state_change_if(enum daemon_status
> > oldstate,
> >                                             unsigned long ms);
> > +void schedule_reconfigure(void);
> >  int need_to_delay_reconfig (struct vectors *);
> >  int reconfigure (struct vectors *);
> >  int ev_add_path (struct path *, struct vectors *, int);
> > @@ -44,7 +45,7 @@ int ev_remove_path (struct path *, struct vectors
> > *, int);
> >  int ev_add_map (char *, const char *, struct vectors *);
> >  int ev_remove_map (char *, char *, int, struct vectors *);
> >  int flush_map(struct multipath *, struct vectors *, int);
> > -int set_config_state(enum daemon_status);
> > +enum daemon_status set_config_state(enum daemon_status);
> 
> Can't we just remove set_config_state from main.h, and make it
> static?
> Other than that, everything looks fine.

Good point, will do.

Thanks, Martin



--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel