* [dm-devel] [PATCH 1/6] mpathpersist: Fix Register and Ignore with 0x00 SARK
2020-12-18 23:06 [dm-devel] [PATCH 0/6] More misc multipath patches Benjamin Marzinski
@ 2020-12-18 23:06 ` Benjamin Marzinski
2020-12-18 23:18 ` Martin Wilck
2020-12-18 23:06 ` [dm-devel] [PATCH 2/6] mpathpersist: update prkeys file on changing registrations Benjamin Marzinski
` (4 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Benjamin Marzinski @ 2020-12-18 23:06 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [dm-devel] [PATCH 1/6] mpathpersist: Fix Register and Ignore with 0x00 SARK
2020-12-18 23:06 ` [dm-devel] [PATCH 1/6] mpathpersist: Fix Register and Ignore with 0x00 SARK Benjamin Marzinski
@ 2020-12-18 23:18 ` Martin Wilck
0 siblings, 0 replies; 18+ messages in thread
From: Martin Wilck @ 2020-12-18 23:18 UTC (permalink / raw)
To: bmarzins, christophe.varoqui; +Cc: dm-devel
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* [dm-devel] [PATCH 2/6] mpathpersist: update prkeys file on changing registrations
2020-12-18 23:06 [dm-devel] [PATCH 0/6] More misc multipath patches Benjamin Marzinski
2020-12-18 23:06 ` [dm-devel] [PATCH 1/6] mpathpersist: Fix Register and Ignore with 0x00 SARK Benjamin Marzinski
@ 2020-12-18 23:06 ` Benjamin Marzinski
2020-12-18 23:23 ` Martin Wilck
2020-12-18 23:06 ` [dm-devel] [PATCH 3/6] libmultipath: warn about missing braces at end of multipath.conf Benjamin Marzinski
` (3 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Benjamin Marzinski @ 2020-12-18 23:06 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [dm-devel] [PATCH 2/6] mpathpersist: update prkeys file on changing registrations
2020-12-18 23:06 ` [dm-devel] [PATCH 2/6] mpathpersist: update prkeys file on changing registrations Benjamin Marzinski
@ 2020-12-18 23:23 ` Martin Wilck
0 siblings, 0 replies; 18+ messages in thread
From: Martin Wilck @ 2020-12-18 23:23 UTC (permalink / raw)
To: bmarzins, christophe.varoqui; +Cc: dm-devel
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* [dm-devel] [PATCH 3/6] libmultipath: warn about missing braces at end of multipath.conf
2020-12-18 23:06 [dm-devel] [PATCH 0/6] More misc multipath patches Benjamin Marzinski
2020-12-18 23:06 ` [dm-devel] [PATCH 1/6] mpathpersist: Fix Register and Ignore with 0x00 SARK Benjamin Marzinski
2020-12-18 23:06 ` [dm-devel] [PATCH 2/6] mpathpersist: update prkeys file on changing registrations Benjamin Marzinski
@ 2020-12-18 23:06 ` Benjamin Marzinski
2020-12-18 23:28 ` Martin Wilck
2020-12-18 23:06 ` [dm-devel] [PATCH 4/6] libmultipath: ignore multipaths sections without wwid option Benjamin Marzinski
` (2 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Benjamin Marzinski @ 2020-12-18 23:06 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* [dm-devel] [PATCH 4/6] libmultipath: ignore multipaths sections without wwid option
2020-12-18 23:06 [dm-devel] [PATCH 0/6] More misc multipath patches Benjamin Marzinski
` (2 preceding siblings ...)
2020-12-18 23:06 ` [dm-devel] [PATCH 3/6] libmultipath: warn about missing braces at end of multipath.conf Benjamin Marzinski
@ 2020-12-18 23:06 ` Benjamin Marzinski
2020-12-18 23:30 ` Martin Wilck
2020-12-18 23:06 ` [dm-devel] [PATCH 5/6] multipathd: Fix multipathd stopping on shutdown Benjamin Marzinski
2020-12-18 23:06 ` [dm-devel] [PATCH 6/6] multipath.conf.5: Improve checker_timeout description Benjamin Marzinski
5 siblings, 1 reply; 18+ messages in thread
From: Benjamin Marzinski @ 2020-12-18 23:06 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck
"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
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [dm-devel] [PATCH 4/6] libmultipath: ignore multipaths sections without wwid option
2020-12-18 23:06 ` [dm-devel] [PATCH 4/6] libmultipath: ignore multipaths sections without wwid option Benjamin Marzinski
@ 2020-12-18 23:30 ` Martin Wilck
2021-01-04 19:24 ` Benjamin Marzinski
0 siblings, 1 reply; 18+ messages in thread
From: Martin Wilck @ 2020-12-18 23:30 UTC (permalink / raw)
To: bmarzins, christophe.varoqui; +Cc: dm-devel
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
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [dm-devel] [PATCH 4/6] libmultipath: ignore multipaths sections without wwid option
2020-12-18 23:30 ` Martin Wilck
@ 2021-01-04 19:24 ` Benjamin Marzinski
0 siblings, 0 replies; 18+ messages in thread
From: Benjamin Marzinski @ 2021-01-04 19:24 UTC (permalink / raw)
To: Martin Wilck; +Cc: dm-devel
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* [dm-devel] [PATCH 5/6] multipathd: Fix multipathd stopping on shutdown
2020-12-18 23:06 [dm-devel] [PATCH 0/6] More misc multipath patches Benjamin Marzinski
` (3 preceding siblings ...)
2020-12-18 23:06 ` [dm-devel] [PATCH 4/6] libmultipath: ignore multipaths sections without wwid option Benjamin Marzinski
@ 2020-12-18 23:06 ` Benjamin Marzinski
2020-12-18 23:31 ` Martin Wilck
2020-12-18 23:06 ` [dm-devel] [PATCH 6/6] multipath.conf.5: Improve checker_timeout description Benjamin Marzinski
5 siblings, 1 reply; 18+ messages in thread
From: Benjamin Marzinski @ 2020-12-18 23:06 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [dm-devel] [PATCH 5/6] multipathd: Fix multipathd stopping on shutdown
2020-12-18 23:06 ` [dm-devel] [PATCH 5/6] multipathd: Fix multipathd stopping on shutdown Benjamin Marzinski
@ 2020-12-18 23:31 ` Martin Wilck
2021-01-27 1:16 ` Benjamin Marzinski
0 siblings, 1 reply; 18+ messages in thread
From: Martin Wilck @ 2020-12-18 23:31 UTC (permalink / raw)
To: bmarzins, christophe.varoqui; +Cc: dm-devel
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [dm-devel] [PATCH 5/6] multipathd: Fix multipathd stopping on shutdown
2020-12-18 23:31 ` Martin Wilck
@ 2021-01-27 1:16 ` Benjamin Marzinski
2021-01-27 9:41 ` Martin Wilck
0 siblings, 1 reply; 18+ messages in thread
From: Benjamin Marzinski @ 2021-01-27 1:16 UTC (permalink / raw)
To: Martin Wilck; +Cc: dm-devel
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* [dm-devel] [PATCH 6/6] multipath.conf.5: Improve checker_timeout description
2020-12-18 23:06 [dm-devel] [PATCH 0/6] More misc multipath patches Benjamin Marzinski
` (4 preceding siblings ...)
2020-12-18 23:06 ` [dm-devel] [PATCH 5/6] multipathd: Fix multipathd stopping on shutdown Benjamin Marzinski
@ 2020-12-18 23:06 ` Benjamin Marzinski
2020-12-18 23:56 ` Martin Wilck
5 siblings, 1 reply; 18+ messages in thread
From: Benjamin Marzinski @ 2020-12-18 23:06 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [dm-devel] [PATCH 6/6] multipath.conf.5: Improve checker_timeout description
2020-12-18 23:06 ` [dm-devel] [PATCH 6/6] multipath.conf.5: Improve checker_timeout description Benjamin Marzinski
@ 2020-12-18 23:56 ` Martin Wilck
2021-01-05 0:33 ` Benjamin Marzinski
0 siblings, 1 reply; 18+ messages in thread
From: Martin Wilck @ 2020-12-18 23:56 UTC (permalink / raw)
To: bmarzins, christophe.varoqui; +Cc: dm-devel
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [dm-devel] [PATCH 6/6] multipath.conf.5: Improve checker_timeout description
2020-12-18 23:56 ` Martin Wilck
@ 2021-01-05 0:33 ` Benjamin Marzinski
2021-01-05 10:55 ` Martin Wilck
0 siblings, 1 reply; 18+ messages in thread
From: Benjamin Marzinski @ 2021-01-05 0:33 UTC (permalink / raw)
To: Martin Wilck; +Cc: dm-devel
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [dm-devel] [PATCH 6/6] multipath.conf.5: Improve checker_timeout description
2021-01-05 0:33 ` Benjamin Marzinski
@ 2021-01-05 10:55 ` Martin Wilck
0 siblings, 0 replies; 18+ messages in thread
From: Martin Wilck @ 2021-01-05 10:55 UTC (permalink / raw)
To: bmarzins; +Cc: dm-devel
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
^ permalink raw reply [flat|nested] 18+ messages in thread