From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 23CA71FF0AF for ; Thu, 24 Sep 2026 18:16:02 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 26AB021673; Thu, 24 Sep 2026 18:15:38 +0200 (CEST) From: =?UTF-8?q?Michael=20K=C3=B6ppl?= To: pve-devel@lists.proxmox.com Subject: [PATCH many v6 00/18] add option to prevent suggesting previously used VMIDs Date: Thu, 24 Sep 2026 18:14:52 +0200 Message-ID: <20260924161510.847362-1-m.koeppl@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1790266512417 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.475 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: 7DQNZ7ZNG7JHG6W3QUKU2HO3VXHLCW2N X-Message-ID-Hash: 7DQNZ7ZNG7JHG6W3QUKU2HO3VXHLCW2N X-MailFrom: m.koeppl@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: 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 v5 [1]. The last patch for cluster, guest-common, qemu-server and container and the last two for 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. 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. - manager needs versioned dependencies on libpve-cluster-perl (next-id 'unique' and 'enforce') and libpve-guest-common-perl (get_next_unused_id() and check_enforced_id()). Feedback regarding the "forced" merging of ranges to keep the file size in check would be much appreciated. The merging could continue until the file size is below the lower threshold to avoid the warning being printed too soon again, but I wanted to avoid unnecessarily marking guest IDs as used. Tests were also added for the pve-guest-common GuestID module specifically since the logic behind it is complex enough to warrant some unit testts. Changes since v5 (thanks @Fiona for the extensive feedback): - Moved the file to virtual-guest/used-guest-ids. - Keep used IDs as ranges in memory as well. get_next_unused_id() returns the lowest ID that is neither in use nor was used before. - Renamed the module to PVE::GuestID, use signatures, and renamed add_id() to register_used_id(). - Record IDs in create_and_lock_config(), which also covers qm importovf, and when remote migration with --delete removes a guest. - Failing to record an ID only warns while 'unique' is off. - Close the smallest gaps once the file would exceed 768 KiB, and warn above 512 KiB. Are these thresholds fine? - 'unique' is now a sub-property of next-id. - New optional 'enforce' sub-property of next-id. - Added tests for for the GuestID module. Changes since v4: - File in pmxcfs has been renamed to used-guest-ids to make it a bit more general (both VMs and CTs) while still making clear which IDs are meant. - Moved the `UsedVmidList` module from pve-manager to pve-guest-common and renamed it `UsedGuestIDs`. Also renamed some of the functions there for more consistent naming. - Implemented `write_id_list` more closely aligned with Fabian's feedback. - Also record used VMIDs when creating, cloning, destroying guests as well as incoming remote migrations. (Thanks, @Fiona, for the off-list input and feedback!) - Moved the `add_id` calls for recording used IDs to before anything is persisted during any of the above actions. - Rephrased some commit messages. [0] https://lore.proxmox.com/pve-devel/mailman.62.1731030290.372.pve-devel@lists.proxmox.com/ [1] https://lore.proxmox.com/pve-devel/20260921155410.938337-1-m.koeppl@proxmox.com/ cluster: Michael Köppl (2): datacenter config: add unique subproperty to next-id datacenter config: next-id: add enforce subproperty Severen Redwood (1): cluster files: add virtual-guest/used-guest-ids src/PVE/Cluster.pm | 1 + src/PVE/DataCenterConfig.pm | 15 +++++++++++++++ src/pmxcfs/status.c | 1 + 3 files changed, 17 insertions(+) guest-common: Michael Köppl (6): add module to track previously used guest IDs tests: add tests for used guest ID tracking abstract config: register used guest ID when creating config guest id: keep used ID list below the pmxcfs file size limit tests: add tests for used-guest-ids max file size handling guest id: optionally enforce the next-id range and uniqueness debian/control | 1 + src/Makefile | 1 + src/PVE/AbstractConfig.pm | 5 +- src/PVE/GuestID.pm | 251 +++++++++++++++++++++++++++++ src/tests/Makefile | 7 +- src/tests/guest-id-tests.pl | 303 ++++++++++++++++++++++++++++++++++++ 6 files changed, 565 insertions(+), 3 deletions(-) create mode 100644 src/PVE/GuestID.pm create mode 100755 src/tests/guest-id-tests.pl qemu-server: Michael Köppl (2): api: record VM ID as used on destruction and remote migration api, remote migrate: exempt existing VMs from next-id enforcement src/PVE/API2/Qemu.pm | 9 +++++++++ src/PVE/QemuMigrate.pm | 4 ++++ 2 files changed, 13 insertions(+) container: Michael Köppl (2): api: record CT ID as used on destruction and remote migration api, migrate: exempt existing CTs from next-id enforcement src/PVE/API2/LXC.pm | 5 +++++ src/PVE/LXC/Migrate.pm | 4 ++++ 2 files changed, 9 insertions(+) manager: Michael Köppl (4): fix #4369: api: optionally only suggest unique IDs ui: dc options: rename VMID to guest ID api: nextid: reject IDs forbidden by next-id enforcement ui: dc options: add option to enforce next free guest ID settings Severen Redwood (1): fix #4369: ui: dc options: add option for unique VM/CT IDs PVE/API2/Cluster.pm | 19 +++++++++++++++---- www/manager6/dc/OptionView.js | 18 +++++++++++++++++- 2 files changed, 32 insertions(+), 5 deletions(-) Summary over all repositories: 15 files changed, 636 insertions(+), 8 deletions(-) -- Generated by murpp 0.12.1