[pve-devel] [PATCH proxmox-perl-rs 04/11] fabrics: add CRUD and generate fabrics methods
Gabriel Goller
g.goller at proxmox.com
Wed Mar 5 11:20:06 CET 2025
>> [snip]
>> + impl PerlSectionConfig<OpenFabricSectionConfig> {
>> + pub fn add_fabric(&self, new_config: AddFabric) -> Result<(), anyhow::Error> {
>> + let fabricid = FabricId::from(new_config.name).to_string();
>
>Could we simplify this method and the ones below by just using the
>concrete types (here FabricId) inside the argument structs (AddFabric)?
>There's potential for quite a few here afaict, also with the
>Option<u16>'s. Would save us a lot of conversion / validation logic if
>we just did it at deserialization.
Yep, that would work. We just need to change the serde deserialize
override to be generic.
>I pointed out some instances below.
>
>I guess the error messages would be a bit worse then?
Nope, they are quite the same. We can just wrap them in serde custom
errors.
>> + let new_fabric = OpenFabricSectionConfig::Fabric(FabricSection {
>> + hello_interval: new_config
>> + .hello_interval
>> + .map(|x| x.try_into())
>> + .transpose()?,
>> + });
>> + let mut config = self.section_config.lock().unwrap();
>> + if config.sections.contains_key(&fabricid) {
>> + anyhow::bail!("fabric already exists");
>> + }
>> + config.sections.insert(fabricid, new_fabric);
>
>try_insert instead of contains_key + insert?
still deprecated :)
>> [snip]
>> + let mut config = self.section_config.lock().unwrap();
>> + if !config.sections.contains_key(&nodeid) {
>> + anyhow::bail!("node not found");
>> + }
>> + config.sections.entry(nodeid).and_modify(|n| {
>> + if let OpenFabricSectionConfig::Node(n) = n {
>> + n.net = net;
>> + n.interface = interfaces;
>> + }
>> + });
>
>wouldn't get_mut be easier here? also would save the extra contains_key
Agree, also changed everywhere else.
Thanks!
More information about the pve-devel
mailing list