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 A0D111FF185 for ; Mon, 4 Aug 2025 14:54:50 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 72F8C30D79; Mon, 4 Aug 2025 14:56:21 +0200 (CEST) Date: Mon, 4 Aug 2025 14:55:48 +0200 From: Wolfgang Bumiller To: Stefan Hanreich Message-ID: <4bs5sbsu3jnjtxtz3jiryeepw6od4nd2dd6anzdcc4n66lthbv@eqqjtpokkw2m> References: <20250731140855.573717-1-s.hanreich@proxmox.com> <20250731140855.573717-5-s.hanreich@proxmox.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20250731140855.573717-5-s.hanreich@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1754312129833 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.076 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pbs-devel] [PATCH proxmox v4 3/3] network-api: add rename_interfaces method X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Cc: pbs-devel@lists.proxmox.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" On Thu, Jul 31, 2025 at 04:08:47PM +0200, Stefan Hanreich wrote: > Used for batch renaming interfaces in the /e/n/i configuration file by > the proxmox-network-interface-pinning tool. > > Signed-off-by: Stefan Hanreich > Tested-by: Christian Ebner > --- > proxmox-network-api/src/config/mod.rs | 68 +++++++++++++++++++++++++++ > 1 file changed, 68 insertions(+) > > diff --git a/proxmox-network-api/src/config/mod.rs b/proxmox-network-api/src/config/mod.rs > index e8cb81d1..3b2ffdc2 100644 > --- a/proxmox-network-api/src/config/mod.rs > +++ b/proxmox-network-api/src/config/mod.rs > @@ -267,6 +267,74 @@ impl NetworkConfig { > Ok(interface) > } > > + pub fn rename_interfaces(&mut self, mapping: &HashMap) -> Result<(), Error> { > + for (old_name, new_name) in mapping.iter() { > + self.interfaces > + .remove(old_name) > + .map(|interface| self.interfaces.insert(new_name.to_string(), interface)); A freestanding `.map` with the `map` doing a thing like that is bad style. And shouldn't we also update `.name` right away? Also consider that we should probably issue a warning if the new name already existed? if let Some(mut interface) == self.interfaces.remove(old_name) { interface.name = new_name.clone(); if self.interfaces.insert(new_name.to_string(), interface).is_some() { warn about this } } > + > + if let Some(idx) = self > + .order > + .iter() > + .position(|elem| matches!(elem, NetworkOrderEntry::Iface(name) if name == new_name)) Shouldn't this check for `old_name`? Also, could use `iter_mut().find()` and then assign to resulting mut ref if let Some(elem) = self.order.iter_mut().find(...) { *elem = ... } > + { > + self.order[idx] = NetworkOrderEntry::Iface(new_name.to_string()); > + } > + } > + > + for interface in self.interfaces.values_mut() { > + if let Some(new_name) = mapping.get(&interface.name) { > + interface.name = new_name.to_string(); > + } > + > + if let Some(bridge_ports) = interface.bridge_ports.take() { > + interface.bridge_ports = Some( > + bridge_ports > + .into_iter() > + .map(|interface| { > + mapping > + .get(&interface) > + .map(String::from) could just use `.cloned()` > + .unwrap_or(interface) > + }) > + .collect(), > + ) > + } > + > + if let Some(vlan_raw_device) = interface.vlan_raw_device.take() { > + if let Some(new_name) = mapping.get(&vlan_raw_device) { > + interface.vlan_raw_device = Some(new_name.to_string()); > + } else { > + interface.vlan_raw_device = Some(vlan_raw_device); > + } > + } could skip the else if we modify in-place with `.as_mut()`: if let Some(vlan_raw_device) = interface.vlan_raw_device.as_mut() { if let Some(new_name) = mapping.get(&*vlan_raw_device) { // note the extra '*' deref *vlan_raw_device = Some(new_name.clone()); } } > + > + if let Some(slaves) = interface.slaves.take() { > + interface.slaves = Some( > + slaves > + .into_iter() > + .map(|interface| { > + mapping > + .get(&interface) > + .map(String::from) > + .unwrap_or(interface) > + }) > + .collect(), > + ) > + } > + > + if let Some(bond_primary) = interface.bond_primary.take() { same as the vlan_raw_device case > + if let Some(new_name) = mapping.get(&bond_primary) { > + interface.bond_primary = Some(new_name.to_string()); > + } else { > + interface.bond_primary = Some(bond_primary); > + } > + } > + } > + > + Ok(()) > + } > + > /// Check that there is no other gateway. > /// > /// The gateway property is only allowed on passed 'iface'. This should be > -- > 2.47.2 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel