Re: [PATCH many v7 00/24] add option to prevent suggesting previously used VMIDs

[Fiona Ebner <f.ebner@proxmox.com>]

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 <f.ebner@proxmox.com>
Tested-by: Fiona Ebner <f.ebner@proxmox.com>