On Mon, 2026-10-26 at 10:26 +0100, Martin Wilck wrote: > On Fri, 2026-10-23 at 19:40 -0400, Benjamin Marzinski wrote: > > > > I do still waver a bit on whether or not we could leave > > force_readonly > > set on failed reloads. Once a device turns read-only, it should stay > > that way until we get another uevent, at which point we will call > > needs_ro_update() again. Perhaps we just need to make sure that we > > clear > > force_readonly when needs_ro_update() returns false. > > AFAICS, this is exactly what Brian's v4 patch does. No?
Mostly, yes. v4 sets force_readonly back in dm_addmap_reload() when a reload fails, so the next reload tries read-only again. needs_ro_update() doesn't clear it on every false return, though. It clears it in the dmi.read_only == ro case only when both the map and the uevent are read-write and none of the map's paths is read-only. If a path is still read-only, the flag stays set, so a map whose read-only reload failed keeps retrying instead of staying read-write. About dm_addmap_create(): agreed that the flag isn't needed for correctness there, since a new map opens all paths fresh and a read-write create fails with EROFS anyway. It only saves that first attempt. Create still clears the flag, so that a value set by ev_add_path() can't carry over to a later reload. I can drop the read-only shortcut in create if you'd prefer. Uday Shankar, who did 430f8f89, also suggested dropping force_readonly altogether, and tracking the ro state per path in sysfs_pathinfo() sounds like the right direction to me. But it changes behavior: every reload, not just the one done for a DISK_RO uevent, would follow the paths' ro state, and so would "multipath -r", since dm_addmap_reload() is in libmultipath. So I'd rather do it as a separate series on top of this fix, and keep this one small enough to backport. Brian
