[Devel] [PATCH vz7 v3] ploop: properly restore old delta kobject on replace error

[Alexander Atanasov]

Current code removes the old_delta kobject before the new delta
is completely ready to be used, in case there is an error while
reading BAT, the "too short BAT" error we have seen, the new delta
and its objects are destroyed properly but the original delta kobject
is not restored, so pdelta sysfs dir stay empty and userspace tools
get confused since they consult this files before performing operations
on the ploop device.

To fix this instead of deleting the delta's kobject early move it at
the end and on error restore back original kobject.

Extract code to rename kobject into a function and cleanup its users.
Since kobject_add can fail, get an extra reference on error so later
kobject_del/kobject_put would not destroy it unexpectedly. Object
should be intact except that it would not be visible in sysfs.

Fixes:
5f3ee110e6f4 (ploop: Repopulate holes_bitmap on changing delta, 2019-04-17)

https://jira.vzint.dev/browse/PSBM-146797
Signed-off-by: Alexander Atanasov <alexander.atanasov at virtuozzo.com>
---
 drivers/block/ploop/dev.c | 54 ++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 26 deletions(-)

v1->v2: Addressing review comments. Removed the rename to minimize
possibility of errors. added getting extra refs after failed add.

v2->v3: dropped the idea of extra refs since they are not necessary.
double kobject_put is not an issue  since parent is cleared and
kset objects are not used.

diff --git a/drivers/block/ploop/dev.c b/drivers/block/ploop/dev.c
index 6eb22168b5fe..0d6272b39863 100644
--- a/drivers/block/ploop/dev.c
+++ b/drivers/block/ploop/dev.c
@@ -3594,27 +3594,24 @@ static int ploop_replace_delta(struct ploop_device * plo, unsigned long arg)
 	if (err)
 		goto out_destroy;
 
-	kobject_del(&old_delta->kobj);
-
-	err = KOBJECT_ADD(&delta->kobj, kobject_get(&plo->kobj),
-			  "%d", delta->level);
-	/* _put below is a pair for _get for OLD delta */
-	kobject_put(&plo->kobj);
-
-	if (err < 0) {
-		kobject_put(&plo->kobj);
-		goto out_close;
-	}
-
 	ploop_quiesce(plo);
 	if (delta->ops->replace_delta) {
 		err = delta->ops->replace_delta(delta);
 		if (err) {
 			ploop_relax(plo);
-			goto out_kobj_del;
+			goto out_close;
 		}
 	}
 
+	/* Remove old delta kobj to avoid name collision with the new one */
+	kobject_del(&old_delta->kobj);
+	err = KOBJECT_ADD(&delta->kobj, kobject_get(&plo->kobj),
+			  "%d", delta->level);
+	if (err < 0) {
+		/* _put for failed _ADD */
+		kobject_put(&plo->kobj);
+		goto out_kobj_restore;
+	}
 	ploop_map_destroy(&plo->map);
 	list_replace_init(&old_delta->list, &delta->list);
 	ploop_delta_list_changed(plo);
@@ -3632,11 +3629,14 @@ static int ploop_replace_delta(struct ploop_device * plo, unsigned long arg)
 	old_delta->ops->stop(old_delta);
 	old_delta->ops->destroy(old_delta);
 	kobject_put(&old_delta->kobj);
-	return 0;
-
-out_kobj_del:
-	kobject_del(&delta->kobj);
 	kobject_put(&plo->kobj);
+	return 0;
+out_kobj_restore:
+	/* we haven't dropped our plo->kobj ref just add back */
+	err = KOBJECT_ADD(&old_delta->kobj, &plo->kobj, "%d", old_delta->level);
+	if (err < 0)
+		/* Nothing we can do unfortunately */
+		PL_ERR(plo, "Failed to restore old delta kobject\n");
 out_close:
 	delta->ops->stop(delta);
 out_destroy:
@@ -3965,6 +3965,16 @@ static void renumber_deltas(struct ploop_device * plo)
 	}
 }
 
+/*
+ * Have to implement own version of kobject_rename since it is GPL only symbol
+ */
+static int ploop_rename_delta(struct ploop_delta *delta, int level, char *pref)
+{
+	kobject_del(&delta->kobj);
+	return KOBJECT_ADD(&delta->kobj, &delta->plo->kobj,
+			  "%s%d", pref ? : "", level);
+}
+
 static void rename_deltas(struct ploop_device * plo, int level)
 {
 	struct ploop_delta * delta;
@@ -3974,15 +3984,7 @@ static void rename_deltas(struct ploop_device * plo, int level)
 
 		if (delta->level < level)
 			continue;
-#if 0
-		/* Oops, kobject_rename() is not exported! */
-		sprintf(nname, "%d", delta->level);
-		err = kobject_rename(&delta->kobj, nname);
-#else
-		kobject_del(&delta->kobj);
-		err = KOBJECT_ADD(&delta->kobj, &plo->kobj,
-				  "%d", delta->level);
-#endif
+		err = ploop_rename_delta(delta, delta->level, NULL);
 		if (err)
 			PL_WARN(plo, "rename_deltas: %d %d %d\n", err, level, delta->level);
 	}
-- 
2.39.1