[PATCH 0/2] Changes in Monitor

[PATCH 0/2] Changes in Monitor

[Blazej Kucman]

Hi, Jes, Coly
In first patch blocked is starting monitor when
--scan mode and MD devices list are combiened,
to prevent undefined behaviors.
Second patch containst monitor manual update.

Blazej Kucman (2):
  Monitor: block if monitor modes are combined.
  Update mdadm Monitor manual.

 Monitor.c  |  7 +++++-
 mdadm.8.in | 71 ++++++++++++++++++++++++++++++++++++++----------------
 2 files changed, 56 insertions(+), 22 deletions(-)

-- 
2.35.3

[PATCH 1/2] Monitor: block if monitor modes are combined.

[Blazej Kucman]

Block monitoring start if --scan mode and MD devices list are combined.

Signed-off-by: Blazej Kucman <blazej.kucman@intel.com>
Change-Id: Ic2b90662dbd297e8e2c8e88194155d65110ef517
---
 Monitor.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Monitor.c b/Monitor.c
index 7d7dc4d2..119e17d8 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -123,7 +123,7 @@ int Monitor(struct mddev_dev *devlist,
 	 *  and if we can get_disk_info and find a name
 	 *  Then we hot-remove and hot-add to the other array
 	 *
-	 * If devlist is NULL, then we can monitor everything because --scan
+	 * If devlist is NULL, then we can monitor everything if --scan
 	 * was given.  We get an initial list from config file and add anything
 	 * that appears in /proc/mdstat
 	 */
@@ -136,6 +136,11 @@ int Monitor(struct mddev_dev *devlist,
 	struct mddev_ident *mdlist;
 	int delay_for_event = c->delay;
 
+	if (devlist && c->scan) {
+		pr_err("Devices list and --scan option cannot be combined - not monitoring.\n");
+		return 1;
+	}
+
 	if (!mailaddr)
 		mailaddr = conf_get_mailaddr();
 
-- 
2.35.3

Re: [PATCH 1/2] Monitor: block if monitor modes are combined.

[Coly Li]

> 2022年12月19日 17:58，Blazej Kucman <blazej.kucman@intel.com> 写道：
> 
> Block monitoring start if --scan mode and MD devices list are combined.
> 
> Signed-off-by: Blazej Kucman <blazej.kucman@intel.com>
> Change-Id: Ic2b90662dbd297e8e2c8e88194155d65110ef517

I guess Change-ID is unnecessary for mdadm upstream?

Otherwise, it is fine to me,

Acked-by: Coly Li <colyli@suse.de <mailto:colyli@suse.de>>

Thanks.

Coly Li


> ---
> Monitor.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Monitor.c b/Monitor.c
> index 7d7dc4d2..119e17d8 100644
> --- a/Monitor.c
> +++ b/Monitor.c
> @@ -123,7 +123,7 @@ int Monitor(struct mddev_dev *devlist,
> *  and if we can get_disk_info and find a name
> *  Then we hot-remove and hot-add to the other array
> *
> - * If devlist is NULL, then we can monitor everything because --scan
> + * If devlist is NULL, then we can monitor everything if --scan
> * was given.  We get an initial list from config file and add anything
> * that appears in /proc/mdstat
> */
> @@ -136,6 +136,11 @@ int Monitor(struct mddev_dev *devlist,
> struct mddev_ident *mdlist;
> int delay_for_event = c->delay;
> 
> + if (devlist && c->scan) {
> + pr_err("Devices list and --scan option cannot be combined - not monitoring.\n");
> + return 1;
> + }
> +
> if (!mailaddr)
> mailaddr = conf_get_mailaddr();
> 
> -- 
> 2.35.3
> 

Re: [PATCH 1/2] Monitor: block if monitor modes are combined.

[Coly Li]

> 2022年12月19日 17:58，Blazej Kucman <blazej.kucman@intel.com> 写道：
> 
> Block monitoring start if --scan mode and MD devices list are combined.
> 
> Signed-off-by: Blazej Kucman <blazej.kucman@intel.com>
> Change-Id: Ic2b90662dbd297e8e2c8e88194155d65110ef517


I guess Change-ID is unnecessary for mdadm upstream?

Otherwise, it is fine to me,

Acked-by: Coly Li <colyli@suse.de>

Thanks.

Coly Li



> ---
> Monitor.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Monitor.c b/Monitor.c
> index 7d7dc4d2..119e17d8 100644
> --- a/Monitor.c
> +++ b/Monitor.c
> @@ -123,7 +123,7 @@ int Monitor(struct mddev_dev *devlist,
> *  and if we can get_disk_info and find a name
> *  Then we hot-remove and hot-add to the other array
> *
> - * If devlist is NULL, then we can monitor everything because --scan
> + * If devlist is NULL, then we can monitor everything if --scan
> * was given.  We get an initial list from config file and add anything
> * that appears in /proc/mdstat
> */
> @@ -136,6 +136,11 @@ int Monitor(struct mddev_dev *devlist,
> struct mddev_ident *mdlist;
> int delay_for_event = c->delay;
> 
> + if (devlist && c->scan) {
> + pr_err("Devices list and --scan option cannot be combined - not monitoring.\n");
> + return 1;
> + }
> +
> if (!mailaddr)
> mailaddr = conf_get_mailaddr();
> 
> -- 
> 2.35.3
> 

[PATCH 2/2] Update mdadm Monitor manual.

[Blazej Kucman]

- describe monitor work modes,
- clarify the turning off condition,
- describe the mdmonitor.service as a prefered management way.

Signed-off-by: Blazej Kucman <blazej.kucman@intel.com>
Change-Id: Id5a1d5e60b958954f48d3e0285dfeb0c6f54a9d4
---
 mdadm.8.in | 71 ++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 50 insertions(+), 21 deletions(-)

diff --git a/mdadm.8.in b/mdadm.8.in
index 70c79d1e..64f71ed1 100644
--- a/mdadm.8.in
+++ b/mdadm.8.in
@@ -2548,13 +2548,33 @@ Usage:
 .I options... devices...
 
 .PP
-This usage causes
+Monitor option can work in two modes:
+.IP \(bu 4
+system wide mode, follow all md devices based on
+.B /proc/mdstat,
+.IP \(bu 4
+follow only specified MD devices in command line.
+.PP
+
+.B \-\-scan -
+indicates system wide mode. Option causes the
+.I monitor
+to track all md devices that appear in
+.B /proc/mdstat.
+If it is not set, then at least one
+.B device
+must be specified.
+
+Monitor usage causes
 .I mdadm
 to periodically poll a number of md arrays and to report on any events
 noticed.
-.I mdadm
-will never exit once it decides that there are arrays to be checked,
-so it should normally be run in the background.
+
+In both modes,
+.I monitor
+will work as long as there is an active array with redundancy and it is defined to follow (for
+.B \-\-scan
+every array is followed).
 
 As well as reporting events,
 .I mdadm
@@ -2565,15 +2585,6 @@ or
 .B domain
 and if the destination array has a failed drive but no spares.
 
-If any devices are listed on the command line,
-.I mdadm
-will only monitor those devices, otherwise, all arrays listed in the
-configuration file will be monitored.  Further, if
-.B \-\-scan
-is given, then any other md devices that appear in
-.B /proc/mdstat
-will also be monitored.
-
 The result of monitoring the arrays is the generation of events.
 These events are passed to a separate program (if specified) and may
 be mailed to a given E-mail address.
@@ -2586,16 +2597,34 @@ device if relevant (such as a component device that has failed).
 
 If
 .B \-\-scan
-is given, then a program or an E-mail address must be specified on the
-command line or in the config file.  If neither are available, then
+is given, then a
+.B program
+or an
+.B e-mail
+address must be specified on the
+command line or in the config file. If neither are available, then
 .I mdadm
 will not monitor anything.
-Without
-.B \-\-scan,
-.I mdadm
-will continue monitoring as long as something was found to monitor.  If
-no program or email is given, then each event is reported to
-.BR stdout .
+For devices given directly in command line, without
+.B program
+or
+.B email
+specified, each event is reported to
+.BR stdout.
+
+Note: For systems where
+.If mdadm monitor
+is configured via systemd,
+.B mdmonitor(mdmonitor.service)
+should be configured. The service is designed to be primary solution for array monitoring,
+it is configured to work in system wide mode.
+It is automatically started and stopped according to current state and types of MD arrays in system.
+The service may require additional configuration, like
+.B e-mail
+or
+.B delay.
+That should be done in
+.B mdadm.conf.
 
 The different events are:
 
-- 
2.35.3

Re: [PATCH 2/2] Update mdadm Monitor manual.

[Coly Li]

> 2022年12月19日 17:58，Blazej Kucman <blazej.kucman@intel.com> 写道：
> 
> - describe monitor work modes,
> - clarify the turning off condition,
> - describe the mdmonitor.service as a prefered management way.
> 
> Signed-off-by: Blazej Kucman <blazej.kucman@intel.com>
> Change-Id: Id5a1d5e60b958954f48d3e0285dfeb0c6f54a9d4

Except for the Change-Id part, the patch itself is fine to me.

Acked-by: Coly Li <colyli@suse.de>

Thanks.

Coly Li



> ---
> mdadm.8.in | 71 ++++++++++++++++++++++++++++++++++++++----------------
> 1 file changed, 50 insertions(+), 21 deletions(-)
> 
> diff --git a/mdadm.8.in b/mdadm.8.in
> index 70c79d1e..64f71ed1 100644
> --- a/mdadm.8.in
> +++ b/mdadm.8.in
> @@ -2548,13 +2548,33 @@ Usage:
> .I options... devices...
> 
> .PP
> -This usage causes
> +Monitor option can work in two modes:
> +.IP \(bu 4
> +system wide mode, follow all md devices based on
> +.B /proc/mdstat,
> +.IP \(bu 4
> +follow only specified MD devices in command line.
> +.PP
> +
> +.B \-\-scan -
> +indicates system wide mode. Option causes the
> +.I monitor
> +to track all md devices that appear in
> +.B /proc/mdstat.
> +If it is not set, then at least one
> +.B device
> +must be specified.
> +
> +Monitor usage causes
> .I mdadm
> to periodically poll a number of md arrays and to report on any events
> noticed.
> -.I mdadm
> -will never exit once it decides that there are arrays to be checked,
> -so it should normally be run in the background.
> +
> +In both modes,
> +.I monitor
> +will work as long as there is an active array with redundancy and it is defined to follow (for
> +.B \-\-scan
> +every array is followed).
> 
> As well as reporting events,
> .I mdadm
> @@ -2565,15 +2585,6 @@ or
> .B domain
> and if the destination array has a failed drive but no spares.
> 
> -If any devices are listed on the command line,
> -.I mdadm
> -will only monitor those devices, otherwise, all arrays listed in the
> -configuration file will be monitored.  Further, if
> -.B \-\-scan
> -is given, then any other md devices that appear in
> -.B /proc/mdstat
> -will also be monitored.
> -
> The result of monitoring the arrays is the generation of events.
> These events are passed to a separate program (if specified) and may
> be mailed to a given E-mail address.
> @@ -2586,16 +2597,34 @@ device if relevant (such as a component device that has failed).
> 
> If
> .B \-\-scan
> -is given, then a program or an E-mail address must be specified on the
> -command line or in the config file.  If neither are available, then
> +is given, then a
> +.B program
> +or an
> +.B e-mail
> +address must be specified on the
> +command line or in the config file. If neither are available, then
> .I mdadm
> will not monitor anything.
> -Without
> -.B \-\-scan,
> -.I mdadm
> -will continue monitoring as long as something was found to monitor.  If
> -no program or email is given, then each event is reported to
> -.BR stdout .
> +For devices given directly in command line, without
> +.B program
> +or
> +.B email
> +specified, each event is reported to
> +.BR stdout.
> +
> +Note: For systems where
> +.If mdadm monitor
> +is configured via systemd,
> +.B mdmonitor(mdmonitor.service)
> +should be configured. The service is designed to be primary solution for array monitoring,
> +it is configured to work in system wide mode.
> +It is automatically started and stopped according to current state and types of MD arrays in system.
> +The service may require additional configuration, like
> +.B e-mail
> +or
> +.B delay.
> +That should be done in
> +.B mdadm.conf.
> 
> The different events are:
> 
> -- 
> 2.35.3
>