diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 6e2475d875..36ab33dc58 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -5,6 +5,10 @@ members = [ "agama-derive", "agama-lib", "agama-locale-data", - "agama-settings" + "agama-settings", ] +resolver = "2" +[workspace.package] +rust-version = "1.74" +edition = "2021" diff --git a/rust/agama-dbus-server/Cargo.toml b/rust/agama-dbus-server/Cargo.toml index 4079ee01ca..6877d621f4 100644 --- a/rust/agama-dbus-server/Cargo.toml +++ b/rust/agama-dbus-server/Cargo.toml @@ -2,13 +2,14 @@ name = "agama-dbus-server" version = "0.1.0" edition = "2021" +rust-version.workspace = true # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] anyhow = "1.0" -agama-locale-data = { path="../agama-locale-data" } -agama-lib = { path="../agama-lib" } +agama-locale-data = { path = "../agama-locale-data" } +agama-lib = { path = "../agama-lib" } log = "0.4" simplelog = "0.12.1" systemd-journal-logger = "1.0" diff --git a/rust/agama-dbus-server/src/network/action.rs b/rust/agama-dbus-server/src/network/action.rs index 568410d579..0b3d00b5c2 100644 --- a/rust/agama-dbus-server/src/network/action.rs +++ b/rust/agama-dbus-server/src/network/action.rs @@ -23,11 +23,17 @@ pub enum Action { ), /// Gets a connection GetConnection(Uuid, Responder>), + /// Gets a connection + GetConnectionPath(String, Responder>), + /// Get connections paths + GetConnectionsPaths(Responder>), /// Gets a controller connection GetController( Uuid, Responder>, ), + /// Get devices paths + GetDevicesPaths(Responder>), /// Sets a controller's ports. It uses the Uuid of the controller and the IDs or interface names /// of the ports. SetPorts( diff --git a/rust/agama-dbus-server/src/network/adapter.rs b/rust/agama-dbus-server/src/network/adapter.rs index b5d24c4250..c96ce8e649 100644 --- a/rust/agama-dbus-server/src/network/adapter.rs +++ b/rust/agama-dbus-server/src/network/adapter.rs @@ -1,8 +1,10 @@ use crate::network::NetworkState; +use async_trait::async_trait; use std::error::Error; /// A trait for the ability to read/write from/to a network service +#[async_trait] pub trait Adapter { - fn read(&self) -> Result>; - fn write(&self, network: &NetworkState) -> Result<(), Box>; + async fn read(&self) -> Result>; + async fn write(&self, network: &NetworkState) -> Result<(), Box>; } diff --git a/rust/agama-dbus-server/src/network/dbus.rs b/rust/agama-dbus-server/src/network/dbus.rs index ba95f4c9ac..30fe51b7aa 100644 --- a/rust/agama-dbus-server/src/network/dbus.rs +++ b/rust/agama-dbus-server/src/network/dbus.rs @@ -8,4 +8,4 @@ pub mod service; mod tree; pub use service::NetworkService; -pub(crate) use tree::{ObjectsRegistry, Tree}; +pub(crate) use tree::Tree; diff --git a/rust/agama-dbus-server/src/network/dbus/interfaces/connections.rs b/rust/agama-dbus-server/src/network/dbus/interfaces/connections.rs index e8df645ec5..9fa57d7e6f 100644 --- a/rust/agama-dbus-server/src/network/dbus/interfaces/connections.rs +++ b/rust/agama-dbus-server/src/network/dbus/interfaces/connections.rs @@ -9,23 +9,21 @@ use zbus::{ }; use super::common::ConnectionInterface; -use crate::network::{dbus::ObjectsRegistry, error::NetworkStateError, model::MacAddress, Action}; +use crate::network::{error::NetworkStateError, model::MacAddress, Action}; /// D-Bus interface for the set of connections. /// /// It offers an API to query the connections collection. pub struct Connections { actions: Arc>>, - objects: Arc>, } impl Connections { /// Creates a Connections interface object. /// /// * `objects`: Objects paths registry. - pub fn new(objects: Arc>, actions: UnboundedSender) -> Self { + pub fn new(actions: UnboundedSender) -> Self { Self { - objects, actions: Arc::new(Mutex::new(actions)), } } @@ -34,13 +32,12 @@ impl Connections { #[dbus_interface(name = "org.opensuse.Agama1.Network.Connections")] impl Connections { /// Returns the D-Bus paths of the network connections. - pub async fn get_connections(&self) -> Vec { - let objects = self.objects.lock().await; - objects - .connections_paths() - .iter() - .filter_map(|c| ObjectPath::try_from(c.clone()).ok()) - .collect() + pub async fn get_connections(&self) -> zbus::fdo::Result> { + let actions = self.actions.lock().await; + let (tx, rx) = oneshot::channel(); + actions.send(Action::GetConnectionsPaths(tx)).unwrap(); + let result = rx.await.unwrap(); + Ok(result) } /// Adds a new network connection. @@ -67,11 +64,16 @@ impl Connections { /// /// * `id`: connection ID. pub async fn get_connection(&self, id: &str) -> zbus::fdo::Result { - let objects = self.objects.lock().await; - match objects.connection_path(id) { - Some(path) => Ok(path.into()), - None => Err(NetworkStateError::UnknownConnection(id.to_string()).into()), - } + let actions = self.actions.lock().await; + let (tx, rx) = oneshot::channel(); + actions + .send(Action::GetConnectionPath(id.to_string(), tx)) + .unwrap(); + let path = rx + .await + .unwrap() + .ok_or(NetworkStateError::UnknownConnection(id.to_string()))?; + Ok(path) } /// Removes a network connection. diff --git a/rust/agama-dbus-server/src/network/dbus/interfaces/devices.rs b/rust/agama-dbus-server/src/network/dbus/interfaces/devices.rs index 41339e01e5..8f29d84f35 100644 --- a/rust/agama-dbus-server/src/network/dbus/interfaces/devices.rs +++ b/rust/agama-dbus-server/src/network/dbus/interfaces/devices.rs @@ -1,34 +1,35 @@ -use crate::network::{dbus::ObjectsRegistry, model::Device as NetworkDevice}; +use crate::network::{model::Device as NetworkDevice, Action}; use std::sync::Arc; -use tokio::sync::Mutex; -use zbus::{dbus_interface, zvariant::ObjectPath}; +use tokio::sync::{mpsc::UnboundedSender, oneshot, Mutex}; +use zbus::{dbus_interface, zvariant::OwnedObjectPath}; /// D-Bus interface for the network devices collection /// /// It offers an API to query the devices collection. pub struct Devices { - objects: Arc>, + actions: Arc>>, } impl Devices { /// Creates a Devices interface object. /// /// * `objects`: Objects paths registry. - pub fn new(objects: Arc>) -> Self { - Self { objects } + pub fn new(actions: UnboundedSender) -> Self { + Self { + actions: Arc::new(Mutex::new(actions)), + } } } #[dbus_interface(name = "org.opensuse.Agama1.Network.Devices")] impl Devices { /// Returns the D-Bus paths of the network devices. - pub async fn get_devices(&self) -> Vec { - let objects = self.objects.lock().await; - objects - .devices_paths() - .iter() - .filter_map(|c| ObjectPath::try_from(c.clone()).ok()) - .collect() + pub async fn get_devices(&self) -> zbus::fdo::Result> { + let actions = self.actions.lock().await; + let (tx, rx) = oneshot::channel(); + actions.send(Action::GetDevicesPaths(tx)).unwrap(); + let result = rx.await.unwrap(); + Ok(result) } } diff --git a/rust/agama-dbus-server/src/network/dbus/tree.rs b/rust/agama-dbus-server/src/network/dbus/tree.rs index d6080508d9..149d9ab1ef 100644 --- a/rust/agama-dbus-server/src/network/dbus/tree.rs +++ b/rust/agama-dbus-server/src/network/dbus/tree.rs @@ -1,10 +1,9 @@ use agama_lib::error::ServiceError; -use tokio::sync::Mutex; use zbus::zvariant::{ObjectPath, OwnedObjectPath}; use crate::network::{action::Action, dbus::interfaces, model::*}; use log; -use std::{collections::HashMap, sync::Arc}; +use std::collections::HashMap; use tokio::sync::mpsc::UnboundedSender; const CONNECTIONS_PATH: &str = "/org/opensuse/Agama1/Network/connections"; @@ -14,7 +13,7 @@ const DEVICES_PATH: &str = "/org/opensuse/Agama1/Network/devices"; pub struct Tree { connection: zbus::Connection, actions: UnboundedSender, - objects: Arc>, + objects: ObjectsRegistry, } impl Tree { @@ -37,7 +36,7 @@ impl Tree { /// /// * `connections`: list of connections. pub async fn set_connections( - &self, + &mut self, connections: &mut [Connection], ) -> Result<(), ServiceError> { self.remove_connections().await?; @@ -63,15 +62,11 @@ impl Tree { let path = ObjectPath::try_from(path.as_str()).unwrap(); self.add_interface(&path, interfaces::Device::new(dev.clone())) .await?; - let mut objects = self.objects.lock().await; - objects.register_device(&dev.name, path); + self.objects.register_device(&dev.name, path); } - self.add_interface( - DEVICES_PATH, - interfaces::Devices::new(Arc::clone(&self.objects)), - ) - .await?; + self.add_interface(DEVICES_PATH, interfaces::Devices::new(self.actions.clone())) + .await?; Ok(()) } @@ -81,17 +76,16 @@ impl Tree { /// * `conn`: connection to add. /// * `notify`: whether to notify the added connection pub async fn add_connection( - &self, + &mut self, conn: &mut Connection, ) -> Result { - let mut objects = self.objects.lock().await; - let uuid = conn.uuid; - let (id, path) = objects.register_connection(conn); + let (id, path) = self.objects.register_connection(conn); if id != conn.id { conn.id = id.clone(); } - log::info!("Publishing network connection '{}'", id); + let path: OwnedObjectPath = path.into(); + log::info!("Publishing network connection '{}' on '{}'", id, &path); self.add_interface( &path, @@ -115,33 +109,49 @@ impl Tree { .await?; } - Ok(path.into()) + Ok(path) } /// Removes a connection from the tree /// /// * `id`: connection ID. pub async fn remove_connection(&mut self, id: &str) -> Result<(), ServiceError> { - let mut objects = self.objects.lock().await; - let Some(path) = objects.connection_path(id) else { + let Some(path) = self.objects.connection_path(id) else { return Ok(()); }; self.remove_connection_on(path.as_str()).await?; - objects.deregister_connection(id).unwrap(); + self.objects.deregister_connection(id).unwrap(); Ok(()) } + /// Returns all devices paths. + pub fn devices_paths(&self) -> Vec { + self.objects.devices_paths() + } + + /// Returns all connection paths. + pub fn connections_paths(&self) -> Vec { + self.objects.connections_paths() + } + + pub fn connection_path(&self, id: &str) -> Option { + self.objects.connection_path(id).map(|o| o.into()) + } + /// Adds connections to the D-Bus tree. /// /// * `connections`: list of connections. - async fn add_connections(&self, connections: &mut [Connection]) -> Result<(), ServiceError> { + async fn add_connections( + &mut self, + connections: &mut [Connection], + ) -> Result<(), ServiceError> { for conn in connections.iter_mut() { self.add_connection(conn).await?; } self.add_interface( CONNECTIONS_PATH, - interfaces::Connections::new(Arc::clone(&self.objects), self.actions.clone()), + interfaces::Connections::new(self.actions.clone()), ) .await?; @@ -149,25 +159,23 @@ impl Tree { } /// Clears all the connections from the tree. - async fn remove_connections(&self) -> Result<(), ServiceError> { - let mut objects = self.objects.lock().await; - for path in objects.connections.values() { + async fn remove_connections(&mut self) -> Result<(), ServiceError> { + for path in self.objects.connections.values() { self.remove_connection_on(path.as_str()).await?; } - objects.connections.clear(); + self.objects.connections.clear(); Ok(()) } /// Clears all the devices from the tree. async fn remove_devices(&mut self) -> Result<(), ServiceError> { let object_server = self.connection.object_server(); - let mut objects = self.objects.lock().await; - for path in objects.devices.values() { + for path in self.objects.devices.values() { object_server .remove::(path.as_str()) .await?; } - objects.devices.clear(); + self.objects.devices.clear(); Ok(()) } @@ -185,7 +193,7 @@ impl Tree { Ok(()) } - async fn add_interface(&self, path: &str, iface: T) -> Result + async fn add_interface(&mut self, path: &str, iface: T) -> Result where T: zbus::Interface, { @@ -198,7 +206,7 @@ impl Tree { /// /// Connections are indexed by its Id, which is expected to be unique. #[derive(Debug, Default)] -pub struct ObjectsRegistry { +struct ObjectsRegistry { /// device_name (eth0) -> object_path devices: HashMap, /// id -> object_path @@ -246,13 +254,13 @@ impl ObjectsRegistry { } /// Returns all devices paths. - pub fn devices_paths(&self) -> Vec { - self.devices.values().map(|p| p.to_string()).collect() + pub fn devices_paths(&self) -> Vec { + self.devices.values().cloned().collect() } /// Returns all connection paths. - pub fn connections_paths(&self) -> Vec { - self.connections.values().map(|p| p.to_string()).collect() + pub fn connections_paths(&self) -> Vec { + self.connections.values().cloned().collect() } /// Proposes a connection ID. diff --git a/rust/agama-dbus-server/src/network/nm/adapter.rs b/rust/agama-dbus-server/src/network/nm/adapter.rs index 6aa590fb0e..eb42b36afb 100644 --- a/rust/agama-dbus-server/src/network/nm/adapter.rs +++ b/rust/agama-dbus-server/src/network/nm/adapter.rs @@ -4,8 +4,8 @@ use crate::network::{ Adapter, }; use agama_lib::error::ServiceError; +use async_trait::async_trait; use log; -use tokio::{runtime::Handle, task}; /// An adapter for NetworkManager pub struct NetworkManagerAdapter<'a> { @@ -27,16 +27,12 @@ impl<'a> NetworkManagerAdapter<'a> { } } +#[async_trait] impl<'a> Adapter for NetworkManagerAdapter<'a> { - fn read(&self) -> Result> { - task::block_in_place(|| { - Handle::current().block_on(async { - let devices = self.client.devices().await?; - let connections = self.client.connections().await?; - - Ok(NetworkState::new(devices, connections)) - }) - }) + async fn read(&self) -> Result> { + let devices = self.client.devices().await?; + let connections = self.client.connections().await?; + Ok(NetworkState::new(devices, connections)) } /// Writes the connections to NetworkManager. @@ -46,31 +42,27 @@ impl<'a> Adapter for NetworkManagerAdapter<'a> { /// simpler approach. /// /// * `network`: network model. - fn write(&self, network: &NetworkState) -> Result<(), Box> { + async fn write(&self, network: &NetworkState) -> Result<(), Box> { // By now, traits do not support async functions. Using `task::block_on` allows // to use 'await'. - task::block_in_place(|| { - Handle::current().block_on(async { - for conn in ordered_connections(network) { - if !Self::is_writable(conn) { - continue; - } + for conn in ordered_connections(network) { + if !Self::is_writable(conn) { + continue; + } - let result = if conn.is_removed() { - self.client.remove_connection(conn.uuid).await - } else { - let ctrl = conn - .controller - .and_then(|uuid| network.get_connection_by_uuid(uuid)); - self.client.add_or_update_connection(conn, ctrl).await - }; + let result = if conn.is_removed() { + self.client.remove_connection(conn.uuid).await + } else { + let ctrl = conn + .controller + .and_then(|uuid| network.get_connection_by_uuid(uuid)); + self.client.add_or_update_connection(conn, ctrl).await + }; - if let Err(e) = result { - log::error!("Could not process the connection {}: {}", conn.id, e); - } - } - }) - }); + if let Err(e) = result { + log::error!("Could not process the connection {}: {}", conn.id, e); + } + } // FIXME: indicate which connections could not be written. Ok(()) } diff --git a/rust/agama-dbus-server/src/network/system.rs b/rust/agama-dbus-server/src/network/system.rs index b0850064e5..416c9e261f 100644 --- a/rust/agama-dbus-server/src/network/system.rs +++ b/rust/agama-dbus-server/src/network/system.rs @@ -33,8 +33,8 @@ impl NetworkSystem { /// Writes the network configuration. pub async fn write(&mut self) -> Result<(), Box> { - self.adapter.write(&self.state)?; - self.state = self.adapter.read()?; + self.adapter.write(&self.state).await?; + self.state = self.adapter.read().await?; Ok(()) } @@ -47,7 +47,7 @@ impl NetworkSystem { /// Populates the D-Bus tree with the known devices and connections. pub async fn setup(&mut self) -> Result<(), Box> { - self.state = self.adapter.read()?; + self.state = self.adapter.read().await?; self.tree .set_connections(&mut self.state.connections) .await?; @@ -77,10 +77,16 @@ impl NetworkSystem { let conn = self.state.get_connection_by_uuid(uuid); tx.send(conn.cloned()).unwrap(); } + Action::GetConnectionPath(id, tx) => { + let path = self.tree.connection_path(&id); + tx.send(path).unwrap(); + } Action::GetController(uuid, tx) => { let result = self.get_controller_action(uuid); tx.send(result).unwrap() } + Action::GetDevicesPaths(tx) => tx.send(self.tree.devices_paths()).unwrap(), + Action::GetConnectionsPaths(tx) => tx.send(self.tree.connections_paths()).unwrap(), Action::SetPorts(uuid, ports, rx) => { let result = self.set_ports_action(uuid, *ports); rx.send(result).unwrap(); diff --git a/rust/agama-dbus-server/tests/network.rs b/rust/agama-dbus-server/tests/network.rs index 71bbf8f638..616067d651 100644 --- a/rust/agama-dbus-server/tests/network.rs +++ b/rust/agama-dbus-server/tests/network.rs @@ -11,6 +11,7 @@ use agama_lib::network::{ types::DeviceType, NetworkClient, }; +use async_trait::async_trait; use cidr::IpInet; use std::error::Error; use tokio::test; @@ -18,12 +19,16 @@ use tokio::test; #[derive(Default)] pub struct NetworkTestAdapter(network::NetworkState); +#[async_trait] impl Adapter for NetworkTestAdapter { - fn read(&self) -> Result> { + async fn read(&self) -> Result> { Ok(self.0.clone()) } - fn write(&self, _network: &network::NetworkState) -> Result<(), Box> { + async fn write( + &self, + _network: &network::NetworkState, + ) -> Result<(), Box> { unimplemented!("Not used in tests"); } } diff --git a/rust/agama-lib/src/error.rs b/rust/agama-lib/src/error.rs index b1fc9ba579..f01a929837 100644 --- a/rust/agama-lib/src/error.rs +++ b/rust/agama-lib/src/error.rs @@ -6,9 +6,9 @@ use zbus; #[derive(Error, Debug)] pub enum ServiceError { - #[error("D-Bus service error")] + #[error("D-Bus service error: {0}")] DBus(#[from] zbus::Error), - #[error("Could not connect to Agama bus at '{0}'")] + #[error("Could not connect to Agama bus at '{0}': {1}")] DBusConnectionError(String, #[source] zbus::Error), // it's fine to say only "Error" because the original // specific error will be printed too @@ -20,7 +20,7 @@ pub enum ServiceError { FailedRegistration(String), #[error("Failed to find these patterns: {0:?}")] UnknownPatterns(Vec), - #[error("Error: {0}")] + #[error("Could not perform action '{0}'")] UnsuccessfulAction(String), }