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?
>From f7e006cca8d4f41def8ac319e7fa9daf1cd0dc90 Mon Sep 17 00:00:00 2001 From: Lauri Tirkkonen <la...@tuxera.com> Date: Tue, 10 Apr 2018 17:58:34 +0300 Subject: [PATCH] use PVE::Cluster::alloc_vmid Signed-off-by: Lauri Tirkkonen <la...@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
>From 29443e827855d24dcc60684973afa9d78cd6c66d Mon Sep 17 00:00:00 2001 From: Lauri Tirkkonen <la...@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
>From 4aca643a7f727588f51ad7c7e2d18a96912f90a5 Mon Sep 17 00:00:00 2001 From: Lauri Tirkkonen <la...@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 <la...@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
>From 2542955a2650be3568a8aed40fdb9be5d5bfbabd Mon Sep 17 00:00:00 2001 From: Lauri Tirkkonen <la...@tuxera.com> Date: Tue, 10 Apr 2018 18:24:25 +0300 Subject: [PATCH] use PVE::Cluster::alloc_vmid Signed-off-by: Lauri Tirkkonen <la...@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
_______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel