From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
	by lore.proxmox.com (Postfix) with ESMTPS id 7F4A71FF173
	for <inbox@lore.proxmox.com>; Mon, 27 Jan 2025 13:16:41 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 9C15929B34;
	Mon, 27 Jan 2025 13:16:33 +0100 (CET)
Date: Mon, 27 Jan 2025 13:16:30 +0100 (CET)
From: =?UTF-8?Q?Fabian_Gr=C3=BCnbichler?= <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Message-ID: <1494603895.9133.1737980190457@webmail.proxmox.com>
In-Reply-To: <mailman.64.1731030431.372.pve-devel@lists.proxmox.com>
References: <mailman.62.1731030290.372.pve-devel@lists.proxmox.com>
 <20241108014620.73352-1-severen.redwood@sitehost.co.nz>
 <mailman.64.1731030431.372.pve-devel@lists.proxmox.com>
MIME-Version: 1.0
X-Priority: 3
Importance: Normal
X-Mailer: Open-Xchange Mailer v7.10.6-Rev72
X-Originating-Client: open-xchange-appsuite
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.051 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
 T_SCC_BODY_TEXT_LINE    -0.01 -
Subject: Re: [pve-devel] [PATCH container v4] api: record CT ID as used
 after a container is destroyed
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Cc: t.lamprecht@proxmox.com
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>


> Severen Redwood via pve-devel <pve-devel@lists.proxmox.com> hat am 08.11.2024 02:46 CET geschrieben:
> After a container is destroyed, record that its ID has been used via the
> `PVE::UsedVmidList` module so that the `/cluster/nextids` endpoint can
> later optionally avoid suggesting previously used IDs.
> 
> Co-authored-by: Daniel Krambrock <krambrock@hrz.uni-marburg.de>
> Signed-off-by: Severen Redwood <severen.redwood@sitehost.co.nz>
> Tested-by: Aaron Lauterer <a.lauterer@proxmox.com>
> Reviewed-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> Changed since v3 is removing an unnecessary `qw(add_vmid)` after
> `use PVE::UsedVmidList`.
> 
>  src/PVE/API2/LXC.pm | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
> index 213e518..e35b26d 100644
> --- a/src/PVE/API2/LXC.pm
> +++ b/src/PVE/API2/LXC.pm
> @@ -28,6 +28,7 @@ use PVE::API2::LXC::Config;
>  use PVE::API2::LXC::Status;
>  use PVE::API2::LXC::Snapshot;
>  use PVE::JSONSchema qw(get_standard_option);
> +use PVE::UsedVmidList;
>  use base qw(PVE::RESTHandler);
>  
>  BEGIN {
> @@ -794,7 +795,9 @@ __PACKAGE__->register_method({
>  		}
>  	    }
>  
> -	    # only now remove the zombie config, else we can have reuse race
> +	    # only now mark the CT ID as previously used and remove the zombie
> +	    # config, else we can have reuse race
> +	    PVE::UsedVmidList::add_vmid($vmid);

if we only add it when the VM is removed via the API, we might miss those where an admin just does "rm /etc/pve/../XXX.conf" (which might happen if something is blocking regular removal)? wouldn't it be safer to add them when creating the VM? or potentially in both cases to increase the likelihood of a VM being registered?

alternatively, a call to nextid already has both lists and could warn if there is a mismatch or even fix it up? i.e., if we don't only add on removal, but upon creation, then the intersection of currently used VMIDs and previously used VMIDs should be identical to the currently used VMIDs. if not, then any missing ones can be added/registered?

all of this obviously applies to containers and VMs..

>  	    PVE::LXC::Config->destroy_config($vmid);
>  	};
>  
> -- 
> 2.47.0


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel