[snip] diff --git a/proxmox-ve-config/src/sdn/fabric/mod.rs b/proxmox-ve-config/src/sdn/fabric/mod.rs index 3342a7053d3f..a2132e8aff3f 100644 --- a/proxmox-ve-config/src/sdn/fabric/mod.rs +++ b/proxmox-ve-config/src/sdn/fabric/mod.rs @@ -1,11 +1,13 @@ pub mod section_config;-use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashSet}; use std::marker::PhantomData; use std::ops::Deref; use serde::{Deserialize, Serialize}; +use crate::common::valid::Validatable; + use crate::sdn::fabric::section_config::{ fabric::{ Fabric, FabricDeletableProperties, FabricId, FabricSection, FabricSectionUpdater, @@ -34,15 +36,38 @@ pub enum FabricConfigError { FabricDoesNotExist(String), #[error("node '{0}' does not exist in fabric '{1}'")] NodeDoesNotExist(String, String), + #[error("node IP {0} is outside the IP prefix {1} of the fabric")] + NodeIpOutsideFabricRange(String, String), #[error("node has a different protocol than the referenced fabric")] ProtocolMismatch, #[error("fabric '{0}' already exists in config")] DuplicateFabric(String), #[error("node '{0}' already exists in config for fabric {1}")] DuplicateNode(String, String), + #[error("fabric {0} contains nodes with duplicated IPs")] + DuplicateNodeIp(String), + #[error("fabric '{0}' does not have an IP prefix configured for the node IP {1}")] + FabricNoIpPrefixForNode(String, String), + #[error("node '{0}' does not have an IP configured for this fabric prefix {1}")] + NodeNoIpForFabricPrefix(String, String), + #[error("fabric '{0}' does not have an IP prefix configured")] + FabricNoIpPrefix(String), + #[error("node '{0}' does not have an IP configured")] + NodeNoIp(String), + #[error("interface is already in use by another fabric")] + DuplicateInterface, + #[error("IPv6 is currently not supported for protocol {0}")] + NoIpv6(String),^ "NoIpv6" is a weird variant name for this. Maybe use Ipv6Unsupported.
Agree, changed it.
I also wonder if we really need all those messages as separate variants. As a followup to the previous thoughts on error types: Generally I think only errors which are worth distinguishing need their own variants. We definitely don't need one for every single message.
IMO what we could do is split it up in multiple enums and have a sort of tree structure at the end.
The ones here could probably all just be a single `Invalid(String)`. (Or if we don't want to expose `String` here, `Validation(ValidationError)`, with the inner one being a newtype wrapper around the `String`.
Hmm but then why create custom error types at all? This is the same as having strings where we can't distinguish between errors.
Internally there can be a helper `fn FabricConfigError::validation(impl Into<String>)`.// should usually not occur, but we still check for it nonetheless #[error("mismatched fabric_id")] FabricIdMismatch, + // this is technically possible, but we don't allow it + #[error("duplicate OSPF area")] + DuplicateOspfArea, + #[error("IP prefix {0} in fabric '{1}' overlaps with IPv4 prefix {2} in fabric '{3}'")] + OverlappingIp4Prefix(String, String, String, String), + #[error("IPv6 prefix {0} in fabric '{1}' overlaps with IPv6 prefix {2} in fabric '{3}'")] + OverlappingIp6Prefix(String, String, String, String), } /// An entry in a [`FabricConfig`]. @@ -367,6 +392,93 @@ impl From<Fabric> for FabricEntry { } } [snip]
Thanks! _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
