[PATCH guest-common v6 09/18] 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        | 49 ++++++++++++++++++++++++++++++++++-----
 2 files changed, 45 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 4ab6f2d..02abf24 100644
--- a/src/PVE/GuestID.pm
+++ b/src/PVE/GuestID.pm
@@ -187,26 +187,63 @@ my sub insert_id($ranges, $id) {
     return 1;
 }
 
-sub register_used_id($id) {
-    cfs_lock_file(
+my sub check_range($next_id, $id) {
+    my $lower = $next_id->{lower};
+    my $upper = $next_id->{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;
+}
+
+# Dies if the next-id datacenter option enforces its range or uniqueness
+# and a new guest must not use $id. Existing guests are not considered.
+sub check_enforced_id($id) {
+    my $next_id = cfs_read_file('datacenter.cfg')->{'next-id'} // {};
+    return if !$next_id->{enforce};
+
+    check_range($next_id, $id);
+
+    if ($next_id->{unique}) {
+        my $ranges = cfs_read_file($FILENAME);
+        die "guest ID $id was used before\n" if next_unused($ranges, $id) != $id;
+    }
+}
+
+# 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 = cfs_read_file('datacenter.cfg')->{'next-id'} // {};
+    # never reject an existing guest
+    my $enforce = $next_id->{enforce} && !$opts->{existing};
+
+    check_range($next_id, $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->{unique};
         warn "$emsg - $err";
+
+        return;
     }
+
+    die "guest ID $id was used before\n"
+        if $enforce && $next_id->{unique} && !$recorded;
 }
 
 cfs_register_file($FILENAME, \&parse_id_list, \&write_id_list);
-- 
2.47.3