[Devel] [PATCH] ploop: Fix maintaince_type obtaining design problem

Kirill Tkhai ktkhai at virtuozzo.com
Thu May 23 14:13:45 MSK 2019


"ploop-balloon status <dev>" made in parallel with "pcompact"
may break its logic and hang in unpredictable state.

Read comment in the function.

https://jira.sw.ru/browse/PSBM-94727

Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
---
 drivers/block/ploop/dev.c      |   31 +++++++++++++++++++++++++++++++
 include/linux/ploop/ploop_if.h |    3 +++
 2 files changed, 34 insertions(+)

diff --git a/drivers/block/ploop/dev.c b/drivers/block/ploop/dev.c
index d896ddec766a..d07928552291 100644
--- a/drivers/block/ploop/dev.c
+++ b/drivers/block/ploop/dev.c
@@ -4481,6 +4481,35 @@ static int ploop_balloon_ioc(struct ploop_device *plo, unsigned long arg)
 	if (ctl.inflate && ctl.keep_intact)
 		return -EINVAL;
 
+	/*
+	 * Userspace may use this IOC to get current maintaince mode
+	 * without changing it, or to change it. The ctl.keep_intact
+	 * modifier is used to differ, what the userspace wants.
+	 *
+	 * The problem was that:
+	 * 1)userspace sometimes passes the modifier set to 1, when
+	 *   the state *must* be changed;
+	 * 2)kernel completely ignores the modifier in case of device
+	 *   is in PLOOP_MNTN_DISCARD, PLOOP_MNTN_FBLOADED, PLOOP_MNTN_RELOC
+	 *   or PLOOP_MNTN_OFF mode.
+	 *
+	 * So, the harmless R/O command "ploop-balloon state <dev>"
+	 * made in parallel with "pcompact" switches the device in
+	 * unexpected maintaince mode, and "pcompact" lose its logic
+	 * and hangs.
+	 *
+	 * It's not possible to fix the problem in one of userspace
+	 * or kernel, since both of them suck. So, we introduce new
+	 * PLOOP_KEEP_INTACK_ALWAYS for newer pcompact to never change
+	 * current state and to fix the problem. Userspace will use
+	 * it. Old kernel and new userspace will work OK together,
+	 * since there is boolean !0 comparison in the kernel.
+	 *
+	 * Old code may be seen at least in ploop 7.0.140.2.
+	 */
+	if (ctl.keep_intact == PLOOP_KEEP_INTACK_ALWAYS)
+		goto mntn_type;
+
 	switch (plo->maintenance_type) {
 	case PLOOP_MNTN_DISCARD:
 		if (!test_bit(PLOOP_S_DISCARD_LOADED, &plo->state))
@@ -4515,6 +4544,8 @@ static int ploop_balloon_ioc(struct ploop_device *plo, unsigned long arg)
 			ploop_relax(plo);
 		}
 	}
+
+mntn_type:
 	ctl.mntn_type = plo->maintenance_type;
 
 	return copy_to_user((void*)arg, &ctl, sizeof(ctl));
diff --git a/include/linux/ploop/ploop_if.h b/include/linux/ploop/ploop_if.h
index 3b5928cfb69e..9c5e541f5f02 100644
--- a/include/linux/ploop/ploop_if.h
+++ b/include/linux/ploop/ploop_if.h
@@ -245,6 +245,9 @@ enum {
 	PLOOP_MNTN_PUSH_BACKUP, /* push backup is in progress */
 };
 
+#define PLOOP_KEEP_INTACK_OLD		1
+#define PLOOP_KEEP_INTACK_ALWAYS	2
+
 /*
  * This define should be in sync with enum above.
  * NB: PLOOP_MNTN_TRACK is handled separately because



More information about the Devel mailing list