From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 6C68183099 for ; Thu, 2 Dec 2021 08:50:17 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5ACEA15553 for ; Thu, 2 Dec 2021 08:49:47 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 5443115548 for ; Thu, 2 Dec 2021 08:49:46 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 26BA24414F for ; Thu, 2 Dec 2021 08:49:46 +0100 (CET) Message-ID: Date: Thu, 2 Dec 2021 08:49:45 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.2 Content-Language: en-US To: Fabian Ebner , pve-devel@lists.proxmox.com References: <20211126101938.3992163-1-a.lauterer@proxmox.com> <0badc51b-ae20-d3e1-ede4-bf2e95dbb74f@proxmox.com> From: Aaron Lauterer In-Reply-To: <0badc51b-ae20-d3e1-ede4-bf2e95dbb74f@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 1.720 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -3.3 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH container 0/2] Improve volume deactivation X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Dec 2021 07:50:17 -0000 On 12/2/21 08:40, Fabian Ebner wrote: > 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. No need to excuse yourself, I came across much meaner than I intended to. :) I'll have to think about how to handle the second patch better to avoid these issues of potentially deactivating a storage when we shouldn't. >>> >>>> 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(-) >>>>