From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 9266D1FF186 for ; Tue, 5 Nov 2024 16:53:44 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 2AF5B33242; Tue, 5 Nov 2024 16:53:53 +0100 (CET) Message-ID: Date: Tue, 5 Nov 2024 16:53:48 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: Severen Redwood , pve-devel@lists.proxmox.com References: <20241105020054.215734-1-severen.redwood@sitehost.co.nz> From: Aaron Lauterer In-Reply-To: <20241105020054.215734-1-severen.redwood@sitehost.co.nz> X-SPAM-LEVEL: Spam detection results: 0 AWL -0.037 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH manager v3 1/2] close #4369: api: optionally only suggest unique IDs X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Cc: t.lamprecht@proxmox.com Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" besides some smaller things that could be cleaned up in a follow-up, consider this: Tested-By: Aaron Lauterer Reviewed-By: Aaron Lauterer On 2024-11-05 03:00, 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 > Signed-off-by: Severen Redwood > --- > There are no changes to this patch since v2. > > 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 c2a7a946..a3e89484 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; > @@ -866,12 +867,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); > + The following block is valid perl, though a bit unusual for our codebase. used_ids could be defined as $used_ids = {}; > + my %used_ids; > + my @lines = split(/\n/, $raw); > + foreach my $line (@lines) { > + if ($line =~ m/^(\d+)$/) { > + $used_ids{$1} = 1; Then the above line would be $used_ids->{$1} > + } elsif ($line =~ m/^(\d+)-(\d+)$/) { > + foreach my $id ($1..$2) { > + $used_ids{$id} = 1; same here > + } > + } else { > + warn "Skipping invalid entry in used_vmids.list: $line\n"; > + } > + } > + > + return \%used_ids; and we don't need to manually send a reference on return but can do 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