diff --git a/Cargo.lock b/Cargo.lock index e7f4772..0f1b13b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -402,9 +402,9 @@ dependencies = [ [[package]] name = "cap-std-ext" -version = "5.1.1" +version = "5.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "56f9d7cf114dea582f663f03f4c563d0fc5ca2c8fa4c496eb538d8f01981ea51" +checksum = "6c6c5ac70c4465b47c3a322330f87c2df31e42ac527497d94dc1f1d1cb8989f9" dependencies = [ "cap-primitives", "cap-tempfile", @@ -1479,11 +1479,9 @@ dependencies = [ "cfg-if", "dirs", "itest", - "libc", "linkme", "rand", "regex", - "rustix", "scopeguard", "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index de28b15..8519c02 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,7 +11,7 @@ rand = "0.9" tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["fmt", "env-filter"] } tracing-error = { version = "0.2.0" } -cap-std-ext = "5.1" +cap-std-ext = "5.1.2" xshell = "0.2.7" [workspace.lints.rust] diff --git a/crates/bcvk-qemu/src/qemu.rs b/crates/bcvk-qemu/src/qemu.rs index 5c88e07..634a991 100644 --- a/crates/bcvk-qemu/src/qemu.rs +++ b/crates/bcvk-qemu/src/qemu.rs @@ -14,7 +14,7 @@ use std::sync::Arc; use std::time::Duration; use camino::{Utf8Path, Utf8PathBuf}; -use cap_std_ext::cmdext::CapStdExtCommandExt; +use cap_std_ext::cmdext::{CapStdExtCommandExt, CmdFds}; use color_eyre::eyre::{eyre, Context}; use color_eyre::Result; use libc::{VMADDR_CID_ANY, VMADDR_PORT_ANY}; @@ -571,12 +571,13 @@ fn spawn( cmd.arg("-no-reboot"); } + let mut cmd_fds = CmdFds::new(); for (idx, fd) in config.fdset.iter().enumerate() { let fd_id = 100 + idx as u32; // Start at 100 to avoid conflicts let set_id = idx + 1; // fdset starts at 1 // Pass the write FD to QEMU - cmd.take_fd_n(Arc::clone(fd), fd_id as i32); + cmd_fds.take_fd_n(Arc::clone(fd), fd_id as i32); cmd.args(["-add-fd", &format!("fd={},set={}", fd_id, set_id)]); } @@ -736,7 +737,7 @@ fn spawn( // Add AF_VSOCK device if enabled if let Some((vhostfd, guest_cid)) = vsock { debug!("Adding AF_VSOCK device with guest CID: {}", guest_cid); - cmd.take_fd_n(Arc::new(vhostfd), 42); + cmd_fds.take_fd_n(Arc::new(vhostfd), 42); cmd.args([ "-device", &format!("vhost-vsock-pci,guest-cid={},vhostfd=42", guest_cid), @@ -773,6 +774,8 @@ fn spawn( } } + cmd.take_fds(cmd_fds); + tracing::debug!("{cmd:?}"); cmd.spawn().context("Failed to spawn QEMU") diff --git a/crates/integration-tests/Cargo.toml b/crates/integration-tests/Cargo.toml index 05a9fbd..275f726 100644 --- a/crates/integration-tests/Cargo.toml +++ b/crates/integration-tests/Cargo.toml @@ -38,8 +38,6 @@ regex = "1" rand = { workspace = true } scopeguard = "1" cap-std-ext = { workspace = true } -libc = "0.2" -rustix = { version = "1", features = ["process"] } [lints] workspace = true diff --git a/crates/integration-tests/src/tests/varlink.rs b/crates/integration-tests/src/tests/varlink.rs index f28757c..d113f5d 100644 --- a/crates/integration-tests/src/tests/varlink.rs +++ b/crates/integration-tests/src/tests/varlink.rs @@ -11,11 +11,10 @@ //! If something is not working, use assert/unwrap to fail hard. use std::os::unix::net::UnixStream; -use std::os::unix::process::CommandExt; use std::process::Command; use std::sync::{Arc, OnceLock}; -use cap_std_ext::cmdext::CapStdExtCommandExt; +use cap_std_ext::cmdext::{CapStdExtCommandExt, CmdFds, SystemdFdName}; use itest::TestResult; use serde::Deserialize; @@ -190,13 +189,11 @@ struct ActivatedBcvk { /// Spawn bcvk with socket activation and return a zlink connection. /// -/// Creates a Unix socketpair and spawns bcvk directly with socket-activation -/// env vars (`LISTEN_FDS`, `LISTEN_PID`, `LISTEN_FDNAMES`) set via -/// `libc::setenv` in a `pre_exec` hook. We avoid `Command::env()` because -/// Rust's std overwrites the global `environ` pointer *after* `pre_exec` -/// callbacks run, which would discard our `LISTEN_PID` value (which must -/// equal `getpid()` for libsystemd's `receive_descriptors` to accept it). -/// See `library/std/src/sys/process/unix/unix.rs` in rust-lang/rust. +/// Creates a Unix socketpair and spawns bcvk with socket-activation +/// fd passing using `CmdFds::new_systemd_fds()`. This atomically places +/// the socket at fd 3 and sets `LISTEN_PID`, `LISTEN_FDS`, and +/// `LISTEN_FDNAMES` in the child process -- replacing the previous manual +/// `pre_exec` + `libc::setenv` approach. /// /// The child process is bound to the calling thread via /// `lifecycle_bind_to_parent_thread`, so it is automatically killed when the @@ -207,21 +204,10 @@ fn activated_connection() -> anyhow::Result { let theirs_fd: Arc = Arc::new(theirs.into()); let mut cmd = Command::new(&bck); - // Do NOT use cmd.env() here -- it causes Rust's Command to build an - // envp array that replaces environ after our pre_exec setenv calls. - cmd.take_fd_n(theirs_fd, 3) - .lifecycle_bind_to_parent_thread(); - #[allow(unsafe_code)] - unsafe { - cmd.pre_exec(|| { - let pid = rustix::process::getpid(); - let pid_dec = rustix::path::DecInt::new(pid.as_raw_nonzero().get()); - libc::setenv(c"LISTEN_PID".as_ptr(), pid_dec.as_c_str().as_ptr(), 1); - libc::setenv(c"LISTEN_FDS".as_ptr(), c"1".as_ptr(), 1); - libc::setenv(c"LISTEN_FDNAMES".as_ptr(), c"varlink".as_ptr(), 1); - Ok(()) - }); - } + let fds = CmdFds::new_systemd_fds([(theirs_fd, SystemdFdName::new("varlink"))]); + // Do NOT use cmd.env() here -- it would cause Rust's Command to build + // an envp array that replaces environ after take_fds' setenv calls. + cmd.take_fds(fds).lifecycle_bind_to_parent_thread(); let _child = cmd.spawn()?; ours.set_nonblocking(true)?;