[dm-devel] [PATCH v2 6/6] multipathd: use symbolic returns for ev_remove_path()
Benjamin Marzinski
bmarzins at redhat.com
Thu May 13 17:23:15 UTC 2021
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 at redhat.com>
---
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
More information about the dm-devel
mailing list