ev_add_path() sets mpp->force_readonly when a newly added path is read-only, but never clears it. If a map is created or reloaded while its storage is write protected, the flag stays set. When write protect is cleared, the reload that uev_update_path() does for the DISK_RO=0 uevent uses the stale flag and loads the map read-only, even though all of its paths are read-write. uev_update_path() clears the flag after that reload, so the map only recovers if another DISK_RO=0 uevent reloads it again. If none comes, the map stays read-only, and since mpp->dmi.read_only is then 1, needs_ro_update() also ignores later DISK_RO=1 uevents for it.
Add a helper, map_has_ro_path(), that checks whether any of a map's paths is read-only in sysfs, and use it to set force_readonly: - In ev_add_path(), set force_readonly from all of the map's paths rather than only the path being added, and clear it once the map has been loaded, as uev_update_path() already does after its reload. With need_do_map == 0 the flag is left set for the call that loads the map. - In uev_update_path(), set force_readonly from the paths' current state in sysfs instead of from the uevent. The kernel updates the ro attribute before it sends the DISK_RO uevent, so sysfs is current when the uevent is handled. - In update_map(), which does reloads that were delayed while the map was waiting for udev (UDEV_WAIT_RELOAD), recompute force_readonly before setting up the map, since the paths' state may have changed since it was last set. Signed-off-by: Brian Bunker <[email protected]> --- multipathd/main.c | 44 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/multipathd/main.c b/multipathd/main.c index 9bce4d6b..68916256 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -709,6 +709,22 @@ void cleanup_reset_vec(struct vector_s **v) vector_reset(*v); } +static int sysfs_get_ro(struct path *pp); + +/* A map must be loaded read-only if any of its paths is read-only. */ +static bool +map_has_ro_path(struct multipath *mpp) +{ + struct path *pp; + unsigned int i; + + vector_foreach_slot (mpp->paths, pp, i) { + if (sysfs_get_ro(pp) == 1) + return true; + } + return false; +} + static int update_map (struct multipath *mpp, struct vectors *vecs, int new_map) { @@ -732,6 +748,13 @@ retry: mpp->action = ACT_RELOAD; + /* + * This may be a delayed reload, and the paths' write protect + * state may have changed since force_readonly was last set. + * Recompute it from the current paths. + */ + mpp->force_readonly = map_has_ro_path(mpp); + if (setup_map(mpp, ¶ms, vecs)) { condlog(0, "%s: failed to setup new map in update", mpp->alias); retries = -1; @@ -1305,7 +1328,6 @@ ev_add_path (struct path * pp, struct vectors * vecs, int need_do_map) int retries = 3; int start_waiter = 0; int ret; - int ro; unsigned char prflag = PR_UNSET; /* @@ -1370,10 +1392,11 @@ rescan: goto fail; /* leave path added to pathvec */ } - /* 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; + /* + * Load the map read-only if any of its paths is read-only, + * not just the one being added. + */ + mpp->force_readonly = map_has_ro_path(mpp); if (!need_do_map) return 0; @@ -1400,6 +1423,8 @@ rescan: sleep(1); ret = domap(mpp, params, 1); } + /* force_readonly only applies to this load, as in uev_update_path() */ + mpp->force_readonly = 0; if (ret == DOMAP_FAIL || ret == DOMAP_RETRY) { condlog(0, "%s: failed in domap for addition of new " "path %s", mpp->alias, pp->dev); @@ -1732,8 +1757,13 @@ uev_update_path (struct uevent *uev, struct vectors * vecs) if (mpp->wait_for_udev != UDEV_WAIT_DONE) mpp->wait_for_udev = UDEV_WAIT_RELOAD; else { - if (ro == 1) - pp->mpp->force_readonly = 1; + /* + * force_readonly may be stale from ev_add_path(). + * Set it from the paths' current state in sysfs, + * which the kernel updates before sending the + * uevent, rather than from the uevent itself. + */ + pp->mpp->force_readonly = map_has_ro_path(mpp); retval = reload_and_sync_map(mpp, vecs); if (retval == 2) condlog(2, "%s: map removed during reload", pp->dev); -- 2.56.0
