On 10/1/25 9:22 AM, Hannes Laimer wrote: > On 9/25/25 14:22, Stefan Hanreich wrote: >> Enables iterating over the network devices in order, without >> performing any additional sorting at the call sites. This makes the >> output in proxmox-firewall stable, which is useful for test cases as >> well as for comparing the output of different proxmox-firewall runs. >> > > Are we actually explicitly sorting anywhere? I didn't really find any > places. If there should be, we can drop those. Nonetheless this makes a > lot if sense.
no, we're currently not - the value is used here [1] to generate the ipfilter sets or here [2] for MAC address filtering. The generated JSON for the ruleset is unstable if there's no sorting somewhere along the line which leads to flakey integration tests. I preferred BTreeMaps over HashMaps + sorting, since then the call site doesn't need to remember sorting and if we have multiple users then sorting only need to happen once. [1] https://git.proxmox.com/?p=proxmox-firewall.git;a=blob;f=proxmox-firewall/src/firewall.rs;h=8cac190170a8d69e6f2c9479390f7ec0fea50a88;hb=HEAD#l793 [2] https://git.proxmox.com/?p=proxmox-firewall.git;a=blob;f=proxmox-firewall/src/firewall.rs;h=8cac190170a8d69e6f2c9479390f7ec0fea50a88;hb=HEAD#l598 >> Signed-off-by: Stefan Hanreich <[email protected]> >> --- >> proxmox-ve-config/src/guest/vm.rs | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/proxmox-ve-config/src/guest/vm.rs b/proxmox-ve-config/ >> src/guest/vm.rs >> index cc97781..baff1b8 100644 >> --- a/proxmox-ve-config/src/guest/vm.rs >> +++ b/proxmox-ve-config/src/guest/vm.rs >> @@ -1,4 +1,4 @@ >> -use std::collections::HashMap; >> +use std::collections::BTreeMap; >> use std::io; >> use std::str::FromStr; >> @@ -278,7 +278,7 @@ impl FromStr for NetworkDevice { >> #[derive(Debug, Default)] >> #[cfg_attr(test, derive(Eq, PartialEq))] >> pub struct NetworkConfig { >> - network_devices: HashMap<i64, NetworkDevice>, >> + network_devices: BTreeMap<i64, NetworkDevice>, >> } >> impl NetworkConfig { >> @@ -300,12 +300,12 @@ impl NetworkConfig { >> bail!("No index found in net key string: {key}") >> } >> - pub fn network_devices(&self) -> &HashMap<i64, NetworkDevice> { >> + pub fn network_devices(&self) -> &BTreeMap<i64, NetworkDevice> { >> &self.network_devices >> } >> pub fn parse<R: io::BufRead>(input: R) -> Result<Self, Error> { >> - let mut network_devices = HashMap::new(); >> + let mut network_devices = BTreeMap::new(); >> for line in input.lines() { >> let line = line?; > _______________________________________________ pve-devel mailing list [email protected] https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
