All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/8] Mdmonitor refactor and udev event handling improvements
@ 2023-01-19 13:30 Mateusz Grzonka
  2023-01-19 13:30 ` [PATCH 1/8] Mdmonitor: Make alert_info global Mateusz Grzonka
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Mateusz Grzonka @ 2023-01-19 13:30 UTC (permalink / raw)
  To: linux-raid; +Cc: jes

Along the way we observed many problems with current approach to event handling in mdmonitor.
It frequently doesn't report Fail and DeviceDisappeared events.
It's due to time races with udev, and too long delay in some cases.
While there was a patch intending to address time races with udev, it didn't remove them completely.
This patch series presents alternative approach, where mdmonitor wakes up on udev events, so that
there should be no more conflicts with udev we saw before.

Additionally some code quality improvements were done, to make the code more maintainable.

v2:
Fixed mismatched comment style and rebased onto master.

Mateusz Grzonka (8):
  Mdmonitor: Make alert_info global
  Mdmonitor: Pass events to alert() using enums instead of strings
  Mdmonitor: Add helper functions
  Add helpers to determine whether directories or files are soft links
  Mdmonitor: Refactor write_autorebuild_pid()
  Mdmonitor: Refactor check_one_sharer() for better error handling
  Mdmonitor: Improve udev event handling
  udev: Move udev_block() and udev_unblock() into udev.c

 Create.c  |   1 +
 Makefile  |   3 +-
 Manage.c  |   3 +-
 Monitor.c | 623 +++++++++++++++++++++++++++++++++---------------------
 lib.c     |  42 ----
 mdadm.h   |   5 +-
 mdopen.c  |  19 +-
 udev.c    | 191 +++++++++++++++++
 udev.h    |  38 ++++
 util.c    |  45 ++++
 10 files changed, 668 insertions(+), 302 deletions(-)
 create mode 100644 udev.c
 create mode 100644 udev.h

-- 
2.26.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/8] Mdmonitor: Make alert_info global
  2023-01-19 13:30 [PATCH v2 0/8] Mdmonitor refactor and udev event handling improvements Mateusz Grzonka
@ 2023-01-19 13:30 ` Mateusz Grzonka
  2023-01-19 13:30 ` [PATCH 2/8] Mdmonitor: Pass events to alert() using enums instead of strings Mateusz Grzonka
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Mateusz Grzonka @ 2023-01-19 13:30 UTC (permalink / raw)
  To: linux-raid; +Cc: jes

Move information about --test flag and hostname into alert_info.

Signed-off-by: Mateusz Grzonka <mateusz.grzonka@intel.com>
---
 Monitor.c | 124 +++++++++++++++++++++++++++---------------------------
 1 file changed, 61 insertions(+), 63 deletions(-)

diff --git a/Monitor.c b/Monitor.c
index 188cb8be..9ef4dab8 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -58,21 +58,20 @@ struct state {
 };
 
 struct alert_info {
+	char hostname[HOST_NAME_MAX];
 	char *mailaddr;
 	char *mailfrom;
 	char *alert_cmd;
 	int dosyslog;
-};
+	int test;
+} info;
 static int make_daemon(char *pidfile);
 static int check_one_sharer(int scan);
 static void write_autorebuild_pid(void);
-static void alert(const char *event, const char *dev, const char *disc, struct alert_info *info);
-static int check_array(struct state *st, struct mdstat_ent *mdstat,
-		       int test, struct alert_info *info,
-		       int increments, char *prefer);
-static int add_new_arrays(struct mdstat_ent *mdstat, struct state **statelist,
-			  int test, struct alert_info *info);
-static void try_spare_migration(struct state *statelist, struct alert_info *info);
+static void alert(const char *event, const char *dev, const char *disc);
+static int check_array(struct state *st, struct mdstat_ent *mdstat, int increments, char *prefer);
+static int add_new_arrays(struct mdstat_ent *mdstat, struct state **statelist);
+static void try_spare_migration(struct state *statelist);
 static void link_containers_with_subarrays(struct state *list);
 static void free_statelist(struct state *statelist);
 #ifndef NO_LIBUDEV
@@ -132,7 +131,6 @@ int Monitor(struct mddev_dev *devlist,
 	int finished = 0;
 	struct mdstat_ent *mdstat = NULL;
 	char *mailfrom;
-	struct alert_info info;
 	struct mddev_ident *mdlist;
 	int delay_for_event = c->delay;
 
@@ -166,6 +164,13 @@ int Monitor(struct mddev_dev *devlist,
 	info.mailaddr = mailaddr;
 	info.mailfrom = mailfrom;
 	info.dosyslog = dosyslog;
+	info.test = c->test;
+
+	if (gethostname(info.hostname, sizeof(info.hostname)) != 0) {
+		pr_err("Cannot get hostname.\n");
+		return 1;
+	}
+	info.hostname[sizeof(info.hostname) - 1] = '\0';
 
 	if (share){
 		if (check_one_sharer(c->scan))
@@ -241,8 +246,7 @@ int Monitor(struct mddev_dev *devlist,
 		mdstat = mdstat_read(oneshot ? 0 : 1, 0);
 
 		for (st = statelist; st; st = st->next) {
-			if (check_array(st, mdstat, c->test, &info,
-					increments, c->prefer))
+			if (check_array(st, mdstat, increments, c->prefer))
 				anydegraded = 1;
 			/* for external arrays, metadata is filled for
 			 * containers only
@@ -255,15 +259,14 @@ int Monitor(struct mddev_dev *devlist,
 
 		/* now check if there are any new devices found in mdstat */
 		if (c->scan)
-			new_found = add_new_arrays(mdstat, &statelist, c->test,
-						   &info);
+			new_found = add_new_arrays(mdstat, &statelist);
 
 		/* If an array has active < raid && spare == 0 && spare_group != NULL
 		 * Look for another array with spare > 0 and active == raid and same spare_group
 		 * if found, choose a device and hotremove/hotadd
 		 */
 		if (share && anydegraded)
-			try_spare_migration(statelist, &info);
+			try_spare_migration(statelist);
 		if (!new_found) {
 			if (oneshot)
 				break;
@@ -294,7 +297,7 @@ int Monitor(struct mddev_dev *devlist,
 				mdstat_close();
 			}
 		}
-		c->test = 0;
+		info.test = 0;
 
 		for (stp = &statelist; (st = *stp) != NULL; ) {
 			if (st->from_auto && st->err > 5) {
@@ -412,7 +415,7 @@ static void write_autorebuild_pid()
 	}
 }
 
-static void execute_alert_cmd(const char *event, const char *dev, const char *disc, struct alert_info *info)
+static void execute_alert_cmd(const char *event, const char *dev, const char *disc)
 {
 	int pid = fork();
 
@@ -424,15 +427,14 @@ static void execute_alert_cmd(const char *event, const char *dev, const char *di
 		pr_err("Cannot fork to execute alert command");
 		break;
 	case 0:
-		execl(info->alert_cmd, info->alert_cmd, event, dev, disc, NULL);
+		execl(info.alert_cmd, info.alert_cmd, event, dev, disc, NULL);
 		exit(2);
 	}
 }
 
-static void send_event_email(const char *event, const char *dev, const char *disc, struct alert_info *info)
+static void send_event_email(const char *event, const char *dev, const char *disc)
 {
 	FILE *mp, *mdstat;
-	char hname[256];
 	char buf[BUFSIZ];
 	int n;
 
@@ -442,14 +444,13 @@ static void send_event_email(const char *event, const char *dev, const char *dis
 		return;
 	}
 
-	gethostname(hname, sizeof(hname));
 	signal(SIGPIPE, SIG_IGN);
-	if (info->mailfrom)
-		fprintf(mp, "From: %s\n", info->mailfrom);
+	if (info.mailfrom)
+		fprintf(mp, "From: %s\n", info.mailfrom);
 	else
 		fprintf(mp, "From: %s monitoring <root>\n", Name);
-	fprintf(mp, "To: %s\n", info->mailaddr);
-	fprintf(mp, "Subject: %s event on %s:%s\n\n", event, dev, hname);
+	fprintf(mp, "To: %s\n", info.mailaddr);
+	fprintf(mp, "Subject: %s event on %s:%s\n\n", event, dev, info.hostname);
 	fprintf(mp, "This is an automatically generated mail message. \n");
 	fprintf(mp, "A %s event had been detected on md device %s.\n\n", event, dev);
 
@@ -501,37 +502,36 @@ static void log_event_to_syslog(const char *event, const char *dev, const char *
 		syslog(priority, "%s event detected on md device %s", event, dev);
 }
 
-static void alert(const char *event, const char *dev, const char *disc, struct alert_info *info)
+static void alert(const char *event, const char *dev, const char *disc)
 {
-	if (!info->alert_cmd && !info->mailaddr && !info->dosyslog) {
+	if (!info.alert_cmd && !info.mailaddr && !info.dosyslog) {
 		time_t now = time(0);
 
 		printf("%1.15s: %s on %s %s\n", ctime(&now) + 4,
 		       event, dev, disc?disc:"unknown device");
 	}
-	if (info->alert_cmd)
-		execute_alert_cmd(event, dev, disc, info);
+	if (info.alert_cmd)
+		execute_alert_cmd(event, dev, disc);
 
-	if (info->mailaddr && (strncmp(event, "Fail", 4) == 0 ||
+	if (info.mailaddr && (strncmp(event, "Fail", 4) == 0 ||
 			       strncmp(event, "Test", 4) == 0 ||
 			       strncmp(event, "Spares", 6) == 0 ||
 			       strncmp(event, "Degrade", 7) == 0)) {
-		send_event_email(event, dev, disc, info);
+		send_event_email(event, dev, disc);
 	}
 
-	if (info->dosyslog)
+	if (info.dosyslog)
 		log_event_to_syslog(event, dev, disc);
 }
 
 static int check_array(struct state *st, struct mdstat_ent *mdstat,
-		       int test, struct alert_info *ainfo,
 		       int increments, char *prefer)
 {
 	/* Update the state 'st' to reflect any changes shown in mdstat,
 	 * or found by directly examining the array, and return
 	 * '1' if the array is degraded, or '0' if it is optimal (or dead).
 	 */
-	struct { int state, major, minor; } info[MAX_DISKS];
+	struct { int state, major, minor; } disks_info[MAX_DISKS];
 	struct mdinfo *sra = NULL;
 	mdu_array_info_t array;
 	struct mdstat_ent *mse = NULL, *mse2;
@@ -545,8 +545,8 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 	int is_container = 0;
 	unsigned long redundancy_only_flags = 0;
 
-	if (test)
-		alert("TestMessage", dev, NULL, ainfo);
+	if (info.test)
+		alert("TestMessage", dev, NULL);
 
 	retval = 0;
 
@@ -595,7 +595,7 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 	 */
 	if (sra->array.level == 0 || sra->array.level == -1) {
 		if (!st->err && !st->from_config)
-			alert("DeviceDisappeared", dev, " Wrong-Level", ainfo);
+			alert("DeviceDisappeared", dev, " Wrong-Level");
 		st->err++;
 		goto out;
 	}
@@ -612,7 +612,7 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 		st->percent = RESYNC_NONE;
 		new_array = 1;
 		if (!is_container)
-			alert("NewArray", st->devname, NULL, ainfo);
+			alert("NewArray", st->devname, NULL);
 	}
 
 	if (st->utime == array.utime && st->failed == sra->array.failed_disks &&
@@ -625,14 +625,14 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 	}
 	if (st->utime == 0 && /* new array */
 	    mse->pattern && strchr(mse->pattern, '_') /* degraded */)
-		alert("DegradedArray", dev, NULL, ainfo);
+		alert("DegradedArray", dev, NULL);
 
 	if (st->utime == 0 && /* new array */ st->expected_spares > 0 &&
 	    sra->array.spare_disks < st->expected_spares)
-		alert("SparesMissing", dev, NULL, ainfo);
+		alert("SparesMissing", dev, NULL);
 	if (st->percent < 0 && st->percent != RESYNC_UNKNOWN &&
 	    mse->percent >= 0)
-		alert("RebuildStarted", dev, NULL, ainfo);
+		alert("RebuildStarted", dev, NULL);
 	if (st->percent >= 0 && mse->percent >= 0 &&
 	    (mse->percent / increments) > (st->percent / increments)) {
 		char percentalert[18];
@@ -647,7 +647,7 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 			snprintf(percentalert, sizeof(percentalert),
 				 "Rebuild%02d", mse->percent);
 
-		alert(percentalert, dev, NULL, ainfo);
+		alert(percentalert, dev, NULL);
 	}
 
 	if (mse->percent == RESYNC_NONE && st->percent >= 0) {
@@ -660,9 +660,9 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 			snprintf(cnt, sizeof(cnt),
 				 " mismatches found: %d (on raid level %d)",
 				 sra->mismatch_cnt, sra->array.level);
-			alert("RebuildFinished", dev, cnt, ainfo);
+			alert("RebuildFinished", dev, cnt);
 		} else
-			alert("RebuildFinished", dev, NULL, ainfo);
+			alert("RebuildFinished", dev, NULL);
 	}
 	st->percent = mse->percent;
 
@@ -671,13 +671,13 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 		mdu_disk_info_t disc;
 		disc.number = i;
 		if (md_get_disk_info(fd, &disc) >= 0) {
-			info[i].state = disc.state;
-			info[i].major = disc.major;
-			info[i].minor = disc.minor;
+			disks_info[i].state = disc.state;
+			disks_info[i].major = disc.major;
+			disks_info[i].minor = disc.minor;
 			if (disc.major || disc.minor)
 				remaining_disks --;
 		} else
-			info[i].major = info[i].minor = 0;
+			disks_info[i].major = disks_info[i].minor = 0;
 	}
 	last_disk = i;
 
@@ -700,13 +700,13 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 		int change;
 		char *dv = NULL;
 		disc.number = i;
-		if (i < last_disk && (info[i].major || info[i].minor)) {
-			newstate = info[i].state;
-			dv = map_dev_preferred(info[i].major, info[i].minor, 1,
+		if (i < last_disk && (disks_info[i].major || disks_info[i].minor)) {
+			newstate = disks_info[i].state;
+			dv = map_dev_preferred(disks_info[i].major, disks_info[i].minor, 1,
 					       prefer);
 			disc.state = newstate;
-			disc.major = info[i].major;
-			disc.minor = info[i].minor;
+			disc.major = disks_info[i].major;
+			disc.minor = disks_info[i].minor;
 		} else
 			newstate = (1 << MD_DISK_REMOVED);
 
@@ -716,14 +716,14 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 		change = newstate ^ st->devstate[i];
 		if (st->utime && change && !st->err && !new_array) {
 			if ((st->devstate[i]&change) & (1 << MD_DISK_SYNC))
-				alert("Fail", dev, dv, ainfo);
+				alert("Fail", dev, dv);
 			else if ((newstate & (1 << MD_DISK_FAULTY)) &&
 				 (disc.major || disc.minor) &&
 				 st->devid[i] == makedev(disc.major,
 							 disc.minor))
-				alert("FailSpare", dev, dv, ainfo);
+				alert("FailSpare", dev, dv);
 			else if ((newstate&change) & (1 << MD_DISK_SYNC))
-				alert("SpareActive", dev, dv, ainfo);
+				alert("SpareActive", dev, dv);
 		}
 		st->devstate[i] = newstate;
 		st->devid[i] = makedev(disc.major, disc.minor);
@@ -747,13 +747,12 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 
  disappeared:
 	if (!st->err && !is_container)
-		alert("DeviceDisappeared", dev, NULL, ainfo);
+		alert("DeviceDisappeared", dev, NULL);
 	st->err++;
 	goto out;
 }
 
-static int add_new_arrays(struct mdstat_ent *mdstat, struct state **statelist,
-			  int test, struct alert_info *info)
+static int add_new_arrays(struct mdstat_ent *mdstat, struct state **statelist)
 {
 	struct mdstat_ent *mse;
 	int new_found = 0;
@@ -806,8 +805,8 @@ static int add_new_arrays(struct mdstat_ent *mdstat, struct state **statelist,
 			} else
 				st->parent_devnm[0] = 0;
 			*statelist = st;
-			if (test)
-				alert("TestMessage", st->devname, NULL, info);
+			if (info.test)
+				alert("TestMessage", st->devname, NULL);
 			new_found = 1;
 		}
 	return new_found;
@@ -971,7 +970,7 @@ static dev_t container_choose_spare(struct state *from, struct state *to,
 	return dev;
 }
 
-static void try_spare_migration(struct state *statelist, struct alert_info *info)
+static void try_spare_migration(struct state *statelist)
 {
 	struct state *from;
 	struct state *st;
@@ -1030,8 +1029,7 @@ static void try_spare_migration(struct state *statelist, struct alert_info *info
 				if (devid > 0 &&
 				    move_spare(from->devname, to->devname,
 					       devid)) {
-					alert("MoveSpare", to->devname,
-					      from->devname, info);
+					alert("MoveSpare", to->devname, from->devname);
 					break;
 				}
 			}
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/8] Mdmonitor: Pass events to alert() using enums instead of strings
  2023-01-19 13:30 [PATCH v2 0/8] Mdmonitor refactor and udev event handling improvements Mateusz Grzonka
  2023-01-19 13:30 ` [PATCH 1/8] Mdmonitor: Make alert_info global Mateusz Grzonka
@ 2023-01-19 13:30 ` Mateusz Grzonka
  2023-01-19 13:30 ` [PATCH 3/8] Mdmonitor: Add helper functions Mateusz Grzonka
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Mateusz Grzonka @ 2023-01-19 13:30 UTC (permalink / raw)
  To: linux-raid; +Cc: jes

Add events enum, and mapping_t struct, that maps them to strings, so
that enums are passed around instead of strings.

Signed-off-by: Mateusz Grzonka <mateusz.grzonka@intel.com>
---
 Monitor.c | 136 +++++++++++++++++++++++++++++++++---------------------
 1 file changed, 83 insertions(+), 53 deletions(-)

diff --git a/Monitor.c b/Monitor.c
index 9ef4dab8..029e9efd 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -32,6 +32,8 @@
 #include	<libudev.h>
 #endif
 
+#define EVENT_NAME_MAX 32
+
 struct state {
 	char devname[MD_NAME_MAX + sizeof("/dev/md/")];	/* length of "/dev/md/" + device name + terminating byte*/
 	char devnm[MD_NAME_MAX];	/* to sync with mdstat info */
@@ -65,10 +67,43 @@ struct alert_info {
 	int dosyslog;
 	int test;
 } info;
+
+enum event {
+	EVENT_SPARE_ACTIVE = 0,
+	EVENT_NEW_ARRAY,
+	EVENT_MOVE_SPARE,
+	EVENT_TEST_MESSAGE,
+	EVENT_REBUILD_STARTED,
+	EVENT_REBUILD,
+	EVENT_REBUILD_FINISHED,
+	EVENT_SPARES_MISSING,
+	EVENT_DEVICE_DISAPPEARED,
+	EVENT_FAIL,
+	EVENT_FAIL_SPARE,
+	EVENT_DEGRADED_ARRAY,
+	EVENT_UNKNOWN
+};
+
+mapping_t events_map[] = {
+	{"SpareActive", EVENT_SPARE_ACTIVE},
+	{"NewArray", EVENT_NEW_ARRAY},
+	{"MoveSpare", EVENT_MOVE_SPARE},
+	{"TestMessage", EVENT_TEST_MESSAGE},
+	{"RebuildStarted", EVENT_REBUILD_STARTED},
+	{"Rebuild", EVENT_REBUILD},
+	{"RebuildFinished", EVENT_REBUILD_FINISHED},
+	{"SparesMissing", EVENT_SPARES_MISSING},
+	{"DeviceDisappeared", EVENT_DEVICE_DISAPPEARED},
+	{"Fail", EVENT_FAIL},
+	{"FailSpare", EVENT_FAIL_SPARE},
+	{"DegradedArray", EVENT_DEGRADED_ARRAY},
+	{NULL, EVENT_UNKNOWN}
+};
+
 static int make_daemon(char *pidfile);
 static int check_one_sharer(int scan);
 static void write_autorebuild_pid(void);
-static void alert(const char *event, const char *dev, const char *disc);
+static void alert(const enum event event_enum, const unsigned int progress, const char *dev, const char *disc);
 static int check_array(struct state *st, struct mdstat_ent *mdstat, int increments, char *prefer);
 static int add_new_arrays(struct mdstat_ent *mdstat, struct state **statelist);
 static void try_spare_migration(struct state *statelist);
@@ -415,7 +450,7 @@ static void write_autorebuild_pid()
 	}
 }
 
-static void execute_alert_cmd(const char *event, const char *dev, const char *disc)
+static void execute_alert_cmd(const char *event_name, const char *dev, const char *disc)
 {
 	int pid = fork();
 
@@ -427,12 +462,12 @@ static void execute_alert_cmd(const char *event, const char *dev, const char *di
 		pr_err("Cannot fork to execute alert command");
 		break;
 	case 0:
-		execl(info.alert_cmd, info.alert_cmd, event, dev, disc, NULL);
+		execl(info.alert_cmd, info.alert_cmd, event_name, dev, disc, NULL);
 		exit(2);
 	}
 }
 
-static void send_event_email(const char *event, const char *dev, const char *disc)
+static void send_event_email(const char *event_name, const char *dev, const char *disc)
 {
 	FILE *mp, *mdstat;
 	char buf[BUFSIZ];
@@ -450,9 +485,9 @@ static void send_event_email(const char *event, const char *dev, const char *dis
 	else
 		fprintf(mp, "From: %s monitoring <root>\n", Name);
 	fprintf(mp, "To: %s\n", info.mailaddr);
-	fprintf(mp, "Subject: %s event on %s:%s\n\n", event, dev, info.hostname);
+	fprintf(mp, "Subject: %s event on %s:%s\n\n", event_name, dev, info.hostname);
 	fprintf(mp, "This is an automatically generated mail message. \n");
-	fprintf(mp, "A %s event had been detected on md device %s.\n\n", event, dev);
+	fprintf(mp, "A %s event had been detected on md device %s.\n\n", event_name, dev);
 
 	if (disc && disc[0] != ' ')
 		fprintf(mp,
@@ -474,20 +509,20 @@ static void send_event_email(const char *event, const char *dev, const char *dis
 	pclose(mp);
 }
 
-static void log_event_to_syslog(const char *event, const char *dev, const char *disc)
+static void log_event_to_syslog(const enum event event_enum, const char *event_name, const char *dev, const char *disc)
 {
 	int priority;
 	/* Log at a different severity depending on the event.
 	 *
 	 * These are the critical events:  */
-	if (strncmp(event, "Fail", 4) == 0 ||
-		strncmp(event, "Degrade", 7) == 0 ||
-		strncmp(event, "DeviceDisappeared", 17) == 0)
+	if (event_enum == EVENT_FAIL ||
+	    event_enum == EVENT_DEGRADED_ARRAY ||
+	    event_enum == EVENT_DEVICE_DISAPPEARED)
 		priority = LOG_CRIT;
 	/* Good to know about, but are not failures: */
-	else if (strncmp(event, "Rebuild", 7) == 0 ||
-			strncmp(event, "MoveSpare", 9) == 0 ||
-			strncmp(event, "Spares", 6) != 0)
+	else if (event_enum == EVENT_REBUILD ||
+		 event_enum == EVENT_MOVE_SPARE ||
+		 event_enum == EVENT_SPARES_MISSING)
 		priority = LOG_WARNING;
 	/* Everything else: */
 	else
@@ -495,33 +530,37 @@ static void log_event_to_syslog(const char *event, const char *dev, const char *
 
 	if (disc && disc[0] != ' ')
 		syslog(priority,
-			"%s event detected on md device %s, component device %s", event, dev, disc);
+		       "%s event detected on md device %s, component device %s",
+		       event_name, dev, disc);
 	else if (disc)
-		syslog(priority, "%s event detected on md device %s: %s", event, dev, disc);
+		syslog(priority, "%s event detected on md device %s: %s", event_name, dev, disc);
 	else
-		syslog(priority, "%s event detected on md device %s", event, dev);
+		syslog(priority, "%s event detected on md device %s", event_name, dev);
 }
 
-static void alert(const char *event, const char *dev, const char *disc)
+static void alert(const enum event event_enum, const unsigned int progress, const char *dev, const char *disc)
 {
-	if (!info.alert_cmd && !info.mailaddr && !info.dosyslog) {
-		time_t now = time(0);
+	char event_name[EVENT_NAME_MAX];
 
-		printf("%1.15s: %s on %s %s\n", ctime(&now) + 4,
-		       event, dev, disc?disc:"unknown device");
+	if (event_enum == EVENT_REBUILD) {
+		snprintf(event_name, sizeof(event_name), "%s%02d",
+			 map_num_s(events_map, EVENT_REBUILD), progress);
+	} else {
+		snprintf(event_name, sizeof(event_name), "%s", map_num_s(events_map, event_enum));
 	}
+
 	if (info.alert_cmd)
-		execute_alert_cmd(event, dev, disc);
+		execute_alert_cmd(event_name, dev, disc);
 
-	if (info.mailaddr && (strncmp(event, "Fail", 4) == 0 ||
-			       strncmp(event, "Test", 4) == 0 ||
-			       strncmp(event, "Spares", 6) == 0 ||
-			       strncmp(event, "Degrade", 7) == 0)) {
-		send_event_email(event, dev, disc);
+	if (info.mailaddr && (event_enum == EVENT_FAIL ||
+			      event_enum == EVENT_TEST_MESSAGE ||
+			      event_enum == EVENT_SPARES_MISSING ||
+			      event_enum == EVENT_DEGRADED_ARRAY)) {
+		send_event_email(event_name, dev, disc);
 	}
 
 	if (info.dosyslog)
-		log_event_to_syslog(event, dev, disc);
+		log_event_to_syslog(event_enum, event_name, dev, disc);
 }
 
 static int check_array(struct state *st, struct mdstat_ent *mdstat,
@@ -546,7 +585,7 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 	unsigned long redundancy_only_flags = 0;
 
 	if (info.test)
-		alert("TestMessage", dev, NULL);
+		alert(EVENT_TEST_MESSAGE, 0, dev, NULL);
 
 	retval = 0;
 
@@ -595,7 +634,7 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 	 */
 	if (sra->array.level == 0 || sra->array.level == -1) {
 		if (!st->err && !st->from_config)
-			alert("DeviceDisappeared", dev, " Wrong-Level");
+			alert(EVENT_DEVICE_DISAPPEARED, 0, dev, " Wrong-Level");
 		st->err++;
 		goto out;
 	}
@@ -612,7 +651,7 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 		st->percent = RESYNC_NONE;
 		new_array = 1;
 		if (!is_container)
-			alert("NewArray", st->devname, NULL);
+			alert(EVENT_NEW_ARRAY, 0, st->devname, NULL);
 	}
 
 	if (st->utime == array.utime && st->failed == sra->array.failed_disks &&
@@ -625,29 +664,20 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 	}
 	if (st->utime == 0 && /* new array */
 	    mse->pattern && strchr(mse->pattern, '_') /* degraded */)
-		alert("DegradedArray", dev, NULL);
+		alert(EVENT_DEGRADED_ARRAY, 0, dev, NULL);
 
 	if (st->utime == 0 && /* new array */ st->expected_spares > 0 &&
 	    sra->array.spare_disks < st->expected_spares)
-		alert("SparesMissing", dev, NULL);
+		alert(EVENT_SPARES_MISSING, 0, dev, NULL);
 	if (st->percent < 0 && st->percent != RESYNC_UNKNOWN &&
 	    mse->percent >= 0)
-		alert("RebuildStarted", dev, NULL);
+		alert(EVENT_REBUILD_STARTED, 0, dev, NULL);
 	if (st->percent >= 0 && mse->percent >= 0 &&
 	    (mse->percent / increments) > (st->percent / increments)) {
-		char percentalert[18];
-		/*
-		 * "RebuildNN" (10 chars) or "RebuildStarted" (15 chars)
-		 */
-
 		if((mse->percent / increments) == 0)
-			snprintf(percentalert, sizeof(percentalert),
-				 "RebuildStarted");
+			alert(EVENT_REBUILD_STARTED, 0, dev, NULL);
 		else
-			snprintf(percentalert, sizeof(percentalert),
-				 "Rebuild%02d", mse->percent);
-
-		alert(percentalert, dev, NULL);
+			alert(EVENT_REBUILD, mse->percent, dev, NULL);
 	}
 
 	if (mse->percent == RESYNC_NONE && st->percent >= 0) {
@@ -660,9 +690,9 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 			snprintf(cnt, sizeof(cnt),
 				 " mismatches found: %d (on raid level %d)",
 				 sra->mismatch_cnt, sra->array.level);
-			alert("RebuildFinished", dev, cnt);
+			alert(EVENT_REBUILD_FINISHED, 0, dev, cnt);
 		} else
-			alert("RebuildFinished", dev, NULL);
+			alert(EVENT_REBUILD_FINISHED, 0, dev, NULL);
 	}
 	st->percent = mse->percent;
 
@@ -716,14 +746,14 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 		change = newstate ^ st->devstate[i];
 		if (st->utime && change && !st->err && !new_array) {
 			if ((st->devstate[i]&change) & (1 << MD_DISK_SYNC))
-				alert("Fail", dev, dv);
+				alert(EVENT_FAIL, 0, dev, dv);
 			else if ((newstate & (1 << MD_DISK_FAULTY)) &&
 				 (disc.major || disc.minor) &&
 				 st->devid[i] == makedev(disc.major,
 							 disc.minor))
-				alert("FailSpare", dev, dv);
+				alert(EVENT_FAIL_SPARE, 0, dev, dv);
 			else if ((newstate&change) & (1 << MD_DISK_SYNC))
-				alert("SpareActive", dev, dv);
+				alert(EVENT_SPARE_ACTIVE, 0, dev, dv);
 		}
 		st->devstate[i] = newstate;
 		st->devid[i] = makedev(disc.major, disc.minor);
@@ -747,7 +777,7 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 
  disappeared:
 	if (!st->err && !is_container)
-		alert("DeviceDisappeared", dev, NULL);
+		alert(EVENT_DEVICE_DISAPPEARED, 0, dev, NULL);
 	st->err++;
 	goto out;
 }
@@ -806,7 +836,7 @@ static int add_new_arrays(struct mdstat_ent *mdstat, struct state **statelist)
 				st->parent_devnm[0] = 0;
 			*statelist = st;
 			if (info.test)
-				alert("TestMessage", st->devname, NULL);
+				alert(EVENT_TEST_MESSAGE, 0, st->devname, NULL);
 			new_found = 1;
 		}
 	return new_found;
@@ -1029,7 +1059,7 @@ static void try_spare_migration(struct state *statelist)
 				if (devid > 0 &&
 				    move_spare(from->devname, to->devname,
 					       devid)) {
-					alert("MoveSpare", to->devname, from->devname);
+					alert(EVENT_MOVE_SPARE, 0, to->devname, from->devname);
 					break;
 				}
 			}
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/8] Mdmonitor: Add helper functions
  2023-01-19 13:30 [PATCH v2 0/8] Mdmonitor refactor and udev event handling improvements Mateusz Grzonka
  2023-01-19 13:30 ` [PATCH 1/8] Mdmonitor: Make alert_info global Mateusz Grzonka
  2023-01-19 13:30 ` [PATCH 2/8] Mdmonitor: Pass events to alert() using enums instead of strings Mateusz Grzonka
@ 2023-01-19 13:30 ` Mateusz Grzonka
  2023-01-19 13:30 ` [PATCH 4/8] Add helpers to determine whether directories or files are soft links Mateusz Grzonka
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Mateusz Grzonka @ 2023-01-19 13:30 UTC (permalink / raw)
  To: linux-raid; +Cc: jes

Add functions:
- is_email_event(),
- get_syslog_event_priority(),
- sprint_event_message(),
with kernel style comments containing more detailed descriptions.

Also update event syslog priorities to be consistent with man. MoveSpare event was described in man as priority info, while implemented as warning. Move event data into a struct, so that it is passed between different functions if needed.
Sort function declarations alphabetically and remove redundant alert() declaration.

Signed-off-by: Mateusz Grzonka <mateusz.grzonka@intel.com>
---
 Monitor.c | 228 +++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 158 insertions(+), 70 deletions(-)

diff --git a/Monitor.c b/Monitor.c
index 029e9efd..39598ba0 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -73,10 +73,12 @@ enum event {
 	EVENT_NEW_ARRAY,
 	EVENT_MOVE_SPARE,
 	EVENT_TEST_MESSAGE,
+	__SYSLOG_PRIORITY_WARNING,
 	EVENT_REBUILD_STARTED,
 	EVENT_REBUILD,
 	EVENT_REBUILD_FINISHED,
 	EVENT_SPARES_MISSING,
+	__SYSLOG_PRIORITY_CRITICAL,
 	EVENT_DEVICE_DISAPPEARED,
 	EVENT_FAIL,
 	EVENT_FAIL_SPARE,
@@ -100,18 +102,31 @@ mapping_t events_map[] = {
 	{NULL, EVENT_UNKNOWN}
 };
 
-static int make_daemon(char *pidfile);
-static int check_one_sharer(int scan);
-static void write_autorebuild_pid(void);
-static void alert(const enum event event_enum, const unsigned int progress, const char *dev, const char *disc);
-static int check_array(struct state *st, struct mdstat_ent *mdstat, int increments, char *prefer);
+struct event_data {
+	enum event event_enum;
+	/*
+	 * @event_name: Rebuild event name must be in form "RebuildXX", where XX is rebuild progress.
+	 */
+	char event_name[EVENT_NAME_MAX];
+	char message[BUFSIZ];
+	const char *description;
+	const char *dev;
+	const char *disc;
+};
+
 static int add_new_arrays(struct mdstat_ent *mdstat, struct state **statelist);
 static void try_spare_migration(struct state *statelist);
 static void link_containers_with_subarrays(struct state *list);
 static void free_statelist(struct state *statelist);
+static int check_array(struct state *st, struct mdstat_ent *mdstat, int increments, char *prefer);
+static int check_one_sharer(int scan);
 #ifndef NO_LIBUDEV
 static int check_udev_activity(void);
 #endif
+static void link_containers_with_subarrays(struct state *list);
+static int make_daemon(char *pidfile);
+static void try_spare_migration(struct state *statelist);
+static void write_autorebuild_pid(void);
 
 int Monitor(struct mddev_dev *devlist,
 	    char *mailaddr, char *alert_cmd,
@@ -450,7 +465,80 @@ static void write_autorebuild_pid()
 	}
 }
 
-static void execute_alert_cmd(const char *event_name, const char *dev, const char *disc)
+#define BASE_MESSAGE "%s event detected on md device %s"
+#define COMPONENT_DEVICE_MESSAGE ", component device %s"
+#define DESCRIPTION_MESSAGE ": %s"
+/*
+ * sprint_event_message() - Writes basic message about detected event to destination ptr.
+ * @dest: message destination, should be at least the size of BUFSIZ
+ * @data: event data
+ *
+ * Return: 0 on success, 1 on error
+ */
+static int sprint_event_message(char *dest, const struct event_data *data)
+{
+	if (!dest || !data)
+		return 1;
+
+	if (data->disc && data->description)
+		snprintf(dest, BUFSIZ, BASE_MESSAGE COMPONENT_DEVICE_MESSAGE DESCRIPTION_MESSAGE,
+			 data->event_name, data->dev, data->disc, data->description);
+	else if (data->disc)
+		snprintf(dest, BUFSIZ, BASE_MESSAGE COMPONENT_DEVICE_MESSAGE,
+			 data->event_name, data->dev, data->disc);
+	else if (data->description)
+		snprintf(dest, BUFSIZ, BASE_MESSAGE DESCRIPTION_MESSAGE,
+			 data->event_name, data->dev, data->description);
+	else
+		snprintf(dest, BUFSIZ, BASE_MESSAGE, data->event_name, data->dev);
+
+	return 0;
+}
+
+/*
+ * get_syslog_event_priority() - Determines event priority.
+ * @event_enum: event to be checked
+ *
+ * Return: LOG_CRIT, LOG_WARNING or LOG_INFO
+ */
+static int get_syslog_event_priority(const enum event event_enum)
+{
+	if (event_enum > __SYSLOG_PRIORITY_CRITICAL)
+		return LOG_CRIT;
+	if (event_enum > __SYSLOG_PRIORITY_WARNING)
+		return LOG_WARNING;
+	return LOG_INFO;
+}
+
+/*
+ * is_email_event() - Determines whether email for event should be sent or not.
+ * @event_enum: event to be checked
+ *
+ * Return: true if email should be sent, false otherwise
+ */
+static bool is_email_event(const enum event event_enum)
+{
+	static const enum event email_events[] = {
+	EVENT_FAIL,
+	EVENT_FAIL_SPARE,
+	EVENT_DEGRADED_ARRAY,
+	EVENT_SPARES_MISSING,
+	EVENT_TEST_MESSAGE
+	};
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(email_events); ++i) {
+		if (event_enum == email_events[i])
+			return true;
+	}
+	return false;
+}
+
+/*
+ * execute_alert_cmd() - Forks and executes command provided as alert_cmd.
+ * @data: event data
+ */
+static void execute_alert_cmd(const struct event_data *data)
 {
 	int pid = fork();
 
@@ -462,12 +550,16 @@ static void execute_alert_cmd(const char *event_name, const char *dev, const cha
 		pr_err("Cannot fork to execute alert command");
 		break;
 	case 0:
-		execl(info.alert_cmd, info.alert_cmd, event_name, dev, disc, NULL);
+		execl(info.alert_cmd, info.alert_cmd, data->event_name, data->dev, data->disc, NULL);
 		exit(2);
 	}
 }
 
-static void send_event_email(const char *event_name, const char *dev, const char *disc)
+/*
+ * send_event_email() - Sends an email about event detected by monitor.
+ * @data: event data
+ */
+static void send_event_email(const struct event_data *data)
 {
 	FILE *mp, *mdstat;
 	char buf[BUFSIZ];
@@ -485,15 +577,9 @@ static void send_event_email(const char *event_name, const char *dev, const char
 	else
 		fprintf(mp, "From: %s monitoring <root>\n", Name);
 	fprintf(mp, "To: %s\n", info.mailaddr);
-	fprintf(mp, "Subject: %s event on %s:%s\n\n", event_name, dev, info.hostname);
-	fprintf(mp, "This is an automatically generated mail message. \n");
-	fprintf(mp, "A %s event had been detected on md device %s.\n\n", event_name, dev);
-
-	if (disc && disc[0] != ' ')
-		fprintf(mp,
-			"It could be related to component device %s.\n\n", disc);
-	if (disc && disc[0] == ' ')
-		fprintf(mp, "Extra information:%s.\n\n", disc);
+	fprintf(mp, "Subject: %s event on %s:%s\n\n", data->event_name, data->dev, info.hostname);
+	fprintf(mp, "This is an automatically generated mail message.\n");
+	fprintf(mp, "%s\n", data->message);
 
 	mdstat = fopen("/proc/mdstat", "r");
 	if (!mdstat) {
@@ -509,58 +595,60 @@ static void send_event_email(const char *event_name, const char *dev, const char
 	pclose(mp);
 }
 
-static void log_event_to_syslog(const enum event event_enum, const char *event_name, const char *dev, const char *disc)
+/*
+ * log_event_to_syslog() - Logs an event into syslog.
+ * @data: event data
+ */
+static void log_event_to_syslog(const struct event_data *data)
 {
 	int priority;
-	/* Log at a different severity depending on the event.
-	 *
-	 * These are the critical events:  */
-	if (event_enum == EVENT_FAIL ||
-	    event_enum == EVENT_DEGRADED_ARRAY ||
-	    event_enum == EVENT_DEVICE_DISAPPEARED)
-		priority = LOG_CRIT;
-	/* Good to know about, but are not failures: */
-	else if (event_enum == EVENT_REBUILD ||
-		 event_enum == EVENT_MOVE_SPARE ||
-		 event_enum == EVENT_SPARES_MISSING)
-		priority = LOG_WARNING;
-	/* Everything else: */
-	else
-		priority = LOG_INFO;
-
-	if (disc && disc[0] != ' ')
-		syslog(priority,
-		       "%s event detected on md device %s, component device %s",
-		       event_name, dev, disc);
-	else if (disc)
-		syslog(priority, "%s event detected on md device %s: %s", event_name, dev, disc);
-	else
-		syslog(priority, "%s event detected on md device %s", event_name, dev);
+
+	priority = get_syslog_event_priority(data->event_enum);
+
+	syslog(priority, "%s\n", data->message);
 }
 
-static void alert(const enum event event_enum, const unsigned int progress, const char *dev, const char *disc)
+/*
+ * alert() - Alerts about the monitor event.
+ * @event_enum: event to be sent
+ * @description: event description
+ * @progress: rebuild progress
+ * @dev: md device name
+ * @disc: component device
+ *
+ * If needed function executes alert command, sends an email or logs event to syslog.
+ */
+static void alert(const enum event event_enum, const char *description, const uint8_t progress,
+		  const char *dev, const char *disc)
 {
-	char event_name[EVENT_NAME_MAX];
+	struct event_data data = {.dev = dev, .disc = disc, .description = description};
+
+	if (!dev)
+		return;
 
 	if (event_enum == EVENT_REBUILD) {
-		snprintf(event_name, sizeof(event_name), "%s%02d",
+		snprintf(data.event_name, sizeof(data.event_name), "%s%02d",
 			 map_num_s(events_map, EVENT_REBUILD), progress);
 	} else {
-		snprintf(event_name, sizeof(event_name), "%s", map_num_s(events_map, event_enum));
+		snprintf(data.event_name, sizeof(data.event_name), "%s", map_num_s(events_map, event_enum));
 	}
 
-	if (info.alert_cmd)
-		execute_alert_cmd(event_name, dev, disc);
+	data.event_enum = event_enum;
 
-	if (info.mailaddr && (event_enum == EVENT_FAIL ||
-			      event_enum == EVENT_TEST_MESSAGE ||
-			      event_enum == EVENT_SPARES_MISSING ||
-			      event_enum == EVENT_DEGRADED_ARRAY)) {
-		send_event_email(event_name, dev, disc);
+	if (sprint_event_message(data.message, &data) != 0) {
+		pr_err("Cannot create event message.\n");
+		return;
 	}
+	pr_err("%s\n", data.message);
+
+	if (info.alert_cmd)
+		execute_alert_cmd(&data);
+
+	if (info.mailaddr && is_email_event(event_enum))
+		send_event_email(&data);
 
 	if (info.dosyslog)
-		log_event_to_syslog(event_enum, event_name, dev, disc);
+		log_event_to_syslog(&data);
 }
 
 static int check_array(struct state *st, struct mdstat_ent *mdstat,
@@ -585,7 +673,7 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 	unsigned long redundancy_only_flags = 0;
 
 	if (info.test)
-		alert(EVENT_TEST_MESSAGE, 0, dev, NULL);
+		alert(EVENT_TEST_MESSAGE, NULL, 0, dev, NULL);
 
 	retval = 0;
 
@@ -634,7 +722,7 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 	 */
 	if (sra->array.level == 0 || sra->array.level == -1) {
 		if (!st->err && !st->from_config)
-			alert(EVENT_DEVICE_DISAPPEARED, 0, dev, " Wrong-Level");
+			alert(EVENT_DEVICE_DISAPPEARED, "Wrong-Level", 0, dev, NULL);
 		st->err++;
 		goto out;
 	}
@@ -651,7 +739,7 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 		st->percent = RESYNC_NONE;
 		new_array = 1;
 		if (!is_container)
-			alert(EVENT_NEW_ARRAY, 0, st->devname, NULL);
+			alert(EVENT_NEW_ARRAY, NULL, 0, st->devname, NULL);
 	}
 
 	if (st->utime == array.utime && st->failed == sra->array.failed_disks &&
@@ -664,20 +752,20 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 	}
 	if (st->utime == 0 && /* new array */
 	    mse->pattern && strchr(mse->pattern, '_') /* degraded */)
-		alert(EVENT_DEGRADED_ARRAY, 0, dev, NULL);
+		alert(EVENT_DEGRADED_ARRAY, NULL, 0, dev, NULL);
 
 	if (st->utime == 0 && /* new array */ st->expected_spares > 0 &&
 	    sra->array.spare_disks < st->expected_spares)
-		alert(EVENT_SPARES_MISSING, 0, dev, NULL);
+		alert(EVENT_SPARES_MISSING, NULL, 0, dev, NULL);
 	if (st->percent < 0 && st->percent != RESYNC_UNKNOWN &&
 	    mse->percent >= 0)
-		alert(EVENT_REBUILD_STARTED, 0, dev, NULL);
+		alert(EVENT_REBUILD_STARTED, NULL, 0, dev, NULL);
 	if (st->percent >= 0 && mse->percent >= 0 &&
 	    (mse->percent / increments) > (st->percent / increments)) {
 		if((mse->percent / increments) == 0)
-			alert(EVENT_REBUILD_STARTED, 0, dev, NULL);
+			alert(EVENT_REBUILD_STARTED, NULL, 0, dev, NULL);
 		else
-			alert(EVENT_REBUILD, mse->percent, dev, NULL);
+			alert(EVENT_REBUILD, NULL, mse->percent, dev, NULL);
 	}
 
 	if (mse->percent == RESYNC_NONE && st->percent >= 0) {
@@ -690,9 +778,9 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 			snprintf(cnt, sizeof(cnt),
 				 " mismatches found: %d (on raid level %d)",
 				 sra->mismatch_cnt, sra->array.level);
-			alert(EVENT_REBUILD_FINISHED, 0, dev, cnt);
+			alert(EVENT_REBUILD_FINISHED, NULL, 0, dev, cnt);
 		} else
-			alert(EVENT_REBUILD_FINISHED, 0, dev, NULL);
+			alert(EVENT_REBUILD_FINISHED, NULL, 0, dev, NULL);
 	}
 	st->percent = mse->percent;
 
@@ -746,14 +834,14 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 		change = newstate ^ st->devstate[i];
 		if (st->utime && change && !st->err && !new_array) {
 			if ((st->devstate[i]&change) & (1 << MD_DISK_SYNC))
-				alert(EVENT_FAIL, 0, dev, dv);
+				alert(EVENT_FAIL, NULL, 0, dev, dv);
 			else if ((newstate & (1 << MD_DISK_FAULTY)) &&
 				 (disc.major || disc.minor) &&
 				 st->devid[i] == makedev(disc.major,
 							 disc.minor))
-				alert(EVENT_FAIL_SPARE, 0, dev, dv);
+				alert(EVENT_FAIL_SPARE, NULL, 0, dev, dv);
 			else if ((newstate&change) & (1 << MD_DISK_SYNC))
-				alert(EVENT_SPARE_ACTIVE, 0, dev, dv);
+				alert(EVENT_SPARE_ACTIVE, NULL, 0, dev, dv);
 		}
 		st->devstate[i] = newstate;
 		st->devid[i] = makedev(disc.major, disc.minor);
@@ -777,7 +865,7 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 
  disappeared:
 	if (!st->err && !is_container)
-		alert(EVENT_DEVICE_DISAPPEARED, 0, dev, NULL);
+		alert(EVENT_DEVICE_DISAPPEARED, NULL, 0, dev, NULL);
 	st->err++;
 	goto out;
 }
@@ -836,7 +924,7 @@ static int add_new_arrays(struct mdstat_ent *mdstat, struct state **statelist)
 				st->parent_devnm[0] = 0;
 			*statelist = st;
 			if (info.test)
-				alert(EVENT_TEST_MESSAGE, 0, st->devname, NULL);
+				alert(EVENT_TEST_MESSAGE, NULL, 0, st->devname, NULL);
 			new_found = 1;
 		}
 	return new_found;
@@ -1059,7 +1147,7 @@ static void try_spare_migration(struct state *statelist)
 				if (devid > 0 &&
 				    move_spare(from->devname, to->devname,
 					       devid)) {
-					alert(EVENT_MOVE_SPARE, 0, to->devname, from->devname);
+					alert(EVENT_MOVE_SPARE, NULL, 0, to->devname, from->devname);
 					break;
 				}
 			}
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/8] Add helpers to determine whether directories or files are soft links
  2023-01-19 13:30 [PATCH v2 0/8] Mdmonitor refactor and udev event handling improvements Mateusz Grzonka
                   ` (2 preceding siblings ...)
  2023-01-19 13:30 ` [PATCH 3/8] Mdmonitor: Add helper functions Mateusz Grzonka
@ 2023-01-19 13:30 ` Mateusz Grzonka
  2023-01-19 13:30 ` [PATCH 5/8] Mdmonitor: Refactor write_autorebuild_pid() Mateusz Grzonka
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Mateusz Grzonka @ 2023-01-19 13:30 UTC (permalink / raw)
  To: linux-raid; +Cc: jes

Signed-off-by: Mateusz Grzonka <mateusz.grzonka@intel.com>
---
 mdadm.h |  2 ++
 util.c  | 45 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/mdadm.h b/mdadm.h
index 13f8b4cb..1674ce13 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -1777,6 +1777,8 @@ extern void set_dlm_hooks(void);
 #define MSEC_TO_NSEC(msec) ((msec) * 1000000)
 #define USEC_TO_NSEC(usec) ((usec) * 1000)
 extern void sleep_for(unsigned int sec, long nsec, bool wake_after_interrupt);
+extern bool is_directory(const char *path);
+extern bool is_file(const char *path);
 
 #define _ROUND_UP(val, base)	(((val) + (base) - 1) & ~(base - 1))
 #define ROUND_UP(val, base)	_ROUND_UP(val, (typeof(val))(base))
diff --git a/util.c b/util.c
index 9cd89fa4..5afb7c08 100644
--- a/util.c
+++ b/util.c
@@ -2396,3 +2396,48 @@ void sleep_for(unsigned int sec, long nsec, bool wake_after_interrupt)
 		}
 	} while (!wake_after_interrupt && errno == EINTR);
 }
+
+/* is_directory() - Checks if directory provided by path is indeed a regular directory.
+ * @path: directory path to be checked
+ *
+ * Doesn't accept symlinks.
+ *
+ * Return: true if is a directory, false if not
+ */
+bool is_directory(const char *path)
+{
+	struct stat st;
+
+	if (lstat(path, &st) != 0) {
+		pr_err("%s: %s\n", strerror(errno), path);
+		return false;
+	}
+
+	if (!S_ISDIR(st.st_mode))
+		return false;
+
+	return true;
+}
+
+/*
+ * is_file() - Checks if file provided by path is indeed a regular file.
+ * @path: file path to be checked
+ *
+ * Doesn't accept symlinks.
+ *
+ * Return: true if is  a file, false if not
+ */
+bool is_file(const char *path)
+{
+	struct stat st;
+
+	if (lstat(path, &st) != 0) {
+		pr_err("%s: %s\n", strerror(errno), path);
+		return false;
+	}
+
+	if (!S_ISREG(st.st_mode))
+		return false;
+
+	return true;
+}
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/8] Mdmonitor: Refactor write_autorebuild_pid()
  2023-01-19 13:30 [PATCH v2 0/8] Mdmonitor refactor and udev event handling improvements Mateusz Grzonka
                   ` (3 preceding siblings ...)
  2023-01-19 13:30 ` [PATCH 4/8] Add helpers to determine whether directories or files are soft links Mateusz Grzonka
@ 2023-01-19 13:30 ` Mateusz Grzonka
  2023-01-19 13:30 ` [PATCH 6/8] Mdmonitor: Refactor check_one_sharer() for better error handling Mateusz Grzonka
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Mateusz Grzonka @ 2023-01-19 13:30 UTC (permalink / raw)
  To: linux-raid; +Cc: jes

Add better error handling and check for symlinks when opening MDMON_DIR.

Signed-off-by: Mateusz Grzonka <mateusz.grzonka@intel.com>
---
 Monitor.c | 55 ++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 36 insertions(+), 19 deletions(-)

diff --git a/Monitor.c b/Monitor.c
index 39598ba0..14a2dfe5 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -33,6 +33,7 @@
 #endif
 
 #define EVENT_NAME_MAX 32
+#define AUTOREBUILD_PID_PATH MDMON_DIR "/autorebuild.pid"
 
 struct state {
 	char devname[MD_NAME_MAX + sizeof("/dev/md/")];	/* length of "/dev/md/" + device name + terminating byte*/
@@ -126,7 +127,7 @@ static int check_udev_activity(void);
 static void link_containers_with_subarrays(struct state *list);
 static int make_daemon(char *pidfile);
 static void try_spare_migration(struct state *statelist);
-static void write_autorebuild_pid(void);
+static int write_autorebuild_pid(void);
 
 int Monitor(struct mddev_dev *devlist,
 	    char *mailaddr, char *alert_cmd,
@@ -234,7 +235,8 @@ int Monitor(struct mddev_dev *devlist,
 	}
 
 	if (share)
-		write_autorebuild_pid();
+		if (write_autorebuild_pid() != 0)
+			return 1;
 
 	if (devlist == NULL) {
 		mdlist = conf_get_ident(NULL);
@@ -440,29 +442,44 @@ static int check_one_sharer(int scan)
 	return 0;
 }
 
-static void write_autorebuild_pid()
+/*
+ * write_autorebuild_pid() - Writes pid to autorebuild.pid file.
+ *
+ * Return: 0 on success, 1 on error
+ */
+static int write_autorebuild_pid(void)
 {
-	char path[PATH_MAX];
-	int pid;
-	FILE *fp = NULL;
-	sprintf(path, "%s/autorebuild.pid", MDMON_DIR);
+	FILE *fp;
+	int fd;
 
 	if (mkdir(MDMON_DIR, 0700) < 0 && errno != EEXIST) {
-		pr_err("Can't create autorebuild.pid file\n");
-	} else {
-		int fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0700);
+		pr_err("%s: %s\n", strerror(errno), MDMON_DIR);
+		return 1;
+	}
 
-		if (fd >= 0)
-			fp = fdopen(fd, "w");
+	if (!is_directory(MDMON_DIR)) {
+		pr_err("%s is not a regular directory.\n", MDMON_DIR);
+		return 1;
+	}
 
-		if (!fp)
-			pr_err("Can't create autorebuild.pid file\n");
-		else {
-			pid = getpid();
-			fprintf(fp, "%d\n", pid);
-			fclose(fp);
-		}
+	fd = open(AUTOREBUILD_PID_PATH, O_WRONLY | O_CREAT | O_TRUNC, 0700);
+
+	if (fd < 0) {
+		pr_err("Error opening %s file.\n", AUTOREBUILD_PID_PATH);
+		return 1;
 	}
+
+	fp = fdopen(fd, "w");
+
+	if (!fp) {
+		pr_err("Error opening fd for %s file.\n", AUTOREBUILD_PID_PATH);
+		return 1;
+	}
+
+	fprintf(fp, "%d\n", getpid());
+
+	fclose(fp);
+	return 0;
 }
 
 #define BASE_MESSAGE "%s event detected on md device %s"
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 6/8] Mdmonitor: Refactor check_one_sharer() for better error handling
  2023-01-19 13:30 [PATCH v2 0/8] Mdmonitor refactor and udev event handling improvements Mateusz Grzonka
                   ` (4 preceding siblings ...)
  2023-01-19 13:30 ` [PATCH 5/8] Mdmonitor: Refactor write_autorebuild_pid() Mateusz Grzonka
@ 2023-01-19 13:30 ` Mateusz Grzonka
  2023-01-19 13:30 ` [PATCH 7/8] Mdmonitor: Improve udev event handling Mateusz Grzonka
  2023-01-19 13:30 ` [PATCH 8/8] udev: Move udev_block() and udev_unblock() into udev.c Mateusz Grzonka
  7 siblings, 0 replies; 9+ messages in thread
From: Mateusz Grzonka @ 2023-01-19 13:30 UTC (permalink / raw)
  To: linux-raid; +Cc: jes

Also check if autorebuild.pid is a symlink, which we shouldn't accept.

Signed-off-by: Mateusz Grzonka <mateusz.grzonka@intel.com>
---
 Monitor.c | 89 ++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 62 insertions(+), 27 deletions(-)

diff --git a/Monitor.c b/Monitor.c
index 14a2dfe5..44918184 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -32,6 +32,7 @@
 #include	<libudev.h>
 #endif
 
+#define TASK_COMM_LEN 16
 #define EVENT_NAME_MAX 32
 #define AUTOREBUILD_PID_PATH MDMON_DIR "/autorebuild.pid"
 
@@ -224,7 +225,7 @@ int Monitor(struct mddev_dev *devlist,
 	info.hostname[sizeof(info.hostname) - 1] = '\0';
 
 	if (share){
-		if (check_one_sharer(c->scan))
+		if (check_one_sharer(c->scan) == 2)
 			return 1;
 	}
 
@@ -406,39 +407,73 @@ static int make_daemon(char *pidfile)
 	return -1;
 }
 
+/*
+ * check_one_sharer() - Checks for other mdmon processes running.
+ *
+ * Return:
+ * 0 - no other processes running,
+ * 1 - warning,
+ * 2 - error, or when scan mode is enabled, and one mdmon process already exists
+ */
 static int check_one_sharer(int scan)
 {
 	int pid;
-	FILE *comm_fp;
-	FILE *fp;
+	FILE *fp, *comm_fp;
 	char comm_path[PATH_MAX];
-	char path[PATH_MAX];
-	char comm[20];
-
-	sprintf(path, "%s/autorebuild.pid", MDMON_DIR);
-	fp = fopen(path, "r");
-	if (fp) {
-		if (fscanf(fp, "%d", &pid) != 1)
-			pid = -1;
-		snprintf(comm_path, sizeof(comm_path),
-			 "/proc/%d/comm", pid);
-		comm_fp = fopen(comm_path, "r");
-		if (comm_fp) {
-			if (fscanf(comm_fp, "%19s", comm) &&
-			    strncmp(basename(comm), Name, strlen(Name)) == 0) {
-				if (scan) {
-					pr_err("Only one autorebuild process allowed in scan mode, aborting\n");
-					fclose(comm_fp);
-					fclose(fp);
-					return 1;
-				} else {
-					pr_err("Warning: One autorebuild process already running.\n");
-				}
-			}
+	char comm[TASK_COMM_LEN];
+
+	if (!is_directory(MDMON_DIR)) {
+		pr_err("%s is not a regular directory.\n", MDMON_DIR);
+		return 2;
+	}
+
+	if (access(AUTOREBUILD_PID_PATH, F_OK) != 0)
+		return 0;
+
+	if (!is_file(AUTOREBUILD_PID_PATH)) {
+		pr_err("%s is not a regular file.\n", AUTOREBUILD_PID_PATH);
+		return 2;
+	}
+
+	fp = fopen(AUTOREBUILD_PID_PATH, "r");
+	if (!fp) {
+		pr_err("Cannot open %s file.\n", AUTOREBUILD_PID_PATH);
+		return 2;
+	}
+
+	if (fscanf(fp, "%d", &pid) != 1) {
+		pr_err("Cannot read pid from %s file.\n", AUTOREBUILD_PID_PATH);
+		fclose(fp);
+		return 2;
+	}
+
+	snprintf(comm_path, sizeof(comm_path), "/proc/%d/comm", pid);
+
+	comm_fp = fopen(comm_path, "r");
+	if (!comm_fp) {
+		dprintf("Warning: Cannot open %s, continuing\n", comm_path);
+		fclose(fp);
+		return 1;
+	}
+
+	if (fscanf(comm_fp, "%15s", comm) == 0) {
+		dprintf("Warning: Cannot read comm from %s, continuing\n", comm_path);
+		fclose(comm_fp);
+		fclose(fp);
+		return 1;
+	}
+
+	if (strncmp(basename(comm), Name, strlen(Name)) == 0) {
+		if (scan) {
+			pr_err("Only one autorebuild process allowed in scan mode, aborting\n");
 			fclose(comm_fp);
+			fclose(fp);
+			return 2;
 		}
-		fclose(fp);
+		pr_err("Warning: One autorebuild process already running.\n");
 	}
+	fclose(comm_fp);
+	fclose(fp);
 	return 0;
 }
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 7/8] Mdmonitor: Improve udev event handling
  2023-01-19 13:30 [PATCH v2 0/8] Mdmonitor refactor and udev event handling improvements Mateusz Grzonka
                   ` (5 preceding siblings ...)
  2023-01-19 13:30 ` [PATCH 6/8] Mdmonitor: Refactor check_one_sharer() for better error handling Mateusz Grzonka
@ 2023-01-19 13:30 ` Mateusz Grzonka
  2023-01-19 13:30 ` [PATCH 8/8] udev: Move udev_block() and udev_unblock() into udev.c Mateusz Grzonka
  7 siblings, 0 replies; 9+ messages in thread
From: Mateusz Grzonka @ 2023-01-19 13:30 UTC (permalink / raw)
  To: linux-raid; +Cc: jes

Mdmonitor is waiting for udev queue to become empty.
Even if the queue becomes empty, udev might still be processing last event.
However we want to wait and wake up mdmonitor when udev finished
processing events..

Also, the udev queue interface is considered legacy and should not be
used outside of udev.

Use udev monitor instead, and wake up mdmonitor on every event triggered
by udev for md block device.

We need to generate more change events from kernel, because they are
missing in some situations, for example, when rebuild started.
This will be addressed in a separate patch.

Move udev specific code into separate functions, and place them in udev.c file.
Also move use_udev() logic from lib.c into newly created file.

Signed-off-by: Mateusz Grzonka <mateusz.grzonka@intel.com>
---
 Makefile  |   3 +-
 Manage.c  |   3 +-
 Monitor.c | 137 +++++++++++++++++++-------------------------------
 lib.c     |  13 -----
 mdadm.h   |   1 -
 mdopen.c  |   7 +--
 udev.c    | 147 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 udev.h    |  36 +++++++++++++
 8 files changed, 241 insertions(+), 106 deletions(-)
 create mode 100644 udev.c
 create mode 100644 udev.h

diff --git a/Makefile b/Makefile
index 5eac1a4e..c4c40832 100644
--- a/Makefile
+++ b/Makefile
@@ -124,6 +124,7 @@ LDLIBS = -ldl
 # To explicitly disable libudev, set -DNO_LIBUDEV in CXFLAGS
 ifeq (, $(findstring -DNO_LIBUDEV,  $(CXFLAGS)))
 	LDLIBS += -ludev
+	OBJS = udev.o
 endif
 
 INSTALL = /usr/bin/install
@@ -145,7 +146,7 @@ else
 	ECHO=:
 endif
 
-OBJS =  mdadm.o config.o policy.o mdstat.o  ReadMe.o uuid.o util.o maps.o lib.o \
+OBJS += mdadm.o config.o policy.o mdstat.o  ReadMe.o uuid.o util.o maps.o lib.o \
 	Manage.o Assemble.o Build.o \
 	Create.o Detail.o Examine.o Grow.o Monitor.o dlink.o Kill.o Query.o \
 	Incremental.o Dump.o \
diff --git a/Manage.c b/Manage.c
index fde6aba3..9c7b1b9c 100644
--- a/Manage.c
+++ b/Manage.c
@@ -25,6 +25,7 @@
 #include "mdadm.h"
 #include "md_u.h"
 #include "md_p.h"
+#include "udev.h"
 #include <ctype.h>
 
 int Manage_ro(char *devname, int fd, int readonly)
@@ -472,7 +473,7 @@ done:
 			sysfs_uevent(mdi, "change");
 	}
 
-	if (devnm[0] && use_udev()) {
+	if (devnm[0] && udev_is_available()) {
 		struct map_ent *mp = map_by_devnm(&map, devnm);
 		remove_devices(devnm, mp ? mp->path : NULL);
 	}
diff --git a/Monitor.c b/Monitor.c
index 44918184..01133003 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -23,18 +23,17 @@
  */
 
 #include	"mdadm.h"
+#include	"udev.h"
 #include	"md_p.h"
 #include	"md_u.h"
 #include	<sys/wait.h>
 #include	<limits.h>
 #include	<syslog.h>
-#ifndef NO_LIBUDEV
-#include	<libudev.h>
-#endif
 
 #define TASK_COMM_LEN 16
 #define EVENT_NAME_MAX 32
 #define AUTOREBUILD_PID_PATH MDMON_DIR "/autorebuild.pid"
+#define FALLBACK_DELAY 5
 
 struct state {
 	char devname[MD_NAME_MAX + sizeof("/dev/md/")];	/* length of "/dev/md/" + device name + terminating byte*/
@@ -122,12 +121,11 @@ static void link_containers_with_subarrays(struct state *list);
 static void free_statelist(struct state *statelist);
 static int check_array(struct state *st, struct mdstat_ent *mdstat, int increments, char *prefer);
 static int check_one_sharer(int scan);
-#ifndef NO_LIBUDEV
-static int check_udev_activity(void);
-#endif
 static void link_containers_with_subarrays(struct state *list);
 static int make_daemon(char *pidfile);
 static void try_spare_migration(struct state *statelist);
+static void wait_for_events(int *delay_for_event, int c_delay);
+static void wait_for_events_mdstat(int *delay_for_event, int c_delay);
 static int write_autorebuild_pid(void);
 
 int Monitor(struct mddev_dev *devlist,
@@ -323,32 +321,12 @@ int Monitor(struct mddev_dev *devlist,
 		if (!new_found) {
 			if (oneshot)
 				break;
-			else if (!anyredundant) {
+			if (!anyredundant) {
 				pr_err("No array with redundancy detected, stopping\n");
 				break;
 			}
-			else {
-#ifndef NO_LIBUDEV
-				/*
-				 * Wait for udevd to finish new devices
-				 * processing.
-				 */
-				if (mdstat_wait(delay_for_event) &&
-				    check_udev_activity())
-					pr_err("Error while waiting for UDEV to complete new devices processing\n");
-#else
-				int wait_result = mdstat_wait(delay_for_event);
-				/*
-				 * Give chance to process new device
-				 */
-				if (wait_result != 0) {
-					if (c->delay > 5)
-						delay_for_event = 5;
-				} else
-					delay_for_event = c->delay;
-#endif
-				mdstat_close();
-			}
+
+			wait_for_events(&delay_for_event, c->delay);
 		}
 		info.test = 0;
 
@@ -371,6 +349,49 @@ int Monitor(struct mddev_dev *devlist,
 	return 0;
 }
 
+/*
+ * wait_for_events() - Waits for events on md devices.
+ * @delay_for_event: pointer to current event delay
+ * @c_delay: delay from config
+ */
+static void wait_for_events(int *delay_for_event, int c_delay)
+{
+#ifndef NO_LIBUDEV
+	if (udev_is_available()) {
+		if (udev_wait_for_events(*delay_for_event) == UDEV_STATUS_ERROR)
+			pr_err("Error while waiting for udev events.\n");
+		return;
+	}
+#endif
+	wait_for_events_mdstat(delay_for_event, c_delay);
+}
+
+/*
+ * wait_for_events_mdstat() - Waits for events on mdstat.
+ * @delay_for_event: pointer to current event delay
+ * @c_delay: delay from config
+ */
+static void wait_for_events_mdstat(int *delay_for_event, int c_delay)
+{
+	int wait_result = mdstat_wait(*delay_for_event);
+
+	if (wait_result < 0) {
+		pr_err("Error while waiting for events on mdstat.\n");
+		return;
+	}
+
+	/*
+	 * Give chance to process new device
+	 */
+	if (wait_result != 0) {
+		if (c_delay > FALLBACK_DELAY)
+			*delay_for_event = FALLBACK_DELAY;
+	} else {
+		*delay_for_event = c_delay;
+	}
+	mdstat_close();
+}
+
 static int make_daemon(char *pidfile)
 {
 	/* Return:
@@ -1251,64 +1272,6 @@ static void free_statelist(struct state *statelist)
 	}
 }
 
-#ifndef NO_LIBUDEV
-/* function: check_udev_activity
- * Description: Function waits for udev to finish
- * events processing.
- * Returns:
- *		1 - detected error while opening udev
- *		2 - timeout
- *		0 - successfull completion
- */
-static int check_udev_activity(void)
-{
-	struct udev *udev = NULL;
-	struct udev_queue *udev_queue = NULL;
-	int timeout_cnt = 30;
-	int rc = 0;
-
-	/*
-	 * In rare cases systemd may not have udevm,
-	 * in such cases just exit with rc 0
-	 */
-	if (!use_udev())
-		goto out;
-
-	udev = udev_new();
-	if (!udev) {
-		rc = 1;
-		goto out;
-	}
-
-	udev_queue = udev_queue_new(udev);
-	if (!udev_queue) {
-		rc = 1;
-		goto out;
-	}
-
-	if (udev_queue_get_queue_is_empty(udev_queue))
-		goto out;
-
-	while (!udev_queue_get_queue_is_empty(udev_queue)) {
-		sleep(1);
-
-		if (timeout_cnt)
-			timeout_cnt--;
-		else {
-			rc = 2;
-			goto out;
-		}
-	}
-
-out:
-	if (udev_queue)
-		udev_queue_unref(udev_queue);
-	if (udev)
-		udev_unref(udev);
-	return rc;
-}
-#endif
-
 /* Not really Monitor but ... */
 int Wait(char *dev)
 {
diff --git a/lib.c b/lib.c
index e395b28d..96ba6e8d 100644
--- a/lib.c
+++ b/lib.c
@@ -495,19 +495,6 @@ int check_env(char *name)
 	return 0;
 }
 
-int use_udev(void)
-{
-	static int use = -1;
-	struct stat stb;
-
-	if (use < 0) {
-		use = ((stat("/dev/.udev", &stb) == 0 ||
-			stat("/run/udev", &stb) == 0) &&
-		       check_env("MDADM_NO_UDEV") == 0);
-	}
-	return use;
-}
-
 unsigned long GCD(unsigned long a, unsigned long b)
 {
 	while (a != b) {
diff --git a/mdadm.h b/mdadm.h
index 1674ce13..23c10a52 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -1624,7 +1624,6 @@ extern char *conf_line(FILE *file);
 extern char *conf_word(FILE *file, int allow_key);
 extern void print_quoted(char *str);
 extern void print_escape(char *str);
-extern int use_udev(void);
 extern unsigned long GCD(unsigned long a, unsigned long b);
 extern int conf_name_is_free(char *name);
 extern int conf_verify_devnames(struct mddev_ident *array_list);
diff --git a/mdopen.c b/mdopen.c
index d18c9319..afec34a4 100644
--- a/mdopen.c
+++ b/mdopen.c
@@ -23,6 +23,7 @@
  */
 
 #include "mdadm.h"
+#include "udev.h"
 #include "md_p.h"
 #include <ctype.h>
 
@@ -176,7 +177,7 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
 	char devnm[32];
 	char cbuf[400];
 
-	if (!use_udev())
+	if (!udev_is_available())
 		block_udev = 0;
 
 	if (chosen == NULL)
@@ -383,7 +384,7 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
 	 * If we cannot detect udev, we need to make
 	 * devices and links ourselves.
 	 */
-	if (!use_udev()) {
+	if (!udev_is_available()) {
 		/* Make sure 'devname' exists and 'chosen' is a symlink to it */
 		if (lstat(devname, &stb) == 0) {
 			/* Must be the correct device, else error */
@@ -507,7 +508,7 @@ char *find_free_devnm(int use_partitions)
 			continue;
 		if (!conf_name_is_free(devnm))
 			continue;
-		if (!use_udev()) {
+		if (!udev_is_available()) {
 			/* make sure it is new to /dev too, at least as a
 			 * non-standard */
 			dev_t devid = devnm2devid(devnm);
diff --git a/udev.c b/udev.c
new file mode 100644
index 00000000..72a38f47
--- /dev/null
+++ b/udev.c
@@ -0,0 +1,147 @@
+/*
+ * mdadm - manage Linux "md" devices aka RAID arrays.
+ *
+ * Copyright (C) 2022 Mateusz Grzonka <mateusz.grzonka@intel.com>
+ *
+ *    This program is free software; you can redistribute it and/or modify
+ *    it under the terms of the GNU General Public License as published by
+ *    the Free Software Foundation; either version 2 of the License, or
+ *    (at your option) any later version.
+ *
+ *    This program is distributed in the hope that it will be useful,
+ *    but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *    GNU General Public License for more details.
+ *
+ *    You should have received a copy of the GNU General Public License
+ *    along with this program; if not, write to the Free Software
+ *    Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include	"mdadm.h"
+#include	"udev.h"
+#include	"md_p.h"
+#include	"md_u.h"
+#include	<sys/wait.h>
+#include	<signal.h>
+#include	<limits.h>
+#include	<syslog.h>
+#include	<libudev.h>
+
+static struct udev *udev;
+static struct udev_monitor *udev_monitor;
+
+/*
+ * udev_is_available() - Checks for udev in the system.
+ *
+ * Function looks whether udev binaries are available and MDADM_NO_UDEV env defined.
+ *
+ * Return:
+ * true if udev is available,
+ * false if not
+ */
+bool udev_is_available(void)
+{
+	struct stat stb;
+
+	if (stat("/dev/.udev", &stb) != 0 &&
+	    stat("/run/udev", &stb) != 0)
+		return false;
+	if (check_env("MDADM_NO_UDEV") == 1)
+		return false;
+	return true;
+}
+
+/*
+ * udev_initialize() - Initializes udev and udev_monitor structures.
+ *
+ * Function initializes udev, udev_monitor, and sets udev_monitor filter for block devices.
+ *
+ * Return:
+ * UDEV_STATUS_SUCCESS on success
+ * UDEV_STATUS_ERROR on error
+ * UDEV_STATUS_ERROR_NO_UDEV when udev not available
+ */
+enum udev_status udev_initialize(void)
+{
+	if (!udev_is_available()) {
+		pr_err("No udev.\n");
+		return UDEV_STATUS_ERROR_NO_UDEV;
+	}
+
+	udev = udev_new();
+	if (!udev) {
+		pr_err("Cannot initialize udev.\n");
+		return UDEV_STATUS_ERROR;
+	}
+
+	udev_monitor = udev_monitor_new_from_netlink(udev, "udev");
+	if (!udev_monitor) {
+		pr_err("Cannot initialize udev monitor.\n");
+		udev = udev_unref(udev);
+		return UDEV_STATUS_ERROR;
+	}
+
+	if (udev_monitor_filter_add_match_subsystem_devtype(udev_monitor, "block", 0) < 0) {
+		pr_err("Cannot add udev monitor event filter for md devices.\n");
+		udev_release();
+		return UDEV_STATUS_ERROR;
+	}
+	if (udev_monitor_enable_receiving(udev_monitor) < 0) {
+		pr_err("Cannot enable receiving udev events through udev monitor.\n");
+		udev_release();
+		return UDEV_STATUS_ERROR;
+	}
+	atexit(udev_release);
+	return UDEV_STATUS_SUCCESS;
+}
+
+/*
+ * udev_wait_for_events() - Waits for events from udev.
+ * @seconds: Timeout in seconds.
+ *
+ * Function waits udev events, wakes up on event or timeout.
+ *
+ * Return:
+ * UDEV_STATUS_SUCCESS on detected event
+ * UDEV_STATUS_TIMEOUT on timeout
+ * UDEV_STATUS_ERROR on error
+ */
+enum udev_status udev_wait_for_events(int seconds)
+{
+	int fd;
+	fd_set readfds;
+	struct timeval tv;
+	int ret;
+
+	if (!udev || !udev_monitor) {
+		ret = udev_initialize();
+		if (ret != UDEV_STATUS_SUCCESS)
+			return ret;
+	}
+
+	fd = udev_monitor_get_fd(udev_monitor);
+	if (fd < 0) {
+		pr_err("Cannot access file descriptor associated with udev monitor.\n");
+		return UDEV_STATUS_ERROR;
+	}
+
+	FD_ZERO(&readfds);
+	FD_SET(fd, &readfds);
+	tv.tv_sec = seconds;
+	tv.tv_usec = 0;
+
+	if (select(fd + 1, &readfds, NULL, NULL, &tv) > 0 && FD_ISSET(fd, &readfds))
+		if (udev_monitor_receive_device(udev_monitor))
+			return UDEV_STATUS_SUCCESS; /* event detected */
+	return UDEV_STATUS_TIMEOUT;
+}
+
+/*
+ * udev_release() - Drops references of udev and udev_monitor.
+ */
+void udev_release(void)
+{
+	udev_monitor_unref(udev_monitor);
+	udev_unref(udev);
+}
diff --git a/udev.h b/udev.h
new file mode 100644
index 00000000..515366e7
--- /dev/null
+++ b/udev.h
@@ -0,0 +1,36 @@
+/*
+ * mdadm - manage Linux "md" devices aka RAID arrays.
+ *
+ * Copyright (C) 2022 Mateusz Grzonka <mateusz.grzonka@intel.com>
+ *
+ *    This program is free software; you can redistribute it and/or modify
+ *    it under the terms of the GNU General Public License as published by
+ *    the Free Software Foundation; either version 2 of the License, or
+ *    (at your option) any later version.
+ *
+ *    This program is distributed in the hope that it will be useful,
+ *    but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *    GNU General Public License for more details.
+ *
+ *    You should have received a copy of the GNU General Public License
+ *    along with this program; if not, write to the Free Software
+ *    Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifndef MONITOR_UDEV_H
+#define MONITOR_UDEV_H
+
+enum udev_status {
+	UDEV_STATUS_ERROR_NO_UDEV = -2,
+	UDEV_STATUS_ERROR,
+	UDEV_STATUS_SUCCESS = 0,
+	UDEV_STATUS_TIMEOUT
+};
+
+bool udev_is_available(void);
+enum udev_status udev_initialize(void);
+enum udev_status udev_wait_for_events(int seconds);
+void udev_release(void);
+
+#endif
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 8/8] udev: Move udev_block() and udev_unblock() into udev.c
  2023-01-19 13:30 [PATCH v2 0/8] Mdmonitor refactor and udev event handling improvements Mateusz Grzonka
                   ` (6 preceding siblings ...)
  2023-01-19 13:30 ` [PATCH 7/8] Mdmonitor: Improve udev event handling Mateusz Grzonka
@ 2023-01-19 13:30 ` Mateusz Grzonka
  7 siblings, 0 replies; 9+ messages in thread
From: Mateusz Grzonka @ 2023-01-19 13:30 UTC (permalink / raw)
  To: linux-raid; +Cc: jes

Add kernel style comments and better error handling.

Signed-off-by: Mateusz Grzonka <mateusz.grzonka@intel.com>
---
 Create.c |  1 +
 lib.c    | 29 -----------------------------
 mdadm.h  |  2 --
 mdopen.c | 12 ++++++------
 udev.c   | 44 ++++++++++++++++++++++++++++++++++++++++++++
 udev.h   |  2 ++
 6 files changed, 53 insertions(+), 37 deletions(-)

diff --git a/Create.c b/Create.c
index 953e7372..666a8c92 100644
--- a/Create.c
+++ b/Create.c
@@ -23,6 +23,7 @@
  */
 
 #include	"mdadm.h"
+#include	"udev.h"
 #include	"md_u.h"
 #include	"md_p.h"
 #include	<ctype.h>
diff --git a/lib.c b/lib.c
index 96ba6e8d..24cd10e3 100644
--- a/lib.c
+++ b/lib.c
@@ -186,35 +186,6 @@ char *fd2devnm(int fd)
 	return NULL;
 }
 
-/* When we create a new array, we don't want the content to
- * be immediately examined by udev - it is probably meaningless.
- * So create /run/mdadm/creating-mdXXX and expect that a udev
- * rule will noticed this and act accordingly.
- */
-static char block_path[] = "/run/mdadm/creating-%s";
-static char *unblock_path = NULL;
-void udev_block(char *devnm)
-{
-	int fd;
-	char *path = NULL;
-
-	xasprintf(&path, block_path, devnm);
-	fd = open(path, O_CREAT|O_RDWR, 0600);
-	if (fd >= 0) {
-		close(fd);
-		unblock_path = path;
-	} else
-		free(path);
-}
-
-void udev_unblock(void)
-{
-	if (unblock_path)
-		unlink(unblock_path);
-	free(unblock_path);
-	unblock_path = NULL;
-}
-
 /*
  * convert a major/minor pair for a block device into a name in /dev, if possible.
  * On the first call, walk /dev collecting name.
diff --git a/mdadm.h b/mdadm.h
index 23c10a52..5607c599 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -1729,8 +1729,6 @@ extern char *fd2kname(int fd);
 extern char *stat2devnm(struct stat *st);
 bool stat_is_md_dev(struct stat *st);
 extern char *fd2devnm(int fd);
-extern void udev_block(char *devnm);
-extern void udev_unblock(void);
 
 extern int in_initrd(void);
 
diff --git a/mdopen.c b/mdopen.c
index afec34a4..ef34613a 100644
--- a/mdopen.c
+++ b/mdopen.c
@@ -336,8 +336,8 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
 	devnm[0] = 0;
 	if (num < 0 && cname && ci->names) {
 		sprintf(devnm, "md_%s", cname);
-		if (block_udev)
-			udev_block(devnm);
+		if (block_udev && udev_block(devnm) != UDEV_STATUS_SUCCESS)
+			return -1;
 		if (!create_named_array(devnm)) {
 			devnm[0] = 0;
 			udev_unblock();
@@ -345,8 +345,8 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
 	}
 	if (num >= 0) {
 		sprintf(devnm, "md%d", num);
-		if (block_udev)
-			udev_block(devnm);
+		if (block_udev && udev_block(devnm) != UDEV_STATUS_SUCCESS)
+			return -1;
 		if (!create_named_array(devnm)) {
 			devnm[0] = 0;
 			udev_unblock();
@@ -369,8 +369,8 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
 				return -1;
 			}
 		}
-		if (block_udev)
-			udev_block(devnm);
+		if (block_udev && udev_block(devnm) != UDEV_STATUS_SUCCESS)
+			return -1;
 	}
 
 	sprintf(devname, "/dev/%s", devnm);
diff --git a/udev.c b/udev.c
index 72a38f47..5389e5df 100644
--- a/udev.c
+++ b/udev.c
@@ -30,6 +30,7 @@
 
 static struct udev *udev;
 static struct udev_monitor *udev_monitor;
+static char *unblock_path;
 
 /*
  * udev_is_available() - Checks for udev in the system.
@@ -145,3 +146,46 @@ void udev_release(void)
 	udev_monitor_unref(udev_monitor);
 	udev_unref(udev);
 }
+
+/*
+ * udev_block() - Block udev from examining newly created arrays.
+ *
+ * When array is created, we don't want udev to examine it immediately.
+ * Function creates /run/mdadm/creating-mdXXX and expects that udev rule
+ * will notice it and act accordingly.
+ *
+ * Return:
+ * UDEV_STATUS_SUCCESS when successfully blocked udev
+ * UDEV_STATUS_ERROR on error
+ */
+enum udev_status udev_block(char *devnm)
+{
+	int fd;
+	char *path = xcalloc(1, BUFSIZ);
+
+	snprintf(path, BUFSIZ, "/run/mdadm/creating-%s", devnm);
+
+	fd = open(path, O_CREAT | O_RDWR, 0600);
+	if (!is_fd_valid(fd)) {
+		pr_err("Cannot block udev, error creating blocking file.\n");
+		pr_err("%s: %s\n", strerror(errno), path);
+		free(path);
+		return UDEV_STATUS_ERROR;
+	}
+
+	close(fd);
+	unblock_path = path;
+	return UDEV_STATUS_SUCCESS;
+}
+
+/*
+ * udev_unblock() - Unblock udev.
+ */
+void udev_unblock(void)
+{
+	if (unblock_path)
+		unlink(unblock_path);
+	free(unblock_path);
+	unblock_path = NULL;
+}
+
diff --git a/udev.h b/udev.h
index 515366e7..e4da29cc 100644
--- a/udev.h
+++ b/udev.h
@@ -32,5 +32,7 @@ bool udev_is_available(void);
 enum udev_status udev_initialize(void);
 enum udev_status udev_wait_for_events(int seconds);
 void udev_release(void);
+enum udev_status udev_block(char *devnm);
+void udev_unblock(void);
 
 #endif
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-01-19 13:50 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-19 13:30 [PATCH v2 0/8] Mdmonitor refactor and udev event handling improvements Mateusz Grzonka
2023-01-19 13:30 ` [PATCH 1/8] Mdmonitor: Make alert_info global Mateusz Grzonka
2023-01-19 13:30 ` [PATCH 2/8] Mdmonitor: Pass events to alert() using enums instead of strings Mateusz Grzonka
2023-01-19 13:30 ` [PATCH 3/8] Mdmonitor: Add helper functions Mateusz Grzonka
2023-01-19 13:30 ` [PATCH 4/8] Add helpers to determine whether directories or files are soft links Mateusz Grzonka
2023-01-19 13:30 ` [PATCH 5/8] Mdmonitor: Refactor write_autorebuild_pid() Mateusz Grzonka
2023-01-19 13:30 ` [PATCH 6/8] Mdmonitor: Refactor check_one_sharer() for better error handling Mateusz Grzonka
2023-01-19 13:30 ` [PATCH 7/8] Mdmonitor: Improve udev event handling Mateusz Grzonka
2023-01-19 13:30 ` [PATCH 8/8] udev: Move udev_block() and udev_unblock() into udev.c Mateusz Grzonka

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.