Merge branch 'work/whiting/cancellable' into 'master'

Add a ProcessManager so we can cancel/pause/resume.

See merge request holo/steamos-manager!16

Cargo.lock

@@ -588,9 +588,9 @@ checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646"
 
 [[package]]
 name = "libc"
-version = "0.2.153"
+version = "0.2.154"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "9c198f91728a82281a64e1f4f9eeb25d82cb32a5de251c6bd1b5154d63a8e7bd"
+checksum = "ae743338b92ff9146ce83992f766a31066a91a8c84a45e0e9f21e7cf6de6d346"
 
 [[package]]
 name = "linux-raw-sys"
@@ -938,6 +938,7 @@ dependencies = [
  "anyhow",
  "clap",
  "inotify",
+ "libc",
  "nix",
  "tempfile",
  "tokio",

Cargo.toml

@@ -14,8 +14,9 @@ strip="symbols"
 anyhow = "1"
 clap = { version = "4.5", default-features = false, features = ["derive", "help", "std", "usage"] }
 inotify = { version = "0.10", default-features = false, features = ["stream"] }
-nix = { version = "0.28", default-features = false, features = ["fs"] }
+libc = "0.2"
-tokio = { version = "1", default-features = false, features = ["fs", "io-util", "macros", "rt-multi-thread", "signal", "sync"] }
+nix = { version = "0.28", default-features = false, features = ["fs", "signal"] }
+tokio = { version = "1", default-features = false, features = ["fs", "io-std", "io-util", "macros", "process", "rt-multi-thread", "signal", "sync"] }
 tokio-stream = { version = "0.1", default-features = false }
 tokio-util = { version = "0.7", default-features = false }
 tracing = { version = "0.1", default-features = false }

com.steampowered.SteamOSManager1.xml

@@ -105,18 +105,26 @@
 
         Perform a BIOS update.
         
-        Version available: 7
+        @objectpath: An object path that can be used to pause/resume/cancel/kill the operation
+
+        Version available: 8
     -->
-    <method name="UpdateBios" />
+    <method name="UpdateBios">
+      <arg type="o" name="objectpath" direction="out"/>
+    </method>
 
     <!--
         UpdateDock:
 
         Perform a Dock Firmware update.
 
-        Version available: 7
+        @objectpath: An object path that can be used to pause/resume/cancel/kill the operation
+
+        Version available: 8
     -->
-    <method name="UpdateDock" />
+    <method name="UpdateDock">
+      <arg type="o" name="objectpath" direction="out"/>
+    </method>
 
     <!--
         TrimDevices:
@@ -125,9 +133,13 @@
         is important as some devices are not safe to trim unless some kernel
         quirks are available.
 
-        Version available: 7
+        @objectpath: An object path that can be used to pause/resume/cancel/kill the operation
+
+        Version available: 8
     -->
-    <method name="TrimDevices" />
+    <method name="TrimDevices">
+      <arg type="o" name="objectpath" direction="out"/>
+    </method>
 
     <!--
         FormatDevice:
@@ -135,15 +147,17 @@
         @device: Which device to format, e.g. /dev/mmcblk0
         @label: Filesystem label to assign to the formatted device
         @validate: When set runs common checks for conterfeit flash media before formatting, e.g. f3probe
+        @objectpath: An object path that can be used to pause/resume/cancel/kill the operation
 
         Format and optionally validate a storage device to a steam compatible filesystem.
 
-        Version available: 7
+        Version available: 8
     -->
     <method name="FormatDevice">
       <arg type="s" name="device" direction="in"/>
       <arg type="s" name="label" direction="in"/>
       <arg type="b" name="validate" direction="in"/>
+      <arg type="o" name="objectpath" direction="out"/>
     </method>
 
     <!--
@@ -265,4 +279,54 @@
 
   </interface>
 
+  <!--
+      com.steampowered.SteamOSManager1.SubProcess
+      @short_description: Interface to control a subprocess
+
+      Version available: 8
+  -->
+  <interface name="com.steampowered.SteamOSManager1.SubProcess">
+      <!--
+        Pause the operation
+      -->
+      <method name="Pause"/>
+      
+      <!--
+        Resume the operation
+      -->
+      <method name="Resume"/>
+      
+      <!--
+        Cancel the operation
+          
+        Note this sends a SIGTERM to the subprocess, vs Kill which sends SIGKILL.
+      -->
+      <method name="Cancel"/>
+
+      <!--
+        Kill the operation
+        
+        Note this sends a SIGKILL, vs Cancel which sends SIGTERM.
+      -->
+      <method name="Kill"/>
+      
+      <!--
+        Wait for process to end and get exit code, resuming if paused
+
+        @exit_code The exit code
+      -->
+      <method name="Wait">
+        <arg type="i" name="exit_code" direction="out"/>
+      </method>
+
+      <!--
+        Get the exit code of the process if possible.
+
+        @exit_code The exit code
+        -->
+      <method name="ExitCode">
+        <arg type="i" name="exit_code" direction="out"/>
+      </method>
+  </interface>
+
 </node>

src/manager.rs

@@ -17,7 +17,7 @@ use crate::power::{
     get_gpu_clocks, get_gpu_performance_level, get_tdp_limit, set_gpu_clocks,
     set_gpu_performance_level, set_tdp_limit, GPUPerformanceLevel,
 };
-use crate::process::{run_script, script_output};
+use crate::process::{run_script, script_output, ProcessManager};
 use crate::wifi::{
     get_wifi_backend, get_wifi_power_management_state, set_wifi_backend, set_wifi_debug_mode,
     set_wifi_power_management_state, WifiBackend, WifiDebugMode, WifiPowerManagement,
@@ -38,15 +38,17 @@ pub struct SteamOSManager {
     // Whether we should use trace-cmd or not.
     // True on galileo devices, false otherwise
     should_trace: bool,
+    process_manager: ProcessManager,
 }
 
 impl SteamOSManager {
     pub async fn new(connection: Connection) -> Result<Self> {
         Ok(SteamOSManager {
             fan_control: FanControl::new(connection.clone()),
-            connection,
             wifi_debug_mode: WifiDebugMode::Off,
             should_trace: variant().await? == HardwareVariant::Galileo,
+            process_manager: ProcessManager::new(connection.clone()),
+            connection,
         })
     }
 }
@@ -142,47 +144,52 @@ impl SteamOSManager {
         }
     }
 
-    async fn update_bios(&self) -> zbus::fdo::Result<()> {
+    async fn update_bios(&mut self) -> zbus::fdo::Result<zbus::zvariant::OwnedObjectPath> {
         // Update the bios as needed
-        run_script("/usr/bin/jupiter-biosupdate", &["--auto"])
+        self.process_manager
+            .get_command_object_path("/usr/bin/jupiter-biosupdate", &["--auto"], "updating BIOS")
             .await
-            .inspect_err(|message| error!("Error updating BIOS: {message}"))
-            .map_err(to_zbus_fdo_error)
     }
 
-    async fn update_dock(&self) -> zbus::fdo::Result<()> {
+    async fn update_dock(&mut self) -> zbus::fdo::Result<zbus::zvariant::OwnedObjectPath> {
         // Update the dock firmware as needed
-        run_script(
+        self.process_manager
+            .get_command_object_path(
                 "/usr/lib/jupiter-dock-updater/jupiter-dock-updater.sh",
                 &[] as &[String; 0],
+                "updating dock",
             )
             .await
-        .inspect_err(|message| error!("Error updating dock: {message}"))
-        .map_err(to_zbus_fdo_error)
     }
 
-    async fn trim_devices(&self) -> zbus::fdo::Result<()> {
+    async fn trim_devices(&mut self) -> zbus::fdo::Result<zbus::zvariant::OwnedObjectPath> {
         // Run steamos-trim-devices script
-        run_script("/usr/lib/hwsupport/trim-devices.sh", &[] as &[String; 0])
+        self.process_manager
+            .get_command_object_path(
+                "/usr/lib/hwsupport/trim-devices.sh",
+                &[] as &[String; 0],
+                "trimming devices",
+            )
             .await
-            .inspect_err(|message| error!("Error updating trimming devices: {message}"))
-            .map_err(to_zbus_fdo_error)
     }
 
     async fn format_device(
-        &self,
+        &mut self,
         device: &str,
         label: &str,
         validate: bool,
-    ) -> zbus::fdo::Result<()> {
+    ) -> zbus::fdo::Result<zbus::zvariant::OwnedObjectPath> {
         let mut args = vec!["--label", label, "--device", device];
         if !validate {
             args.push("--skip-validation");
         }
-        run_script("/usr/lib/hwsupport/format-device.sh", args.as_ref())
+        self.process_manager
+            .get_command_object_path(
+                "/usr/lib/hwsupport/format-device.sh",
+                args.as_ref(),
+                format!("formatting {device}").as_str(),
+            )
             .await
-            .inspect_err(|message| error!("Error formatting {device}: {message}"))
-            .map_err(to_zbus_fdo_error)
     }
 
     #[zbus(property(emits_changed_signal = "false"))]

src/process.rs

@@ -5,10 +5,171 @@
  * SPDX-License-Identifier: MIT
  */
 
-use anyhow::{anyhow, Result};
+use anyhow::{anyhow, bail, Result};
+use libc::pid_t;
+use nix::sys::signal;
+use nix::sys::signal::Signal;
+use nix::unistd::Pid;
 use std::ffi::OsStr;
-#[cfg(not(test))]
+use tokio::process::{Child, Command};
-use tokio::process::Command;
+use tracing::error;
+use zbus::interface;
+
+use crate::to_zbus_fdo_error;
+
+const PROCESS_PREFIX: &str = "/com/steampowered/SteamOSManager1/Process";
+
+pub struct ProcessManager {
+    // The thing that manages subprocesses.
+    // Keeps a handle to the zbus connection and
+    // what the next process id on the bus should be
+    connection: zbus::Connection,
+    next_process: u32,
+}
+
+pub struct SubProcess {
+    process: Child,
+    paused: bool,
+    exit_code: Option<i32>,
+}
+
+impl ProcessManager {
+    pub fn new(conn: zbus::Connection) -> ProcessManager {
+        ProcessManager {
+            connection: conn,
+            next_process: 0,
+        }
+    }
+
+    pub async fn get_command_object_path(
+        &mut self,
+        executable: &str,
+        args: &[impl AsRef<OsStr>],
+        operation_name: &str,
+    ) -> zbus::fdo::Result<zbus::zvariant::OwnedObjectPath> {
+        // Run the given executable and give back an object path
+        let path = format!("{}{}", PROCESS_PREFIX, self.next_process);
+        self.next_process += 1;
+        let pm = ProcessManager::run_long_command(executable, args)
+            .await
+            .inspect_err(|message| error!("Error {operation_name}: {message}"))
+            .map_err(to_zbus_fdo_error)?;
+        self.connection
+            .object_server()
+            .at(path.as_str(), pm)
+            .await?;
+        zbus::zvariant::OwnedObjectPath::try_from(path).map_err(to_zbus_fdo_error)
+    }
+
+    pub async fn run_long_command(
+        executable: &str,
+        args: &[impl AsRef<OsStr>],
+    ) -> Result<SubProcess> {
+        // Run the given executable with the given arguments
+        // Return an id that can be used later to pause/cancel/resume as needed
+        let child = Command::new(executable).args(args).spawn()?;
+        Ok(SubProcess {
+            process: child,
+            paused: false,
+            exit_code: None,
+        })
+    }
+}
+
+impl SubProcess {
+    fn send_signal(&self, signal: nix::sys::signal::Signal) -> Result<()> {
+        let pid = match self.process.id() {
+            Some(id) => id,
+            None => {
+                bail!("Unable to get pid from command, it likely finished running");
+            }
+        };
+        let pid: pid_t = match pid.try_into() {
+            Ok(pid) => pid,
+            Err(message) => {
+                bail!("Unable to get pid_t from command {message}");
+            }
+        };
+        signal::kill(Pid::from_raw(pid), signal)?;
+        Ok(())
+    }
+
+    async fn exit_code_internal(&mut self) -> Result<i32> {
+        match self.exit_code {
+            // Just give the exit_code if we have it already.
+            Some(code) => Ok(code),
+            None => {
+                // Otherwise wait for the process
+                let status = self.process.wait().await?;
+                match status.code() {
+                    Some(code) => {
+                        self.exit_code = Some(code);
+                        Ok(code)
+                    }
+                    None => bail!("Process exited without giving a code."),
+                }
+            }
+        }
+    }
+}
+
+#[interface(name = "com.steampowered.SteamOSManager1.SubProcess")]
+impl SubProcess {
+    pub async fn pause(&mut self) -> zbus::fdo::Result<()> {
+        if self.paused {
+            return Err(zbus::fdo::Error::Failed("Already paused".to_string()));
+        }
+        // Pause the given process if possible
+        // Return true on success, false otherwise
+        let result = self.send_signal(Signal::SIGSTOP).map_err(to_zbus_fdo_error);
+        self.paused = true;
+        result
+    }
+
+    pub async fn resume(&mut self) -> zbus::fdo::Result<()> {
+        // Resume the given process if possible
+        if !self.paused {
+            return Err(zbus::fdo::Error::Failed("Not paused".to_string()));
+        }
+        let result = self.send_signal(Signal::SIGCONT).map_err(to_zbus_fdo_error);
+        self.paused = false;
+        result
+    }
+
+    pub async fn cancel(&self) -> zbus::fdo::Result<()> {
+        self.send_signal(Signal::SIGTERM).map_err(to_zbus_fdo_error)
+    }
+
+    pub async fn kill(&self) -> zbus::fdo::Result<()> {
+        self.send_signal(signal::SIGKILL).map_err(to_zbus_fdo_error)
+    }
+
+    pub async fn wait(&mut self) -> zbus::fdo::Result<i32> {
+        if self.paused {
+            self.resume().await?;
+        }
+
+        let code = match self.exit_code_internal().await.map_err(to_zbus_fdo_error) {
+            Ok(v) => v,
+            Err(_) => {
+                return Err(zbus::fdo::Error::Failed(
+                    "Unable to get exit code".to_string(),
+                ));
+            }
+        };
+        self.exit_code = Some(code);
+        Ok(code)
+    }
+
+    pub async fn exit_code(&mut self) -> zbus::fdo::Result<i32> {
+        match self.exit_code_internal().await {
+            Ok(i) => Ok(i),
+            Err(_) => Err(zbus::fdo::Error::Failed(
+                "Unable to get exit code.".to_string(),
+            )),
+        }
+    }
+}
 
 #[cfg(not(test))]
 pub async fn script_exit_code(executable: &str, args: &[impl AsRef<OsStr>]) -> Result<i32> {
@@ -72,6 +233,41 @@ mod test {
         Err(anyhow!("oops!"))
     }
 
+    #[tokio::test]
+    async fn test_process_manager() {
+        let _h = testing::start();
+
+        let mut false_process = ProcessManager::run_long_command("/bin/false", &[] as &[String; 0])
+            .await
+            .unwrap();
+        let mut true_process = ProcessManager::run_long_command("/bin/true", &[] as &[String; 0])
+            .await
+            .unwrap();
+
+        let mut pause_process = ProcessManager::run_long_command("/usr/bin/sleep", &["1"])
+            .await
+            .unwrap();
+        let _ = pause_process.pause().await;
+
+        assert_eq!(
+            pause_process.pause().await.unwrap_err(),
+            zbus::fdo::Error::Failed("Already paused".to_string())
+        );
+
+        let _ = pause_process.resume().await;
+
+        assert_eq!(
+            pause_process.resume().await.unwrap_err(),
+            zbus::fdo::Error::Failed("Not paused".to_string())
+        );
+
+        // Sleep gives 0 exit code when done, -1 when we haven't waited for it yet
+        assert_eq!(pause_process.wait().await.unwrap(), 0);
+
+        assert_eq!(false_process.wait().await.unwrap(), 1);
+        assert_eq!(true_process.wait().await.unwrap(), 0);
+    }
+
     #[tokio::test]
     async fn test_run_script() {
         let h = testing::start();

src/user_manager.rs

@@ -144,15 +144,15 @@ impl SteamOSManagerUser {
         }
     }
 
-    async fn update_bios(&self) -> zbus::fdo::Result<()> {
+    async fn update_bios(&self) -> zbus::fdo::Result<zbus::zvariant::OwnedObjectPath> {
         method!(self, "UpdateBios")
     }
 
-    async fn update_dock(&self) -> zbus::fdo::Result<()> {
+    async fn update_dock(&self) -> zbus::fdo::Result<zbus::zvariant::OwnedObjectPath> {
         method!(self, "UpdateDock")
     }
 
-    async fn trim_devices(&self) -> zbus::fdo::Result<()> {
+    async fn trim_devices(&self) -> zbus::fdo::Result<zbus::zvariant::OwnedObjectPath> {
         method!(self, "TrimDevices")
     }
 
@@ -161,7 +161,7 @@ impl SteamOSManagerUser {
         device: &str,
         label: &str,
         validate: bool,
-    ) -> zbus::fdo::Result<()> {
+    ) -> zbus::fdo::Result<zbus::zvariant::OwnedObjectPath> {
         method!(self, "FormatDevice", device, label, validate)
     }
 
@@ -351,7 +351,13 @@ mod test {
             let remote_method = remote_methods.get(*key).expect(key);
 
             assert_eq!(local_method.name(), remote_method.name());
-            assert_eq!(local_method.args().len(), remote_method.args().len());
+            assert_eq!(
+                local_method.args().len(),
+                remote_method.args().len(),
+                "Testing {:?} against {:?}",
+                local_method,
+                remote_method
+            );
             for (local_arg, remote_arg) in
                 zip(local_method.args().iter(), remote_method.args().iter())
             {