[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