Ports will be in the State connections referencing its controller by …

…name

rust/agama-dbus-server/src/network/action.rs

@@ -1,9 +1,5 @@
-use crate::network::model::{Connection, ControllerConfig};
+use crate::network::model::Connection;
 use agama_lib::network::types::DeviceType;
-use std::collections::HashMap;
-use tokio::sync::oneshot;
-
-use super::error::NetworkStateError;
 
 /// Networking actions, like adding, updating or removing connections.
 ///
@@ -15,12 +11,6 @@ pub enum Action {
     AddConnection(String, DeviceType),
     /// Update a connection (replacing the old one).
     UpdateConnection(Connection),
-    /// Update a controller connection (replacing the old one).
-    UpdateControllerConnection(
-        Connection,
-        HashMap<String, ControllerConfig>,
-        oneshot::Sender<Result<Connection, NetworkStateError>>,
-    ),
     /// Remove the connection with the given Uuid.
     RemoveConnection(String),
     /// Apply the current configuration.

rust/agama-dbus-server/src/network/dbus/interfaces.rs

@@ -2,21 +2,20 @@
 //!
 //! This module contains the set of D-Bus interfaces that are exposed by [D-Bus network
 //! service](crate::NetworkService).
-use std::collections::HashMap;
 
 use super::ObjectsRegistry;
 use crate::network::{
     action::Action,
     error::NetworkStateError,
     model::{
-        BondConnection, Connection as NetworkConnection, ControllerConfig, Device as NetworkDevice,
+        BondConnection, Connection as NetworkConnection, Device as NetworkDevice,
         WirelessConnection,
     },
 };
 
 use agama_lib::network::types::SSID;
 use std::sync::Arc;
-use tokio::sync::{mpsc::UnboundedSender, oneshot};
+use tokio::sync::mpsc::UnboundedSender;
 use tokio::sync::{MappedMutexGuard, Mutex, MutexGuard};
 use zbus::{
     dbus_interface,
@@ -329,24 +328,6 @@ impl Bond {
         actions.send(Action::UpdateConnection(connection)).unwrap();
         Ok(())
     }
-
-    /// Updates the controller connection data in the NetworkSystem.
-    ///
-    /// * `connection`: Updated connection.
-    async fn update_controller_connection<'a>(
-        &self,
-        connection: MappedMutexGuard<'a, BondConnection>,
-        settings: HashMap<String, ControllerConfig>,
-    ) -> Result<crate::network::model::Connection, NetworkStateError> {
-        let actions = self.actions.lock().await;
-        let connection = NetworkConnection::Bond(connection.clone());
-        let (tx, rx) = oneshot::channel();
-        actions
-            .send(Action::UpdateControllerConnection(connection, settings, tx))
-            .unwrap();
-
-        rx.await.unwrap()
-    }
 }
 
 #[dbus_interface(name = "org.opensuse.Agama1.Network.Connection.Bond")]
@@ -355,57 +336,42 @@ impl Bond {
     #[dbus_interface(property)]
     pub async fn mode(&self) -> String {
         let connection = self.get_bond().await;
-
         connection.bond.mode.to_string()
     }
 
     #[dbus_interface(property)]
     pub async fn set_mode(&mut self, mode: &str) -> zbus::fdo::Result<()> {
         let mut connection = self.get_bond().await;
-        connection.set_mode(mode)?;
-
+        connection.set_mode(mode.try_into()?);
         self.update_connection(connection).await
     }
 
     /// List of bond ports.
     #[dbus_interface(property)]
     pub async fn options(&self) -> String {
         let connection = self.get_bond().await;
-
         connection.bond.options.to_string()
     }
 
     #[dbus_interface(property)]
     pub async fn set_options(&mut self, opts: &str) -> zbus::fdo::Result<()> {
         let mut connection = self.get_bond().await;
-        connection.set_options(opts)?;
-
+        connection.set_options(opts.try_into()?);
         self.update_connection(connection).await
     }
 
     /// List of bond ports.
     #[dbus_interface(property)]
     pub async fn ports(&self) -> Vec<String> {
         let connection = self.get_bond().await;
-        connection
-            .bond
-            .ports
-            .iter()
-            .map(|port| port.base().id.to_string())
-            .collect()
+        connection.bond.ports.clone()
     }
 
     #[dbus_interface(property)]
     pub async fn set_ports(&mut self, ports: Vec<String>) -> zbus::fdo::Result<()> {
-        let connection = self.get_bond().await;
-        let result = self
-            .update_controller_connection(
-                connection,
-                HashMap::from([("ports".to_string(), ControllerConfig::Ports(ports.clone()))]),
-            )
-            .await;
-        self.connection = Arc::new(Mutex::new(result.unwrap()));
-        Ok(())
+        let mut connection = self.get_bond().await;
+        connection.set_ports(ports);
+        self.update_connection(connection).await
     }
 }
 

rust/agama-dbus-server/src/network/model.rs

@@ -41,9 +41,9 @@ impl NetworkState {
         self.devices.iter().find(|d| d.name == name)
     }
 
-    /// Get connection by UUID
+    /// Get connection by ID
     ///
-    /// * `uuid`: connection UUID
+    /// * `id`: connection ID
     pub fn get_connection(&self, id: &str) -> Option<&Connection> {
         self.connections.iter().find(|c| c.id() == id)
     }
@@ -78,6 +78,16 @@ impl NetworkState {
         };
 
         *old_conn = conn;
+
+        match old_conn {
+            Connection::Bond(ref bond) => {
+                let id = old_conn.id().to_string();
+                let ports = bond.bond.ports.clone();
+                self.update_controller_ports(id, ports)?;
+            }
+            _ => {}
+        };
+
         Ok(())
     }
 
@@ -86,33 +96,17 @@ impl NetworkState {
     /// It uses the `id` to decide which connection to update.
     ///
     /// Additionally, it registers the connection to be removed when the changes are applied.
-    pub fn update_controller_connection(
+    pub fn update_controller_ports(
         &mut self,
-        mut conn: Connection,
-        settings: HashMap<String, ControllerConfig>,
+        controller_id: String,
+        ports: Vec<String>,
     ) -> Result<(), NetworkStateError> {
-        let controller = conn.clone();
-
-        if let Connection::Bond(ref mut bond) = conn {
-            if let Some(ControllerConfig::Ports(ports)) = settings.get("ports") {
-                let new_ports: Vec<_> = ports
-                    .iter()
-                    .map(|port| {
-                        bond.find_port(port).cloned().unwrap_or_else(|| {
-                            ConnectionBuilder::new(port)
-                                .with_type(DeviceType::Ethernet)
-                                .with_interface(port)
-                                .with_controller(controller.id())
-                                .build()
-                        })
-                    })
-                    .collect();
-
-                bond.set_ports(new_ports);
+        for conn in self.connections.iter_mut() {
+            if ports.contains(&conn.id().to_string()) {
+                conn.set_controller(&controller_id);
             }
         }
-
-        self.update_connection(conn)
+        Ok(())
     }
 
     /// Removes a connection from the state.
@@ -361,15 +355,19 @@ impl Connection {
     }
 
     /// Ports controller name, e.g.: bond0, br0
-    pub fn controller(&self) -> &str {
-        self.base().controller.as_str()
+    pub fn is_controlled(&self) -> bool {
+        self.base().controller.is_some()
+    }
+    /// Ports controller name, e.g.: bond0, br0
+    pub fn controller(&self) -> Option<&String> {
+        self.base().controller.as_ref()
     }
 
     /// Sets the ports controller.
     ///
     /// `controller`: Name of the controller (Bridge, Bond, Team), e.g.: bond0.
     pub fn set_controller(&mut self, controller: &str) {
-        self.base_mut().controller = controller.to_string()
+        self.base_mut().controller = Some(controller.to_string());
     }
 
     pub fn uuid(&self) -> Uuid {
@@ -413,7 +411,7 @@ pub struct BaseConnection {
     pub ip_config: IpConfig,
     pub status: Status,
     pub interface: String,
-    pub controller: String,
+    pub controller: Option<String>,
     pub match_config: MatchConfig,
 }
 
@@ -585,29 +583,16 @@ pub struct BondConnection {
 }
 
 impl BondConnection {
-    pub fn find_port(&self, name: &str) -> Option<&Connection> {
-        self.bond.ports.iter().find(|c| c.interface() == name)
-    }
-
-    pub fn set_ports(&mut self, ports: Vec<Connection>) {
+    pub fn set_ports(&mut self, ports: Vec<String>) {
         self.bond.ports = ports;
     }
 
-    pub fn set_mode(&mut self, mode: &str) -> Result<(), NetworkStateError> {
-        self.bond.mode = BondMode::try_from(mode)
-            .map_err(|_e| NetworkStateError::InvalidBondMode(mode.to_string()))?;
-        Ok(())
+    pub fn set_mode(&mut self, mode: BondMode) {
+        self.bond.mode = mode;
     }
 
-    pub fn set_options<T>(&mut self, options: T) -> Result<(), NetworkStateError>
-    where
-        T: TryInto<BondOptions>,
-        <T as TryInto<BondOptions>>::Error: std::fmt::Debug,
-    {
-        self.bond.options = options
-            .try_into()
-            .map_err(|_e| NetworkStateError::InvalidBondOptions)?;
-        Ok(())
+    pub fn set_options(&mut self, options: BondOptions) {
+        self.bond.options = options;
     }
 }
 
@@ -647,7 +632,7 @@ impl fmt::Display for BondOptions {
 #[derive(Debug, Default, PartialEq, Clone)]
 pub struct BondConfig {
     pub mode: BondMode,
-    pub ports: Vec<Connection>,
+    pub ports: Vec<String>,
     pub options: BondOptions,
 }
 

rust/agama-dbus-server/src/network/nm/adapter.rs

@@ -52,8 +52,10 @@ impl<'a> Adapter for NetworkManagerAdapter<'a> {
                         if let Err(e) = self.client.remove_connection(conn.uuid()).await {
                             log::error!("Could not remove the connection {}: {}", conn.id(), e);
                         }
-                    } else if let Err(e) = self.client.add_or_update_connection(conn).await {
-                        log::error!("Could not add/update the connection {}: {}", conn.id(), e);
+                    } else if !conn.is_controlled() {
+                        if let Err(e) = self.client.add_or_update_connection(conn).await {
+                            log::error!("Could not add/update the connection {}: {}", conn.id(), e);
+                        }
                     }
                 }
             })

rust/agama-dbus-server/src/network/nm/client.rs

@@ -1,9 +1,6 @@
 //! NetworkManager client.
-use std::collections::HashMap;
-
 use super::dbus::{
-    cleanup_dbus_connection, connection_from_dbus, connection_to_dbus, controller_from_dbus,
-    merge_dbus_connections,
+    cleanup_dbus_connection, connection_from_dbus, connection_to_dbus, merge_dbus_connections,
 };
 use super::model::NmDeviceType;
 use super::proxies::{ConnectionProxy, DeviceProxy, NetworkManagerProxy, SettingsProxy};
@@ -76,24 +73,15 @@ impl<'a> NetworkManagerClient<'a> {
         let proxy = SettingsProxy::new(&self.connection).await?;
         let paths = proxy.list_connections().await?;
         let mut connections: Vec<Connection> = Vec::with_capacity(paths.len());
-        let mut controllers: HashMap<String, Vec<Connection>> = HashMap::new();
         for path in paths {
             let proxy = ConnectionProxy::builder(&self.connection)
                 .path(path.as_str())?
                 .build()
                 .await?;
             let settings = proxy.get_settings().await?;
             // TODO: log an error if a connection is not found
-
             if let Some(connection) = connection_from_dbus(settings.clone()) {
-                if let Some(controller) = controller_from_dbus(&settings) {
-                    controllers
-                        .entry(controller)
-                        .or_insert_with(Vec::new)
-                        .push(connection)
-                } else {
-                    connections.push(connection);
-                }
+                connections.push(connection);
             }
         }
 
@@ -117,53 +105,6 @@ impl<'a> NetworkManagerClient<'a> {
             proxy.add_connection(new_conn).await?
         };
 
-        if let Connection::Bond(bond) = conn {
-            for port in bond.bond.ports.clone() {
-                self.add_or_update_port_connection(
-                    &port,
-                    bond.base.interface.to_string(),
-                    "bond".to_string(),
-                )
-                .await?;
-            }
-        }
-
-        self.activate_connection(path).await?;
-        Ok(())
-    }
-
-    pub async fn add_or_update_port_connection(
-        &self,
-        conn: &Connection,
-        controller: String,
-        port_type: String,
-    ) -> Result<(), ServiceError> {
-        let mut dbus_conn = connection_to_dbus(conn);
-
-        if let Some(new_conn) = dbus_conn.get_mut("connection") {
-            new_conn.insert("slave-type", port_type.to_string().into());
-            new_conn.insert("master", controller.to_string().into());
-        }
-
-        let path = if let Ok(proxy) = self.get_connection_proxy(conn.uuid()).await {
-            let original = proxy.get_settings().await?;
-            let merged = merge_dbus_connections(&original, &dbus_conn);
-            proxy.update(merged).await?;
-            OwnedObjectPath::from(proxy.path().to_owned())
-        } else {
-            let proxy = SettingsProxy::new(&self.connection).await?;
-            proxy.add_connection(dbus_conn).await?
-        };
-
-        if let Connection::Bond(bond) = conn {
-            let _ = bond.bond.ports.iter().map(|port| {
-                self.add_or_update_port_connection(
-                    port,
-                    bond.base.interface.to_string(),
-                    "bond".to_string(),
-                )
-            });
-        }
         self.activate_connection(path).await?;
         Ok(())
     }