From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 0DD2B1FF0B0 for ; Fri, 09 Oct 2026 15:05:43 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 7530121765; Fri, 09 Oct 2026 15:05:37 +0200 (CEST) Date: Fri, 9 Oct 2026 15:04:46 +0200 From: Arthur Bied-Charreton To: Thomas Ellmenreich Subject: Re: SPAM: [PATCH container/firewall/manager/network/qemu-server v3 00/16] handle dangling references when firewall objects go away Message-ID: <7uosrzzh257p66v2xy2z4eab6o6uufzjjloni7pu3lnvanffcf@3ez6jpssopal> References: <20260925094230.844917-1-a.bied-charreton@proxmox.com> <3szzwo47yubk7wzebw2ljxudwf7f36rgnu5pfudrn7ocffjeay@axig2y7zxujd> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3szzwo47yubk7wzebw2ljxudwf7f36rgnu5pfudrn7ocffjeay@axig2y7zxujd> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1791551087088 X-SPAM-LEVEL: Spam detection results: 0 AWL 1.049 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: UVLEJQTA5YYMZUYECHXPMLCAZ6MSQD2B X-Message-ID-Hash: UVLEJQTA5YYMZUYECHXPMLCAZ6MSQD2B X-MailFrom: a.bied-charreton@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 CC: pve-devel@lists.proxmox.com X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Wed, Sep 30, 2026 at 10:22:54AM +0200, Arthur Bied-Charreton wrote: > On Wed, Sep 30, 2026 at 09:44:25AM +0200, Thomas Ellmenreich wrote: > > Thanks for sending in this series! > > > thanks for having a look :) > > I ran some basic tests on the renaming and dropping functionality and found > > that everything worked well. > > > > Looking at the different operations that now have a new option to deal with > > references, 'disable' is always possible, why is that not the case for rename. > > There might be a technical reason that I'm currently not thinking of, but I > > can see 'disabling' the referencing rules being a worthwhile option when > > performing a rename. > > > good point, I had not considered that. it could be useful when the old > name is going to be reused afterwards. I am not sure how common this > would be in practice, but the helper already implements 'disable', so > wiring it up for rename is cheap. > > I will look into this! > hey, thanks again for the feedback, quick update on the direction this series will be taking. to answer your question first: yes, 'disable' makes sense on rename and it will be added in v4 along with 'drop'. i protoyped a bit and talked to Stefan and Aaron off-list, here is what we agreed on: - force-rename will not be exposed in the UI. it stays in the API as an escape hatch for interrupted renames and will be documented as such. - the option list got long enough to warrant a drop down. for operations on firewall objects themselves, it goes behind an "Advanced" section. for guest destroys and SDN applies it stays inline, next to the hint explaining what is about to break. this also makes it easier to show descriptions right next to the options without having to resort to a tooltip. - a cluster-wide default for these operations will be settable, as a cluster.fw [OPTIONS] entry. - object updates and deletions will become tassks in the firewall API, logging each matched rule and the action taken for better auditability and to get the per-config locking out of the API handler. the next version will also include a documentation patch explaining the different options in more detail, which the edit/remove dialogs can then also reference now that the general approach seems to have mostly settled. > > The tests I performed were: > > > > Renaming of aliases and ipsets: > > For this, I created aliases both at the cluster and vm level. I then > > referenced these aliases in the firewall rules of the cluster, node and vm. > > The aliases and ipsets defined at the vm level were only referenced in vm > > level rules. Performing some renames with the different settings acted > > exactly as expected. I also used an alias in one of the ip sets and the > > rename worked perfectly in that case as well. > > > > Simple benchmark for rename: > > By creating 500 guests and then defining a firewall rule on each guest > > referencing a cluster level alias, I could then benchmark the renaming of > > the alias. On average it took ~2,5 seconds which I find reasonable. > > > > Deletion of vms: > > When deleting a vm, one has the option to keep/disable/delete the > > referencing rules. After creating the 500 vms for the benchmark, I added > > some more rules that reference the IPAM aliases. Using keep/disable/delete > > all worked exactly as expected. > > > thanks a lot for the extensive testing! > > So, aside from the question mentioned at the very beginning, consider this: > > > > Tested-by: Thomas Ellmenreich > > > > On Fri Sep 25, 2026 at 11:42 AM CEST, Arthur Bied-Charreton wrote: > > > > > [...] > > > > [snip] > > > >