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.
Fix this by setting force_readonly before each load and clearing it when a load succeeds: - needs_ro_update() now sets force_readonly whenever it returns true: to 1 for a DISK_RO=1 uevent, to 0 when all of the map's paths are writable, and from the paths' state when the map has no dm info. uev_update_path() no longer sets or clears it itself. - ev_add_path() sets it from all of the map's paths rather than only the path being added, and only when it is about to load the map (need_do_map). mpp->paths is up to date there after adopt_paths(), while mpp->pg doesn't contain the new paths yet. - update_map(), which does reloads that were delayed while the map was waiting for udev (UDEV_WAIT_RELOAD), recomputes it from the map's paths before setting up the map. - dm_addmap() clears it after a successful load, as it does for need_reload, so it can't outlive the load it was set for. Signed-off-by: Brian Bunker <[email protected]> --- libmultipath/devmapper.c | 4 +- multipathd/main.c | 113 +++++++++++++++++++++++++-------------- 2 files changed, 75 insertions(+), 42 deletions(-) diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c index e3870f59..c2939dca 100644 --- a/libmultipath/devmapper.c +++ b/libmultipath/devmapper.c @@ -520,8 +520,10 @@ dm_addmap (int task, const char *target, struct multipath *mpp, if (task == DM_DEVICE_CREATE) libmp_udev_wait(cookie); - if (r) + if (r) { mpp->need_reload = false; + mpp->force_readonly = 0; + } return r; } diff --git a/multipathd/main.c b/multipathd/main.c index 9bce4d6b..605e24ae 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -709,6 +709,42 @@ void cleanup_reset_vec(struct vector_s **v) vector_reset(*v); } +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_ok(pp->udev, "ro", buff, sizeof(buff))) { + 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; +} + +/* A map must be loaded read-only if any of its paths is read-only. */ +static bool +has_ro_path(vector paths) +{ + struct path *pp; + unsigned int i; + + vector_foreach_slot (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 +768,14 @@ 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->paths is up to date + * after adopt_paths(), while mpp->pg isn't until setup_map(). + */ + mpp->force_readonly = has_ro_path(mpp->paths); + if (setup_map(mpp, ¶ms, vecs)) { condlog(0, "%s: failed to setup new map in update", mpp->alias); retries = -1; @@ -1270,28 +1314,6 @@ 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_ok(pp->udev, "ro", buff, sizeof(buff))) { - 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 @@ -1305,7 +1327,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,14 +1391,16 @@ 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; - if (!need_do_map) return 0; + /* + * Load the map read-only if any of its paths is read-only, not + * just the one being added. mpp->paths is up to date after + * adopt_paths(), while mpp->pg doesn't have the new paths yet. + */ + mpp->force_readonly = has_ro_path(mpp->paths); + if (!dm_map_present(mpp->alias)) { mpp->action = ACT_CREATE; start_waiter = 1; @@ -1597,23 +1620,35 @@ static bool needs_ro_update(struct multipath *mpp, int ro) { struct pathgroup * pgp; - struct path * pp; - unsigned int i, j; + unsigned int i; if (!mpp || ro < 0) return false; - if (!has_dm_info(mpp)) + if (!has_dm_info(mpp)) { + mpp->force_readonly = 0; + vector_foreach_slot (mpp->pg, pgp, i) { + if (has_ro_path(pgp->paths)) { + mpp->force_readonly = 1; + break; + } + } return true; + } if (mpp->dmi.read_only == ro) return false; - if (ro == 1) + if (ro == 1) { + mpp->force_readonly = 1; return true; + } vector_foreach_slot (mpp->pg, pgp, i) { - vector_foreach_slot (pgp->paths, pp, j) { - if (sysfs_get_ro(pp) == 1) - return false; - } + if (has_ro_path(pgp->paths)) + return false; } + /* + * Clear force_readonly in case it is still set from an earlier + * failed load. + */ + mpp->force_readonly = 0; return true; } @@ -1732,15 +1767,11 @@ 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; retval = reload_and_sync_map(mpp, vecs); if (retval == 2) condlog(2, "%s: map removed during reload", pp->dev); - else { - pp->mpp->force_readonly = 0; + else condlog(2, "%s: map %s reloaded (retval %d)", uev->kernel, mpp->alias, retval); - } } } if (auto_resize != AUTO_RESIZE_NEVER && mpp && -- 2.56.0
