[Devel] [PATCH RHEL7 COMMIT] ploop: properly restore old delta kobject on replace error

Konstantin Khorenko khorenko at virtuozzo.com
Fri Jun 30 20:56:48 MSK 2023


The commit is pushed to "branch-rh7-3.10.0-1160.90.1.vz7.200.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-1160.90.1.vz7.200.2
------>
commit 72b0b506d642972be8b087aa571ac493ec0e3ccb
Author: Alexander Atanasov <alexander.atanasov at virtuozzo.com>
Date:   Wed Jun 14 23:23:58 2023 +0300

    ploop: properly restore old delta kobject on replace error
    
    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>
    Reviewed-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
---
 drivers/block/ploop/dev.c | 54 ++++++++++++++++++++++++-----------------------
 1 file changed, 28 insertions(+), 26 deletions(-)

diff --git a/drivers/block/ploop/dev.c b/drivers/block/ploop/dev.c
index 75e427927713..7ce951f2f387 100644
--- a/drivers/block/ploop/dev.c
+++ b/drivers/block/ploop/dev.c
@@ -3601,27 +3601,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);
@@ -3639,11 +3636,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:
@@ -3972,6 +3972,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;
@@ -3981,15 +3991,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);
 	}


More information about the Devel mailing list