* [pve-devel] [PATCH manager v2 1/2] close #4369: api: optionally only suggest unique IDs
[not found] <20241004050957.441759-1-severen.redwood@sitehost.co.nz>
@ 2024-10-04 5:07 ` Severen Redwood via pve-devel
2024-10-04 5:07 ` [pve-devel] [PATCH manager v2 2/2] close #4369: ui: add datacenter option for unique VM/CT IDs Severen Redwood via pve-devel
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Severen Redwood via pve-devel @ 2024-10-04 5:07 UTC (permalink / raw)
To: pve-devel; +Cc: Severen Redwood, Thomas Lamprecht
[-- Attachment #1: Type: message/rfc822, Size: 6938 bytes --]
From: Severen Redwood <severen.redwood@sitehost.co.nz>
To: pve-devel@lists.proxmox.com
Cc: Thomas Lamprecht <t.lamprecht@proxmox.com>, Dietmar Maurer <dietmar@proxmox.com>, Severen Redwood <severen.redwood@sitehost.co.nz>, Daniel Krambrock <krambrock@hrz.uni-marburg.de>
Subject: [PATCH manager v2 1/2] close #4369: api: optionally only suggest unique IDs
Date: Fri, 4 Oct 2024 18:07:40 +1300
Message-ID: <20241004050957.441759-2-severen.redwood@sitehost.co.nz>
At the moment, the `/cluster/nextid` API endpoint will return the lowest
available VM/CT ID, which means that it will suggest re-using VM IDs.
This can be undesirable, so add an optional check to ensure that it
chooses an ID which is not and has never been in use.
This optional behaviour is enabled when `unique-next-id: 1` in
the data centre config, and the previously used IDs are tracked as a
list in the file `/etc/pve/used_vmids.list`.
Co-authored-by: Daniel Krambrock <krambrock@hrz.uni-marburg.de>
Signed-off-by: Severen Redwood <severen.redwood@sitehost.co.nz>
---
PVE/API2/Cluster.pm | 13 +++++++--
PVE/Makefile | 1 +
PVE/UsedVmidList.pm | 70 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 82 insertions(+), 2 deletions(-)
create mode 100644 PVE/UsedVmidList.pm
diff --git a/PVE/API2/Cluster.pm b/PVE/API2/Cluster.pm
index 04387ab4..d0b02d78 100644
--- a/PVE/API2/Cluster.pm
+++ b/PVE/API2/Cluster.pm
@@ -20,6 +20,7 @@ use PVE::RPCEnvironment;
use PVE::SafeSyslog;
use PVE::Storage;
use PVE::Tools qw(extract_param);
+use PVE::UsedVmidList;
use PVE::API2::ACMEAccount;
use PVE::API2::ACMEPlugin;
@@ -813,12 +814,20 @@ __PACKAGE__->register_method({
my $dc_conf = PVE::Cluster::cfs_read_file('datacenter.cfg');
my $next_id = $dc_conf->{'next-id'} // {};
+ my $want_unique = $dc_conf->{'unique-next-id'} // 0;
my $lower = $next_id->{lower} // 100;
my $upper = $next_id->{upper} // (1000 * 1000); # note, lower than the schema-maximum
- for (my $i = $lower; $i < $upper; $i++) {
- return $i if !defined($idlist->{$i});
+ if ($want_unique) {
+ my $used_ids = PVE::Cluster::cfs_read_file('used_vmids.list');
+ for (my $i = $lower; $i < $upper; $i++) {
+ return $i if !defined($idlist->{$i}) and !defined($used_ids->{$i});
+ }
+ } else {
+ for (my $i = $lower; $i < $upper; $i++) {
+ return $i if !defined($idlist->{$i});
+ }
}
die "unable to get any free VMID in range [$lower, $upper]\n";
diff --git a/PVE/Makefile b/PVE/Makefile
index efcb250d..29775e78 100644
--- a/PVE/Makefile
+++ b/PVE/Makefile
@@ -15,6 +15,7 @@ PERLSOURCE = \
NodeConfig.pm \
PullMetric.pm \
Report.pm \
+ UsedVmidList.pm \
VZDump.pm
all: pvecfg.pm $(SUBDIRS)
diff --git a/PVE/UsedVmidList.pm b/PVE/UsedVmidList.pm
new file mode 100644
index 00000000..b88a8681
--- /dev/null
+++ b/PVE/UsedVmidList.pm
@@ -0,0 +1,70 @@
+package PVE::UsedVmidList;
+
+use strict;
+use warnings;
+
+use PVE::Cluster;
+
+my $read_id_list = sub {
+ my ($filename, $raw) = @_;
+
+ return {} if !defined($raw);
+
+ my %used_ids;
+ my @lines = split(/\n/, $raw);
+ foreach my $line (@lines) {
+ if ($line =~ m/^(\d+)$/) {
+ $used_ids{$1} = 1;
+ } elsif ($line =~ m/^(\d+)-(\d+)$/) {
+ foreach my $id ($1..$2) {
+ $used_ids{$id} = 1;
+ }
+ } else {
+ warn "Skipping invalid entry in used_vmids.list: $line\n";
+ }
+ }
+
+ return \%used_ids;
+};
+
+my $write_id_list = sub {
+ my ($filename, $used_ids) = @_;
+ my @used_ids = sort {$a <=> $b} keys(%$used_ids);
+
+ my @lines;
+ my $len = scalar(@used_ids);
+ for (my $i = 0; $i < $len; $i++) {
+ my $line = "$used_ids[$i]";
+
+ my $j = $i;
+ while ($j + 1 < $len and $used_ids[$j] + 1 == $used_ids[$j + 1]) {
+ $j++;
+ }
+
+ # If we find a range of consecutive IDs, write $ids[$i]-$ids[$j] to
+ # denote the range so that we avoid storing each individual integer.
+ if ($i != $j) {
+ $line .= "-$used_ids[$j]";
+ }
+
+ $i = $j;
+ push(@lines, $line);
+ }
+
+ return join("\n", @lines) . "\n";
+};
+
+PVE::Cluster::cfs_register_file('used_vmids.list', $read_id_list, $write_id_list);
+
+sub add_vmid {
+ my ($vmid) = @_;
+
+ PVE::Cluster::cfs_lock_file('used_vmids.list', 10, sub {
+ my $used_ids = PVE::Cluster::cfs_read_file('used_vmids.list');
+
+ $used_ids->{$vmid} = 1;
+ PVE::Cluster::cfs_write_file('used_vmids.list', $used_ids);
+ });
+}
+
+1;
--
2.46.2
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* [pve-devel] [PATCH manager v2 2/2] close #4369: ui: add datacenter option for unique VM/CT IDs
[not found] <20241004050957.441759-1-severen.redwood@sitehost.co.nz>
2024-10-04 5:07 ` [pve-devel] [PATCH manager v2 1/2] close #4369: api: optionally only suggest unique IDs Severen Redwood via pve-devel
@ 2024-10-04 5:07 ` Severen Redwood via pve-devel
2024-10-04 5:07 ` [pve-devel] [PATCH container v2] api: record CT ID as used after a container is destroyed Severen Redwood via pve-devel
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Severen Redwood via pve-devel @ 2024-10-04 5:07 UTC (permalink / raw)
To: pve-devel; +Cc: Severen Redwood, Thomas Lamprecht
[-- Attachment #1: Type: message/rfc822, Size: 3989 bytes --]
From: Severen Redwood <severen.redwood@sitehost.co.nz>
To: pve-devel@lists.proxmox.com
Cc: Thomas Lamprecht <t.lamprecht@proxmox.com>, Dietmar Maurer <dietmar@proxmox.com>, Severen Redwood <severen.redwood@sitehost.co.nz>, Daniel Krambrock <krambrock@hrz.uni-marburg.de>
Subject: [PATCH manager v2 2/2] close #4369: ui: add datacenter option for unique VM/CT IDs
Date: Fri, 4 Oct 2024 18:07:41 +1300
Message-ID: <20241004050957.441759-3-severen.redwood@sitehost.co.nz>
Add a 'suggest unique VMIDs' row to the datacenter options page that
allows choosing whether the `/cluster/nextid` API endpoint (and thereby
any UI elements that suggest IDs) should avoid suggesting previously
used IDs. This option defaults to off to ensure that this change in
behaviour is opt in.
Co-authored-by: Daniel Krambrock <krambrock@hrz.uni-marburg.de>
Signed-off-by: Severen Redwood <severen.redwood@sitehost.co.nz>
---
www/manager6/dc/OptionView.js | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/www/manager6/dc/OptionView.js b/www/manager6/dc/OptionView.js
index b200fd12..feace344 100644
--- a/www/manager6/dc/OptionView.js
+++ b/www/manager6/dc/OptionView.js
@@ -339,6 +339,10 @@ Ext.define('PVE.dc.OptionView', {
submitValue: true,
}],
});
+ me.add_boolean_row('unique-next-id', gettext('Suggest unique VMIDs'), {
+ defaultValue: 0,
+ deleteDefaultValue: true,
+ });
me.rows['tag-style'] = {
required: true,
renderer: (value) => {
--
2.46.2
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* [pve-devel] [PATCH container v2] api: record CT ID as used after a container is destroyed
[not found] <20241004050957.441759-1-severen.redwood@sitehost.co.nz>
2024-10-04 5:07 ` [pve-devel] [PATCH manager v2 1/2] close #4369: api: optionally only suggest unique IDs Severen Redwood via pve-devel
2024-10-04 5:07 ` [pve-devel] [PATCH manager v2 2/2] close #4369: ui: add datacenter option for unique VM/CT IDs Severen Redwood via pve-devel
@ 2024-10-04 5:07 ` Severen Redwood via pve-devel
2024-10-04 5:07 ` [pve-devel] [PATCH qemu-server v2] api: record VM ID as used after a virtual machine " Severen Redwood via pve-devel
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Severen Redwood via pve-devel @ 2024-10-04 5:07 UTC (permalink / raw)
To: pve-devel; +Cc: Severen Redwood, Thomas Lamprecht
[-- Attachment #1: Type: message/rfc822, Size: 3891 bytes --]
From: Severen Redwood <severen.redwood@sitehost.co.nz>
To: pve-devel@lists.proxmox.com
Cc: Thomas Lamprecht <t.lamprecht@proxmox.com>, Dietmar Maurer <dietmar@proxmox.com>, Severen Redwood <severen.redwood@sitehost.co.nz>, Daniel Krambrock <krambrock@hrz.uni-marburg.de>
Subject: [PATCH container v2] api: record CT ID as used after a container is destroyed
Date: Fri, 4 Oct 2024 18:07:42 +1300
Message-ID: <20241004050957.441759-4-severen.redwood@sitehost.co.nz>
After a container is destroyed, record that its ID has been used via the
`PVE::UsedVmidList` module so that the `/cluster/nextids` endpoint can
later optionally avoid suggesting previously used IDs.
Co-authored-by: Daniel Krambrock <krambrock@hrz.uni-marburg.de>
Signed-off-by: Severen Redwood <severen.redwood@sitehost.co.nz>
---
src/PVE/API2/LXC.pm | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 918e719..ee0e3e9 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -794,7 +794,9 @@ __PACKAGE__->register_method({
}
}
- # only now remove the zombie config, else we can have reuse race
+ # only now mark the CT ID as previously used and remove the zombie
+ # config, else we can have reuse race
+ PVE::UsedVmidList::add_vmid($vmid);
PVE::LXC::Config->destroy_config($vmid);
};
--
2.46.2
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* [pve-devel] [PATCH qemu-server v2] api: record VM ID as used after a virtual machine is destroyed
[not found] <20241004050957.441759-1-severen.redwood@sitehost.co.nz>
` (2 preceding siblings ...)
2024-10-04 5:07 ` [pve-devel] [PATCH container v2] api: record CT ID as used after a container is destroyed Severen Redwood via pve-devel
@ 2024-10-04 5:07 ` Severen Redwood via pve-devel
2024-10-04 5:07 ` [pve-devel] [PATCH cluster v2 5/6] cluster files: add used_vmids.list Severen Redwood via pve-devel
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Severen Redwood via pve-devel @ 2024-10-04 5:07 UTC (permalink / raw)
To: pve-devel; +Cc: Severen Redwood, Thomas Lamprecht
[-- Attachment #1: Type: message/rfc822, Size: 3887 bytes --]
From: Severen Redwood <severen.redwood@sitehost.co.nz>
To: pve-devel@lists.proxmox.com
Cc: Thomas Lamprecht <t.lamprecht@proxmox.com>, Dietmar Maurer <dietmar@proxmox.com>, Severen Redwood <severen.redwood@sitehost.co.nz>, Daniel Krambrock <krambrock@hrz.uni-marburg.de>
Subject: [PATCH qemu-server v2] api: record VM ID as used after a virtual machine is destroyed
Date: Fri, 4 Oct 2024 18:07:43 +1300
Message-ID: <20241004050957.441759-5-severen.redwood@sitehost.co.nz>
After a virtual machine is destroyed, record that its ID has been used
via the `PVE::UsedVmidList` module so that the `/cluster/nextids`
endpoint can later optionally avoid suggesting previously used IDs.
Co-authored-by: Daniel Krambrock <krambrock@hrz.uni-marburg.de>
Signed-off-by: Severen Redwood <severen.redwood@sitehost.co.nz>
---
PVE/API2/Qemu.pm | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index d25a79fe..a01dec2c 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -2335,7 +2335,9 @@ __PACKAGE__->register_method({
}
}
- # only now remove the zombie config, else we can have reuse race
+ # only now mark the VM ID as previously used and remove the
+ # zombie config, else we can have reuse race
+ PVE::UsedVmidList::add_vmid($vmid);
PVE::QemuConfig->destroy_config($vmid);
});
};
--
2.46.2
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* [pve-devel] [PATCH cluster v2 5/6] cluster files: add used_vmids.list
[not found] <20241004050957.441759-1-severen.redwood@sitehost.co.nz>
` (3 preceding siblings ...)
2024-10-04 5:07 ` [pve-devel] [PATCH qemu-server v2] api: record VM ID as used after a virtual machine " Severen Redwood via pve-devel
@ 2024-10-04 5:07 ` Severen Redwood via pve-devel
2024-10-04 5:07 ` [pve-devel] [PATCH cluster v2 6/6] datacenter config: add unique-next-id to schema Severen Redwood via pve-devel
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Severen Redwood via pve-devel @ 2024-10-04 5:07 UTC (permalink / raw)
To: pve-devel; +Cc: Severen Redwood, Thomas Lamprecht
[-- Attachment #1: Type: message/rfc822, Size: 4144 bytes --]
From: Severen Redwood <severen.redwood@sitehost.co.nz>
To: pve-devel@lists.proxmox.com
Cc: Thomas Lamprecht <t.lamprecht@proxmox.com>, Dietmar Maurer <dietmar@proxmox.com>, Severen Redwood <severen.redwood@sitehost.co.nz>, Daniel Krambrock <krambrock@hrz.uni-marburg.de>
Subject: [PATCH cluster v2 5/6] cluster files: add used_vmids.list
Date: Fri, 4 Oct 2024 18:07:44 +1300
Message-ID: <20241004050957.441759-6-severen.redwood@sitehost.co.nz>
Add `/etc/pve/used_vmids.list` to the list of cluster files, which will
be used for recording previously used VM/CT IDs. This is required so
that we can optionally ensure that such IDs are not suggested by the
`/cluster/nextid` API endpoint.
Co-authored-by: Daniel Krambrock <krambrock@hrz.uni-marburg.de>
Signed-off-by: Severen Redwood <severen.redwood@sitehost.co.nz>
---
src/PVE/Cluster.pm | 1 +
src/pmxcfs/status.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/src/PVE/Cluster.pm b/src/PVE/Cluster.pm
index f899dbe..059c7af 100644
--- a/src/PVE/Cluster.pm
+++ b/src/PVE/Cluster.pm
@@ -84,6 +84,7 @@ my $observed = {
'virtual-guest/profiles.cfg' => 1,
'mapping/pci.cfg' => 1,
'mapping/usb.cfg' => 1,
+ 'used_vmids.list' => 1,
};
sub prepare_observed_file_basedirs {
diff --git a/src/pmxcfs/status.c b/src/pmxcfs/status.c
index dc44464..7f60145 100644
--- a/src/pmxcfs/status.c
+++ b/src/pmxcfs/status.c
@@ -114,6 +114,7 @@ static memdb_change_t memdb_change_array[] = {
{ .path = "firewall/cluster.fw" },
{ .path = "mapping/pci.cfg" },
{ .path = "mapping/usb.cfg" },
+ { .path = "used_vmids.list" },
};
static GMutex mutex;
--
2.46.2
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* [pve-devel] [PATCH cluster v2 6/6] datacenter config: add unique-next-id to schema
[not found] <20241004050957.441759-1-severen.redwood@sitehost.co.nz>
` (4 preceding siblings ...)
2024-10-04 5:07 ` [pve-devel] [PATCH cluster v2 5/6] cluster files: add used_vmids.list Severen Redwood via pve-devel
@ 2024-10-04 5:07 ` Severen Redwood via pve-devel
2024-10-30 2:29 ` [pve-devel] [PATCH SERIES v2] Add ability to prevent suggesting previously used VM/CT IDs Severen Redwood via pve-devel
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Severen Redwood via pve-devel @ 2024-10-04 5:07 UTC (permalink / raw)
To: pve-devel; +Cc: Severen Redwood, Thomas Lamprecht
[-- Attachment #1: Type: message/rfc822, Size: 4011 bytes --]
From: Severen Redwood <severen.redwood@sitehost.co.nz>
To: pve-devel@lists.proxmox.com
Cc: Thomas Lamprecht <t.lamprecht@proxmox.com>, Dietmar Maurer <dietmar@proxmox.com>, Severen Redwood <severen.redwood@sitehost.co.nz>, Daniel Krambrock <krambrock@hrz.uni-marburg.de>
Subject: [PATCH cluster v2 6/6] datacenter config: add unique-next-id to schema
Date: Fri, 4 Oct 2024 18:07:45 +1300
Message-ID: <20241004050957.441759-7-severen.redwood@sitehost.co.nz>
Add the `unique-next-id` property to the datacentre config schema to
track whether only unique (ie. neither currently nor previously in use)
VM/CT IDs should be suggested by the `/cluster/nextid` API endpoint.
Co-authored-by: Daniel Krambrock <krambrock@hrz.uni-marburg.de>
Signed-off-by: Severen Redwood <severen.redwood@sitehost.co.nz>
---
src/PVE/DataCenterConfig.pm | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/PVE/DataCenterConfig.pm b/src/PVE/DataCenterConfig.pm
index abd0bbf..d1d4533 100644
--- a/src/PVE/DataCenterConfig.pm
+++ b/src/PVE/DataCenterConfig.pm
@@ -337,6 +337,11 @@ my $datacenter_schema = {
format => $next_id_format,
description => "Control the range for the free VMID auto-selection pool.",
},
+ 'unique-next-id' => {
+ optional => 1,
+ type => 'boolean',
+ description => "Only suggest VMIDs that are neither currently in use nor have previously been used.",
+ },
migration => {
optional => 1,
type => 'string', format => $migration_format,
--
2.46.2
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [pve-devel] [PATCH SERIES v2] Add ability to prevent suggesting previously used VM/CT IDs
[not found] <20241004050957.441759-1-severen.redwood@sitehost.co.nz>
` (5 preceding siblings ...)
2024-10-04 5:07 ` [pve-devel] [PATCH cluster v2 6/6] datacenter config: add unique-next-id to schema Severen Redwood via pve-devel
@ 2024-10-30 2:29 ` Severen Redwood via pve-devel
[not found] ` <20241004050957.441759-2-severen.redwood@sitehost.co.nz>
[not found] ` <20241004050957.441759-4-severen.redwood@sitehost.co.nz>
8 siblings, 0 replies; 10+ messages in thread
From: Severen Redwood via pve-devel @ 2024-10-30 2:29 UTC (permalink / raw)
To: pve-devel; +Cc: Severen Redwood
[-- Attachment #1: Type: message/rfc822, Size: 3873 bytes --]
From: Severen Redwood <severen.redwood@sitehost.co.nz>
To: pve-devel@lists.proxmox.com
Subject: Re: [PATCH SERIES v2] Add ability to prevent suggesting previously used VM/CT IDs
Date: Wed, 30 Oct 2024 15:29:10 +1300
Message-ID: <456426ef-3e12-4da9-9f95-e7a6dfdc5248@sitehost.co.nz>
Hi,
Is someone able to review this updated patch series? I think
the previous comments should be addressed.
Also, the actual patches seem to have ended up disconnected
from the cover letter, so for reference, you can find them
here:
https://lore.proxmox.com/pve-devel/mailman.178.1728018645.332.pve-devel@lists.proxmox.com/T/#t
Thanks,
Severen
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 10+ messages in thread[parent not found: <20241004050957.441759-2-severen.redwood@sitehost.co.nz>]
* Re: [pve-devel] [PATCH manager v2 1/2] close #4369: api: optionally only suggest unique IDs
[not found] ` <20241004050957.441759-2-severen.redwood@sitehost.co.nz>
@ 2024-10-31 13:10 ` Aaron Lauterer
0 siblings, 0 replies; 10+ messages in thread
From: Aaron Lauterer @ 2024-10-31 13:10 UTC (permalink / raw)
To: Severen Redwood, pve-devel; +Cc: Thomas Lamprecht
One thing I noticed, that is important when trying to build this patch
series: make sure to apply and build the pve-cluster patches first. Then
install the updated package on the build machine.
Otherwise the build of pve-manager will fails as the installed
Cluster.pm has no idea about `used_vmids.list`.
At least on my machine that was the case.
On 2024-10-04 07:07, Severen Redwood wrote:
> At the moment, the `/cluster/nextid` API endpoint will return the lowest
> available VM/CT ID, which means that it will suggest re-using VM IDs.
> This can be undesirable, so add an optional check to ensure that it
> chooses an ID which is not and has never been in use.
>
> This optional behaviour is enabled when `unique-next-id: 1` in
> the data centre config, and the previously used IDs are tracked as a
> list in the file `/etc/pve/used_vmids.list`.
>
> Co-authored-by: Daniel Krambrock <krambrock@hrz.uni-marburg.de>
> Signed-off-by: Severen Redwood <severen.redwood@sitehost.co.nz>
> ---
> PVE/API2/Cluster.pm | 13 +++++++--
> PVE/Makefile | 1 +
> PVE/UsedVmidList.pm | 70 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 82 insertions(+), 2 deletions(-)
> create mode 100644 PVE/UsedVmidList.pm
>
> diff --git a/PVE/API2/Cluster.pm b/PVE/API2/Cluster.pm
> index 04387ab4..d0b02d78 100644
> --- a/PVE/API2/Cluster.pm
> +++ b/PVE/API2/Cluster.pm
> @@ -20,6 +20,7 @@ use PVE::RPCEnvironment;
> use PVE::SafeSyslog;
> use PVE::Storage;
> use PVE::Tools qw(extract_param);
> +use PVE::UsedVmidList;
>
> use PVE::API2::ACMEAccount;
> use PVE::API2::ACMEPlugin;
> @@ -813,12 +814,20 @@ __PACKAGE__->register_method({
>
> my $dc_conf = PVE::Cluster::cfs_read_file('datacenter.cfg');
> my $next_id = $dc_conf->{'next-id'} // {};
> + my $want_unique = $dc_conf->{'unique-next-id'} // 0;
>
> my $lower = $next_id->{lower} // 100;
> my $upper = $next_id->{upper} // (1000 * 1000); # note, lower than the schema-maximum
>
> - for (my $i = $lower; $i < $upper; $i++) {
> - return $i if !defined($idlist->{$i});
> + if ($want_unique) {
> + my $used_ids = PVE::Cluster::cfs_read_file('used_vmids.list');
> + for (my $i = $lower; $i < $upper; $i++) {
> + return $i if !defined($idlist->{$i}) and !defined($used_ids->{$i});
> + }
> + } else {
> + for (my $i = $lower; $i < $upper; $i++) {
> + return $i if !defined($idlist->{$i});
> + }
> }
>
> die "unable to get any free VMID in range [$lower, $upper]\n";
> diff --git a/PVE/Makefile b/PVE/Makefile
> index efcb250d..29775e78 100644
> --- a/PVE/Makefile
> +++ b/PVE/Makefile
> @@ -15,6 +15,7 @@ PERLSOURCE = \
> NodeConfig.pm \
> PullMetric.pm \
> Report.pm \
> + UsedVmidList.pm \
> VZDump.pm
>
> all: pvecfg.pm $(SUBDIRS)
> diff --git a/PVE/UsedVmidList.pm b/PVE/UsedVmidList.pm
> new file mode 100644
> index 00000000..b88a8681
> --- /dev/null
> +++ b/PVE/UsedVmidList.pm
> @@ -0,0 +1,70 @@
> +package PVE::UsedVmidList;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::Cluster;
> +
> +my $read_id_list = sub {
> + my ($filename, $raw) = @_;
> +
> + return {} if !defined($raw);
> +
> + my %used_ids;
> + my @lines = split(/\n/, $raw);
> + foreach my $line (@lines) {
> + if ($line =~ m/^(\d+)$/) {
> + $used_ids{$1} = 1;
> + } elsif ($line =~ m/^(\d+)-(\d+)$/) {
> + foreach my $id ($1..$2) {
> + $used_ids{$id} = 1;
> + }
> + } else {
> + warn "Skipping invalid entry in used_vmids.list: $line\n";
> + }
> + }
> +
> + return \%used_ids;
> +};
> +
> +my $write_id_list = sub {
> + my ($filename, $used_ids) = @_;
> + my @used_ids = sort {$a <=> $b} keys(%$used_ids);
> +
> + my @lines;
> + my $len = scalar(@used_ids);
> + for (my $i = 0; $i < $len; $i++) {
> + my $line = "$used_ids[$i]";
> +
> + my $j = $i;
> + while ($j + 1 < $len and $used_ids[$j] + 1 == $used_ids[$j + 1]) {
> + $j++;
> + }
> +
> + # If we find a range of consecutive IDs, write $ids[$i]-$ids[$j] to
> + # denote the range so that we avoid storing each individual integer.
> + if ($i != $j) {
> + $line .= "-$used_ids[$j]";
> + }
> +
> + $i = $j;
> + push(@lines, $line);
> + }
> +
> + return join("\n", @lines) . "\n";
> +};
> +
> +PVE::Cluster::cfs_register_file('used_vmids.list', $read_id_list, $write_id_list);
> +
> +sub add_vmid {
> + my ($vmid) = @_;
> +
> + PVE::Cluster::cfs_lock_file('used_vmids.list', 10, sub {
> + my $used_ids = PVE::Cluster::cfs_read_file('used_vmids.list');
> +
> + $used_ids->{$vmid} = 1;
> + PVE::Cluster::cfs_write_file('used_vmids.list', $used_ids);
> + });
> +}
> +
> +1;
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20241004050957.441759-4-severen.redwood@sitehost.co.nz>]
* Re: [pve-devel] [PATCH container v2] api: record CT ID as used after a container is destroyed
[not found] ` <20241004050957.441759-4-severen.redwood@sitehost.co.nz>
@ 2024-10-31 17:27 ` Aaron Lauterer
2024-11-05 0:53 ` Severen Redwood via pve-devel
0 siblings, 1 reply; 10+ messages in thread
From: Aaron Lauterer @ 2024-10-31 17:27 UTC (permalink / raw)
To: Severen Redwood, pve-devel; +Cc: Thomas Lamprecht
This is missing a
use PVE::UsedVmidList;
at the top of the file. Otherwise running
pct destroy {vmid}
will result in the following error:
Undefined subroutine &PVE::UsedVmidList::add_vmid called at
/usr/share/perl5/PVE/API2/LXC.pm line 799.
This doesn't seem to be necessary in the qemu-server repo, but I did not
investigate further how pulling in dependencies works there.
On 2024-10-04 07:07, Severen Redwood wrote:
> After a container is destroyed, record that its ID has been used via the
> `PVE::UsedVmidList` module so that the `/cluster/nextids` endpoint can
> later optionally avoid suggesting previously used IDs.
>
> Co-authored-by: Daniel Krambrock <krambrock@hrz.uni-marburg.de>
> Signed-off-by: Severen Redwood <severen.redwood@sitehost.co.nz>
> ---
> src/PVE/API2/LXC.pm | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
> index 918e719..ee0e3e9 100644
> --- a/src/PVE/API2/LXC.pm
> +++ b/src/PVE/API2/LXC.pm
> @@ -794,7 +794,9 @@ __PACKAGE__->register_method({
> }
> }
>
> - # only now remove the zombie config, else we can have reuse race
> + # only now mark the CT ID as previously used and remove the zombie
> + # config, else we can have reuse race
> + PVE::UsedVmidList::add_vmid($vmid);
> PVE::LXC::Config->destroy_config($vmid);
> };
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [pve-devel] [PATCH container v2] api: record CT ID as used after a container is destroyed
2024-10-31 17:27 ` [pve-devel] [PATCH container v2] api: record CT ID as used after a container is destroyed Aaron Lauterer
@ 2024-11-05 0:53 ` Severen Redwood via pve-devel
0 siblings, 0 replies; 10+ messages in thread
From: Severen Redwood via pve-devel @ 2024-11-05 0:53 UTC (permalink / raw)
To: a.lauterer; +Cc: Severen Redwood, pve-devel, t.lamprecht
[-- Attachment #1: Type: message/rfc822, Size: 4410 bytes --]
From: Severen Redwood <severen.redwood@sitehost.co.nz>
To: a.lauterer@proxmox.com
Cc: pve-devel@lists.proxmox.com, t.lamprecht@proxmox.com
Subject: Re: [pve-devel] [PATCH container v2] api: record CT ID as used after a container is destroyed
Date: Tue, 5 Nov 2024 13:53:41 +1300
Message-ID: <ba5t63z7iwekm7iejhguzrjxlnpp2d27a4j7flrdcmxturpkfq@lsioezbx572v>
Hi Aaron,
On Thu, Oct 31, 2024 at 06:27:43PM +0100, Aaron Lauterer wrote:
> This is missing a
>
> use PVE::UsedVmidList;
>
> at the top of the file. Otherwise running
>
> pct destroy {vmid}
>
> will result in the following error:
>
> Undefined subroutine &PVE::UsedVmidList::add_vmid called at
> /usr/share/perl5/PVE/API2/LXC.pm line 799.
>
> This doesn't seem to be necessary in the qemu-server repo, but I did not
> investigate further how pulling in dependencies works there.
Thanks for catching this! It's strange that it's not necessary in
`PVE::API2::Qemu`... Either way, I think it makes sense to explicitly use the
module in both files, so I'll update the patches to do that.
Thanks,
Severen
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 10+ messages in thread