[PATCH guest-common v7 10/24] guest id: optionally enforce the next-id range and uniqueness

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

If the 'enforce' subproperty of next-id is enabled, ensure that only
guest IDs from the configured range and, if unique is enabled as well,
only previously unused IDs can be used. By setting existing => 1, the
enforcement of these criteria is disabled for actions on existing
guests, such as destroy, remote migration with --delete, and restore
over an existing guest.

IDs are recorded before a guest exists and stay recorded if its
creation fails later. With both 'enforce' and 'unique' set, retrying a
failed creation with the same ID is therefore rejected.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 src/PVE/AbstractConfig.pm |  4 ++--
 src/PVE/GuestID.pm        | 35 +++++++++++++++++++++++++++++------
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/src/PVE/AbstractConfig.pm b/src/PVE/AbstractConfig.pm
index 229e4bd..2f20596 100644
--- a/src/PVE/AbstractConfig.pm
+++ b/src/PVE/AbstractConfig.pm
@@ -260,9 +260,9 @@ sub create_and_lock_config {
         $vmid,
         5,
         sub {
-            PVE::Cluster::check_vmid_unused($vmid, $allow_existing);
+            my $is_new = PVE::Cluster::check_vmid_unused($vmid, $allow_existing);
 
-            PVE::GuestID::register_used_id($vmid);
+            PVE::GuestID::register_used_id($vmid, { existing => !$is_new });
 
             my $conf = eval { $class->load_config($vmid) } || {};
             $class->check_lock($conf);
diff --git a/src/PVE/GuestID.pm b/src/PVE/GuestID.pm
index 4ab6c1c..ba71e4f 100644
--- a/src/PVE/GuestID.pm
+++ b/src/PVE/GuestID.pm
@@ -195,25 +195,48 @@ my sub insert_id($ranges, $id) {
     return 1;
 }
 
-sub register_used_id($id) {
-    cfs_lock_file(
+my sub assert_id_in_next_id_range($next_id_opts, $id) {
+    my $lower = $next_id_opts->{lower};
+    my $upper = $next_id_opts->{upper};
+
+    die "guest ID $id is below the lower boundary $lower of the next-id range\n"
+        if defined($lower) && $id < $lower;
+    die "guest ID $id is not below the upper boundary $upper of the next-id range\n"
+        if defined($upper) && $id >= $upper;
+}
+
+# Records $id as used. If the next-id datacenter option enforces its
+# range or uniqueness, IDs a new guest must not use are rejected, unless
+# $opts->{existing} marks $id as belonging to an existing guest.
+sub register_used_id($id, $opts = {}) {
+    my $next_id_opts = cfs_read_file('datacenter.cfg')->{'next-id'} // {};
+    # never reject the ID of a currently existing guest
+    my $enforce = $next_id_opts->{enforce} && !$opts->{existing};
+
+    assert_id_in_next_id_range($next_id_opts, $id) if $enforce;
+
+    my $recorded = cfs_lock_file(
         $FILENAME,
         10,
         sub {
             my $ranges = cfs_read_file($FILENAME);
 
-            return if !insert_id($ranges, $id);
+            return 0 if !insert_id($ranges, $id);
 
             cfs_write_file($FILENAME, $ranges);
+            return 1;
         },
     );
     if (my $err = $@) {
-        my $dc_conf = cfs_read_file('datacenter.cfg');
-
         my $emsg = "unable to record guest ID $id as used";
-        die "$emsg - $err" if $dc_conf->{'next-id'}->{unique};
+        die "$emsg - $err" if $next_id_opts->{unique};
         warn "$emsg - $err";
+
+        return;
     }
+
+    die "guest ID $id was used before\n"
+        if $enforce && $next_id_opts->{unique} && !$recorded;
 }
 
 cfs_register_file($FILENAME, \&parse_id_list, \&write_id_list);
-- 
2.47.3