[PATCH 0/7] multipath-tools man pages: add missing information

[PATCH 0/7] multipath-tools man pages: add missing information

[Benjamin Marzinski]

This patchset is inspired by:
https://lore.kernel.org/dm-devel/1673555329-9850-1-git-send-email-bmarzins@redhat.com/

It removes the useless pathgroup wildcards section from the "show
wildcards" output and adds wildcards descriptions to the multipathd.8
man page, along with some issing multipathd commands.  In the process of
doing this, I noticed some instances where either the man page or
multipathd wasn't giving the correct information, and fixed those up.

Benjamin Marzinski (6):
  multipath-tools man pages: don't assume multipath.socket is enabled
  libmultipath: print all values in snprint_failback
  multipathd: Stop double counting map failures for no_path_retry > 0
  multipath-tools man pages: add missing multipathd commands
  libmultipath: change the vend/prod/rev printing
  multipath-tools man pages: Add format wildcard descriptions

Nitin Yewale (1):
  libmultipath: remove pathgroup wildcard options

 libmultipath/print.c       |  22 ++--
 libmultipath/structs_vec.c |   5 +-
 multipathd/multipathd.8.in | 260 +++++++++++++++++++++++++++++++++++--
 3 files changed, 263 insertions(+), 24 deletions(-)

-- 
2.45.0

[PATCH 1/7] libmultipath: remove pathgroup wildcard options

[Benjamin Marzinski]

From: Nitin Yewale <nyewale@redhat.com>

The multipathd command "multipathd show wildcards" shows the pathgroup
format wildcards, but there is no way to use them in a multipathd
command.

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

diff --git a/libmultipath/print.c b/libmultipath/print.c
index efd5a16a..89813517 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -912,13 +912,6 @@ int snprint_wildcards(struct strbuf *buff)
 				       pd[i].wildcard, pd[i].header)) < 0)
 			return rc;
 
-	if ((rc = append_strbuf_str(buff, "\npathgroup format wildcards:\n")) < 0)
-		return rc;
-	for (i = 0; i < ARRAY_SIZE(pgd); i++)
-		if ((rc = print_strbuf(buff, "%%%c  %s\n",
-				       pgd[i].wildcard, pgd[i].header)) < 0)
-			return rc;
-
 	return get_strbuf_len(buff) - initial_len;
 }
 
-- 
2.45.0

[PATCH 2/7] multipath-tools man pages: don't assume multipath.socket is enabled

[Benjamin Marzinski]

Add "If enabled, " to the sentence about multipathd.socket, since it is
no longer enabled by default.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 multipathd/multipathd.8.in | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/multipathd/multipathd.8.in b/multipathd/multipathd.8.in
index 12b77156..06d91c06 100644
--- a/multipathd/multipathd.8.in
+++ b/multipathd/multipathd.8.in
@@ -339,12 +339,13 @@ Stop multipathd.
 .\" ----------------------------------------------------------------------------
 .
 When compiled with systemd support two systemd service files are installed,
-\fImultipathd.service\fR and \fImultipathd.socket\fR The \fImultipathd.socket\fR
-service instructs systemd to intercept the CLI command socket, so that any call
-to the CLI interface will start-up the daemon if required.
-The \fImultipathd.service\fR file carries the definitions for controlling the
-multipath daemon. The daemon itself uses the \fBsd_notify\fR(3) interface to
-communicate with systemd. The following unit keywords are recognized:
+\fImultipathd.service\fR and \fImultipathd.socket\fR. If enabled, the
+\fImultipathd.socket\fR service instructs systemd to intercept the CLI command
+socket, so that any call to the CLI interface will start-up the daemon if
+required. The \fImultipathd.service\fR file carries the definitions for
+controlling the multipath daemon. The daemon itself uses the \fBsd_notify\fR(3)
+interface to communicate with systemd. The following unit keywords are
+recognized:
 .
 .TP
 .B WatchdogSec=
-- 
2.45.0

[PATCH 3/7] libmultipath: print all values in snprint_failback

[Benjamin Marzinski]

Add the missing output for manual failback and print the defferral time
for deferred failbacks, if one isn't currently in progress.

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

diff --git a/libmultipath/print.c b/libmultipath/print.c
index 89813517..14e7e325 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -218,9 +218,13 @@ snprint_failback (struct strbuf *buff, const struct multipath * mpp)
 		return append_strbuf_str(buff, "immediate");
 	if (mpp->pgfailback == -FAILBACK_FOLLOWOVER)
 		return append_strbuf_str(buff, "followover");
+	if (mpp->pgfailback == -FAILBACK_MANUAL)
+		return append_strbuf_str(buff, "manual");
+	if (mpp->pgfailback == FAILBACK_UNDEF)
+		return append_strbuf_str(buff, "undef");
 
 	if (!mpp->failback_tick)
-		return append_strbuf_str(buff, "-");
+		return print_strbuf(buff, "%i", mpp->pgfailback);
 	else
 		return snprint_progress(buff, mpp->failback_tick,
 					mpp->pgfailback);
-- 
2.45.0

[PATCH 4/7] multipathd: Stop double counting map failures for no_path_retry > 0

[Benjamin Marzinski]

If no_path_retry was greater than 0, multipathd was counting a map
failure when recovery mode was entered, and again when queueing was
disabled. The first one is incorrect, since the map is still queueing.

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

diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index c0c5cc90..ccc4efc7 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -844,10 +844,13 @@ int verify_paths(struct multipath *mpp)
 void update_queue_mode_del_path(struct multipath *mpp)
 {
 	int active = count_active_paths(mpp);
+	bool is_queueing = mpp->features &&
+			   strstr(mpp->features, "queue_if_no_path");
 
 	if (active == 0) {
 		enter_recovery_mode(mpp);
-		if (mpp->no_path_retry != NO_PATH_RETRY_QUEUE)
+		if (mpp->no_path_retry == NO_PATH_RETRY_FAIL ||
+		    (mpp->no_path_retry == NO_PATH_RETRY_UNDEF && !is_queueing))
 			mpp->stat_map_failures++;
 	}
 	condlog(2, "%s: remaining active paths: %d", mpp->alias, active);
-- 
2.45.0

[PATCH 5/7] multipath-tools man pages: add missing multipathd commands

[Benjamin Marzinski]

Also, the description for "del map $map" was incorrect.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 multipathd/multipathd.8.in | 42 +++++++++++++++++++++++++++++++++-----
 1 file changed, 37 insertions(+), 5 deletions(-)

diff --git a/multipathd/multipathd.8.in b/multipathd/multipathd.8.in
index 06d91c06..32976052 100644
--- a/multipathd/multipathd.8.in
+++ b/multipathd/multipathd.8.in
@@ -114,18 +114,24 @@ The following commands can be used in interactive mode:
 Show the paths that multipathd is monitoring, and their state.
 .
 .TP
-.B list|show paths format $format
+.B list|show paths [raw] format $format
 Show the paths that multipathd is monitoring, using a format string with path
-format wildcards.
+format wildcards. Adding \fIraw\fR will remove the headers and alignment
+padding from the ouput.
+.
+.TP
+.B list|show path $path
+Show whether path $path is offline or running.
 .
 .TP
 .B list|show maps|multipaths
 Show the multipath devices that the multipathd is monitoring.
 .
 .TP
-.B list|show maps|multipaths format $format
+.B list|show maps|multipaths [raw] format $format
 Show the status of all multipath devices that the multipathd is monitoring,
-using a format string with multipath format wildcards.
+using a format string with multipath format wildcards. Adding \fIraw\fR will
+remove the headers and alignment padding from the output.
 .
 .TP
 .B list|show maps|multipaths status
@@ -138,6 +144,10 @@ Show some statistics of all multipath devices that the multipathd is monitoring.
 .TP
 .B list|show maps|multipaths topology
 Show the current multipath topology. Same as '\fImultipath \-ll\fR'.
+.TP
+.
+.B list|show maps|multipaths json
+Show information about all multipath devices in JSON format.
 .
 .TP
 .B list|show topology
@@ -149,6 +159,16 @@ Show topology of a single multipath device specified by $map, for example
 36005076303ffc56200000000000010aa. This map could be obtained from '\fIlist maps\fR'.
 .
 .TP
+.B list|show map|multipath $map [raw] format $format.
+Show the status of multipath device $map, using a format string with multipath
+format wildcards. Adding \fIraw\fR will remove the headers and alignment
+padding from the output.
+.
+.TP
+.B list|show map|multipath $map json
+Show information about multipath device $map in JSON format.
+.
+.TP
 .B list|show wildcards
 Show the format wildcards used in interactive commands taking $format.
 .
@@ -182,6 +202,14 @@ paths, and whether multipathd is currently handling a uevent.
 Show the current state of the multipathd daemon.
 .
 .TP
+.B reset maps|multipaths stats
+Reset the statistics of all multipath devices.
+.
+.TP
+.B reset map|multipath $map stats
+Reset the statistics of multipath device $map.
+.
+.TP
 .B add path $path
 Add a path to the list of monitored paths. $path is as listed in /sys/block (e.g. sda).
 .
@@ -197,8 +225,12 @@ for the multipath device (e.g. mpath1) or the uid of the multipath device
 (e.g. 36005076303ffc56200000000000010aa).
 .
 .TP
+.B remove|del maps|multipaths
+Remove all multipath devices.
+.
+.TP
 .B remove|del map|multipath $map
-Stop monitoring a multipath device.
+Remove the multipath device $map.
 .
 .TP
 .B resize map|multipath $map
-- 
2.45.0

[PATCH 6/7] libmultipath: change the vend/prod/rev printing

[Benjamin Marzinski]

The %s multipath and path wildcards both say they print the device
vend/prod/rev string, but neither of them do. The multipath wildcards
already provide a way to print the revision string and the %s wildcard
is used in the multipath -l output, so leave the wildcard output alone,
and change the description to only mention the vendor and product. There
is no other way to print the revision by path, and the path %s wildcard
is only used in the verbose multipath output, so make it actually print
the revision. Also check for unset strings, and print "##" instead of
nothing.

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

diff --git a/libmultipath/print.c b/libmultipath/print.c
index 14e7e325..cccfe5a3 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -330,7 +330,7 @@ snprint_multipath_uuid (struct strbuf *buff, const struct multipath * mpp)
 }
 
 static int
-snprint_multipath_vpr (struct strbuf *buff, const struct multipath * mpp)
+snprint_multipath_vp (struct strbuf *buff, const struct multipath * mpp)
 {
 	struct pathgroup * pgp;
 	struct path * pp;
@@ -591,7 +591,10 @@ static int snprint_initialized(struct strbuf *buff, const struct path * pp)
 static int
 snprint_vpr (struct strbuf *buff, const struct path * pp)
 {
-	return print_strbuf(buff, "%s,%s", pp->vendor_id, pp->product_id);
+	return print_strbuf(buff, "%s,%s,%s",
+			    strlen(pp->vendor_id) ? pp->vendor_id : "##",
+			    strlen(pp->product_id) ? pp->product_id : "##",
+			    strlen(pp->rev) ? pp->rev : "##");
 }
 
 static int
@@ -849,7 +852,7 @@ static const struct multipath_data mpd[] = {
 	{'2', "map_loads",     snprint_map_loads},
 	{'3', "total_q_time",  snprint_total_q_time},
 	{'4', "q_timeouts",    snprint_q_timeouts},
-	{'s', "vend/prod/rev", snprint_multipath_vpr},
+	{'s', "vend/prod",     snprint_multipath_vp},
 	{'v', "vend",          snprint_multipath_vend},
 	{'p', "prod",          snprint_multipath_prod},
 	{'e', "rev",           snprint_multipath_rev},
-- 
2.45.0

[PATCH 7/7] multipath-tools man pages: Add format wildcard descriptions

[Benjamin Marzinski]

Suggested-by: Nitin Yewale <nyewale@redhat.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 multipathd/multipathd.8.in | 211 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 207 insertions(+), 4 deletions(-)

diff --git a/multipathd/multipathd.8.in b/multipathd/multipathd.8.in
index 32976052..ff150f3d 100644
--- a/multipathd/multipathd.8.in
+++ b/multipathd/multipathd.8.in
@@ -117,7 +117,7 @@ Show the paths that multipathd is monitoring, and their state.
 .B list|show paths [raw] format $format
 Show the paths that multipathd is monitoring, using a format string with path
 format wildcards. Adding \fIraw\fR will remove the headers and alignment
-padding from the ouput.
+padding from the output. See "Path format wildcards" below.
 .
 .TP
 .B list|show path $path
@@ -131,7 +131,8 @@ Show the multipath devices that the multipathd is monitoring.
 .B list|show maps|multipaths [raw] format $format
 Show the status of all multipath devices that the multipathd is monitoring,
 using a format string with multipath format wildcards. Adding \fIraw\fR will
-remove the headers and alignment padding from the output.
+remove the headers and alignment padding from the output. See "Multipath
+format wildcards" below.
 .
 .TP
 .B list|show maps|multipaths status
@@ -162,7 +163,7 @@ Show topology of a single multipath device specified by $map, for example
 .B list|show map|multipath $map [raw] format $format.
 Show the status of multipath device $map, using a format string with multipath
 format wildcards. Adding \fIraw\fR will remove the headers and alignment
-padding from the output.
+padding from the output. See "Multipath format wildcards" below.
 .
 .TP
 .B list|show map|multipath $map json
@@ -170,7 +171,8 @@ Show information about multipath device $map in JSON format.
 .
 .TP
 .B list|show wildcards
-Show the format wildcards used in interactive commands taking $format.
+Show the format wildcards used in interactive commands taking $format. See
+"Format Wildcards" below.
 .
 .TP
 .B list|show config
@@ -367,6 +369,207 @@ Stop multipathd.
 .
 .
 .\" ----------------------------------------------------------------------------
+.SH "Format Wildcards"
+.\" ----------------------------------------------------------------------------
+.
+Multipathd commands that take a $format option require a format string. This
+string controls how a device is printed and should include format wildcards.
+When the devices are printed, these wildcards will be replaced by the
+appropriate device information. The following wildcards are supported.
+.TP
+.B Multipath format wildcards
+.RS
+.TP 12
+.B %n
+The device name.
+.TP
+.B %w
+The device WWID (uuid).
+.TP
+.B %d
+The device sysfs name (dm-<minor_nr>).
+.TP
+.B %F
+The device \fBfailback\fR setting. For deferred failbacks, it will either
+print the configured time if a deferred failback is not in progress, or
+it will show the current progress of a deferred failback.
+.TP
+.B %Q
+The device \fBno_path_retry\fR setting. If no_path_retry is set to a
+number of retires, it will either print the configured number of checker
+retries if the device is not in recovery mode, the number of seconds until
+queueing is disabled if the device is queueing in recovery mode, or \fIoff\fR
+if the device has disabled queueing.
+.TP
+.B %N
+The number of active paths for the device.
+.TP
+.B %r
+The device write-protect setting, either \fIro\fR or \fIrw\fR.
+.TP
+.B %t
+The device-mapper state of the device, either \fIsuspend\fR or \fIactive\fR.
+.TP
+.B %S
+The device size.
+.TP
+.B %f
+The device table features string.
+.TP
+.B %x
+The number of times the device has entered a state where it will fail IO.
+This is an alias for the \fB%4\fR wildcard.
+This value can be reset with the '\fIreset map $map stats\fR' command.
+.TP
+.B %h
+The device table hardware handler string.
+.TP
+.B %A
+The last action multipathd took on the device. This wildcard is for debugging
+use, as understanding its meaning requires looking at the code.
+.TP
+.B %0
+The number of times a path in the device has failed.
+This value can be reset with the '\fIreset map $map stats\fR' command.
+.TP
+.B %1
+The number of times multipathd has initiated a pathgroup switch for the device.
+This value can be reset with the '\fIreset map $map stats\fR' command.
+.TP
+.B %2
+The number of times multipathd has loaded a new table for the device.
+This value can be reset with the '\fIreset map $map stats\fR' command.
+.TP
+.B %3
+The approximate number of seconds that multipathd has spent queueing with
+no usable paths. This value can be reset with the '\fIreset map $map stats\fR'
+command.
+.TP
+.B %4
+The number of times the device has entered a state where it will fail IO.
+This is an alias for the \fB%x\fR wildcard.
+This value can be reset with the '\fIreset map $map stats\fR' command.
+.TP
+.B %s
+The vendor/product string for the device.
+.TP
+.B %v
+The array vendor string for the device.
+.TP
+.B %p
+The array product string for the device.
+.TP
+.B %e
+The array firmware revision string for the device.
+.TP
+.B %G
+The foreign library used for the device, or \fB--\fR for native device-mapper
+multipath devices.
+.TP
+.B %g
+Data from vendor specific vpd pages for the device, if any.
+.TP
+.B %k
+The actual max_sectors_kb setting for the device (which may be different from
+the configured one).
+.RE
+.
+.
+.TP
+.B Path format wildcards
+.RS
+.TP 12
+.B %w
+The device WWID (uuid).
+.TP
+.B %i
+The device Host:Channel:Id:Lun
+.TP
+.B %d
+The device sysfs name.
+.TP
+.B %D
+The device major:minor
+.TP
+.B %t
+The device-mapper state of the device, either \fIactive\fR or \fIfailed\fR.
+.TP
+.B %o
+Whether the device is \fIoffline\fR or \fIrunning\fR.
+.TP
+.B %T
+The multipathd path checker state of the device.
+.TP
+.B %s
+The vendor/product/revision string for the device.
+.TP
+.B %c
+The device's path checker name.
+.TP
+.B %C
+The progress towards the next path checker run on the device.
+.TP
+.B %p
+The device priority.
+.TP
+.B %S
+The device size.
+.TP
+.B %z
+The device serial number.
+.TP
+.B %M
+The device marginal state, either \fImarginal\fR or \fInormal\fR.
+.TP
+.B %m
+The multipath device that this device is a path of.
+.TP
+.B %N
+The host World Wide Node Name (WWNN) of the device.
+.TP
+.B %n
+The target World Wide Node Name (WWNN) of the device.
+.TP
+.B %R
+The host World Wide Port Name (WWPN) of the device.
+.TP
+.B %r
+The target World Wide Port Name (WWPN) of the device.
+.TP
+.B %a
+The host adapter name for the device (only SCSI devices).
+.TP
+.B %G
+The foreign library used for the device, or \fB--\fR for paths of native
+device-mapper multipath devices.
+.TP
+.B %g
+Data from vendor specific vpd pages for the device, if any.
+.TP
+.B %0
+The number of times this device has failed.
+.TP
+.B %P
+The device protocol. This output can be used for \fIprotocol\fR blacklist
+entries.
+.TP
+.B %I
+The device initialization state. Devices that have been fully initialized
+are shown as \fIok\fR.
+.TP
+.B %L
+The device SCSI LUN ID in hexadecimal format
+.TP
+.B %A
+The ALUA Target Port Group ID for the device, if applicable.
+.TP
+.B %k
+The actual max_sectors_kb setting for the device (which may be different than
+the configured one).
+.RE
+.
+.
+.\" ----------------------------------------------------------------------------
 .SH "SYSTEMD INTEGRATION"
 .\" ----------------------------------------------------------------------------
 .
-- 
2.45.0

Re: [PATCH 7/7] multipath-tools man pages: Add format wildcard descriptions

[Benjamin Marzinski]

On Mon, May 13, 2024 at 01:36:46PM -0400, Benjamin Marzinski wrote:
> Suggested-by: Nitin Yewale <nyewale@redhat.com>
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Oops. I forgot to update the man page date. Martin, either I can send a
new patch or you can feel free to update the date when you apply this
one, whichever you'd prefer.

-Ben


> ---
>  multipathd/multipathd.8.in | 211 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 207 insertions(+), 4 deletions(-)
> 
> diff --git a/multipathd/multipathd.8.in b/multipathd/multipathd.8.in
> index 32976052..ff150f3d 100644
> --- a/multipathd/multipathd.8.in
> +++ b/multipathd/multipathd.8.in
> @@ -117,7 +117,7 @@ Show the paths that multipathd is monitoring, and their state.
>  .B list|show paths [raw] format $format
>  Show the paths that multipathd is monitoring, using a format string with path
>  format wildcards. Adding \fIraw\fR will remove the headers and alignment
> -padding from the ouput.
> +padding from the output. See "Path format wildcards" below.
>  .
>  .TP
>  .B list|show path $path
> @@ -131,7 +131,8 @@ Show the multipath devices that the multipathd is monitoring.
>  .B list|show maps|multipaths [raw] format $format
>  Show the status of all multipath devices that the multipathd is monitoring,
>  using a format string with multipath format wildcards. Adding \fIraw\fR will
> -remove the headers and alignment padding from the output.
> +remove the headers and alignment padding from the output. See "Multipath
> +format wildcards" below.
>  .
>  .TP
>  .B list|show maps|multipaths status
> @@ -162,7 +163,7 @@ Show topology of a single multipath device specified by $map, for example
>  .B list|show map|multipath $map [raw] format $format.
>  Show the status of multipath device $map, using a format string with multipath
>  format wildcards. Adding \fIraw\fR will remove the headers and alignment
> -padding from the output.
> +padding from the output. See "Multipath format wildcards" below.
>  .
>  .TP
>  .B list|show map|multipath $map json
> @@ -170,7 +171,8 @@ Show information about multipath device $map in JSON format.
>  .
>  .TP
>  .B list|show wildcards
> -Show the format wildcards used in interactive commands taking $format.
> +Show the format wildcards used in interactive commands taking $format. See
> +"Format Wildcards" below.
>  .
>  .TP
>  .B list|show config
> @@ -367,6 +369,207 @@ Stop multipathd.
>  .
>  .
>  .\" ----------------------------------------------------------------------------
> +.SH "Format Wildcards"
> +.\" ----------------------------------------------------------------------------
> +.
> +Multipathd commands that take a $format option require a format string. This
> +string controls how a device is printed and should include format wildcards.
> +When the devices are printed, these wildcards will be replaced by the
> +appropriate device information. The following wildcards are supported.
> +.TP
> +.B Multipath format wildcards
> +.RS
> +.TP 12
> +.B %n
> +The device name.
> +.TP
> +.B %w
> +The device WWID (uuid).
> +.TP
> +.B %d
> +The device sysfs name (dm-<minor_nr>).
> +.TP
> +.B %F
> +The device \fBfailback\fR setting. For deferred failbacks, it will either
> +print the configured time if a deferred failback is not in progress, or
> +it will show the current progress of a deferred failback.
> +.TP
> +.B %Q
> +The device \fBno_path_retry\fR setting. If no_path_retry is set to a
> +number of retires, it will either print the configured number of checker
> +retries if the device is not in recovery mode, the number of seconds until
> +queueing is disabled if the device is queueing in recovery mode, or \fIoff\fR
> +if the device has disabled queueing.
> +.TP
> +.B %N
> +The number of active paths for the device.
> +.TP
> +.B %r
> +The device write-protect setting, either \fIro\fR or \fIrw\fR.
> +.TP
> +.B %t
> +The device-mapper state of the device, either \fIsuspend\fR or \fIactive\fR.
> +.TP
> +.B %S
> +The device size.
> +.TP
> +.B %f
> +The device table features string.
> +.TP
> +.B %x
> +The number of times the device has entered a state where it will fail IO.
> +This is an alias for the \fB%4\fR wildcard.
> +This value can be reset with the '\fIreset map $map stats\fR' command.
> +.TP
> +.B %h
> +The device table hardware handler string.
> +.TP
> +.B %A
> +The last action multipathd took on the device. This wildcard is for debugging
> +use, as understanding its meaning requires looking at the code.
> +.TP
> +.B %0
> +The number of times a path in the device has failed.
> +This value can be reset with the '\fIreset map $map stats\fR' command.
> +.TP
> +.B %1
> +The number of times multipathd has initiated a pathgroup switch for the device.
> +This value can be reset with the '\fIreset map $map stats\fR' command.
> +.TP
> +.B %2
> +The number of times multipathd has loaded a new table for the device.
> +This value can be reset with the '\fIreset map $map stats\fR' command.
> +.TP
> +.B %3
> +The approximate number of seconds that multipathd has spent queueing with
> +no usable paths. This value can be reset with the '\fIreset map $map stats\fR'
> +command.
> +.TP
> +.B %4
> +The number of times the device has entered a state where it will fail IO.
> +This is an alias for the \fB%x\fR wildcard.
> +This value can be reset with the '\fIreset map $map stats\fR' command.
> +.TP
> +.B %s
> +The vendor/product string for the device.
> +.TP
> +.B %v
> +The array vendor string for the device.
> +.TP
> +.B %p
> +The array product string for the device.
> +.TP
> +.B %e
> +The array firmware revision string for the device.
> +.TP
> +.B %G
> +The foreign library used for the device, or \fB--\fR for native device-mapper
> +multipath devices.
> +.TP
> +.B %g
> +Data from vendor specific vpd pages for the device, if any.
> +.TP
> +.B %k
> +The actual max_sectors_kb setting for the device (which may be different from
> +the configured one).
> +.RE
> +.
> +.
> +.TP
> +.B Path format wildcards
> +.RS
> +.TP 12
> +.B %w
> +The device WWID (uuid).
> +.TP
> +.B %i
> +The device Host:Channel:Id:Lun
> +.TP
> +.B %d
> +The device sysfs name.
> +.TP
> +.B %D
> +The device major:minor
> +.TP
> +.B %t
> +The device-mapper state of the device, either \fIactive\fR or \fIfailed\fR.
> +.TP
> +.B %o
> +Whether the device is \fIoffline\fR or \fIrunning\fR.
> +.TP
> +.B %T
> +The multipathd path checker state of the device.
> +.TP
> +.B %s
> +The vendor/product/revision string for the device.
> +.TP
> +.B %c
> +The device's path checker name.
> +.TP
> +.B %C
> +The progress towards the next path checker run on the device.
> +.TP
> +.B %p
> +The device priority.
> +.TP
> +.B %S
> +The device size.
> +.TP
> +.B %z
> +The device serial number.
> +.TP
> +.B %M
> +The device marginal state, either \fImarginal\fR or \fInormal\fR.
> +.TP
> +.B %m
> +The multipath device that this device is a path of.
> +.TP
> +.B %N
> +The host World Wide Node Name (WWNN) of the device.
> +.TP
> +.B %n
> +The target World Wide Node Name (WWNN) of the device.
> +.TP
> +.B %R
> +The host World Wide Port Name (WWPN) of the device.
> +.TP
> +.B %r
> +The target World Wide Port Name (WWPN) of the device.
> +.TP
> +.B %a
> +The host adapter name for the device (only SCSI devices).
> +.TP
> +.B %G
> +The foreign library used for the device, or \fB--\fR for paths of native
> +device-mapper multipath devices.
> +.TP
> +.B %g
> +Data from vendor specific vpd pages for the device, if any.
> +.TP
> +.B %0
> +The number of times this device has failed.
> +.TP
> +.B %P
> +The device protocol. This output can be used for \fIprotocol\fR blacklist
> +entries.
> +.TP
> +.B %I
> +The device initialization state. Devices that have been fully initialized
> +are shown as \fIok\fR.
> +.TP
> +.B %L
> +The device SCSI LUN ID in hexadecimal format
> +.TP
> +.B %A
> +The ALUA Target Port Group ID for the device, if applicable.
> +.TP
> +.B %k
> +The actual max_sectors_kb setting for the device (which may be different than
> +the configured one).
> +.RE
> +.
> +.
> +.\" ----------------------------------------------------------------------------
>  .SH "SYSTEMD INTEGRATION"
>  .\" ----------------------------------------------------------------------------
>  .
> -- 
> 2.45.0
> 

Re: [PATCH 3/7] libmultipath: print all values in snprint_failback

[Martin Wilck]

On Mon, 2024-05-13 at 13:36 -0400, Benjamin Marzinski wrote:
> Add the missing output for manual failback and print the defferral
> time
> for deferred failbacks, if one isn't currently in progress.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmultipath/print.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/libmultipath/print.c b/libmultipath/print.c
> index 89813517..14e7e325 100644
> --- a/libmultipath/print.c
> +++ b/libmultipath/print.c
> @@ -218,9 +218,13 @@ snprint_failback (struct strbuf *buff, const
> struct multipath * mpp)
>  		return append_strbuf_str(buff, "immediate");
>  	if (mpp->pgfailback == -FAILBACK_FOLLOWOVER)
>  		return append_strbuf_str(buff, "followover");
> +	if (mpp->pgfailback == -FAILBACK_MANUAL)
> +		return append_strbuf_str(buff, "manual");
> +	if (mpp->pgfailback == FAILBACK_UNDEF)
> +		return append_strbuf_str(buff, "undef");
>  
>  	if (!mpp->failback_tick)
> -		return append_strbuf_str(buff, "-");
> +		return print_strbuf(buff, "%i", mpp->pgfailback);

Perhaps we should print something like "delayed:%i" here?

Martin



>  	else
>  		return snprint_progress(buff, mpp->failback_tick,
>  					mpp->pgfailback);

Re: [PATCH 7/7] multipath-tools man pages: Add format wildcard descriptions

[Martin Wilck]

On Mon, 2024-05-13 at 13:36 -0400, Benjamin Marzinski wrote:
> Suggested-by: Nitin Yewale <nyewale@redhat.com>
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Thanks. This is useful, but I have a few additional suggestions below.

Martin

> ---
>  multipathd/multipathd.8.in | 211
> ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 207 insertions(+), 4 deletions(-)
> 
> diff --git a/multipathd/multipathd.8.in b/multipathd/multipathd.8.in
> index 32976052..ff150f3d 100644
> --- a/multipathd/multipathd.8.in
> +++ b/multipathd/multipathd.8.in
> @@ -117,7 +117,7 @@ Show the paths that multipathd is monitoring, and
> their state.
>  .B list|show paths [raw] format $format
>  Show the paths that multipathd is monitoring, using a format string
> with path
>  format wildcards. Adding \fIraw\fR will remove the headers and
> alignment
> -padding from the ouput.
> +padding from the output. See "Path format wildcards" below.
>  .
>  .TP
>  .B list|show path $path
> @@ -131,7 +131,8 @@ Show the multipath devices that the multipathd is
> monitoring.
>  .B list|show maps|multipaths [raw] format $format
>  Show the status of all multipath devices that the multipathd is
> monitoring,
>  using a format string with multipath format wildcards. Adding
> \fIraw\fR will
> -remove the headers and alignment padding from the output.
> +remove the headers and alignment padding from the output. See
> "Multipath
> +format wildcards" below.
>  .
>  .TP
>  .B list|show maps|multipaths status
> @@ -162,7 +163,7 @@ Show topology of a single multipath device
> specified by $map, for example
>  .B list|show map|multipath $map [raw] format $format.
>  Show the status of multipath device $map, using a format string with
> multipath
>  format wildcards. Adding \fIraw\fR will remove the headers and
> alignment
> -padding from the output.
> +padding from the output. See "Multipath format wildcards" below.
>  .
>  .TP
>  .B list|show map|multipath $map json
> @@ -170,7 +171,8 @@ Show information about multipath device $map in
> JSON format.
>  .
>  .TP
>  .B list|show wildcards
> -Show the format wildcards used in interactive commands taking
> $format.
> +Show the format wildcards used in interactive commands taking
> $format. See
> +"Format Wildcards" below.
>  .
>  .TP
>  .B list|show config
> @@ -367,6 +369,207 @@ Stop multipathd.
>  .
>  .
>  .\" ----------------------------------------------------------------
> ------------
> +.SH "Format Wildcards"
> +.\" ----------------------------------------------------------------
> ------------
> +.
> +Multipathd commands that take a $format option require a format
> string. This
> +string controls how a device is printed and should include format
> wildcards.
> +When the devices are printed, these wildcards will be replaced by
> the
> +appropriate device information. The following wildcards are
> supported.
> +.TP
> +.B Multipath format wildcards
> +.RS
> +.TP 12

I can see that you're using the same ordering as in print.c. Perhaps we
should use alphabetical ordering in the man page? Or do you think it
makes more sense to group the wildcards semantically?

Alternatively, 

> +.B %n
> +The device name.
> +.TP
> +.B %w
> +The device WWID (uuid).
> +.TP
> +.B %d
> +The device sysfs name (dm-<minor_nr>).
> +.TP
> +.B %F
> +The device \fBfailback\fR setting. For deferred failbacks, it will
> either
> +print the configured time if a deferred failback is not in progress,
> or
> +it will show the current progress of a deferred failback.

If you agree to amend 3/7, please don't forget to change this, too.

> +.TP
> +.B %Q
> +The device \fBno_path_retry\fR setting. If no_path_retry is set to a
> +number of retires, it will either print the configured number of

             ^ typo

> checker
> +retries if the device is not in recovery mode, the number of seconds
> until
> +queueing is disabled if the device is queueing in recovery mode, or
> \fIoff\fR
> +if the device has disabled queueing.
> +.TP
> +.B %N
> +The number of active paths for the device.
> +.TP
> +.B %r
> +The device write-protect setting, either \fIro\fR or \fIrw\fR.
> +.TP
> +.B %t
> +The device-mapper state of the device, either \fIsuspend\fR or
> \fIactive\fR.

"suspend" has been chosen to avoid  field overflow, but it sounds
weird.

\fIsuspend\fR if the device is suspended, and \fIactive\fR otherwise ?

> +.TP
> +.B %S
> +The device size.

Explain our "K", "M", "G", "T", "P" suffixes here?

> +.TP
> +.B %f
> +The device table features string.

Suggestion: 'The "features" string of the device-mapper table in the
kernel.'

> +.TP
> +.B %x
> +The number of times the device has entered a state where it will
> fail IO.
> +This is an alias for the \fB%4\fR wildcard.
> +This value can be reset with the '\fIreset map $map stats\fR'
> command.
> +.TP
> +.B %h
> +The device table hardware handler string.
> +.TP
> +.B %A
> +The last action multipathd took on the device. This wildcard is for
> debugging
> +use, as understanding its meaning requires looking at the code.
> +.TP
> +.B %0
> +The number of times a path in the device has failed.
> +This value can be reset with the '\fIreset map $map stats\fR'
> command.
> +.TP
> +.B %1
> +The number of times multipathd has initiated a pathgroup switch for
> the device.
> +This value can be reset with the '\fIreset map $map stats\fR'
> command.
> +.TP
> +.B %2
> +The number of times multipathd has loaded a new table for the
> device.
> +This value can be reset with the '\fIreset map $map stats\fR'
> command.
> +.TP
> +.B %3
> +The approximate number of seconds that multipathd has spent queueing
> with
> +no usable paths. This value can be reset with the '\fIreset map $map
> stats\fR'
> +command.
> +.TP
> +.B %4
> +The number of times the device has entered a state where it will
> fail IO.
> +This is an alias for the \fB%x\fR wildcard.
> +This value can be reset with the '\fIreset map $map stats\fR'
> command.
> +.TP
> +.B %s
> +The vendor/product string for the device.
> +.TP
> +.B %v
> +The array vendor string for the device.
> +.TP
> +.B %p
> +The array product string for the device.
> +.TP
> +.B %e
> +The array firmware revision string for the device.
> +.TP
> +.B %G
> +The foreign library used for the device, or \fB--\fR for native
> device-mapper
> +multipath devices.

Cross-ref to multipath.conf(5) here for an explanation of "foreign"?

> +.TP
> +.B %g
> +Data from vendor specific vpd pages for the device, if any.

"Currently multipathd supports VPD page 0xc0 für HPE 3PAR / Primera /
Alletra storage arrays."

> +.TP
> +.B %k
> +The actual max_sectors_kb setting for the device (which may be
> different from
> +the configured one).
> +.RE
> +.
> +.
> +.TP
> +.B Path format wildcards
> +.RS
> +.TP 12
> +.B %w
> +The device WWID (uuid).
> +.TP
> +.B %i
> +The device Host:Channel:Id:Lun

"for SCSI devices" (maybe explain what the data means for NVMe).

> +.TP
> +.B %d
> +The device sysfs name.
> +.TP
> +.B %D
> +The device major:minor
> +.TP
> +.B %t
> +The device-mapper state of the device, either \fIactive\fR or
> \fIfailed\fR.
> +.TP
> +.B %o
> +Whether the device is \fIoffline\fR or \fIrunning\fR.

'This shows "offline" if the device's "state" attribute in sysfs
is "offline" (for SCSI) or "dead" (for NMVe). For all other sysfs
states, it shows "running".'

> +.TP
> +.B %T
> +The multipathd path checker state of the device.

Should we list or explain the different states here?

> +.TP
> +.B %s
> +The vendor/product/revision string for the device.
> +.TP
> +.B %c
> +The device's path checker name.

"The name of the device's path checking algorithm"?

> +.TP
> +.B %C
> +The progress towards the next path checker run on the device.

'... in seconds'

> +.TP
> +.B %p
> +The device priority.

> +.TP
> +.B %S
> +The device size.

Format? See above

> +.TP
> +.B %z
> +The device serial number.
> +.TP
> +.B %M
> +The device marginal state, either \fImarginal\fR or \fInormal\fR.
> +.TP
> +.B %m
> +The multipath device that this device is a path of.
> +.TP
> +.B %N
> +The host World Wide Node Name (WWNN) of the device.

... if any
> +.TP
> +.B %n
> +The target World Wide Node Name (WWNN) of the device.

... if any

> +.TP
> +.B %R
> +The host World Wide Port Name (WWPN) of the device.

... if any
> +.TP
> +.B %r
> +The target World Wide Port Name (WWPN) of the device.

... if any

> +.TP
> +.B %a
> +The host adapter name for the device (only SCSI devices).
> +.TP
> +.B %G
> +The foreign library used for the device, or \fB--\fR for paths of
> native
> +device-mapper multipath devices.

See above

> +.TP
> +.B %g
> +Data from vendor specific vpd pages for the device, if any.

See above

> +.TP
> +.B %0
> +The number of times this device has failed.
> +.TP
> +.B %P
> +The device protocol. This output can be used for \fIprotocol\fR
> blacklist
> +entries.

I wouldn't mention the blacklist here. Rather, add a cross-ref to
multipath.conf(5).

> +.TP
> +.B %I
> +The device initialization state. Devices that have been fully
> initialized
> +are shown as \fIok\fR.
> +.TP
> +.B %L
> +The device SCSI LUN ID in hexadecimal format

'This is only meaningful for SCSI devices' ?

> +.TP
> +.B %A
> +The ALUA Target Port Group ID for the device, if applicable.
> +.TP
> +.B %k
> +The actual max_sectors_kb setting for the device (which may be
> different than
> +the configured one).
> +.RE
> +.
> +.
> +.\" ----------------------------------------------------------------
> ------------
>  .SH "SYSTEMD INTEGRATION"
>  .\" ----------------------------------------------------------------
> ------------
>  .

Re: [PATCH 0/7] multipath-tools man pages: add missing information

[Martin Wilck]

On Mon, 2024-05-13 at 13:36 -0400, Benjamin Marzinski wrote:
> This patchset is inspired by:
> https://lore.kernel.org/dm-devel/1673555329-9850-1-git-send-email-bmarzins@redhat.com/
> 
> It removes the useless pathgroup wildcards section from the "show
> wildcards" output and adds wildcards descriptions to the multipathd.8
> man page, along with some issing multipathd commands.  In the process
> of
> doing this, I noticed some instances where either the man page or
> multipathd wasn't giving the correct information, and fixed those up.
> 
> Benjamin Marzinski (6):
>   multipath-tools man pages: don't assume multipath.socket is enabled
>   libmultipath: print all values in snprint_failback
>   multipathd: Stop double counting map failures for no_path_retry > 0
>   multipath-tools man pages: add missing multipathd commands
>   libmultipath: change the vend/prod/rev printing
>   multipath-tools man pages: Add format wildcard descriptions
> 
> Nitin Yewale (1):
>   libmultipath: remove pathgroup wildcard options
> 
>  libmultipath/print.c       |  22 ++--
>  libmultipath/structs_vec.c |   5 +-
>  multipathd/multipathd.8.in | 260
> +++++++++++++++++++++++++++++++++++--
>  3 files changed, 263 insertions(+), 24 deletions(-)
> 

For the series, except 3/7 and 7/7:

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

Re: [PATCH 7/7] multipath-tools man pages: Add format wildcard descriptions

[Martin Wilck]

On Mon, 2024-05-13 at 15:11 -0400, Benjamin Marzinski wrote:
> On Mon, May 13, 2024 at 01:36:46PM -0400, Benjamin Marzinski wrote:
> > Suggested-by: Nitin Yewale <nyewale@redhat.com>
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> 
> Oops. I forgot to update the man page date. Martin, either I can send
> a
> new patch or you can feel free to update the date when you apply this
> one, whichever you'd prefer.

No problem, I can do this.
Would you like to have this included in the 0.9.9 release?
As all these changes are small and non-controversial, I tend to include
them, unless Christophe merges 0.9.9 very quickly.

Martin