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 D8A481FF389 for ; Wed, 5 Jun 2024 10:57:22 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A218F1ED96; Wed, 5 Jun 2024 10:57:53 +0200 (CEST) Mime-Version: 1.0 Date: Wed, 05 Jun 2024 10:57:50 +0200 Message-Id: From: "Shannon Sterz" To: "Daniel Krambrock" , X-Mailer: aerc 0.17.0-69-g65571b67d7d3-dirty References: <20240522120553.49114-1-krambrock@hrz.uni-marburg.de> <20240522120553.49114-2-krambrock@hrz.uni-marburg.de> In-Reply-To: <20240522120553.49114-2-krambrock@hrz.uni-marburg.de> X-SPAM-LEVEL: Spam detection results: 0 AWL -0.057 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 T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [jobs.pm, usedvmidlist.pm, report.pm, pvecfg.pm, vzdump.pm, cluster.pm, nodeconfig.pm] Subject: Re: [pve-devel] [PATCH manager] close #4369: make VMID suggestion strategy configurable 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 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" This is a fairly large patch. It could easily be split into a backend and front-end portion. That would make it easier to review, in my opinion. On Wed May 22, 2024 at 2:05 PM CEST, Daniel Krambrock wrote: > VMID suggestion strategy is set in datacenter config > > Signed-off-by: Daniel Krambrock > --- > PVE/API2/Cluster.pm | 16 +++++++++-- > PVE/Makefile | 1 + > PVE/UsedVmidList.pm | 53 +++++++++++++++++++++++++++++++++++ > www/manager6/dc/OptionView.js | 14 +++++++++ > 4 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..be79ddf0 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,23 @@ __PACKAGE__->register_method({ > > my $dc_conf = PVE::Cluster::cfs_read_file('datacenter.cfg'); > my $next_id = $dc_conf->{'next-id'} // {}; > + my $strategy = $dc_conf->{'next-id-strategy'} // "next-free"; > > 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 ($strategy eq "next-free") { > + for (my $i = $lower; $i < $upper; $i++) { > + return $i if !defined($idlist->{$i}); > + } > + } elsif ($strategy eq "max+1") { > + return %$idlist ? (List::Util::max(keys %$idlist) + 1) : $lower > + } elsif ($strategy eq "list") { > + for (my $i = $lower; $i < $upper; $i++) { > + return $i if (!defined($idlist->{$i}) and !PVE::UsedVmidList::is_on_vmid_list($i)) ; Nit: Unnecessary space before semicolon. > + } > + } else { > + die "unable to get any free VMID with strategy $strategy\n"; > } > > die "unable to get any free VMID in range [$lower, $upper]\n"; > diff --git a/PVE/Makefile b/PVE/Makefile > index 660de4d0..fe627296 100644 > --- a/PVE/Makefile > +++ b/PVE/Makefile > @@ -14,6 +14,7 @@ PERLSOURCE = \ > Jobs.pm \ > NodeConfig.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..a60c75d6 > --- /dev/null > +++ b/PVE/UsedVmidList.pm > @@ -0,0 +1,53 @@ > +package PVE::UsedVmidList; > + > +use strict; > +use warnings; > + > +use PVE::Cluster; > + > +my $parse_vmid_list = sub { > + my ($filename, $raw) = @_; > + > + return [] if !defined($raw); > + > + my @parsed; > + my @lines = split(/\n/, $raw); > + foreach my $line (@lines) { > + next if $line =~ m/^\s*$/; > + > + if ($line =~ m/^(\d+)$/) { > + push(@parsed, $1); > + } else { > + warn "Skipping invalid used_vmids.list entry: $line\n"; > + } > + } > + > + return \@parsed; > +}; > + > +my $write_vmid_list = sub { > + my ($filename, @data) = @_; > + > + return join("\n", sort @data); > +}; > + > +PVE::Cluster::cfs_register_file('used_vmids.list', $parse_vmid_list, $write_vmid_list); > + > +sub add_vmid { > + my ($vmid) = @_; > + > + PVE::Cluster::cfs_lock_file('used_vmids.list', 10, sub { > + my $vmid_list = PVE::Cluster::cfs_read_file('used_vmids.list'); > + > + push(@$vmid_list, $vmid); > + PVE::Cluster::cfs_write_file('used_vmids.list', join("\n", @$vmid_list)); > + }); > +} > + > +sub is_on_vmid_list { > + my ($vmid) = @_; > + my $vmid_list = PVE::Cluster::cfs_read_file('used_vmids.list'); > + return scalar(grep { $_ == $vmid } @$vmid_list); > +} > + > +1; > diff --git a/www/manager6/dc/OptionView.js b/www/manager6/dc/OptionView.js > index b200fd12..613a2183 100644 > --- a/www/manager6/dc/OptionView.js > +++ b/www/manager6/dc/OptionView.js > @@ -339,6 +339,20 @@ Ext.define('PVE.dc.OptionView', { > submitValue: true, > }], > }); > + me.add_combobox_row('next-id-strategy', gettext('Next Free VMID Strategy'), { > + renderer: v => { > + if (v === '__default__') {return Proxmox.Utils.defaultText;} > + return v; Nit: The indetation here seems off, both lines should be indented with two tabs. Also, please note that our JS style guide says you should avoid single line if statements in new code. > + }, > + comboItems: [ > + ['__default__', Proxmox.Utils.defaultText + ' (next-free)'], > + ['next-free', 'next-free'], > + ['max+1', 'max+1'], > + ['list', 'list'], > + ], > + defaultValue: '__default__', > + deleteEmpty: true, > + }); > me.rows['tag-style'] = { > required: true, > renderer: (value) => { _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel