Tweak the Subprocess api a bit.

From feedback from Andres changed SubProcess to just Job for
possible further changes to have the manager do things itself
without launching subprocesses.
Changed names objectpath to jobpath, exit_code to result, etc.
Removed Kill and added a force parameter to Cancel instead.
Also ran cargo fmt which tweaked indentation in a couple places.
Alos removed ExitCode, users can just call Wait again if needed.
This commit is contained in:
Jeremy Whiting 2024-05-20 11:05:46 -06:00 committed by Vicki Pfau
parent 6e51d1556b
commit f6298efbc9
4 changed files with 46 additions and 85 deletions

View file

@ -105,12 +105,12 @@
Perform a BIOS update. Perform a BIOS update.
@objectpath: An object path that can be used to pause/resume/cancel/kill the operation @jobpath: An object path that can be used to pause/resume/cancel the operation
Version available: 8 Version available: 8
--> -->
<method name="UpdateBios"> <method name="UpdateBios">
<arg type="o" name="objectpath" direction="out"/> <arg type="o" name="jobpath" direction="out"/>
</method> </method>
<!-- <!--
@ -118,12 +118,12 @@
Perform a Dock Firmware update. Perform a Dock Firmware update.
@objectpath: An object path that can be used to pause/resume/cancel/kill the operation @jobpath: An object path that can be used to pause/resume/cancel the operation
Version available: 8 Version available: 8
--> -->
<method name="UpdateDock"> <method name="UpdateDock">
<arg type="o" name="objectpath" direction="out"/> <arg type="o" name="jobpath" direction="out"/>
</method> </method>
<!-- <!--
@ -133,12 +133,12 @@
is important as some devices are not safe to trim unless some kernel is important as some devices are not safe to trim unless some kernel
quirks are available. quirks are available.
@objectpath: An object path that can be used to pause/resume/cancel/kill the operation @jobpath: An object path that can be used to pause/resume/cancel the operation
Version available: 8 Version available: 8
--> -->
<method name="TrimDevices"> <method name="TrimDevices">
<arg type="o" name="objectpath" direction="out"/> <arg type="o" name="jobpath" direction="out"/>
</method> </method>
<!-- <!--
@ -147,7 +147,7 @@
@device: Which device to format, e.g. /dev/mmcblk0 @device: Which device to format, e.g. /dev/mmcblk0
@label: Filesystem label to assign to the formatted device @label: Filesystem label to assign to the formatted device
@validate: When set runs common checks for conterfeit flash media before formatting, e.g. f3probe @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 @jobpath: An object path that can be used to pause/resume/cancel the operation
Format and optionally validate a storage device to a steam compatible filesystem. Format and optionally validate a storage device to a steam compatible filesystem.
@ -157,7 +157,7 @@
<arg type="s" name="device" direction="in"/> <arg type="s" name="device" direction="in"/>
<arg type="s" name="label" direction="in"/> <arg type="s" name="label" direction="in"/>
<arg type="b" name="validate" direction="in"/> <arg type="b" name="validate" direction="in"/>
<arg type="o" name="objectpath" direction="out"/> <arg type="o" name="jobpath" direction="out"/>
</method> </method>
<!-- <!--
@ -280,12 +280,12 @@
</interface> </interface>
<!-- <!--
com.steampowered.SteamOSManager1.SubProcess com.steampowered.SteamOSManager1.Job
@short_description: Interface to control a subprocess @short_description: Interface to control a job
Version available: 8 Version available: 8
--> -->
<interface name="com.steampowered.SteamOSManager1.SubProcess"> <interface name="com.steampowered.SteamOSManager1.Job">
<!-- <!--
Pause the operation Pause the operation
--> -->
@ -299,35 +299,30 @@
<!-- <!--
Cancel the operation Cancel the operation
Note this sends a SIGTERM to the subprocess, vs Kill which sends SIGKILL. @force Use sigkill if true, sigterm otherwise.
--> -->
<method name="Cancel"/> <method name="Cancel">
<arg type="b" name="force" direction="in"/>
<!-- </method>
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 Wait for process to end and get exit code, resuming if paused
@exit_code The exit code, or negative signal number if the process @result The exit code, or negative signal number if the process
exited via signal. exited via signal.
--> -->
<method name="Wait"> <method name="Wait">
<arg type="i" name="exit_code" direction="out"/> <arg type="i" name="result" direction="out"/>
</method> </method>
<!-- <!--
Get the exit code of the process if possible. Get the exit code of the process if possible.
@exit_code The exit code, or negative signal number if the process @result The exit code, or negative signal number if the process
exited via signal. exited via signal.
--> -->
<method name="ExitCode"> <method name="ExitCode">
<arg type="i" name="exit_code" direction="out"/> <arg type="i" name="result" direction="out"/>
</method> </method>
</interface> </interface>

View file

@ -233,7 +233,7 @@ async fn main() -> Result<()> {
} }
Commands::SetWifiBackend { backend } => { Commands::SetWifiBackend { backend } => {
proxy.set_wifi_backend(*backend as u32).await?; proxy.set_wifi_backend(*backend as u32).await?;
}, }
Commands::GetWifiBackend => { Commands::GetWifiBackend => {
let backend = proxy.wifi_backend().await?; let backend = proxy.wifi_backend().await?;
match WifiBackend::try_from(backend) { match WifiBackend::try_from(backend) {

View file

@ -19,7 +19,7 @@ use zbus::{fdo, interface};
use crate::error::to_zbus_fdo_error; use crate::error::to_zbus_fdo_error;
const PROCESS_PREFIX: &str = "/com/steampowered/SteamOSManager1/Process"; const PROCESS_PREFIX: &str = "/com/steampowered/SteamOSManager1/Job";
pub struct ProcessManager { pub struct ProcessManager {
// The thing that manages subprocesses. // The thing that manages subprocesses.
@ -29,7 +29,7 @@ pub struct ProcessManager {
next_process: u32, next_process: u32,
} }
pub struct SubProcess { pub struct Job {
process: Child, process: Child,
paused: bool, paused: bool,
exit_code: Option<i32>, exit_code: Option<i32>,
@ -63,14 +63,11 @@ impl ProcessManager {
zbus::zvariant::OwnedObjectPath::try_from(path).map_err(to_zbus_fdo_error) zbus::zvariant::OwnedObjectPath::try_from(path).map_err(to_zbus_fdo_error)
} }
pub async fn run_long_command( pub async fn run_long_command(executable: &str, args: &[impl AsRef<OsStr>]) -> Result<Job> {
executable: &str,
args: &[impl AsRef<OsStr>],
) -> Result<SubProcess> {
// Run the given executable with the given arguments // Run the given executable with the given arguments
// Return an id that can be used later to pause/cancel/resume as needed // Return an id that can be used later to pause/cancel/resume as needed
let child = Command::new(executable).args(args).spawn()?; let child = Command::new(executable).args(args).spawn()?;
Ok(SubProcess { Ok(Job {
process: child, process: child,
paused: false, paused: false,
exit_code: None, exit_code: None,
@ -78,7 +75,7 @@ impl ProcessManager {
} }
} }
impl SubProcess { impl Job {
fn send_signal(&self, signal: nix::sys::signal::Signal) -> Result<()> { fn send_signal(&self, signal: nix::sys::signal::Signal) -> Result<()> {
let pid = match self.process.id() { let pid = match self.process.id() {
Some(id) => id, Some(id) => id,
@ -114,7 +111,7 @@ impl SubProcess {
Ok(self.exit_code) Ok(self.exit_code)
} }
async fn exit_code_internal(&mut self) -> Result<i32> { async fn wait_internal(&mut self) -> Result<i32> {
if let Some(code) = self.exit_code { if let Some(code) = self.exit_code {
// Just give the exit_code if we have it already // Just give the exit_code if we have it already
Ok(code) Ok(code)
@ -126,8 +123,8 @@ impl SubProcess {
} }
} }
#[interface(name = "com.steampowered.SteamOSManager1.SubProcess")] #[interface(name = "com.steampowered.SteamOSManager1.Job")]
impl SubProcess { impl Job {
pub async fn pause(&mut self) -> fdo::Result<()> { pub async fn pause(&mut self) -> fdo::Result<()> {
if self.paused { if self.paused {
return Err(fdo::Error::Failed("Already paused".to_string())); return Err(fdo::Error::Failed("Already paused".to_string()));
@ -149,9 +146,12 @@ impl SubProcess {
result result
} }
pub async fn cancel(&mut self) -> fdo::Result<()> { pub async fn cancel(&mut self, force: bool) -> fdo::Result<()> {
if self.try_wait().map_err(to_zbus_fdo_error)?.is_none() { if self.try_wait().map_err(to_zbus_fdo_error)?.is_none() {
self.send_signal(Signal::SIGTERM) self.send_signal(match force {
true => Signal::SIGKILL,
false => Signal::SIGTERM,
})
.map_err(to_zbus_fdo_error)?; .map_err(to_zbus_fdo_error)?;
if self.paused { if self.paused {
self.resume().await?; self.resume().await?;
@ -160,19 +160,12 @@ impl SubProcess {
Ok(()) Ok(())
} }
pub async fn kill(&mut self) -> fdo::Result<()> {
match self.try_wait().map_err(to_zbus_fdo_error)? {
Some(_) => Ok(()),
None => self.send_signal(signal::SIGKILL).map_err(to_zbus_fdo_error),
}
}
pub async fn wait(&mut self) -> fdo::Result<i32> { pub async fn wait(&mut self) -> fdo::Result<i32> {
if self.paused { if self.paused {
self.resume().await?; self.resume().await?;
} }
let code = match self.exit_code_internal().await.map_err(to_zbus_fdo_error) { let code = match self.wait_internal().await.map_err(to_zbus_fdo_error) {
Ok(v) => v, Ok(v) => v,
Err(_) => { Err(_) => {
return Err(fdo::Error::Failed("Unable to get exit code".to_string())); return Err(fdo::Error::Failed("Unable to get exit code".to_string()));
@ -181,15 +174,6 @@ impl SubProcess {
self.exit_code = Some(code); self.exit_code = Some(code);
Ok(code) Ok(code)
} }
pub async fn exit_code(&mut self) -> fdo::Result<i32> {
match self.try_wait() {
Ok(Some(i)) => Ok(i),
_ => Err(zbus::fdo::Error::Failed(
"Unable to get exit code. The process may still be running.".to_string(),
)),
}
}
} }
#[cfg(not(test))] #[cfg(not(test))]
@ -297,10 +281,10 @@ mod test {
let mut sleep_process = ProcessManager::run_long_command("/usr/bin/sleep", &["0.1"]) let mut sleep_process = ProcessManager::run_long_command("/usr/bin/sleep", &["0.1"])
.await .await
.unwrap(); .unwrap();
sleep_process.kill().await.expect("kill"); sleep_process.cancel(true).await.expect("kill");
// Killing a process should be idempotent // Killing a process should be idempotent
sleep_process.kill().await.expect("kill"); sleep_process.cancel(true).await.expect("kill");
assert_eq!( assert_eq!(
sleep_process.wait().await.unwrap(), sleep_process.wait().await.unwrap(),
@ -319,28 +303,13 @@ mod test {
assert_eq!(pause_process.try_wait().expect("try_wait"), None); assert_eq!(pause_process.try_wait().expect("try_wait"), None);
// Canceling a process should unpause it // Canceling a process should unpause it
pause_process.cancel().await.expect("pause"); pause_process.cancel(false).await.expect("pause");
assert_eq!( assert_eq!(
pause_process.wait().await.unwrap(), pause_process.wait().await.unwrap(),
-(Signal::SIGTERM as i32) -(Signal::SIGTERM as i32)
); );
} }
#[tokio::test]
async fn test_exit_code_early() {
let _h = testing::start();
let mut sleep_process = ProcessManager::run_long_command("/usr/bin/sleep", &["0.1"])
.await
.unwrap();
assert_eq!(
sleep_process.exit_code().await.unwrap_err().to_string(),
"org.freedesktop.DBus.Error.Failed: Unable to get exit code. The process may still be running."
);
assert_eq!(sleep_process.wait().await.unwrap(), 0);
}
#[tokio::test] #[tokio::test]
async fn test_run_script() { async fn test_run_script() {
let h = testing::start(); let h = testing::start();

View file

@ -123,12 +123,12 @@ trait Manager {
#[proxy( #[proxy(
default_service = "com.steampowered.SteamOSManager1", default_service = "com.steampowered.SteamOSManager1",
interface = "com.steampowered.SteamOSManager1.SubProcess", interface = "com.steampowered.SteamOSManager1.Job",
assume_defaults = true assume_defaults = true
)] )]
trait SubProcess { trait Job {
/// Cancel method /// Cancel method
fn cancel(&self) -> zbus::Result<()>; fn cancel(&self, force: bool) -> zbus::Result<()>;
/// ExitCode method /// ExitCode method
fn exit_code(&self) -> zbus::Result<i32>; fn exit_code(&self) -> zbus::Result<i32>;
@ -136,9 +136,6 @@ trait SubProcess {
/// WaitForExitCode method /// WaitForExitCode method
fn wait_for_exit_code(&self) -> zbus::Result<i32>; fn wait_for_exit_code(&self) -> zbus::Result<i32>;
/// Kill method
fn kill(&self) -> zbus::Result<()>;
/// Pause method /// Pause method
fn pause(&self) -> zbus::Result<()>; fn pause(&self) -> zbus::Result<()>;