[dm-devel] [PATCH] multipath-tools: update mpp->force_readonly in ev_add_path

* [dm-devel] [PATCH] multipath-tools: update mpp->force_readonly in ev_add_path
@ 2022-03-09 20:03 Uday Shankar
  2022-03-23 23:10 ` Benjamin Marzinski
  0 siblings, 1 reply; 6+ messages in thread
From: Uday Shankar @ 2022-03-09 20:03 UTC (permalink / raw)
  To: dm-devel; +Cc: Uday Shankar

When NVMe disks are added to the system, no uevent containing the
DISK_RO property is generated. As a result, dm-* nodes backed by
readonly NVMe disks will not have their RO state set properly. The
result looks like this:

$ multipath -l
eui.00c92c091fd6564424a9376600011bd1 dm-3 NVME,Pure Storage FlashArray
size=1.0T features='0' hwhandler='0' wp=rw
|-+- policy='service-time 0' prio=0 status=active
| `- 0:2:2:72657 nvme0n2 259:4 active undef running
`-+- policy='service-time 0' prio=0 status=enabled
  `- 1:0:2:72657 nvme1n2 259:1 active undef running
$ cat /sys/block/dm-3/ro
0
$ cat /sys/block/nvme*n2/ro
1
1

This is not a problem for SCSI disks, since the kernel will emit change
uevents containing the DISK_RO property when the disk is added to the
system. See the following thread for my initial attempt to fix this
issue at the kernel level:
https://lore.kernel.org/linux-block/Yib8GqCA5e3SQYty@infradead.org/T/#t

Fix the issue by picking up the path ro state from sysfs in ev_add_path,
setting the mpp->force_readonly accordingly, and changing
dm_addmap_create to be aware of mpp->force_readonly.

Signed-off-by: Uday Shankar <ushankar@purestorage.com>
---
 libmultipath/devmapper.c |  2 +-
 multipathd/main.c        | 50 ++++++++++++++++++++++------------------
 2 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 2507f77f..9819e29b 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -540,7 +540,7 @@ int dm_addmap_create (struct multipath *mpp, char * params)
 	int ro;
 	uint16_t udev_flags = build_udev_flags(mpp, 0);
 
-	for (ro = 0; ro <= 1; ro++) {
+	for (ro = mpp->force_readonly ? 1 : 0; ro <= 1; ro++) {
 		int err;
 
 		if (dm_addmap(DM_DEVICE_CREATE, TGT_MPATH, mpp, params, ro,
diff --git a/multipathd/main.c b/multipathd/main.c
index f2c0b280..a67865df 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1130,6 +1130,28 @@ out:
 	return ret;
 }
 
+static int
+sysfs_get_ro (struct path *pp)
+{
+	int ro;
+	char buff[3]; /* Either "0\n\0" or "1\n\0" */
+
+	if (!pp->udev)
+		return -1;
+
+	if (sysfs_attr_get_value(pp->udev, "ro", buff, sizeof(buff)) <= 0) {
+		condlog(3, "%s: Cannot read ro attribute in sysfs", pp->dev);
+		return -1;
+	}
+
+	if (sscanf(buff, "%d\n", &ro) != 1 || ro < 0 || ro > 1) {
+		condlog(3, "%s: Cannot parse ro attribute", pp->dev);
+		return -1;
+	}
+
+	return ro;
+}
+
 /*
  * returns:
  * 0: added
@@ -1143,6 +1165,7 @@ ev_add_path (struct path * pp, struct vectors * vecs, int need_do_map)
 	int retries = 3;
 	int start_waiter = 0;
 	int ret;
+	int ro;
 
 	/*
 	 * need path UID to go any further
@@ -1207,6 +1230,11 @@ rescan:
 	/* persistent reservation check*/
 	mpath_pr_event_handle(pp);
 
+	/* ro check - if new path is ro, force map to be ro as well */
+	ro = sysfs_get_ro(pp);
+	if (ro == 1)
+		mpp->force_readonly = 1;
+
 	if (!need_do_map)
 		return 0;
 
@@ -1446,28 +1474,6 @@ finish_path_init(struct path *pp, struct vectors * vecs)
 	return -1;
 }
 
-static int
-sysfs_get_ro (struct path *pp)
-{
-	int ro;
-	char buff[3]; /* Either "0\n\0" or "1\n\0" */
-
-	if (!pp->udev)
-		return -1;
-
-	if (sysfs_attr_get_value(pp->udev, "ro", buff, sizeof(buff)) <= 0) {
-		condlog(3, "%s: Cannot read ro attribute in sysfs", pp->dev);
-		return -1;
-	}
-
-	if (sscanf(buff, "%d\n", &ro) != 1 || ro < 0 || ro > 1) {
-		condlog(3, "%s: Cannot parse ro attribute", pp->dev);
-		return -1;
-	}
-
-	return ro;
-}
-
 static bool
 needs_ro_update(struct multipath *mpp, int ro)
 {
-- 
2.25.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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