[pve-devel] avoiding VMID reuse

Lauri Tirkkonen lauri at tuxera.com
Wed Apr 11 14:12:17 CEST 2018


On Tue, Mar 13 2018 11:15:23 +0200, Lauri Tirkkonen wrote:
> > Sorry if I misunderstood you but VMIDs are already _guaranteed_ to be
> > unique cluster wide, so also unique per node?
> 
> I'll try to clarify: if I create a VM that gets assigned vmid 100, and
> use zfs for storage, its first disk image is called
> <zfsstorage>/vm-100-disk-1. If I then later remove vmid 100, and create
> another new VM, /nextid will suggest that the new vmid should be 100,
> and its disk image will also be called vm-100-disk-1. We're backing up
> our disk images using zfs snapshots and sends (on other ZFS hosts too,
> not just PVE), so it's quite bad if we reuse a name for a completely
> different dataset - it'll require manual admin intevention. So, we want
> to avoid using any vmid that has been used in the past.
> 
> > Your approach, allowing different nodes from a cluster to alloc
> > the same VMID also should not work, our clustered configuration
> > file system (pmxcfs) does not allows this, i.e. no VMID.conf
> > file can exist multiple times in the qemu-server and lxc folders
> > ()i.e., the folders holding CT/VM configuration files)
> 
> Right. We're not actually running PVE in a cluster configuration, so I
> might've been a little confused there - if the VMID's are unique in the
> cluster anyway, then the storage for the counter shouldn't be under
> "local/", I suppose.

I took another stab at this, dropping the local/ and making it generally
less error prone. So to recap, it:
 - stores next unused vm id in /etc/pve/nextid
 - returns that stored id in API requests for /cluster/nextid (or
   highest current existing vmid+1, if nextid is lower and thus out of
   sync)
 - PVE::Cluster::alloc_vmid allocates the requested vm id, by storing
   it into /etc/pve/nextid if it is higher than what there is currently
   (using lower, non-existing id's is still allowed)

Thoughts?
-------------- next part --------------
>From f7e006cca8d4f41def8ac319e7fa9daf1cd0dc90 Mon Sep 17 00:00:00 2001
From: Lauri Tirkkonen <lauri at tuxera.com>
Date: Tue, 10 Apr 2018 17:58:34 +0300
Subject: [PATCH] use PVE::Cluster::alloc_vmid

Signed-off-by: Lauri Tirkkonen <lauri at tuxera.com>
---
 src/PVE/API2/LXC.pm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index bce5fa3..6e516e0 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -334,6 +334,7 @@ __PACKAGE__->register_method({
 	    &$check_vmid_usage(); # final check after locking
 	    my $old_conf;
 
+	    PVE::Cluster::alloc_vmid($vmid);
 	    my $config_fn = PVE::LXC::Config->config_file($vmid);
 	    if (-f $config_fn) {
 		die "container exists" if !$restore; # just to be sure
@@ -1322,6 +1323,7 @@ __PACKAGE__->register_method({
 
 		my $src_conf = $snapname ? $src_conf->{snapshots}->{$snapname} : $src_conf;
 
+		PVE::Cluster::alloc_vmid($newid);
 		$conffile = PVE::LXC::Config->config_file($newid);
 		die "unable to create CT $newid: config file already exists\n"
 		    if -f $conffile;
-- 
2.16.2

-------------- next part --------------
>From 29443e827855d24dcc60684973afa9d78cd6c66d Mon Sep 17 00:00:00 2001
From: Lauri Tirkkonen <lauri at tuxera.com>
Date: Tue, 10 Apr 2018 18:38:25 +0300
Subject: [PATCH] use /etc/pve/nextid for /cluster/nextid

---
 PVE/API2/Cluster.pm | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/PVE/API2/Cluster.pm b/PVE/API2/Cluster.pm
index 7f38e61c..b9beec9a 100644
--- a/PVE/API2/Cluster.pm
+++ b/PVE/API2/Cluster.pm
@@ -505,11 +505,21 @@ __PACKAGE__->register_method({
 	    raise_param_exc({ vmid => "VM $vmid already exists" });
 	}
 
-	for (my $i = 100; $i < 10000; $i++) {
-	    return $i if !defined($idlist->{$i});
-	}
-
-	die "unable to get any free VMID\n";
+        my $i = 0;
+	if (open(my $fh, '<', '/etc/pve/nextid')) {
+            $i = 0 + readline($fh);
+            close($fh);
+        }
+        my $highest = List::Util::max(keys %$idlist) or 0;
+        $highest = 0 unless defined($highest);
+        if ($i <= $highest) {
+            # nextid is out of sync with reality; suggest current highest
+            # vmid+1
+            $i = $highest + 1;
+        }
+        $i = 100 if $i < 100;
+
+	return $i;
     }});
 
 1;
-- 
2.16.2

-------------- next part --------------
>From 4aca643a7f727588f51ad7c7e2d18a96912f90a5 Mon Sep 17 00:00:00 2001
From: Lauri Tirkkonen <lauri at tuxera.com>
Date: Tue, 10 Apr 2018 18:01:57 +0300
Subject: [PATCH] implement PVE::Cluster::alloc_vmid

this is used to ensure the next allocated vmid is always greater than
anything previously selected.

Signed-off-by: Lauri Tirkkonen <lauri at tuxera.com>
---
 data/PVE/Cluster.pm | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/data/PVE/Cluster.pm b/data/PVE/Cluster.pm
index fabf5bc..c83a65d 100644
--- a/data/PVE/Cluster.pm
+++ b/data/PVE/Cluster.pm
@@ -1004,6 +1004,25 @@ sub log_msg {
    syslog("err", "writing cluster log failed: $@") if $@;
 }
 
+sub alloc_vmid {
+    my ($vmid) = @_;
+    check_vmid_unused($vmid);
+    my $next = 0;
+    if (open(my $fh, '<', '/etc/pve/nextid')) {
+        $next = 0 + readline($fh);
+        close($fh);
+    }
+    if ($vmid >= $next) {
+        $next = 1 + $vmid;
+        if (open(my $fh, '>', '/etc/pve/nextid')) {
+            print $fh "$next\n";
+            close($fh);
+        }
+    } else {
+        warn "allocated id $vmid < nextid $next\n";
+    }
+}
+
 sub check_vmid_unused {
     my ($vmid, $noerr) = @_;
 
-- 
2.16.2

-------------- next part --------------
>From 2542955a2650be3568a8aed40fdb9be5d5bfbabd Mon Sep 17 00:00:00 2001
From: Lauri Tirkkonen <lauri at tuxera.com>
Date: Tue, 10 Apr 2018 18:24:25 +0300
Subject: [PATCH] use PVE::Cluster::alloc_vmid

Signed-off-by: Lauri Tirkkonen <lauri at tuxera.com>
---
 PVE/API2/Qemu.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 0f27d29..5cd6d46 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -566,7 +566,7 @@ __PACKAGE__->register_method({
 	my $createfn = sub {
 
 	    # test after locking
-	    PVE::Cluster::check_vmid_unused($vmid);
+	    PVE::Cluster::alloc_vmid($vmid);
 
 	    # ensure no old replication state are exists
 	    PVE::ReplicationState::delete_guest_states($vmid);
@@ -2587,6 +2587,7 @@ __PACKAGE__->register_method({
 	    # we also try to do all tests before we fork the worker
 
 	    my $conf = PVE::QemuConfig->load_config($vmid);
+	    PVE::Cluster::alloc_vmid($vmid);
 
 	    PVE::QemuConfig->check_lock($conf);
 
-- 
2.16.2



More information about the pve-devel mailing list