Re: [pve-devel] [PATCH container 0/2] Improve volume deactivation

[Fabian Ebner <f.ebner@proxmox.com>]

Am 01.12.21 um 17:27 schrieb Aaron Lauterer:
> 
> 
> On 12/1/21 11:12, Fabian Ebner wrote:
>> Am 26.11.21 um 11:19 schrieb Aaron Lauterer:
>>> While working on the reassign feature we (F.Ebner & I) discovered that
>>> it is possible, mainly with RBD volumes, to get into situations where it
>>> is not possible to remove that volume as an old orphaned RBD mapping
>>> still exists.
>>>
>>> Mainly when converting a container on RBD storage to a template and when
>>> adding a new MP to a container that is not running and reassigning that
>>> MP right away to another container.
>>>
>>
>> I feel like cleaning up such things should be the responsibility of 
>> the storage plugin itself. It knows best when a volume gets a new name 
>> and what needs to happen if there is still something using the old 
>> name around.
>>
>> For example, after a full clone, volumes from both containers will be 
>> active and then reassigning or converting to template will lead to the 
>> issue again. There are likely other places where we don't cleanly 
>> deactivate. Of course we could try and hunt them all down ;), but 
>> quoting from [0]:
>>
>> this is fundamentally how volume activation works in PVE - we activate 
>> (and skip the expensive parts if already activated) often, but are 
>> very careful about de-activating only where necessary (shared volumes 
>> when migrating) or clearly 100% right (error handling before removing 
>> a newly allocated volume for example).
>>
>> [0]: https://bugzilla.proxmox.com/show_bug.cgi?id=3756#c3
> 
> Hmm okay yeah, definitely valid regarding the second patch. But the 
> first one would still be valid AFAIU because I don't understand why we 
> activate the volumes when creating a template for containers only, but 
> not for VMs if we don't need to do anything in the volume. So not 
> activating it in the first place would help at least in that case.
> 

Sorry, I didn't mean to imply that the patches were wrong, just wanted 
to point out that they don't fully address the issue.

>>
>>> Aaron Lauterer (2):
>>>    template_create: remove volume activation
>>>    apply_pending_mountpoint: deactivate volumes if not running
>>>
>>>   src/PVE/LXC.pm        | 2 --
>>>   src/PVE/LXC/Config.pm | 2 ++
>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>