From: Aaron Lauterer <a.lauterer@proxmox.com>
To: Severen Redwood <severen.redwood@sitehost.co.nz>,
pve-devel@lists.proxmox.com
Cc: Thomas Lamprecht <t.lamprecht@proxmox.com>
Subject: Re: [pve-devel] [PATCH manager v2 1/2] close #4369: api: optionally only suggest unique IDs
Date: Thu, 31 Oct 2024 14:10:27 +0100 [thread overview]
Message-ID: <38dc9512-3164-40b2-ab3b-897896ad3c1d@proxmox.com> (raw)
In-Reply-To: <20241004050957.441759-2-severen.redwood@sitehost.co.nz>
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
next prev parent reply other threads:[~2024-10-31 13:10 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
[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
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 ` [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 ` [pve-devel] [PATCH cluster v2 5/6] cluster files: add used_vmids.list 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
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
[not found] ` <20241004050957.441759-2-severen.redwood@sitehost.co.nz>
2024-10-31 13:10 ` Aaron Lauterer [this message]
[not found] ` <20241004050957.441759-4-severen.redwood@sitehost.co.nz>
2024-10-31 17:27 ` [pve-devel] [PATCH container v2] api: record CT ID as used after a container is destroyed Aaron Lauterer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=38dc9512-3164-40b2-ab3b-897896ad3c1d@proxmox.com \
--to=a.lauterer@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
--cc=severen.redwood@sitehost.co.nz \
--cc=t.lamprecht@proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox