diff mbox series

[64/74] multipathd: check_path(): set retrigger_delay if necessary

Message ID 20200709105145.9211-12-mwilck@suse.com (mailing list archive)
State Not Applicable, archived
Delegated to: christophe varoqui
Headers show
Series multipath-tools series part V: removed path handling | expand

Commit Message

Martin Wilck July 9, 2020, 10:51 a.m. UTC
From: Martin Wilck <mwilck@suse.com>

In a follow up patch, we will set INIT_MISSING_UDEV and set tick=1
(minimal) at the same time. In this case, which is new, check_path()
must reset the delay when it first triggers an uevent.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipathd/main.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Benjamin Marzinski July 19, 2020, 5:07 a.m. UTC | #1
On Thu, Jul 09, 2020 at 12:51:35PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
> 
> In a follow up patch, we will set INIT_MISSING_UDEV and set tick=1
> (minimal) at the same time. In this case, which is new, check_path()
> must reset the delay when it first triggers an uevent.

Maybe I'm just being obtuse here, but I don't get what this does.
pp->tick will always be 0 for any path that makes it to the check

if (!pp->mpp && pp->initialized == INIT_MISSING_UDEV) {

And then if it's not out of retries, the path will get set to
INIT_REQUESTED_UDEV, and pathinfo() will take care of resetting pp->tick
when it gets called by the new uevent.

If it is out of retries, the path won't get pp->tick reset, which seems
wrong, but it that case it should probably be set to max_checkint, like
happens when the "add missing paths" code fails.

Or like I said, maybe I'm just missing something.
-Ben

> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  multipathd/main.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 7b2d320..0cd0ee6 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -2019,6 +2019,7 @@ check_path (struct vectors * vecs, struct path * pp, unsigned int ticks)
>  	int disable_reinstate = 0;
>  	int oldchkrstate = pp->chkrstate;
>  	int retrigger_tries, verbosity;
> +	unsigned int retrigger_delay;
>  	unsigned int checkint, max_checkint;
>  	struct config *conf;
>  	int marginal_pathgroups, marginal_changed = 0;
> @@ -2036,6 +2037,7 @@ check_path (struct vectors * vecs, struct path * pp, unsigned int ticks)
>  
>  	conf = get_multipath_config();
>  	retrigger_tries = conf->retrigger_tries;
> +	retrigger_delay = conf->retrigger_delay;
>  	checkint = conf->checkint;
>  	max_checkint = conf->max_checkint;
>  	verbosity = conf->verbosity;
> @@ -2048,6 +2050,8 @@ check_path (struct vectors * vecs, struct path * pp, unsigned int ticks)
>  	};
>  
>  	if (!pp->mpp && pp->initialized == INIT_MISSING_UDEV) {
> +		if (pp->tick != retrigger_delay)
> +			pp->tick = conf->retrigger_delay;
>  		if (pp->retriggers < retrigger_tries) {
>  			condlog(2, "%s: triggering change event to reinitialize",
>  				pp->dev);
> -- 
> 2.26.2

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Martin Wilck Aug. 5, 2020, 4:37 p.m. UTC | #2
On Sun, 2020-07-19 at 00:07 -0500, Benjamin Marzinski wrote:
> On Thu, Jul 09, 2020 at 12:51:35PM +0200, mwilck@suse.com wrote:
> > From: Martin Wilck <mwilck@suse.com>
> > 
> > In a follow up patch, we will set INIT_MISSING_UDEV and set tick=1
> > (minimal) at the same time. In this case, which is new,
> > check_path()
> > must reset the delay when it first triggers an uevent.
> 
> Maybe I'm just being obtuse here, but I don't get what this does.
> pp->tick will always be 0 for any path that makes it to the check
> 
> if (!pp->mpp && pp->initialized == INIT_MISSING_UDEV) {
> 
> And then if it's not out of retries, the path will get set to
> INIT_REQUESTED_UDEV, and pathinfo() will take care of resetting pp-
> >tick
> when it gets called by the new uevent.
> 
> If it is out of retries, the path won't get pp->tick reset, which
> seems
> wrong, but it that case it should probably be set to max_checkint,
> like
> happens when the "add missing paths" code fails.
> 
> Or like I said, maybe I'm just missing something.

You're not. This was just plain stupid.

Thanks
Martin


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
diff mbox series

Patch

diff --git a/multipathd/main.c b/multipathd/main.c
index 7b2d320..0cd0ee6 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -2019,6 +2019,7 @@  check_path (struct vectors * vecs, struct path * pp, unsigned int ticks)
 	int disable_reinstate = 0;
 	int oldchkrstate = pp->chkrstate;
 	int retrigger_tries, verbosity;
+	unsigned int retrigger_delay;
 	unsigned int checkint, max_checkint;
 	struct config *conf;
 	int marginal_pathgroups, marginal_changed = 0;
@@ -2036,6 +2037,7 @@  check_path (struct vectors * vecs, struct path * pp, unsigned int ticks)
 
 	conf = get_multipath_config();
 	retrigger_tries = conf->retrigger_tries;
+	retrigger_delay = conf->retrigger_delay;
 	checkint = conf->checkint;
 	max_checkint = conf->max_checkint;
 	verbosity = conf->verbosity;
@@ -2048,6 +2050,8 @@  check_path (struct vectors * vecs, struct path * pp, unsigned int ticks)
 	};
 
 	if (!pp->mpp && pp->initialized == INIT_MISSING_UDEV) {
+		if (pp->tick != retrigger_delay)
+			pp->tick = conf->retrigger_delay;
 		if (pp->retriggers < retrigger_tries) {
 			condlog(2, "%s: triggering change event to reinitialize",
 				pp->dev);