On Fri, 2023-07-28 at 14:05 -0500, Benjamin Marzinski wrote: > by the time get_state() is rechecking the sysfs timeout value, > c->timeout has already been set. The only reason why this check > exists > is to deal with the possiblity that the sysfs value has changed. If > the > sysfs value doesn't exist (which likely means that the device is not > a > scsi device), then there's no reason to reset the default value, > since > that can't have changed. > > Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx> Reviewed-by: Martin Wilck <mwilck@xxxxxxxx> Thinking about it, I am not sure if it's wise to re-read the timeout from sysfs every time we retrieve a path state. It's inefficient, and I wonder if we even want to do this if someone modifies SCSI timeouts in sysfs while multipathd is running. It would be cleaner to set checker_timeout and reload multipathd. But that's no reason to reject this patch. > --- > libmultipath/discovery.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c > index 5626d48d..2b1a11d5 100644 > --- a/libmultipath/discovery.c > +++ b/libmultipath/discovery.c > @@ -1965,9 +1965,8 @@ get_state (struct path * pp, struct config > *conf, int daemon, int oldstate) > checker_set_async(c); > else > checker_set_sync(c); > - if (!conf->checker_timeout && > - sysfs_get_timeout(pp, &(c->timeout)) <= 0) > - c->timeout = DEF_TIMEOUT; > + if (!conf->checker_timeout) > + sysfs_get_timeout(pp, &(c->timeout)); > state = checker_check(c, oldstate); > condlog(3, "%s: %s state = %s", pp->dev, > checker_name(c), checker_state_name(state)); -- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel
