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

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

Thanks for having another look at this series!

On Fri Oct 9, 2026 at 6:22 PM CEST, Fiona Ebner wrote:
> 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.

Good point, that makes more sense than making every call site pass it
in anyway! Will adapt it in a v8, thanks!

>
>>
>> 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.

Should only be relevant if there's a version bump before enforcement,
yes. That was formulated a bit too drastically, but wanted to point it
out in some way.

In any case, I'll use your suggestion from above anyway, so should be
fine. Thanks!

>
>> - 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>