On Wed, Mar 09, 2022 at 01:03:26PM -0700, Uday Shankar wrote: > 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/[email protected]/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 <[email protected]>
Reviewed-by: Benjamin Marzinski <[email protected]> > --- > 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 > [email protected] > https://listman.redhat.com/mailman/listinfo/dm-devel -- dm-devel mailing list [email protected] https://listman.redhat.com/mailman/listinfo/dm-devel
