There are many possible outcomes of calling ev_remove_path(), and not all callers agree on which outcomes are a success and which are a failure. So ev_remove_path() should simply return a different value for each outcome, and the callers can decide how to deal with them. Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx> --- multipathd/cli_handlers.c | 14 ++++++++++++-- multipathd/main.c | 35 +++++++++++++++++++---------------- multipathd/main.h | 9 +++++++++ 3 files changed, 40 insertions(+), 18 deletions(-) diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c index 1de6ad8e..1462ea84 100644 --- a/multipathd/cli_handlers.c +++ b/multipathd/cli_handlers.c @@ -752,7 +752,8 @@ cli_add_path (void * v, char ** reply, int * len, void * data) /* Have the checker reinstate this path asap */ pp->tick = 1; return 0; - } else if (!ev_remove_path(pp, vecs, true)) + } else if (ev_remove_path(pp, vecs, true) & + REMOVE_PATH_SUCCESS) /* Path removed in ev_remove_path() */ pp = NULL; else { @@ -813,6 +814,7 @@ cli_del_path (void * v, char ** reply, int * len, void * data) struct vectors * vecs = (struct vectors *)data; char * param = get_keyparam(v, PATH); struct path *pp; + int ret; param = convert_dev(param, 1); condlog(2, "%s: remove path (operator)", param); @@ -821,7 +823,15 @@ cli_del_path (void * v, char ** reply, int * len, void * data) condlog(0, "%s: path already removed", param); return 1; } - return ev_remove_path(pp, vecs, 1); + ret = ev_remove_path(pp, vecs, 1); + if (ret == REMOVE_PATH_DELAY) { + *reply = strdup("delayed\n"); + *len = strlen(*reply) + 1; + } else if (ret == REMOVE_PATH_MAP_ERROR) { + *reply = strdup("map reload error. removed\n"); + *len = strlen(*reply) + 1; + } + return (ret == REMOVE_PATH_FAILURE); } int diff --git a/multipathd/main.c b/multipathd/main.c index 4bdf14bd..72fb7e38 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -838,7 +838,7 @@ handle_path_wwid_change(struct path *pp, struct vectors *vecs) return; udd = udev_device_ref(pp->udev); - if (ev_remove_path(pp, vecs, 1) != 0 && pp->mpp) { + if (!(ev_remove_path(pp, vecs, 1) & REMOVE_PATH_SUCCESS) && pp->mpp) { pp->dmstate = PSTATE_FAILED; dm_fail_path(pp->mpp->alias, pp->dev_t); } @@ -948,8 +948,8 @@ uev_add_path (struct uevent *uev, struct vectors * vecs, int need_do_map) * Make another attempt to remove the path */ pp->mpp = prev_mpp; - ret = ev_remove_path(pp, vecs, true); - if (ret != 0) { + if (!(ev_remove_path(pp, vecs, true) & + REMOVE_PATH_SUCCESS)) { /* * Failure in ev_remove_path will keep * path in pathvec in INIT_REMOVED state @@ -960,6 +960,7 @@ uev_add_path (struct uevent *uev, struct vectors * vecs, int need_do_map) dm_fail_path(pp->mpp->alias, pp->dev_t); condlog(1, "%s: failed to re-add path still mapped in %s", pp->dev, pp->mpp->alias); + ret = 1; } else if (r == PATHINFO_OK) /* * Path successfully freed, move on to @@ -1167,7 +1168,7 @@ static int uev_remove_path (struct uevent *uev, struct vectors * vecs, int need_do_map) { struct path *pp; - int ret; + int ret = 0; condlog(3, "%s: remove path (uevent)", uev->kernel); delete_foreign(uev->udev); @@ -1176,8 +1177,8 @@ uev_remove_path (struct uevent *uev, struct vectors * vecs, int need_do_map) lock(&vecs->lock); pthread_testcancel(); pp = find_path_by_dev(vecs->pathvec, uev->kernel); - if (pp) - ret = ev_remove_path(pp, vecs, need_do_map); + if (pp && ev_remove_path(pp, vecs, need_do_map) == REMOVE_PATH_FAILURE) + ret = 1; lock_cleanup_pop(vecs->lock); if (!pp) { /* Not an error; path might have been purged earlier */ @@ -1191,7 +1192,7 @@ int ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) { struct multipath * mpp; - int i, retval = 0; + int i, retval = REMOVE_PATH_SUCCESS; char params[PARAMS_SIZE] = {0}; /* @@ -1245,7 +1246,6 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) condlog(2, "%s: removed map after" " removing all paths", alias); - retval = 0; /* flush_map() has freed the path */ goto out; } @@ -1262,11 +1262,14 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) if (mpp->wait_for_udev) { mpp->wait_for_udev = 2; + retval = REMOVE_PATH_DELAY; goto out; } - if (!need_do_map) + if (!need_do_map) { + retval = REMOVE_PATH_DELAY; goto out; + } /* * reload the map */ @@ -1275,7 +1278,7 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) condlog(0, "%s: failed in domap for " "removal of path %s", mpp->alias, pp->dev); - retval = 1; + retval = REMOVE_PATH_FAILURE; } else { /* * update our state from kernel @@ -1283,12 +1286,12 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) char devt[BLK_DEV_SIZE]; strlcpy(devt, pp->dev_t, sizeof(devt)); + + /* setup_multipath will free the path + * regardless of whether it succeeds or + * fails */ if (setup_multipath(vecs, mpp)) - return 0; - /* - * Successful map reload without this path: - * sync_map_state() will free it. - */ + return REMOVE_PATH_MAP_ERROR; sync_map_state(mpp); condlog(2, "%s: path removed from map %s", @@ -1307,7 +1310,7 @@ fail: condlog(0, "%s: error removing path. removing map %s", pp->dev, mpp->alias); remove_map_and_stop_waiter(mpp, vecs); - return 0; + return REMOVE_PATH_MAP_ERROR; } static int diff --git a/multipathd/main.h b/multipathd/main.h index ddd953f9..24d050c8 100644 --- a/multipathd/main.h +++ b/multipathd/main.h @@ -13,6 +13,15 @@ enum daemon_status { DAEMON_STATUS_SIZE, }; +#define REMOVE_PATH_FAILURE 0x0 /* path was not removed */ +#define REMOVE_PATH_SUCCESS 0x1 /* path was removed */ +#define REMOVE_PATH_DELAY 0x2 /* path is set to be removed later. it + * currently still exists and is part of the + * kernel map */ +#define REMOVE_PATH_MAP_ERROR 0x5 /* map was removed because of error. value + * includes REMOVE_PATH_SUCCESS bit + * because the path was also removed */ + struct prout_param_descriptor; struct prin_resp; -- 2.17.2 -- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel