[PATCH many v8 00/22] add option to prevent suggesting previously used VMIDs

["Michael Köppl" <m.koeppl@proxmox.com>]

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 v7 [3].

The last patch for cluster 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.

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
`Originally-by` (except for the UI patch for the 'unique' checkbox,
which I rewrote). 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().
- 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()).

Changes since v7 [3] (thanks @Fiona for the review and testing):
- guest-common:
  - get_next_unused_id() no longer restarts the traversal of the used ID
    ranges for every existing guest. next_unused() is inlined and
    assert_id_satisfies_next_id_settings() uses get_next_unused_id()
    instead.
  - register_used_id() detects existing guests itself through
    check_vmid_unused() instead of taking an 'existing' option. This
    drops the qemu-server and pve-container patches that passed the
    param in.
- manager: keep the translated message for IDs of guests known to the UI
  in the guest ID field. Only show the nextid error otherwise.
- Added Fiona's R-b and T-b to all patches except 5/22, since I
  considered the rework of get_next_unused_id() substantial enough to
  not want to suggest that this has actually been reviewed and tested by
  her.

Changes since v6 [2] (thanks @Fiona for the feedback):
- guest-common:
  - Add preparatory commit, sorting the install list in src/Makefile
    alphabetically.
  - Renamed check_range() to assert_id_in_next_id_range() and
    check_enforced_id() to assert_id_satisfies_next_id_settings(), and
    moved the latter into its own commit.
  - Improved naming of variables for better clarity
  - Expanded the example comment for close_smallest_gaps() (thanks,
    @Fiona)
  - Tests: added cases for a missing trailing newline and for merging
    ranges with adjacent IDs, use only valid guest IDs, and dropped
    checks of the order in which equal gaps are closed.
- manager:
  - New patches to show the error returned by the nextid endpoint in
    the guest ID field, instead of always reporting the ID as in use.
  - nextid: the error for IDs of existing guests now refers to guest
    IDs instead of VMs.
- docs: new patches to list the used-guest-ids file in pmxcfs.adoc and
  to describe the 'unique' and 'enforce' options in pvecm.adoc.

Changes since v5 [1] (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/
[2] https://lore.proxmox.com/pve-devel/20260924161510.847362-1-m.koeppl@proxmox.com/
[3] https://lore.proxmox.com/pve-devel/20261005144805.825538-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 (8):
  src/makefile: order files alphabetically
  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
  guest id: add helper to check guest IDs against next-id enforcement

 debian/control              |   1 +
 src/Makefile                |   7 +-
 src/PVE/AbstractConfig.pm   |   3 +
 src/PVE/GuestID.pm          | 253 ++++++++++++++++++++++++++++++
 src/tests/Makefile          |   7 +-
 src/tests/guest-id-tests.pl | 304 ++++++++++++++++++++++++++++++++++++
 6 files changed, 570 insertions(+), 5 deletions(-)
 create mode 100644 src/PVE/GuestID.pm
 create mode 100755 src/tests/guest-id-tests.pl


qemu-server:

Michael Köppl (1):
  api: record VM ID as used on destruction and remote migration

 src/PVE/API2/Qemu.pm   | 9 +++++++++
 src/PVE/QemuMigrate.pm | 4 ++++
 2 files changed, 13 insertions(+)


container:

Michael Köppl (1):
  api: record CT ID as used on destruction and remote migration

 src/PVE/API2/LXC.pm    | 5 +++++
 src/PVE/LXC/Migrate.pm | 4 ++++
 2 files changed, 9 insertions(+)


manager:

Michael Köppl (7):
  ui: guest ID selector: rename exists flag to rejected
  ui: guest ID selector: show the error returned by nextid
  fix #4369: api: optionally only suggest unique IDs
  ui: dc options: rename VMID to guest ID
  fix #4369: ui: dc options: add option for unique VM/CT IDs
  api: nextid: reject IDs forbidden by next-id enforcement
  ui: dc options: add option to enforce next free guest ID settings

 PVE/API2/Cluster.pm                  | 19 +++++++++++++++----
 www/manager6/dc/OptionView.js        | 18 +++++++++++++++++-
 www/manager6/form/GuestIDSelector.js | 19 ++++++++++++++-----
 3 files changed, 46 insertions(+), 10 deletions(-)


docs:

Michael Köppl (2):
  pmxcfs: files: add virtual-guest/used-guest-ids
  pvecm: next-id: document unique and enforce options

 pmxcfs.adoc |  1 +
 pvecm.adoc  | 33 +++++++++++++++++++++++++++++++--
 2 files changed, 32 insertions(+), 2 deletions(-)


Summary over all repositories:
  18 files changed, 687 insertions(+), 17 deletions(-)

-- 
Generated by murpp 0.12.1