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.atana...@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 _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel
