From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 63CFA1FF0B0 for ; Fri, 09 Oct 2026 18:23:19 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 5BE9121682; Fri, 09 Oct 2026 18:23:03 +0200 (CEST) Message-ID: <2165b7be-5371-4922-b4ce-c783306aedde@proxmox.com> Date: Fri, 9 Oct 2026 18:22:59 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH many v7 00/24] add option to prevent suggesting previously used VMIDs To: =?UTF-8?Q?Michael_K=C3=B6ppl?= , pve-devel@lists.proxmox.com References: <20261005144805.825538-1-m.koeppl@proxmox.com> Content-Language: en-US From: Fiona Ebner In-Reply-To: <20261005144805.825538-1-m.koeppl@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1791562979081 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.445 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: I2HK55MBWSUA6V3TE224552747LX22MY X-Message-ID-Hash: I2HK55MBWSUA6V3TE224552747LX22MY X-MailFrom: f.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Am 05.10.26 um 4:49 PM schrieb Michael Köppl: > This is based on the original series [0] by Severen Redwood and Daniel > Krambrock. It is rebased on the latest master branches and incorporates > the feedback from v6 [2]. > > The last patch for cluster, qemu-server and container and the last two > for guest-common and manager add the optional enforcement of the > next-id settings. The rest of the series works without them. Since IDs > are recorded before a guest exists and stay recorded if its creation > fails, retrying a failed creation with the same ID is rejected when > both 'enforce' and 'unique' are set. Can we do a PVE::Cluster::check_vmid_unused($vmid, 1) call in PVE::GuestID::register_used_id() rather than passing along the state if existing from the call sites? Would keep the interface a bit cleaner and avoid the need for the last qemu-server and container patches. And GuestID already depends on Cluster. > > I kept the trailers from the original series where I made only minimal > changes, but added markers (MK: ...) in those cases. For bigger > changes, I changed the `Co-authored-by` and `Signed-off-by` trailers to > `Signed-off-by`. I removed Aaron's T-b and R-b trailers since it's been > a long time and the trailers might be misleading considering the series > has changed in some of its implementation details overall. > > Dependencies: > - guest-common needs a versioned build and runtime dependency on > libpve-cluster-perl with virtual-guest/used-guest-ids registered, > since loading PVE::AbstractConfig now registers the file. > - qemu-server and container need a versioned dependency on > libpve-guest-common-perl with register_used_id() and its 'existing' > option. > - guest-common with enforcement should break older qemu-server and > container, which do not exempt destroying existing guests from it. Is this only relevant if we would bump before enforcement? If all changes come in the same bump it won't matter, or? And if going with my suggestion above it should also be fine. > - manager needs versioned dependencies on libpve-cluster-perl (next-id > 'unique' and 'enforce') and libpve-guest-common-perl > (get_next_unused_id() and assert_id_satisfies_next_id_settings()). > Two more minor issues in 5/24 and 17/24. Other than that, consider the series: Reviewed-by: Fiona Ebner Tested-by: Fiona Ebner