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

[Alexander Atanasov]

On 6.06.23 13:09, Pavel Tikhomirov wrote:

... [snip] ...
>> @@ -3594,15 +3604,9 @@ 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);
>> -
>> +    err = ploop_rename_delta(old_delta, old_delta->level, "old_");
>>       if (err < 0) {
>> -        kobject_put(&plo->kobj);
>> +        PL_WARN(plo, "Failed to rename old delta kobj\n");
>>           goto out_close;
> 
> I don't like this error path, if ploop_rename_delta failed it means that 
> we did kobject_del(&old_delta->kobj); but didn't finish corresponding 
> KOBJECT_ADD, and in out_close, we would do ploop_rename_delta again 
> where we would do second kobject_del(&old_delta->jobj) and corresponding 
> KOBJECT_ADD. This would do double 
> kobject_del->kobj_kset_leave->kset_put, not sure if it would break 
> something or not though.
> 
> I mean, maybe it's better to preserve original out_close, and add a new 
> label out_restore_rename.


My goal was to not have a situation where kobj is not linked so it will 
not be seen as missing in case of a problem. But seems it can cause 
another issue, so i will rework as you suggest.


-- 
Regards,
Alexander Atanasov
P.S. other patch too