public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Severen Redwood via pve-devel <pve-devel@lists.proxmox.com>
To: pve-devel@lists.proxmox.com
Cc: Severen Redwood <severen.redwood@sitehost.co.nz>
Subject: [pve-devel] [PATCH SERIES] Add ability to prevent suggesting previously used VM/CT IDs
Date: Fri, 27 Sep 2024 01:52:27 +1200	[thread overview]
Message-ID: <mailman.89.1727359009.332.pve-devel@lists.proxmox.com> (raw)

[-- Attachment #1: Type: message/rfc822, Size: 6203 bytes --]

From: Severen Redwood <severen.redwood@sitehost.co.nz>
To: pve-devel@lists.proxmox.com
Subject: [PATCH SERIES] Add ability to prevent suggesting previously used VM/CT IDs
Date: Fri, 27 Sep 2024 01:52:27 +1200
Message-ID: <20240926135516.117065-1-severen.redwood@sitehost.co.nz>

Hi everyone,

This patch series is a reworking of Daniel Krambrock's patches [1] which
allow for configuring the strategy used for suggesting VM IDs. As
discussed with him [2], I have removed the 'max + 1' strategy as it is
fundamentally flawed given that the goal is preventing re-use of IDs
that can cause issues like #4369 [3]. This leaves two strategies, namely
the current behaviour of suggesting the lowest free ID and the new
optional behaviour of suggesting the lowest free *and* not previously
used ID. Beyond this, I have also fixed a bug where IDs would be
recorded as used multiple times and removed sorting the stored list of
IDs on write as it is not required.

I believe that the comments from Shannon Sterz relating to code style
should now be addressed, though there is still their comment on tracking
the used IDs in `/etc/pve/used_vmids.list`:

> Not sure if tracking the used VM/CT IDs in a separate file is the most
> elegant solution here. Especially as this is a somewhat niche usecase.

If this needs to be changed, then where should it go? I am not familiar
enough with the codebase to know what the preference would be. Note that
we *must* store a list of used IDs rather than a simple counter because
PVE allows you to choose any free ID you wish when creating a VM or
container.

As I re-wrote the commit messages and it felt strange to ghost-write
messages under someone else's name, the commit author on all patches is
myself with Daniel listed as a co-author using `Co-authored-by` trailers
in the commit message bodies. Please let me know if this isn't the right
convention :)

I have also sent in a signed CLA to cover my contributions.

Thanks,
Severen

[1]: https://lore.proxmox.com/pve-devel/D1RYIAHXBOIH.RM5K01KGND9T@proxmox.com/t/
[2]: https://lore.proxmox.com/pve-devel/mailman.472.1724973432.302.pve-devel@lists.proxmox.com/T/#u
[3]: https://bugzilla.proxmox.com/show_bug.cgi?id=4369#c13

pve-manager
-----------
Severen Redwood (2):
  close #4369: api: optionally only suggest unique
  close #4369: ui: add datacenter option for unique

 PVE/API2/Cluster.pm           | 12 ++++++--
 PVE/Makefile                  |  1 +
 PVE/UsedVmidList.pm           | 55 +++++++++++++++++++++++++++++++++++
 www/manager6/dc/OptionView.js |  4 +++
 4 files changed, 70 insertions(+), 2 deletions(-)
 create mode 100644 PVE/UsedVmidList.pm

pve-container
-------------
Severen Redwood (1):
  api: record CT ID as used after a container is destroyed

 src/PVE/API2/LXC.pm | 1 +
 1 file changed, 1 insertion(+)

qemu-server
-----------
Severen Redwood (1):
  api: record VM ID as used after a virtual machine is destroyed

 PVE/API2/Qemu.pm | 1 +
 1 file changed, 1 insertion(+)

pve-cluster
-----------
Severen Redwood (2):
  cluster files: add used_vmids.list
  datacenter config: add unique-next-id to schema

 src/PVE/Cluster.pm          | 1 +
 src/PVE/DataCenterConfig.pm | 5 +++++
 src/pmxcfs/status.c         | 1 +
 3 files changed, 7 insertions(+)



[-- 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

                 reply	other threads:[~2024-09-26 13:56 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=mailman.89.1727359009.332.pve-devel@lists.proxmox.com \
    --to=pve-devel@lists.proxmox.com \
    --cc=severen.redwood@sitehost.co.nz \
    /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
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal