All of lore.kernel.org
 help / color / mirror / Atom feed
From: mwilck@suse.com
To: Christophe Varoqui <christophe.varoqui@opensvc.com>,
	Benjamin Marzinski <bmarzins@redhat.com>
Cc: dm-devel@redhat.com, Martin Wilck <mwilck@suse.com>
Subject: [PATCH 63/74] multipathd: deal with INIT_REMOVED during path addition
Date: Thu,  9 Jul 2020 12:51:34 +0200	[thread overview]
Message-ID: <20200709105145.9211-11-mwilck@suse.com> (raw)
In-Reply-To: <20200709105145.9211-1-mwilck@suse.com>

From: Martin Wilck <mwilck@suse.com>

With the introduction of INIT_REMOVED, we have to deal with the situation
when a path is re-added in this state. This enables us to detect the
situation where a path is added while still part of a map after a failed
removal, which we couldn't before. Dealing with this correctly requires
some additional logic. There's a good case (re-added path is still mapped
by a map with matching WWID) and a bad case (non-matching WWID).

The logic is very similar in uev_add_path() and cli_add_path().

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipathd/cli_handlers.c | 49 +++++++++++++++++++++++++++++++++--
 multipathd/main.c         | 54 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 97 insertions(+), 6 deletions(-)

diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 782bb00..679fd57 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -713,11 +713,56 @@ cli_add_path (void * v, char ** reply, int * len, void * data)
 		goto blacklisted;
 
 	pp = find_path_by_dev(vecs->pathvec, param);
-	if (pp) {
+	if (pp && pp->initialized != INIT_REMOVED) {
 		condlog(2, "%s: path already in pathvec", param);
 		if (pp->mpp)
 			return 0;
-	} else {
+	} else if (pp) {
+		/* Trying to add a path in INIT_REMOVED state */
+		struct multipath *prev_mpp;
+
+		prev_mpp = pp->mpp;
+		pp->mpp = NULL;
+		pp->initialized = INIT_NEW;
+		pp->wwid[0] = '\0';
+		conf = get_multipath_config();
+		pthread_cleanup_push(put_multipath_config, conf);
+		r = pathinfo(pp, conf, DI_ALL | DI_BLACKLIST);
+		pthread_cleanup_pop(1);
+
+		if (prev_mpp) {
+			/* Similar logic as in uev_add_path() */
+			pp->mpp = prev_mpp;
+			if (r == PATHINFO_OK &&
+			    !strncmp(prev_mpp->wwid, pp->wwid, WWID_SIZE)) {
+				condlog(2, "%s: path re-added to %s", pp->dev,
+					pp->mpp->alias);
+				return 0;
+			} else if (!ev_remove_path(pp, vecs, true))
+				/* Path removed in ev_remove_path() */
+				pp = NULL;
+			else {
+				/* Init state is now INIT_REMOVED again */
+				pp->dmstate = PSTATE_FAILED;
+				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);
+				return 1;
+			}
+		} else {
+			switch (r) {
+			case PATHINFO_SKIPPED:
+				goto blacklisted;
+			case PATHINFO_OK:
+				break;
+			default:
+				condlog(0, "%s: failed to get pathinfo", param);
+				return 1;
+			}
+		}
+	}
+
+	if (!pp) {
 		struct udev_device *udevice;
 
 		udevice = udev_device_new_from_subsystem_sysname(udev,
diff --git a/multipathd/main.c b/multipathd/main.c
index 545eb6d..7b2d320 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -842,9 +842,21 @@ uev_add_path (struct uevent *uev, struct vectors * vecs, int need_do_map)
 	pp = find_path_by_dev(vecs->pathvec, uev->kernel);
 	if (pp) {
 		int r;
+		struct multipath *prev_mpp = NULL;
+		bool was_removed = pp->initialized == INIT_REMOVED;
+
+		if (was_removed) {
+			condlog(3, "%s: re-adding removed path", pp->dev);
+			pp->initialized = INIT_NEW;
+			prev_mpp = pp->mpp;
+			pp->mpp = NULL;
+			/* make sure get_uid() is called */
+			pp->wwid[0] = '\0';
+		} else
+			condlog(3,
+				"%s: spurious uevent, path already in pathvec",
+				uev->kernel);
 
-		condlog(3, "%s: spurious uevent, path already in pathvec",
-			uev->kernel);
 		if (!pp->mpp && !strlen(pp->wwid)) {
 			condlog(3, "%s: reinitialize path", uev->kernel);
 			udev_device_unref(pp->udev);
@@ -854,9 +866,43 @@ uev_add_path (struct uevent *uev, struct vectors * vecs, int need_do_map)
 			r = pathinfo(pp, conf,
 				     DI_ALL | DI_BLACKLIST);
 			pthread_cleanup_pop(1);
-			if (r == PATHINFO_OK)
+			if (r == PATHINFO_OK && !prev_mpp)
 				ret = ev_add_path(pp, vecs, need_do_map);
-			else if (r == PATHINFO_SKIPPED) {
+			else if (r == PATHINFO_OK &&
+				 !strncmp(pp->wwid, prev_mpp->wwid, WWID_SIZE)) {
+				/*
+				 * Path was unsuccessfully removed, but now
+				 * re-added, and still belongs to the right map
+				 * - all fine.
+				 */
+				pp->mpp = prev_mpp;
+				ret = 0;
+			} else if (prev_mpp) {
+				/*
+				 * Bad: re-added path still hangs in wrong map
+				 * Make another attempt to remove the path
+				 */
+				pp->mpp = prev_mpp;
+				ret = ev_remove_path(pp, vecs, true);
+				if (r == PATHINFO_OK && !ret)
+					/*
+					 * Path successfully freed, move on to
+					 * "new path" code path below
+					 */
+					pp = NULL;
+				else {
+					/*
+					 * Failure in ev_remove_path will keep
+					 * path in pathvec in INIT_REMOVED state
+					 * Fail the path to make sure it isn't
+					 * used any more.
+					 */
+					pp->dmstate = PSTATE_FAILED;
+					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);
+				}
+			} else if (r == PATHINFO_SKIPPED) {
 				condlog(3, "%s: remove blacklisted path",
 					uev->kernel);
 				i = find_slot(vecs->pathvec, (void *)pp);
-- 
2.26.2

  parent reply	other threads:[~2020-07-09 10:51 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-09 10:51 [PATCH 00/74] multipath-tools series part V: removed path handling mwilck
2020-07-09 10:51 ` [PATCH 54/74] libmultipath: protect use of pp->udev mwilck
2020-07-09 10:51 ` [PATCH 55/74] libmultipath: add uninitialize_path() mwilck
2020-07-09 10:51 ` [PATCH 56/74] multipath-tools: introduce INIT_REMOVED state mwilck
2020-07-09 10:51 ` [PATCH 57/74] libmultipath: update_mpp_paths(): handle INIT_REMOVED mwilck
2020-07-09 10:51 ` [PATCH 58/74] libmultipath: verify_paths(): don't delete paths from pathvec mwilck
2020-07-09 10:51 ` [PATCH 59/74] libmultipath: sync_paths(): handle INIT_REMOVED mwilck
2020-07-09 10:51 ` [PATCH 60/74] libmultipath: orphan_paths(): delete paths in INIT_REMOVED state mwilck
2020-07-09 10:51 ` [PATCH 61/74] libmultipath: adopt_paths(): skip removed paths mwilck
2020-07-18  4:28   ` Benjamin Marzinski
2020-07-09 10:51 ` [PATCH 62/74] multipathd: ev_remove_path(): use INIT_REMOVED mwilck
2020-07-09 10:51 ` mwilck [this message]
2020-07-18  5:46   ` [PATCH 63/74] multipathd: deal with INIT_REMOVED during path addition Benjamin Marzinski
2020-07-09 10:51 ` [PATCH 64/74] multipathd: check_path(): set retrigger_delay if necessary mwilck
2020-07-19  5:07   ` Benjamin Marzinski
2020-08-05 16:37     ` Martin Wilck
2020-07-09 10:51 ` [PATCH 65/74] libmultipath: add update_pathvec_from_dm() mwilck
2020-07-19  4:46   ` Benjamin Marzinski
2020-08-05 20:12     ` Martin Wilck
2020-07-09 10:51 ` [PATCH 66/74] libmultipath: update_pathvec_from_dm: handle pp->mpp mismatch mwilck
2020-07-19  5:12   ` Benjamin Marzinski
2020-08-05 19:44     ` Martin Wilck
2020-07-09 10:51 ` [PATCH 67/74] libmultipath: disassemble_map(): always search paths by dev_t mwilck
2020-07-09 10:51 ` [PATCH 68/74] libmultipath: disassemble_map(): require non-NULL pathvec mwilck
2020-07-09 10:51 ` [PATCH 69/74] libmultipath: disassemble_map(): get rid of "is_daemon" argument mwilck
2020-07-19  5:26   ` Benjamin Marzinski
2020-08-05 20:05     ` Martin Wilck
2020-08-11  4:56       ` Benjamin Marzinski
2020-07-09 10:51 ` [PATCH 70/74] libmultipath: disassemble_map(): do not change pathvec and WWIDs mwilck
2020-07-09 10:51 ` [PATCH 71/74] multipath: use update_pathvec_from_dm() mwilck
2020-07-20  2:57   ` Benjamin Marzinski
2020-08-05 20:22     ` Martin Wilck
2020-07-09 10:51 ` [PATCH 72/74] libmpathpersist: " mwilck
2020-07-09 10:51 ` [PATCH 73/74] libmultipath: decrease loglevel in store_path() mwilck
2020-07-09 10:51 ` [PATCH 74/74] libmultipath: dmparser: constify function arguments mwilck
2020-07-20 21:19 ` [PATCH 00/74] multipath-tools series part V: removed path handling Benjamin Marzinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200709105145.9211-11-mwilck@suse.com \
    --to=mwilck@suse.com \
    --cc=bmarzins@redhat.com \
    --cc=christophe.varoqui@opensvc.com \
    --cc=dm-devel@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.