[PATCH 0/2] Manual improvements

[PATCH 0/2] Manual improvements

[Lukasz Florczak]

Patch 1:
- Automatically generate mdadm.conf.5
- Respect Debian default config locations

Patch 2:
- Explain behavior for multiple config files setup
- Add missing Homecluster keyword description

Lukasz Florczak (2):
  mdadm: Respect config file location in man.
  mdadm: Update config man regarding default files and multi-keyword
    behavior.

 .gitignore                      |  1 +
 Makefile                        |  7 ++-
 ReadMe.c                        | 11 ++--
 mdadm.8.in                      | 34 +++++-------
 mdadm.conf.5 => mdadm.conf.5.in | 92 ++++++++++++++++++++++++++++++---
 5 files changed, 112 insertions(+), 33 deletions(-)
 rename mdadm.conf.5 => mdadm.conf.5.in (89%)

-- 
2.27.0

[PATCH 1/2] mdadm: Respect config file location in man.

[Lukasz Florczak]

Default config file location could differ depending on OS (e.g. Debian family).
This patch takes default config file into consideration when creating mdadm.man
file as well as mdadm.conf.man.

Rename mdadm.conf.5 to mdadm.conf.5.in. Now mdadm.conf.5 is generated automatically.

Additionally update config help in ReadMe.c.

Signed-off-by: Lukasz Florczak <lukasz.florczak@linux.intel.com>
---
 .gitignore                      |  1 +
 Makefile                        |  7 ++++++-
 ReadMe.c                        | 11 ++++++-----
 mdadm.8.in                      | 16 ++++++++--------
 mdadm.conf.5 => mdadm.conf.5.in |  2 +-
 5 files changed, 22 insertions(+), 15 deletions(-)
 rename mdadm.conf.5 => mdadm.conf.5.in (99%)

diff --git a/.gitignore b/.gitignore
index 217fe76d..8d791c6f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -3,6 +3,7 @@
 /*-stamp
 /mdadm
 /mdadm.8
+/mdadm.conf.5
 /mdadm.udeb
 /mdassemble
 /mdmon
diff --git a/Makefile b/Makefile
index 2a51d813..bf126033 100644
--- a/Makefile
+++ b/Makefile
@@ -227,7 +227,12 @@ raid6check : raid6check.o mdadm.h $(CHECK_OBJS)
 
 mdadm.8 : mdadm.8.in
 	sed -e 's/{DEFAULT_METADATA}/$(DEFAULT_METADATA)/g' \
-	-e 's,{MAP_PATH},$(MAP_PATH),g'  mdadm.8.in > mdadm.8
+	-e 's,{MAP_PATH},$(MAP_PATH),g' -e 's,{CONFFILE},$(CONFFILE),g' \
+	-e 's,{CONFFILE2},$(CONFFILE2),g'  mdadm.8.in > mdadm.8
+
+mdadm.conf.5 : mdadm.conf.5.in
+	sed -e 's,{CONFFILE},$(CONFFILE),g' \
+	-e 's,{CONFFILE2},$(CONFFILE2),g'  mdadm.conf.5.in > mdadm.conf.5
 
 mdadm.man : mdadm.8
 	man -l mdadm.8 > mdadm.man
diff --git a/ReadMe.c b/ReadMe.c
index 81399765..b7357b97 100644
--- a/ReadMe.c
+++ b/ReadMe.c
@@ -613,7 +613,6 @@ char Help_incr[] =
 ;
 
 char Help_config[] =
-"The /etc/mdadm.conf config file:\n\n"
 " The config file contains, apart from blank lines and comment lines that\n"
 " start with a hash(#), array lines, device lines, and various\n"
 " configuration lines.\n"
@@ -636,10 +635,12 @@ char Help_config[] =
 " than a device must match all of them to be considered.\n"
 "\n"
 " Other configuration lines include:\n"
-"  mailaddr, mailfrom, program     used for --monitor mode\n"
-"  create, auto                    used when creating device names in /dev\n"
-"  homehost, policy, part-policy   used to guide policy in various\n"
-"                                  situations\n"
+"  mailaddr, mailfrom, program, monitordelay    used for --monitor mode\n"
+"  create, auto                                 used when creating device names in /dev\n"
+"  homehost, policy, part-policy                used to guide policy in various\n"
+"                                               situations\n"
+"\n"
+"For more details see mdadm.conf(5).\n"
 "\n"
 ;
 
diff --git a/mdadm.8.in b/mdadm.8.in
index be902dba..d41b3ca7 100644
--- a/mdadm.8.in
+++ b/mdadm.8.in
@@ -267,13 +267,13 @@ the exact meaning of this option in different contexts.
 .TP
 .BR \-c ", " \-\-config=
 Specify the config file or directory.  Default is to use
-.B /etc/mdadm.conf
+.B {CONFFILE}
 and
-.BR /etc/mdadm.conf.d ,
+.BR {CONFFILE}.d ,
 or if those are missing then
-.B /etc/mdadm/mdadm.conf
+.B {CONFFILE2}
 and
-.BR /etc/mdadm/mdadm.conf.d .
+.BR {CONFFILE2}.d .
 If the config file given is
 .B "partitions"
 then nothing will be read, but
@@ -2009,9 +2009,9 @@ The config file is only used if explicitly named with
 or requested with (a possibly implicit)
 .BR \-\-scan .
 In the later case,
-.B /etc/mdadm.conf
+.B {CONFFILE}
 or
-.B /etc/mdadm/mdadm.conf
+.B {CONFFILE2}
 is used.
 
 If
@@ -3339,7 +3339,7 @@ uses this to find arrays when
 is given in Misc mode, and to monitor array reconstruction
 on Monitor mode.
 
-.SS /etc/mdadm.conf
+.SS {CONFFILE} (or {CONFFILE2})
 
 The config file lists which devices may be scanned to see if
 they contain MD super block, and gives identifying information
@@ -3347,7 +3347,7 @@ they contain MD super block, and gives identifying information
 .BR mdadm.conf (5)
 for more details.
 
-.SS /etc/mdadm.conf.d
+.SS {CONFFILE}.d (or {CONFFILE2}.d)
 
 A directory containing configuration files which are read in lexical
 order.
diff --git a/mdadm.conf.5 b/mdadm.conf.5.in
similarity index 99%
rename from mdadm.conf.5
rename to mdadm.conf.5.in
index 74a21c5f..83edd008 100644
--- a/mdadm.conf.5
+++ b/mdadm.conf.5.in
@@ -8,7 +8,7 @@
 .SH NAME
 mdadm.conf \- configuration for management of Software RAID with mdadm
 .SH SYNOPSIS
-/etc/mdadm.conf
+{CONFFILE}
 .SH DESCRIPTION
 .PP
 .I mdadm
-- 
2.27.0

[PATCH 2/2] mdadm: Update config man regarding default files and multi-keyword behavior.

[Lukasz Florczak]

Simplify default and alternative config file and directory location references
from mdadm(8) as references to mdadm.conf(5). Add FILE section in config man
and explain order and conditions in which default and alternative config files
and directories are used.

Update config man behavior regarding parsing order when multiple keywords/config
files are involved.

Additionally add missing HOMECLUSTER keyword description.

Signed-off-by: Lukasz Florczak <lukasz.florczak@linux.intel.com>
---
 mdadm.8.in      | 30 +++++++----------
 mdadm.conf.5.in | 90 +++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 96 insertions(+), 24 deletions(-)

diff --git a/mdadm.8.in b/mdadm.8.in
index d41b3ca7..d6af73b7 100644
--- a/mdadm.8.in
+++ b/mdadm.8.in
@@ -266,14 +266,11 @@ the exact meaning of this option in different contexts.
 
 .TP
 .BR \-c ", " \-\-config=
-Specify the config file or directory.  Default is to use
-.B {CONFFILE}
-and
-.BR {CONFFILE}.d ,
-or if those are missing then
-.B {CONFFILE2}
-and
-.BR {CONFFILE2}.d .
+Specify the config file or directory.  If not specified, default config file
+and default conf.d directory will be used.  See
+.BR mdadm.conf (5)
+for more details.
+
 If the config file given is
 .B "partitions"
 then nothing will be read, but
@@ -2008,11 +2005,9 @@ The config file is only used if explicitly named with
 .B \-\-config
 or requested with (a possibly implicit)
 .BR \-\-scan .
-In the later case,
-.B {CONFFILE}
-or
-.B {CONFFILE2}
-is used.
+In the later case, default config file is used.  See
+.BR mdadm.conf (5)
+for more details.
 
 If
 .B \-\-scan
@@ -3341,16 +3336,15 @@ on Monitor mode.
 
 .SS {CONFFILE} (or {CONFFILE2})
 
-The config file lists which devices may be scanned to see if
-they contain MD super block, and gives identifying information
-(e.g. UUID) about known MD arrays.  See
+Default config file.  See
 .BR mdadm.conf (5)
 for more details.
 
 .SS {CONFFILE}.d (or {CONFFILE2}.d)
 
-A directory containing configuration files which are read in lexical
-order.
+Default directory containing configuration files.  See
+.BR mdadm.conf (5)
+for more details.
 
 .SS {MAP_PATH}
 When
diff --git a/mdadm.conf.5.in b/mdadm.conf.5.in
index 83edd008..7a4e73b7 100644
--- a/mdadm.conf.5.in
+++ b/mdadm.conf.5.in
@@ -88,7 +88,8 @@ but only the major and minor device numbers.  It scans
 .I /dev
 to find the name that matches the numbers.
 
-If no DEVICE line is present, then "DEVICE partitions containers" is assumed.
+If no DEVICE line is present in any config file,
+then "DEVICE partitions containers" is assumed.
 
 For example:
 .IP
@@ -272,6 +273,10 @@ catenated with spaces to form the address.
 Note that this value cannot be set via the
 .I mdadm
 commandline.  It is only settable via the config file.
+There should only be one
+.B MAILADDR
+line and it should have only one address.  Any subsequent addresses
+are silently ignored.
 
 .TP
 .B PROGRAM
@@ -286,7 +291,8 @@ device.
 
 There should only be one
 .B program
-line and it should be give only one program.
+line and it should be given only one program.  Any subsequent programs
+are silently ignored.
 
 
 .TP
@@ -295,7 +301,14 @@ The
 .B create
 line gives default values to be used when creating arrays, new members
 of arrays, and device entries for arrays.
-These include:
+
+There should only be one
+.B create
+line.  Any subsequent lines will override the previous settings.
+
+Keywords used in the
+.I CREATE
+line and supported values are:
 
 .RS 4
 .TP
@@ -426,6 +439,24 @@ from any possible local name. e.g.
 .B /dev/md/1_1
 or
 .BR /dev/md/home_0 .
+
+
+.TP
+.B HOMECLUSTER
+The
+.B homcluster
+line gives a default value for the
+.B \-\-homecluster=
+option to mdadm.  It specifies  the  cluster name for the md device.
+The md device can be assembled only on the cluster which matches
+the name specified. If
+.B homcluster
+is not provided, mdadm tries to detect the cluster name automatically.
+
+There should only be one
+.B homecluster
+line.  Any subsequent lines will be silently ignored.
+
 .TP
 .B AUTO
 A list of names of metadata format can be given, each preceded by a
@@ -475,8 +506,8 @@ The known metadata types are
 
 .B AUTO
 should be given at most once.  Subsequent lines are silently ignored.
-Thus an earlier config file in a config directory will over-ride
-the setting in a later config file.
+Thus a later config file in a config directory will not overwrite
+the setting in an earlier config file.
 
 .TP
 .B POLICY
@@ -501,6 +532,7 @@ To update hot plug configuration it is necessary to execute
 .B mdadm \-\-udev\-rules
 command after changing the config file
 
+
 Keywords used in the
 .I POLICY
 line and supported values are:
@@ -572,6 +604,12 @@ This is similar to
 and accepts the same keyword assignments.  It allows a consistent set
 of policies to applied to each of the partitions of a device.
 
+There can be multiple
+.B PART-POLICY
+lines. Behavior is same as in
+.B POLICY
+so lines parsed later have higher priority.
+
 A
 .B PART-POLICY
 line should set
@@ -594,6 +632,7 @@ The
 line lists custom values of MD device's sysfs attributes which will be
 stored in sysfs after the array is assembled. Multiple lines are allowed and each
 line has to contain the uuid or the name of the device to which it relates.
+Lines are applied in reverse order.
 .RS 4
 .TP
 .B uuid=
@@ -621,7 +660,46 @@ is running in
 .B \-\-monitor
 mode.
 .B \-d/\-\-delay
-command line argument takes precedence over the config file
+command line argument takes precedence over the config file.
+
+If multiple
+.B MINITORDELAY
+lines are provided, only first non-zero value is considered.
+
+.SH FILES
+
+.SS {CONFFILE}
+
+The default config file location, used when
+.I mdadm
+is running without --config option.
+
+.SS {CONFFILE}.d
+
+The default directory with config files. Used when
+.I mdadm
+is running without --config option, after successful reading of the
+.B {CONFFILE}
+default config file. Files in that directory
+are read in lexical order.
+
+
+.SS {CONFFILE2}
+
+Alternative config file that is read, when
+.I mdadm
+is running without --config option and the
+.B {CONFFILE}
+default config file was not opened successfully.
+
+.SS {CONFFILE2}.d
+
+The alternative directory with config files. Used when
+.I mdadm
+is runninng without --config option, after reading the
+.B {CONFFILE2}
+alternative config file whether it was successful or not. Files in
+that directory are read in lexical order.
 
 .SH EXAMPLE
 DEVICE /dev/sd[bcdjkl]1
-- 
2.27.0

Re: [PATCH 2/2] mdadm: Update config man regarding default files and multi-keyword behavior.

[Coly Li]

On 3/15/22 4:55 PM, Lukasz Florczak wrote:
> Simplify default and alternative config file and directory location references
> from mdadm(8) as references to mdadm.conf(5). Add FILE section in config man
> and explain order and conditions in which default and alternative config files
> and directories are used.
>
> Update config man behavior regarding parsing order when multiple keywords/config
> files are involved.
>
> Additionally add missing HOMECLUSTER keyword description.
>
> Signed-off-by: Lukasz Florczak <lukasz.florczak@linux.intel.com>


Hi Lukasz,


This patch doesn't apply on branch 20220315-testing of the mdadm-CI 
tree, could you please rebase this series on

git://git.kernel.org/pub/scm/linux/kernel/git/colyli/mdadm.git 
20220315-testing

Then I will continue to test them.


Thanks.


Coly Li


> ---
>   mdadm.8.in      | 30 +++++++----------
>   mdadm.conf.5.in | 90 +++++++++++++++++++++++++++++++++++++++++++++----
>   2 files changed, 96 insertions(+), 24 deletions(-)
>
> diff --git a/mdadm.8.in b/mdadm.8.in
> index d41b3ca7..d6af73b7 100644
> --- a/mdadm.8.in
> +++ b/mdadm.8.in
> @@ -266,14 +266,11 @@ the exact meaning of this option in different contexts.
>   
>   .TP
>   .BR \-c ", " \-\-config=
> -Specify the config file or directory.  Default is to use
> -.B {CONFFILE}
> -and
> -.BR {CONFFILE}.d ,
> -or if those are missing then
> -.B {CONFFILE2}
> -and
> -.BR {CONFFILE2}.d .
> +Specify the config file or directory.  If not specified, default config file
> +and default conf.d directory will be used.  See
> +.BR mdadm.conf (5)
> +for more details.
> +
>   If the config file given is
>   .B "partitions"
>   then nothing will be read, but
> @@ -2008,11 +2005,9 @@ The config file is only used if explicitly named with
>   .B \-\-config
>   or requested with (a possibly implicit)
>   .BR \-\-scan .
> -In the later case,
> -.B {CONFFILE}
> -or
> -.B {CONFFILE2}
> -is used.
> +In the later case, default config file is used.  See
> +.BR mdadm.conf (5)
> +for more details.
>   
>   If
>   .B \-\-scan
> @@ -3341,16 +3336,15 @@ on Monitor mode.
>   
>   .SS {CONFFILE} (or {CONFFILE2})
>   
> -The config file lists which devices may be scanned to see if
> -they contain MD super block, and gives identifying information
> -(e.g. UUID) about known MD arrays.  See
> +Default config file.  See
>   .BR mdadm.conf (5)
>   for more details.
>   
>   .SS {CONFFILE}.d (or {CONFFILE2}.d)
>   
> -A directory containing configuration files which are read in lexical
> -order.
> +Default directory containing configuration files.  See
> +.BR mdadm.conf (5)
> +for more details.
>   
>   .SS {MAP_PATH}
>   When
> diff --git a/mdadm.conf.5.in b/mdadm.conf.5.in
> index 83edd008..7a4e73b7 100644
> --- a/mdadm.conf.5.in
> +++ b/mdadm.conf.5.in
> @@ -88,7 +88,8 @@ but only the major and minor device numbers.  It scans
>   .I /dev
>   to find the name that matches the numbers.
>   
> -If no DEVICE line is present, then "DEVICE partitions containers" is assumed.
> +If no DEVICE line is present in any config file,
> +then "DEVICE partitions containers" is assumed.
>   
>   For example:
>   .IP
> @@ -272,6 +273,10 @@ catenated with spaces to form the address.
>   Note that this value cannot be set via the
>   .I mdadm
>   commandline.  It is only settable via the config file.
> +There should only be one
> +.B MAILADDR
> +line and it should have only one address.  Any subsequent addresses
> +are silently ignored.
>   
>   .TP
>   .B PROGRAM
> @@ -286,7 +291,8 @@ device.
>   
>   There should only be one
>   .B program
> -line and it should be give only one program.
> +line and it should be given only one program.  Any subsequent programs
> +are silently ignored.
>   
>   
>   .TP
> @@ -295,7 +301,14 @@ The
>   .B create
>   line gives default values to be used when creating arrays, new members
>   of arrays, and device entries for arrays.
> -These include:
> +
> +There should only be one
> +.B create
> +line.  Any subsequent lines will override the previous settings.
> +
> +Keywords used in the
> +.I CREATE
> +line and supported values are:
>   
>   .RS 4
>   .TP
> @@ -426,6 +439,24 @@ from any possible local name. e.g.
>   .B /dev/md/1_1
>   or
>   .BR /dev/md/home_0 .
> +
> +
> +.TP
> +.B HOMECLUSTER
> +The
> +.B homcluster
> +line gives a default value for the
> +.B \-\-homecluster=
> +option to mdadm.  It specifies  the  cluster name for the md device.
> +The md device can be assembled only on the cluster which matches
> +the name specified. If
> +.B homcluster
> +is not provided, mdadm tries to detect the cluster name automatically.
> +
> +There should only be one
> +.B homecluster
> +line.  Any subsequent lines will be silently ignored.
> +
>   .TP
>   .B AUTO
>   A list of names of metadata format can be given, each preceded by a
> @@ -475,8 +506,8 @@ The known metadata types are
>   
>   .B AUTO
>   should be given at most once.  Subsequent lines are silently ignored.
> -Thus an earlier config file in a config directory will over-ride
> -the setting in a later config file.
> +Thus a later config file in a config directory will not overwrite
> +the setting in an earlier config file.
>   
>   .TP
>   .B POLICY
> @@ -501,6 +532,7 @@ To update hot plug configuration it is necessary to execute
>   .B mdadm \-\-udev\-rules
>   command after changing the config file
>   
> +
>   Keywords used in the
>   .I POLICY
>   line and supported values are:
> @@ -572,6 +604,12 @@ This is similar to
>   and accepts the same keyword assignments.  It allows a consistent set
>   of policies to applied to each of the partitions of a device.
>   
> +There can be multiple
> +.B PART-POLICY
> +lines. Behavior is same as in
> +.B POLICY
> +so lines parsed later have higher priority.
> +
>   A
>   .B PART-POLICY
>   line should set
> @@ -594,6 +632,7 @@ The
>   line lists custom values of MD device's sysfs attributes which will be
>   stored in sysfs after the array is assembled. Multiple lines are allowed and each
>   line has to contain the uuid or the name of the device to which it relates.
> +Lines are applied in reverse order.
>   .RS 4
>   .TP
>   .B uuid=
> @@ -621,7 +660,46 @@ is running in
>   .B \-\-monitor
>   mode.
>   .B \-d/\-\-delay
> -command line argument takes precedence over the config file
> +command line argument takes precedence over the config file.
> +
> +If multiple
> +.B MINITORDELAY
> +lines are provided, only first non-zero value is considered.
> +
> +.SH FILES
> +
> +.SS {CONFFILE}
> +
> +The default config file location, used when
> +.I mdadm
> +is running without --config option.
> +
> +.SS {CONFFILE}.d
> +
> +The default directory with config files. Used when
> +.I mdadm
> +is running without --config option, after successful reading of the
> +.B {CONFFILE}
> +default config file. Files in that directory
> +are read in lexical order.
> +
> +
> +.SS {CONFFILE2}
> +
> +Alternative config file that is read, when
> +.I mdadm
> +is running without --config option and the
> +.B {CONFFILE}
> +default config file was not opened successfully.
> +
> +.SS {CONFFILE2}.d
> +
> +The alternative directory with config files. Used when
> +.I mdadm
> +is runninng without --config option, after reading the
> +.B {CONFFILE2}
> +alternative config file whether it was successful or not. Files in
> +that directory are read in lexical order.
>   
>   .SH EXAMPLE
>   DEVICE /dev/sd[bcdjkl]1

Re: [PATCH 1/2] mdadm: Respect config file location in man.

[Paul Menzel]

Dear Lukasz,


Thank you for your patches.

Am 15.03.22 um 09:55 schrieb Lukasz Florczak:

It’d be great if you removed the dot/period at the end of the git commit 
message summaries [1]. (Also in second patch.)

> Default config file location could differ depending on OS (e.g. Debian family).

What is it an Debian?

> This patch takes default config file into consideration when creating mdadm.man
> file as well as mdadm.conf.man.
> 
> Rename mdadm.conf.5 to mdadm.conf.5.in. Now mdadm.conf.5 is generated automatically.
> 
> Additionally update config help in ReadMe.c.
> 
> Signed-off-by: Lukasz Florczak <lukasz.florczak@linux.intel.com>
> ---
>   .gitignore                      |  1 +
>   Makefile                        |  7 ++++++-
>   ReadMe.c                        | 11 ++++++-----
>   mdadm.8.in                      | 16 ++++++++--------
>   mdadm.conf.5 => mdadm.conf.5.in |  2 +-
>   5 files changed, 22 insertions(+), 15 deletions(-)
>   rename mdadm.conf.5 => mdadm.conf.5.in (99%)
> 
> diff --git a/.gitignore b/.gitignore
> index 217fe76d..8d791c6f 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -3,6 +3,7 @@
>   /*-stamp
>   /mdadm
>   /mdadm.8
> +/mdadm.conf.5
>   /mdadm.udeb
>   /mdassemble
>   /mdmon
> diff --git a/Makefile b/Makefile
> index 2a51d813..bf126033 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -227,7 +227,12 @@ raid6check : raid6check.o mdadm.h $(CHECK_OBJS)
>   
>   mdadm.8 : mdadm.8.in
>   	sed -e 's/{DEFAULT_METADATA}/$(DEFAULT_METADATA)/g' \
> -	-e 's,{MAP_PATH},$(MAP_PATH),g'  mdadm.8.in > mdadm.8
> +	-e 's,{MAP_PATH},$(MAP_PATH),g' -e 's,{CONFFILE},$(CONFFILE),g' \
> +	-e 's,{CONFFILE2},$(CONFFILE2),g'  mdadm.8.in > mdadm.8
> +
> +mdadm.conf.5 : mdadm.conf.5.in
> +	sed -e 's,{CONFFILE},$(CONFFILE),g' \
> +	-e 's,{CONFFILE2},$(CONFFILE2),g'  mdadm.conf.5.in > mdadm.conf.5
>   
>   mdadm.man : mdadm.8
>   	man -l mdadm.8 > mdadm.man
> diff --git a/ReadMe.c b/ReadMe.c
> index 81399765..b7357b97 100644
> --- a/ReadMe.c
> +++ b/ReadMe.c
> @@ -613,7 +613,6 @@ char Help_incr[] =
>   ;
>   
>   char Help_config[] =
> -"The /etc/mdadm.conf config file:\n\n"
>   " The config file contains, apart from blank lines and comment lines that\n"
>   " start with a hash(#), array lines, device lines, and various\n"
>   " configuration lines.\n"
> @@ -636,10 +635,12 @@ char Help_config[] =
>   " than a device must match all of them to be considered.\n"
>   "\n"
>   " Other configuration lines include:\n"
> -"  mailaddr, mailfrom, program     used for --monitor mode\n"
> -"  create, auto                    used when creating device names in /dev\n"
> -"  homehost, policy, part-policy   used to guide policy in various\n"
> -"                                  situations\n"
> +"  mailaddr, mailfrom, program, monitordelay    used for --monitor mode\n"

Looks like an independent fix. Please separate into a separate commit.

> +"  create, auto                                 used when creating device names in /dev\n"
> +"  homehost, policy, part-policy                used to guide policy in various\n"
> +"                                               situations\n"
> +"\n"
> +"For more details see mdadm.conf(5).\n"
>   "\n"
>   ;
>   
> diff --git a/mdadm.8.in b/mdadm.8.in
> index be902dba..d41b3ca7 100644
> --- a/mdadm.8.in
> +++ b/mdadm.8.in
> @@ -267,13 +267,13 @@ the exact meaning of this option in different contexts.
>   .TP
>   .BR \-c ", " \-\-config=
>   Specify the config file or directory.  Default is to use
> -.B /etc/mdadm.conf
> +.B {CONFFILE}
>   and
> -.BR /etc/mdadm.conf.d ,
> +.BR {CONFFILE}.d ,
>   or if those are missing then
> -.B /etc/mdadm/mdadm.conf
> +.B {CONFFILE2}
>   and
> -.BR /etc/mdadm/mdadm.conf.d .
> +.BR {CONFFILE2}.d .
>   If the config file given is
>   .B "partitions"
>   then nothing will be read, but
> @@ -2009,9 +2009,9 @@ The config file is only used if explicitly named with
>   or requested with (a possibly implicit)
>   .BR \-\-scan .
>   In the later case,
> -.B /etc/mdadm.conf
> +.B {CONFFILE}
>   or
> -.B /etc/mdadm/mdadm.conf
> +.B {CONFFILE2}
>   is used.
>   
>   If
> @@ -3339,7 +3339,7 @@ uses this to find arrays when
>   is given in Misc mode, and to monitor array reconstruction
>   on Monitor mode.
>   
> -.SS /etc/mdadm.conf
> +.SS {CONFFILE} (or {CONFFILE2})
>   
>   The config file lists which devices may be scanned to see if
>   they contain MD super block, and gives identifying information
> @@ -3347,7 +3347,7 @@ they contain MD super block, and gives identifying information
>   .BR mdadm.conf (5)
>   for more details.
>   
> -.SS /etc/mdadm.conf.d
> +.SS {CONFFILE}.d (or {CONFFILE2}.d)
>   
>   A directory containing configuration files which are read in lexical
>   order.
> diff --git a/mdadm.conf.5 b/mdadm.conf.5.in
> similarity index 99%
> rename from mdadm.conf.5
> rename to mdadm.conf.5.in
> index 74a21c5f..83edd008 100644
> --- a/mdadm.conf.5
> +++ b/mdadm.conf.5.in
> @@ -8,7 +8,7 @@
>   .SH NAME
>   mdadm.conf \- configuration for management of Software RAID with mdadm
>   .SH SYNOPSIS
> -/etc/mdadm.conf
> +{CONFFILE}
>   .SH DESCRIPTION
>   .PP
>   .I mdadm

The rest looks good.


Kind regards,

Paul


[1]: https://chris.beams.io/posts/git-commit/

Re: [PATCH 2/2] mdadm: Update config man regarding default files and multi-keyword behavior.

[Mariusz Tkaczyk]

On Tue, 15 Mar 2022 17:57:09 +0800
Coly Li <colyli@suse.de> wrote:

> On 3/15/22 4:55 PM, Lukasz Florczak wrote:
> > Simplify default and alternative config file and directory location
> > references from mdadm(8) as references to mdadm.conf(5). Add FILE
> > section in config man and explain order and conditions in which
> > default and alternative config files and directories are used.
> >
> > Update config man behavior regarding parsing order when multiple
> > keywords/config files are involved.
> >
> > Additionally add missing HOMECLUSTER keyword description.
> >
> > Signed-off-by: Lukasz Florczak <lukasz.florczak@linux.intel.com>  
> 
> 
> Hi Lukasz,
> 
> 
> This patch doesn't apply on branch 20220315-testing of the mdadm-CI 
> tree, could you please rebase this series on
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/colyli/mdadm.git 
> 20220315-testing
> 
> Then I will continue to test them.
> 

Hi Coly,
This is great to see that something is happening in upstream :)

I can see that you created branch where some patches were merged and
now you are reporting conflicts now. Our patches are based on last
mdadm commit (which is mdadm-4.2 ).
IMO you should try to apply them first on latest master and later
cherry-pick/ rebase them on top of your testing branch. This should
automatically resolve most of conflicts. Could you try that?

This is hard to follow all patches on list (especially that we cannot
determine in which order they will be applied). Preparing patches for
you testing branch (which could be changed in any moment), IMO is not a
good solution.

I really appreciate the work you put to enable upstream testing. If you
need some help, let me know.

Thanks,
Mariusz

Re: [PATCH 2/2] mdadm: Update config man regarding default files and multi-keyword behavior.

[Coly Li]

On 3/16/22 12:00 AM, Mariusz Tkaczyk wrote:
> On Tue, 15 Mar 2022 17:57:09 +0800
> Coly Li <colyli@suse.de> wrote:
>
>> On 3/15/22 4:55 PM, Lukasz Florczak wrote:
>>> Simplify default and alternative config file and directory location
>>> references from mdadm(8) as references to mdadm.conf(5). Add FILE
>>> section in config man and explain order and conditions in which
>>> default and alternative config files and directories are used.
>>>
>>> Update config man behavior regarding parsing order when multiple
>>> keywords/config files are involved.
>>>
>>> Additionally add missing HOMECLUSTER keyword description.
>>>
>>> Signed-off-by: Lukasz Florczak <lukasz.florczak@linux.intel.com>
>>
>> Hi Lukasz,
>>
>>
>> This patch doesn't apply on branch 20220315-testing of the mdadm-CI
>> tree, could you please rebase this series on
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/colyli/mdadm.git
>> 20220315-testing
>>
>> Then I will continue to test them.
>>
> Hi Coly,
> This is great to see that something is happening in upstream :)
>
> I can see that you created branch where some patches were merged and
> now you are reporting conflicts now. Our patches are based on last
> mdadm commit (which is mdadm-4.2 ).
> IMO you should try to apply them first on latest master and later
> cherry-pick/ rebase them on top of your testing branch. This should
> automatically resolve most of conflicts. Could you try that?


The testing branch is updated to latest mdadm upstream. Indeed the 
conflict is about blank line as I see, e.g. it removes some \t from 
empty line, but such issue was removed in latest upstream.

Ineed I can fix the conflict, but I don't know how to make you update 
the change from my side. Does it work if I sand you a diff of the patch?

Thanks.


Coly Li

Re: [PATCH 2/2] mdadm: Update config man regarding default files and multi-keyword behavior.

[Mariusz Tkaczyk]

On Wed, 16 Mar 2022 00:43:57 +0800
Coly Li <colyli@suse.de> wrote:

> On 3/16/22 12:00 AM, Mariusz Tkaczyk wrote:
> > On Tue, 15 Mar 2022 17:57:09 +0800
> > Coly Li <colyli@suse.de> wrote:
> >
> >> On 3/15/22 4:55 PM, Lukasz Florczak wrote:
> >>> Simplify default and alternative config file and directory
> >>> location references from mdadm(8) as references to mdadm.conf(5).
> >>> Add FILE section in config man and explain order and conditions
> >>> in which default and alternative config files and directories are
> >>> used.
> >>>
> >>> Update config man behavior regarding parsing order when multiple
> >>> keywords/config files are involved.
> >>>
> >>> Additionally add missing HOMECLUSTER keyword description.
> >>>
> >>> Signed-off-by: Lukasz Florczak <lukasz.florczak@linux.intel.com>
> >>
> >> Hi Lukasz,
> >>
> >>
> >> This patch doesn't apply on branch 20220315-testing of the mdadm-CI
> >> tree, could you please rebase this series on
> >>
> >> git://git.kernel.org/pub/scm/linux/kernel/git/colyli/mdadm.git
> >> 20220315-testing
> >>
> >> Then I will continue to test them.
> >>
> > Hi Coly,
> > This is great to see that something is happening in upstream :)
> >
> > I can see that you created branch where some patches were merged and
> > now you are reporting conflicts now. Our patches are based on last
> > mdadm commit (which is mdadm-4.2 ).
> > IMO you should try to apply them first on latest master and later
> > cherry-pick/ rebase them on top of your testing branch. This should
> > automatically resolve most of conflicts. Could you try that?
> 
> 
> The testing branch is updated to latest mdadm upstream. Indeed the 
> conflict is about blank line as I see, e.g. it removes some \t from 
> empty line, but such issue was removed in latest upstream.
> 
> Ineed I can fix the conflict, but I don't know how to make you update 
> the change from my side. Does it work if I sand you a diff of the
> patch?
> 
Hi Coly,
Resolving conflicts is a normal maintenance work. Just add you
sign-off and modify whatever is necessary, ofc. if are sure that it is
correct. If not, then ask owner to do that.
IMO you should send a note that you resolved something, we can
verify it ourselves in commit later.

Thanks,
Mariusz

Re: [PATCH 1/2] mdadm: Respect config file location in man.

[Lukasz Florczak]

Dear Paul,
Thanks for reviewing my patch. 

On Tue, 15 Mar 2022 13:39:25 +0100, Paul Menzel <pmenzel@molgen.mpg.de>
wrote:

> Dear Lukasz,
> 
> 
> Thank you for your patches.
> 
> Am 15.03.22 um 09:55 schrieb Lukasz Florczak:
> 
> It’d be great if you removed the dot/period at the end of the git
> commit message summaries [1]. (Also in second patch.)

Noted.

> 
> > Default config file location could differ depending on OS (e.g.
> > Debian family).  
> 
> What is it an Debian?

Could you elaborate?

> 
>  [...]  
> 
> Looks like an independent fix. Please separate into a separate commit.

It's just adding a missing option. I don't think that it deserves a
separate commit. How about I will update the commit body to include
this particular change?

> 
> > +"  create, auto                                 used when creating
> > device names in /dev\n" +"  homehost, policy, part-policy
> >      used to guide policy in various\n" +"
> >                      situations\n" +"\n"
> > +"For more details see mdadm.conf(5).\n"
> >   "\n"
> >   ;
> >   
> > diff --git a/mdadm.8.in b/mdadm.8.in
> > index be902dba..d41b3ca7 100644
> > --- a/mdadm.8.in
> > +++ b/mdadm.8.in
> > @@ -267,13 +267,13 @@ the exact meaning of this option in different
> > contexts. .TP
> >   .BR \-c ", " \-\-config=
> >   Specify the config file or directory.  Default is to use
> > -.B /etc/mdadm.conf
> > +.B {CONFFILE}
> >   and
> > -.BR /etc/mdadm.conf.d ,
> > +.BR {CONFFILE}.d ,
> >   or if those are missing then
> > -.B /etc/mdadm/mdadm.conf
> > +.B {CONFFILE2}
> >   and
> > -.BR /etc/mdadm/mdadm.conf.d .
> > +.BR {CONFFILE2}.d .
> >   If the config file given is
> >   .B "partitions"
> >   then nothing will be read, but
> > @@ -2009,9 +2009,9 @@ The config file is only used if explicitly
> > named with or requested with (a possibly implicit)
> >   .BR \-\-scan .
> >   In the later case,
> > -.B /etc/mdadm.conf
> > +.B {CONFFILE}
> >   or
> > -.B /etc/mdadm/mdadm.conf
> > +.B {CONFFILE2}
> >   is used.
> >   
> >   If
> > @@ -3339,7 +3339,7 @@ uses this to find arrays when
> >   is given in Misc mode, and to monitor array reconstruction
> >   on Monitor mode.
> >   
> > -.SS /etc/mdadm.conf
> > +.SS {CONFFILE} (or {CONFFILE2})
> >   
> >   The config file lists which devices may be scanned to see if
> >   they contain MD super block, and gives identifying information
> > @@ -3347,7 +3347,7 @@ they contain MD super block, and gives
> > identifying information .BR mdadm.conf (5)
> >   for more details.
> >   
> > -.SS /etc/mdadm.conf.d
> > +.SS {CONFFILE}.d (or {CONFFILE2}.d)
> >   
> >   A directory containing configuration files which are read in
> > lexical order.
> > diff --git a/mdadm.conf.5 b/mdadm.conf.5.in
> > similarity index 99%
> > rename from mdadm.conf.5
> > rename to mdadm.conf.5.in
> > index 74a21c5f..83edd008 100644
> > --- a/mdadm.conf.5
> > +++ b/mdadm.conf.5.in
> > @@ -8,7 +8,7 @@
> >   .SH NAME
> >   mdadm.conf \- configuration for management of Software RAID with
> > mdadm .SH SYNOPSIS
> > -/etc/mdadm.conf
> > +{CONFFILE}
> >   .SH DESCRIPTION
> >   .PP
> >   .I mdadm  
> 
> The rest looks good.
> 
> 
> Kind regards,
> 
> Paul
> 
> 
> [1]: https://chris.beams.io/posts/git-commit/

Regards,

Lukasz

Re: [PATCH 1/2] mdadm: Respect config file location in man.

[Paul Menzel]

Dear Lukasz,


Am 16.03.22 um 13:03 schrieb Lukasz Florczak:

> Thanks for reviewing my patch.

Thank you for your reply.

> On Tue, 15 Mar 2022 13:39:25 +0100, Paul Menzel wrote:

>> Am 15.03.22 um 09:55 schrieb Lukasz Florczak:
>>
>> It’d be great if you removed the dot/period at the end of the git
>> commit message summaries [1]. (Also in second patch.)
> 
> Noted.
> 
>>> Default config file location could differ depending on OS (e.g.
>>> Debian family).
>>
>> What is it an Debian?
> 
> Could you elaborate?

Sorry, I meant what is the location/path in Debian based systems, and 
what is the configured path before your patch.

>>   [...]
>>
>> Looks like an independent fix. Please separate into a separate commit.
> 
> It's just adding a missing option. I don't think that it deserves a
> separate commit. How about I will update the commit body to include
> this particular change?

Git makes it easy to handle small commits, so I favor two commits also 
because it wouldn’t be lost in case of a revert. But I have no say in 
the project, and opinions differ, so it’s just an opinion.

[…]


Kind regards,

Paul