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 666E41FF142 for ; Fri, 19 Jun 2026 12:01:54 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id F2D7BAAD8; Fri, 19 Jun 2026 12:01:53 +0200 (CEST) Message-ID: <0d3ca5f9-9003-4553-bc08-2daab29ba0e7@proxmox.com> Date: Fri, 19 Jun 2026 12:01:18 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH access-control/cluster/manager/network/qemu-server 0/9] fix #7294: pool: add SDN VNets as pool members To: David Riley , pve-devel@lists.proxmox.com References: <20260611145935.147788-1-d.riley@proxmox.com> Content-Language: en-US From: Jakob Klocker In-Reply-To: <20260611145935.147788-1-d.riley@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1781863273825 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.806 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [vnetplugin.pm,rpcenvironment.pm,qemumigrate.pm,proxmox.com,pool.pm,sdn.pm,helpers.pm,accesscontrol.pm,plugin.pm] Message-ID-Hash: WTIGNMCVOYPYIFWBWGVAFZN7SYWPG33P X-Message-ID-Hash: WTIGNMCVOYPYIFWBWGVAFZN7SYWPG33P X-MailFrom: j.klocker@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: I've applied the patch and created a test user with Administration permissions on said pool. I've tested: - add a VNet to a pool with my root user - use that VNet with my test user for existing VMs & CTs - remove the VNet - add a VNet to a pool within a cluster with different PVE versions (9.2 & 9.1) - a error message was thrown as expected Two minor things I've noticed: - In the add dropdown under members within the pool the VNet icon is not grayed out like the other items. As discussed this is only visible when PVE is set to light mode. - when a VNet is added to a pool before the cluster is created and a PVE version mismatch exists (9.2 & 9.1 in my case), one PVE instance shows the VNet inside the pool while the other doesn't. As discussed checking this would take a lot of effort and I'm not sure if it's worth handling this edge case. Apart from that everything worked great. Thanks for the patch. Tested-by: Jakob Klocker On 6/11/26 4:59 PM, David Riley wrote: > This series implements support for adding SDN VNets to resource pools, > resolving #7294 [0]. This series depends on the v3 'fix #7520: sdn: > prune orphaned ACLs and handle VNet migrations' [1]. > > It does not, however, add zones as pool members as requested in #7294. > Zones currently share ACL paths for managing the zone itself and > allocating VNets within it. This makes self-service VNet management > without also granting zone management (and its associated > side-effects) difficult. > > This patch series extends the pool section in the user.cfg and > introduces a new network property to the pool configuration which will > hold VNet entries: > * vnet// > * vnet/// > > The type prefix allows future extension to other network resource > types. > > To prevent potential data loss from overwriting newly added VNets, a > cluster-version check is added which ensures all nodes are running a > version that supports this feature. Note: The hardcoded version guard > should be updated to match the final target release when being > applied. > > The existing version check helpers were moved from `qemu-server` to a > new module within `pve-cluster` to make them available for this > implementation, and any future developments that require gatekeeping. > Appropriate attribution has been included for the relocated code. > Please let me know if this organizational move aligns with current > design preferences or additional adjustments are needed. > > [0] https://bugzilla.proxmox.com/show_bug.cgi?id=7294 > [1] https://lore.proxmox.com/pve-devel/20260603145523.120075-1-d.riley@proxmox.com/ > > > pve-manager: > > David Riley (3): > ui: replace var with let to match style guide for variable declaration > fix #7294: api: pool: add SDN VNets as pool members > fix #7294: ui: pool: add SDN VNets as pool members > > PVE/API2/Pool.pm | 137 ++++++++++++++++++++++++++++++- > www/css/ext6-pve.css | 13 +++ > www/manager6/Utils.js | 1 + > www/manager6/grid/PoolMembers.js | 130 +++++++++++++++++++++++++---- > 4 files changed, 265 insertions(+), 16 deletions(-) > > > pve-access-control: > > David Riley (1): > fix #7294: acl: pool: add SDN VNets as pool members > > src/PVE/AccessControl.pm | 88 +++++++++++++++++++++++++++++++++++++-- > src/PVE/RPCEnvironment.pm | 47 +++++++++++++++++++++ > src/test/parser_writer.pl | 53 +++++++++++++++++++---- > 3 files changed, 176 insertions(+), 12 deletions(-) > > > pve-network: > > David Riley (2): > fix #7294: sdn: register api formats for zones and vnets > fix #7294: sdn: vnet: update pool members on vnet migration and > deletion > > src/PVE/Network/SDN.pm | 15 +++++++++++++++ > src/PVE/Network/SDN/VnetPlugin.pm | 23 ++++++++++++++++++++--- > src/PVE/Network/SDN/Zones/Plugin.pm | 23 ++++++++++++++++++++--- > 3 files changed, 55 insertions(+), 6 deletions(-) > > > pve-cluster: > > David Riley (2): > cluster: add helpers module with version comparison functions > fix #7294: cluster: helpers: add cluster-wide version assertion > > debian/pve-cluster.install | 1 + > src/PVE/Cluster/Helpers.pm | 86 ++++++++++++++++++++++++++++++++++++++ > src/PVE/Cluster/Makefile | 2 +- > 3 files changed, 88 insertions(+), 1 deletion(-) > create mode 100644 src/PVE/Cluster/Helpers.pm > > > qemu-server: > > David Riley (1): > fix #7294: helpers: use cluster-wide version helper > > src/PVE/QemuMigrate.pm | 2 +- > src/PVE/QemuServer/Helpers.pm | 40 +---------------------------------- > 2 files changed, 2 insertions(+), 40 deletions(-) > > > Summary over all repositories: > 15 files changed, 586 insertions(+), 75 deletions(-) >