[dm-devel] [PATCH 0/6] More misc multipath patches

[dm-devel] [PATCH 0/6] More misc multipath patches

[Benjamin Marzinski]

The first two patches fix issues with mpathpersist. The second two deal
with minor configuration file issues. The fifth patch deals with a
problem with multipathd.service the could cause multipathd to not get
stopped until after final.target was reached, causing shutdown errors.
The last patch is just a doc clarification, inspired by customer
questions.

Benjamin Marzinski (6):
  mpathpersist: Fix Register and Ignore with 0x00 SARK
  mpathpersist: update prkeys file on changing registrations
  libmultipath: warn about missing braces at end of multipath.conf
  libmultipath: ignore multipaths sections without wwid option
  multipathd: Fix multipathd stopping on shutdown
  multipath.conf.5: Improve checker_timeout description

 libmpathpersist/mpath_persist.c | 10 ++++++----
 libmultipath/config.c           |  7 +++++++
 libmultipath/parser.c           |  5 +++--
 multipath/multipath.conf.5      |  9 +++++++--
 multipathd/multipathd.service   |  2 +-
 5 files changed, 24 insertions(+), 9 deletions(-)

-- 
2.17.2

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

[dm-devel] [PATCH 1/6] mpathpersist: Fix Register and Ignore with 0x00 SARK

[Benjamin Marzinski]

When the Register and Ignore command is run with sg_persist, if a 0x00
Service Action Reservation Key is given or the --param-sark option is
not used at all, sg_persist will clear the registration.  mpathpersist
will fail with an error.  This patch fixes mpathpersist to work like
sg_persist in this case.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmpathpersist/mpath_persist.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
index 79322e86..41789c46 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -304,7 +304,8 @@ int __mpath_persistent_reserve_out ( int fd, int rq_servact, int rq_scope,
 	}
 
 	if (memcmp(paramp->key, &mpp->reservation_key, 8) &&
-	    memcmp(paramp->sa_key, &mpp->reservation_key, 8)) {
+	    memcmp(paramp->sa_key, &mpp->reservation_key, 8) &&
+	    (prkey || rq_servact != MPATH_PROUT_REG_IGN_SA)) {
 		condlog(0, "%s: configured reservation key doesn't match: 0x%" PRIx64, alias, get_be64(mpp->reservation_key));
 		ret = MPATH_PR_SYNTAX_ERROR;
 		goto out1;
-- 
2.17.2

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

[dm-devel] [PATCH 2/6] mpathpersist: update prkeys file on changing registrations

[Benjamin Marzinski]

When the "reservation_key" option is set to "file" and Register command
is run with both the current Reservation Key and a new Service Action
Reservation Key, mpathpersist will change the registration, but will not
update the prkeys file. This means that future paths that come online
will not be able to register, since multipathd is still using the old
reservation key. Fix this.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmpathpersist/mpath_persist.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
index 41789c46..08077936 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -290,9 +290,10 @@ int __mpath_persistent_reserve_out ( int fd, int rq_servact, int rq_scope,
 
 	memcpy(&prkey, paramp->sa_key, 8);
 	if (mpp->prkey_source == PRKEY_SOURCE_FILE && prkey &&
-	    ((!get_be64(mpp->reservation_key) &&
-	      rq_servact == MPATH_PROUT_REG_SA) ||
-	     rq_servact == MPATH_PROUT_REG_IGN_SA)) {
+	    (rq_servact == MPATH_PROUT_REG_IGN_SA ||
+	     (rq_servact == MPATH_PROUT_REG_SA &&
+	      (!get_be64(mpp->reservation_key) ||
+	       memcmp(paramp->key, &mpp->reservation_key, 8) == 0)))) {
 		memcpy(&mpp->reservation_key, paramp->sa_key, 8);
 		if (update_prkey_flags(alias, get_be64(mpp->reservation_key),
 				       paramp->sa_flags)) {
-- 
2.17.2

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

[dm-devel] [PATCH 3/6] libmultipath: warn about missing braces at end of multipath.conf

[Benjamin Marzinski]

Multipath doesn't warn when multipath.conf is missing closing braces at
the end of the file. This has confused people about the correct config
file syntax, so add a warning.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/parser.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libmultipath/parser.c b/libmultipath/parser.c
index 163ffbc9..c70243c3 100644
--- a/libmultipath/parser.c
+++ b/libmultipath/parser.c
@@ -537,7 +537,7 @@ process_stream(struct config *conf, FILE *stream, vector keywords,
 		if (!strcmp(str, EOB)) {
 			if (kw_level > 0) {
 				free_strvec(strvec);
-				break;
+				goto out;
 			}
 			condlog(0, "unmatched '%s' at line %d of %s",
 				EOB, line_nr, file);
@@ -576,7 +576,8 @@ process_stream(struct config *conf, FILE *stream, vector keywords,
 
 		free_strvec(strvec);
 	}
-
+	if (kw_level == 1)
+		condlog(1, "missing '%s' at end of %s", EOB, file);
 out:
 	FREE(buf);
 	free_uniques(uniques);
-- 
2.17.2

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

[dm-devel] [PATCH 4/6] libmultipath: ignore multipaths sections without wwid option

[Benjamin Marzinski]

"multipathd show config local" was crashing in find_mp_by_wwid() if
the multipath configuration included a multipaths section that did
not set a wwid option. There is no reason to keep a mpentry that
didn't set its wwid. Remove it in merge_mptable().

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/config.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/libmultipath/config.c b/libmultipath/config.c
index 9f3cb38d..a643703e 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -509,6 +509,13 @@ void merge_mptable(vector mptable)
 	int i, j;
 
 	vector_foreach_slot(mptable, mp1, i) {
+		/* drop invalid multipath configs */
+		if (!mp1->wwid) {
+			condlog(0, "multipaths config section missing wwid");
+			vector_del_slot(mptable, i--);
+			free_mpe(mp1);
+			continue;
+		}
 		j = i + 1;
 		vector_foreach_slot_after(mptable, mp2, j) {
 			if (strcmp(mp1->wwid, mp2->wwid))
-- 
2.17.2

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

[dm-devel] [PATCH 5/6] multipathd: Fix multipathd stopping on shutdown

[Benjamin Marzinski]

According to man "systemd.special"

"shutdown.target: ... Services that shall be terminated on system
shutdown shall add Conflicts= and Before= dependencies to this unit for
their service unit, which is implicitly done when
DefaultDependencies=yes is set (the default)."

multipathd.service sets DefaultDependencies=no and includes the
Conflits= dependency, but not the Before= one. This can cause multipathd
to continue running past when it is supposed to during shutdown.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 multipathd/multipathd.service | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/multipathd/multipathd.service b/multipathd/multipathd.service
index ba24983e..7d547fa7 100644
--- a/multipathd/multipathd.service
+++ b/multipathd/multipathd.service
@@ -2,7 +2,7 @@
 Description=Device-Mapper Multipath Device Controller
 Wants=systemd-udev-trigger.service systemd-udev-settle.service
 Before=iscsi.service iscsid.service lvm2-activation-early.service
-Before=local-fs-pre.target blk-availability.service
+Before=local-fs-pre.target blk-availability.service shutdown.target
 After=multipathd.socket systemd-udev-trigger.service systemd-udev-settle.service
 DefaultDependencies=no
 Conflicts=shutdown.target
-- 
2.17.2

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

[dm-devel] [PATCH 6/6] multipath.conf.5: Improve checker_timeout description

[Benjamin Marzinski]

I was asked to explain how checker_timeout works for checkers like
directio, that don't issue scsi commands with an explicit timeout

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 multipath/multipath.conf.5 | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index ea66a01e..c7c4184b 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -639,8 +639,13 @@ The default is: \fBno\fR
 .
 .TP
 .B checker_timeout
-Specify the timeout to use for path checkers and prioritizers that issue SCSI
-commands with an explicit timeout, in seconds.
+Specify the timeout to use for path checkers and prioritizers, in seconds.
+Only prioritizers that issue scsi commands use checker_timeout. Checkers
+that support an asynchronous mode (\fItur\fR and \fIdirectio\fR), will
+return shortly after being called by multipathd, regardless of whether the
+storage array responds. If the storage array hasn't responded, mulitpathd will
+check for a response every second, until \fIchecker_timeout\fR seconds have
+elapsed.
 .RS
 .TP
 The default is: in \fB/sys/block/sd<x>/device/timeout\fR
-- 
2.17.2

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

Re: [dm-devel] [PATCH 1/6] mpathpersist: Fix Register and Ignore with 0x00 SARK

[Martin Wilck]

On Fri, 2020-12-18 at 17:06 -0600, Benjamin Marzinski wrote:
> When the Register and Ignore command is run with sg_persist, if a
> 0x00
> Service Action Reservation Key is given or the --param-sark option is
> not used at all, sg_persist will clear the
> registration.  mpathpersist
> will fail with an error.  This patch fixes mpathpersist to work like
> sg_persist in this case.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Reviewed-by: Martin Wilck <mwilck@suse.com>

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE  Software Solutions Germany GmbH
HRB 36809, AG Nürnberg GF: Felix
Imendörffer



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

Re: [dm-devel] [PATCH 2/6] mpathpersist: update prkeys file on changing registrations

[Martin Wilck]

On Fri, 2020-12-18 at 17:06 -0600, Benjamin Marzinski wrote:
> When the "reservation_key" option is set to "file" and Register
> command
> is run with both the current Reservation Key and a new Service Action
> Reservation Key, mpathpersist will change the registration, but will
> not
> update the prkeys file. This means that future paths that come online
> will not be able to register, since multipathd is still using the old
> reservation key. Fix this.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Reviewed-by: Martin Wilck <mwilck@suse.com>

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE  Software Solutions Germany GmbH
HRB 36809, AG Nürnberg GF: Felix
Imendörffer



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

Re: [dm-devel] [PATCH 3/6] libmultipath: warn about missing braces at end of multipath.conf

[Martin Wilck]

On Fri, 2020-12-18 at 17:06 -0600, Benjamin Marzinski wrote:
> Multipath doesn't warn when multipath.conf is missing closing braces
> at
> the end of the file. This has confused people about the correct
> config
> file syntax, so add a warning.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Reviewed-by: Martin Wilck <mwilck@suse.com>

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE  Software Solutions Germany GmbH
HRB 36809, AG Nürnberg GF: Felix
Imendörffer



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

Re: [dm-devel] [PATCH 4/6] libmultipath: ignore multipaths sections without wwid option

[Martin Wilck]

On Fri, 2020-12-18 at 17:06 -0600, Benjamin Marzinski wrote:
> "multipathd show config local" was crashing in find_mp_by_wwid() if
> the multipath configuration included a multipaths section that did
> not set a wwid option. There is no reason to keep a mpentry that
> didn't set its wwid. Remove it in merge_mptable().
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmultipath/config.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/libmultipath/config.c b/libmultipath/config.c
> index 9f3cb38d..a643703e 100644
> --- a/libmultipath/config.c
> +++ b/libmultipath/config.c
> @@ -509,6 +509,13 @@ void merge_mptable(vector mptable)
>  	int i, j;
>  
>  	vector_foreach_slot(mptable, mp1, i) {
> +		/* drop invalid multipath configs */
> +		if (!mp1->wwid) {
> +			condlog(0, "multipaths config section missing
> wwid");
> +			vector_del_slot(mptable, i--);
> +			free_mpe(mp1);
> +			continue;
> +		}
>  		j = i + 1;
>  		vector_foreach_slot_after(mptable, mp2, j) {
>  			if (strcmp(mp1->wwid, mp2->wwid))

Wouldn't you need to check mp2->wwid, too?

Martin

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE  Software Solutions Germany GmbH
HRB 36809, AG Nürnberg GF: Felix
Imendörffer



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

Re: [dm-devel] [PATCH 5/6] multipathd: Fix multipathd stopping on shutdown

[Martin Wilck]

On Fri, 2020-12-18 at 17:06 -0600, Benjamin Marzinski wrote:
> According to man "systemd.special"
> 
> "shutdown.target: ... Services that shall be terminated on system
> shutdown shall add Conflicts= and Before= dependencies to this unit
> for
> their service unit, which is implicitly done when
> DefaultDependencies=yes is set (the default)."
> 
> multipathd.service sets DefaultDependencies=no and includes the
> Conflits= dependency, but not the Before= one. This can cause
> multipathd
> to continue running past when it is supposed to during shutdown.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Reviewed-by: Martin Wilck <mwilck@suse.com>

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE  Software Solutions Germany GmbH
HRB 36809, AG Nürnberg GF: Felix
Imendörffer



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

Re: [dm-devel] [PATCH 6/6] multipath.conf.5: Improve checker_timeout description

[Martin Wilck]

On Fri, 2020-12-18 at 17:06 -0600, Benjamin Marzinski wrote:
> I was asked to explain how checker_timeout works for checkers like
> directio, that don't issue scsi commands with an explicit timeout
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  multipath/multipath.conf.5 | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
> index ea66a01e..c7c4184b 100644
> --- a/multipath/multipath.conf.5
> +++ b/multipath/multipath.conf.5
> @@ -639,8 +639,13 @@ The default is: \fBno\fR
>  .
>  .TP
>  .B checker_timeout
> -Specify the timeout to use for path checkers and prioritizers that
> issue SCSI
> -commands with an explicit timeout, in seconds.
> +Specify the timeout to use for path checkers and prioritizers, in
> seconds.
> +Only prioritizers that issue scsi commands use checker_timeout. 
> Checkers
> +that support an asynchronous mode (\fItur\fR and \fIdirectio\fR),
> will
> +return shortly after being called by multipathd, regardless of
> whether the
> +storage array responds. If the storage array hasn't responded,
> mulitpathd will

typo

> +check for a response every second, until \fIchecker_timeout\fR
> seconds have
> +elapsed.

This is a bit confusing IMHO. Most importantly, checkers will consider
a path down if it doesn't respond to the checker command after the
given timeout. The async behavior doesn't fit too well into this
section. Maybe we should mention and explain the async property in the
path_checkers section, and only refer to that here.

(Btw is the directio checker still deprecated after all the work you
recently put into it? The man page still says so).

Regards,
Martin

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE  Software Solutions Germany GmbH
HRB 36809, AG Nürnberg GF: Felix
Imendörffer



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

Re: [dm-devel] [PATCH 4/6] libmultipath: ignore multipaths sections without wwid option

[Benjamin Marzinski]

On Fri, Dec 18, 2020 at 11:30:41PM +0000, Martin Wilck wrote:
> On Fri, 2020-12-18 at 17:06 -0600, Benjamin Marzinski wrote:
> > "multipathd show config local" was crashing in find_mp_by_wwid() if
> > the multipath configuration included a multipaths section that did
> > not set a wwid option. There is no reason to keep a mpentry that
> > didn't set its wwid. Remove it in merge_mptable().
> > 
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > ---
> >  libmultipath/config.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/libmultipath/config.c b/libmultipath/config.c
> > index 9f3cb38d..a643703e 100644
> > --- a/libmultipath/config.c
> > +++ b/libmultipath/config.c
> > @@ -509,6 +509,13 @@ void merge_mptable(vector mptable)
> >  	int i, j;
> >  
> >  	vector_foreach_slot(mptable, mp1, i) {
> > +		/* drop invalid multipath configs */
> > +		if (!mp1->wwid) {
> > +			condlog(0, "multipaths config section missing
> > wwid");
> > +			vector_del_slot(mptable, i--);
> > +			free_mpe(mp1);
> > +			continue;
> > +		}
> >  		j = i + 1;
> >  		vector_foreach_slot_after(mptable, mp2, j) {
> >  			if (strcmp(mp1->wwid, mp2->wwid))
> 
> Wouldn't you need to check mp2->wwid, too?

Oops. Yeah, strcmp() is undefined with NULL, so that needs a check.
BTW, I noticed that added this one to you upstream-queue branch, even
though you didn't ack it, while not adding patch 5/6 (multipathd: Fix
multipathd stopping on shutdown), even though you did ack it. Was this
intentional?

At any rate, since this is already in upstream-queue, I'll just post a
follow-up that checks mp2->wwid, before calling strcmp()

-Ben
 
> Martin
> 
> -- 
> Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
> SUSE  Software Solutions Germany GmbH
> HRB 36809, AG Nürnberg GF: Felix
> Imendörffer
> 

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

Re: [dm-devel] [PATCH 6/6] multipath.conf.5: Improve checker_timeout description

[Benjamin Marzinski]

On Fri, Dec 18, 2020 at 11:56:47PM +0000, Martin Wilck wrote:
> On Fri, 2020-12-18 at 17:06 -0600, Benjamin Marzinski wrote:
> > I was asked to explain how checker_timeout works for checkers like
> > directio, that don't issue scsi commands with an explicit timeout
> > 
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > ---
> >  multipath/multipath.conf.5 | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
> > index ea66a01e..c7c4184b 100644
> > --- a/multipath/multipath.conf.5
> > +++ b/multipath/multipath.conf.5
> > @@ -639,8 +639,13 @@ The default is: \fBno\fR
> >  .
> >  .TP
> >  .B checker_timeout
> > -Specify the timeout to use for path checkers and prioritizers that
> > issue SCSI
> > -commands with an explicit timeout, in seconds.
> > +Specify the timeout to use for path checkers and prioritizers, in
> > seconds.
> > +Only prioritizers that issue scsi commands use checker_timeout. 
> > Checkers
> > +that support an asynchronous mode (\fItur\fR and \fIdirectio\fR),
> > will
> > +return shortly after being called by multipathd, regardless of
> > whether the
> > +storage array responds. If the storage array hasn't responded,
> > mulitpathd will
> 
> typo
> 
> > +check for a response every second, until \fIchecker_timeout\fR
> > seconds have
> > +elapsed.
> 
> This is a bit confusing IMHO. Most importantly, checkers will consider
> a path down if it doesn't respond to the checker command after the
> given timeout. The async behavior doesn't fit too well into this
> section. Maybe we should mention and explain the async property in the
> path_checkers section, and only refer to that here.

Sounds reasonable.

> (Btw is the directio checker still deprecated after all the work you
> recently put into it? The man page still says so).

No. I'll change that.  There are times when devices claim to be ready
with the tur checker, when in truth, IO to them will fail.  In these
cases, the directio checker is the best way to avoid having paths
bouncing up and down.

-Ben

> 
> Regards,
> Martin
> 
> -- 
> Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
> SUSE  Software Solutions Germany GmbH
> HRB 36809, AG Nürnberg GF: Felix
> Imendörffer
> 

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

Re: [dm-devel] [PATCH 6/6] multipath.conf.5: Improve checker_timeout description

[Martin Wilck]

On Mon, 2021-01-04 at 18:33 -0600, Benjamin Marzinski wrote:
> On Fri, Dec 18, 2020 at 11:56:47PM +0000, Martin Wilck wrote:
> > On Fri, 2020-12-18 at 17:06 -0600, Benjamin Marzinski wrote:
> > > I was asked to explain how checker_timeout works for checkers
> > > like
> > > directio, that don't issue scsi commands with an explicit timeout
> > > 
> > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > > ---
> > >  multipath/multipath.conf.5 | 9 +++++++--
> > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/multipath/multipath.conf.5
> > > b/multipath/multipath.conf.5
> > > index ea66a01e..c7c4184b 100644
> > > --- a/multipath/multipath.conf.5
> > > +++ b/multipath/multipath.conf.5
> > > @@ -639,8 +639,13 @@ The default is: \fBno\fR
> > >  .
> > >  .TP
> > >  .B checker_timeout
> > > -Specify the timeout to use for path checkers and prioritizers
> > > that
> > > issue SCSI
> > > -commands with an explicit timeout, in seconds.
> > > +Specify the timeout to use for path checkers and prioritizers,
> > > in
> > > seconds.
> > > +Only prioritizers that issue scsi commands use checker_timeout. 
> > > Checkers
> > > +that support an asynchronous mode (\fItur\fR and
> > > \fIdirectio\fR),
> > > will
> > > +return shortly after being called by multipathd, regardless of
> > > whether the
> > > +storage array responds. If the storage array hasn't responded,
> > > mulitpathd will
> > 
> > typo
> > 
> > > +check for a response every second, until \fIchecker_timeout\fR
> > > seconds have
> > > +elapsed.
> > 
> > This is a bit confusing IMHO. Most importantly, checkers will
> > consider
> > a path down if it doesn't respond to the checker command after the
> > given timeout. The async behavior doesn't fit too well into this
> > section. Maybe we should mention and explain the async property in
> > the
> > path_checkers section, and only refer to that here.
> 
> Sounds reasonable.
> 
> > (Btw is the directio checker still deprecated after all the work
> > you
> > recently put into it? The man page still says so).
> 
> No. I'll change that.  There are times when devices claim to be ready
> with the tur checker, when in truth, IO to them will fail.  In these
> cases, the directio checker is the best way to avoid having paths
> bouncing up and down.

Right. I recently had one such case with persistent reservations. SPC-4
mandates that the status of TUR commands is independent of PR status
(TUR is always "allowed"), while obviously ordinary IO would fail if
the active PR exclude the current host. That basically makes the TUR
checker inappropriate as soon as PRs on SPC-4 compliant devices are in
use.

As we have support for PR already, I wonder if we could/should extend
the TUR checker to take this into account.

Cheers,
Martin

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Software Solutions Germany GmbH
HRB 36809, AG Nürnberg GF: Felix Imendörffer



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

Re: [dm-devel] [PATCH 5/6] multipathd: Fix multipathd stopping on shutdown

[Benjamin Marzinski]

On Fri, Dec 18, 2020 at 11:31:25PM +0000, Martin Wilck wrote:
> On Fri, 2020-12-18 at 17:06 -0600, Benjamin Marzinski wrote:
> > According to man "systemd.special"
> > 
> > "shutdown.target: ... Services that shall be terminated on system
> > shutdown shall add Conflicts= and Before= dependencies to this unit
> > for
> > their service unit, which is implicitly done when
> > DefaultDependencies=yes is set (the default)."
> > 
> > multipathd.service sets DefaultDependencies=no and includes the
> > Conflits= dependency, but not the Before= one. This can cause
> > multipathd
> > to continue running past when it is supposed to during shutdown.
> > 
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> 
> Reviewed-by: Martin Wilck <mwilck@suse.com>

Martin, I noticed that this commit didn't make it into your queue
branch. I assume it's just an oversight. Right?

-Ben

> 
> -- 
> Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
> SUSE  Software Solutions Germany GmbH
> HRB 36809, AG Nürnberg GF: Felix
> Imendörffer
> 

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

Re: [dm-devel] [PATCH 5/6] multipathd: Fix multipathd stopping on shutdown

[Martin Wilck]

On Tue, 2021-01-26 at 19:16 -0600, Benjamin Marzinski wrote:
> 
> Martin, I noticed that this commit didn't make it into your queue
> branch. I assume it's just an oversight. Right?

Yes, I'm sorry. Fixed now.


Cheers,
Martin

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Software Solutions Germany GmbH
HRB 36809, AG Nürnberg GF: Felix Imendörffer



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