From 9e2c4c1ac51f37d827658fd4cdc5206227313f15 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Oct 2025 09:51:20 -0400 Subject: [PATCH 1/8] qemu: Always set --cache=never for virtiofsd Otherwise we are subject to hit FD exhaustion. See https://github.com/bootc-dev/bcvk/issues/65 Signed-off-by: Colin Walters --- crates/kit/src/qemu.rs | 16 +--------------- crates/kit/src/run_ephemeral.rs | 1 - 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/crates/kit/src/qemu.rs b/crates/kit/src/qemu.rs index e106406a1..8beee2e7e 100644 --- a/crates/kit/src/qemu.rs +++ b/crates/kit/src/qemu.rs @@ -872,8 +872,6 @@ pub struct VirtiofsConfig { pub socket_path: String, /// Host directory to share pub shared_dir: String, - /// Cache mode: always/auto/none - pub cache_mode: String, /// Sandbox: none/namespace/chroot pub sandbox: String, pub debug: bool, @@ -884,7 +882,6 @@ impl Default for VirtiofsConfig { Self { socket_path: "/run/inner-shared/virtiofs.sock".to_string(), shared_dir: "/run/source-image".to_string(), - cache_mode: "always".to_string(), sandbox: "none".to_string(), debug: false, } @@ -928,8 +925,7 @@ pub async fn spawn_virtiofsd_async(config: &VirtiofsConfig) -> Result Result<()> { } } - // Validate cache mode - let valid_cache_modes = ["none", "auto", "always"]; - if !valid_cache_modes.contains(&config.cache_mode.as_str()) { - return Err(eyre!( - "Invalid virtiofsd cache mode: '{}'. Valid options: {}", - config.cache_mode, - valid_cache_modes.join(", ") - )); - } - // Validate sandbox mode let valid_sandbox_modes = ["namespace", "chroot", "none"]; if !valid_sandbox_modes.contains(&config.sandbox.as_str()) { diff --git a/crates/kit/src/run_ephemeral.rs b/crates/kit/src/run_ephemeral.rs index aa0b75cba..9ad0bd8b6 100644 --- a/crates/kit/src/run_ephemeral.rs +++ b/crates/kit/src/run_ephemeral.rs @@ -838,7 +838,6 @@ pub(crate) async fn run_impl(opts: RunEphemeralOpts) -> Result<()> { let virtiofsd_config = qemu::VirtiofsConfig { socket_path: socket_path.clone(), shared_dir: source_path.to_string_lossy().to_string(), - cache_mode: "always".to_string(), sandbox: "none".to_string(), debug: false, }; From f57fab264a2f91fbc23bc86579ff1dd959ca6f32 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Oct 2025 10:28:14 -0400 Subject: [PATCH 2/8] virtiofs: Just hardcode sandbox mode Don't pretend we can configure it. Signed-off-by: Colin Walters --- crates/kit/src/qemu.rs | 9 +++------ crates/kit/src/run_ephemeral.rs | 1 - 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/crates/kit/src/qemu.rs b/crates/kit/src/qemu.rs index 8beee2e7e..42c7f00b5 100644 --- a/crates/kit/src/qemu.rs +++ b/crates/kit/src/qemu.rs @@ -865,15 +865,12 @@ mod tests { } /// VirtiofsD daemon configuration. -/// Cache modes: always(default)/auto/none. Sandbox: none(default)/namespace/chroot. #[derive(Debug, Clone)] pub struct VirtiofsConfig { /// Unix socket for QEMU communication pub socket_path: String, /// Host directory to share pub shared_dir: String, - /// Sandbox: none/namespace/chroot - pub sandbox: String, pub debug: bool, } @@ -882,7 +879,6 @@ impl Default for VirtiofsConfig { Self { socket_path: "/run/inner-shared/virtiofs.sock".to_string(), shared_dir: "/run/source-image".to_string(), - sandbox: "none".to_string(), debug: false, } } @@ -925,9 +921,10 @@ pub async fn spawn_virtiofsd_async(config: &VirtiofsConfig) -> Result Result<()> { let virtiofsd_config = qemu::VirtiofsConfig { socket_path: socket_path.clone(), shared_dir: source_path.to_string_lossy().to_string(), - sandbox: "none".to_string(), debug: false, }; additional_mounts.push((virtiofsd_config, tag.clone())); From deb0410bda21cee7384df6a432a893ca7e403bfc Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Oct 2025 10:13:49 -0400 Subject: [PATCH 3/8] qemu,run_ephemeral: Use Utf8PathBuf for virtiofsd paths Switch VirtiofsConfig to use Utf8PathBuf instead of String for socket_path and shared_dir fields. This provides better type safety for filesystem paths and matches the camino conventions used elsewhere in the codebase. Signed-off-by: Colin Walters --- crates/kit/src/qemu.rs | 43 +++++++++++++++------------------ crates/kit/src/run_ephemeral.rs | 6 ++--- 2 files changed, 22 insertions(+), 27 deletions(-) diff --git a/crates/kit/src/qemu.rs b/crates/kit/src/qemu.rs index 42c7f00b5..4b4724f24 100644 --- a/crates/kit/src/qemu.rs +++ b/crates/kit/src/qemu.rs @@ -11,6 +11,7 @@ use std::process::{Child, Command, Stdio}; use std::sync::Arc; use std::time::Duration; +use camino::Utf8PathBuf; use cap_std_ext::cmdext::CapStdExtCommandExt; use color_eyre::eyre::{eyre, Context}; use color_eyre::Result; @@ -117,7 +118,7 @@ pub enum BootMode { initramfs_path: String, kernel_cmdline: Vec, /// VirtIO-FS socket for root filesystem - virtiofs_socket: String, + virtiofs_socket: Utf8PathBuf, }, #[allow(dead_code)] DiskBoot { @@ -170,7 +171,7 @@ impl QemuConfig { vcpus: u32, kernel_path: String, initramfs_path: String, - virtiofs_socket: String, + virtiofs_socket: Utf8PathBuf, ) -> Self { Self { memory_mb, @@ -290,7 +291,7 @@ impl QemuConfig { pub fn add_virtiofs(&mut self, config: VirtiofsConfig, tag: &str) -> &mut Self { // Also add a corresponding mount so QEMU knows about it self.additional_mounts.push(VirtiofsMount { - socket_path: config.socket_path.clone(), + socket_path: config.socket_path.clone().into(), tag: tag.to_owned(), }); self.virtiofs_configs.push(config); @@ -799,7 +800,8 @@ impl RunningQemu { let process = spawn_virtiofsd_async(main_config).await?; virtiofsd_processes.push(process); // Wait for socket to be ready before proceeding - wait_for_virtiofsd_socket(&main_config.socket_path, Duration::from_secs(10)).await?; + wait_for_virtiofsd_socket(main_config.socket_path.as_str(), Duration::from_secs(10)) + .await?; } // Spawn additional virtiofsd processes @@ -809,8 +811,11 @@ impl RunningQemu { virtiofsd_processes.push(process); // Wait for socket to be ready before proceeding - wait_for_virtiofsd_socket(&virtiofs_config.socket_path, Duration::from_secs(10)) - .await?; + wait_for_virtiofsd_socket( + virtiofs_config.socket_path.as_str(), + Duration::from_secs(10), + ) + .await?; } // Spawn QEMU process with additional VSOCK credential if needed let qemu_process = spawn(&config, &creds, vsockdata)?; @@ -847,7 +852,7 @@ mod tests { 1, "/test/kernel".to_string(), "/test/initramfs".to_string(), - "/test/socket".to_string(), + "/test/socket".into(), ); config .add_virtio_serial_out("serial0", "/tmp/output.txt".to_string(), false) @@ -868,17 +873,17 @@ mod tests { #[derive(Debug, Clone)] pub struct VirtiofsConfig { /// Unix socket for QEMU communication - pub socket_path: String, + pub socket_path: Utf8PathBuf, /// Host directory to share - pub shared_dir: String, + pub shared_dir: Utf8PathBuf, pub debug: bool, } impl Default for VirtiofsConfig { fn default() -> Self { Self { - socket_path: "/run/inner-shared/virtiofs.sock".to_string(), - shared_dir: "/run/source-image".to_string(), + socket_path: "/run/inner-shared/virtiofs.sock".into(), + shared_dir: "/run/source-image".into(), debug: false, } } @@ -918,9 +923,9 @@ pub async fn spawn_virtiofsd_async(config: &VirtiofsConfig) -> Result Result<()> { } // Validate socket path - if config.socket_path.is_empty() { + if config.socket_path.as_str().is_empty() { return Err(eyre!("Virtiofsd socket path cannot be empty")); } @@ -1023,15 +1028,5 @@ pub fn validate_virtiofsd_config(config: &VirtiofsConfig) -> Result<()> { } } - // Validate sandbox mode - let valid_sandbox_modes = ["namespace", "chroot", "none"]; - if !valid_sandbox_modes.contains(&config.sandbox.as_str()) { - return Err(eyre!( - "Invalid virtiofsd sandbox mode: '{}'. Valid options: {}", - config.sandbox, - valid_sandbox_modes.join(", ") - )); - } - Ok(()) } diff --git a/crates/kit/src/run_ephemeral.rs b/crates/kit/src/run_ephemeral.rs index adace1322..fb7116ee5 100644 --- a/crates/kit/src/run_ephemeral.rs +++ b/crates/kit/src/run_ephemeral.rs @@ -817,7 +817,7 @@ pub(crate) async fn run_impl(opts: RunEphemeralOpts) -> Result<()> { let entry = entry?; let mount_name = entry.file_name(); let mount_name_str = mount_name.to_string_lossy(); - let source_path = entry.path(); + let source_path: Utf8PathBuf = entry.path().try_into()?; let mount_path = format!("/run/host-mounts/{}", mount_name_str); // Check if this directory is mounted as read-only @@ -836,8 +836,8 @@ pub(crate) async fn run_impl(opts: RunEphemeralOpts) -> Result<()> { // Store virtiofsd config to be spawned later by QEMU let virtiofsd_config = qemu::VirtiofsConfig { - socket_path: socket_path.clone(), - shared_dir: source_path.to_string_lossy().to_string(), + socket_path: socket_path.clone().into(), + shared_dir: source_path, debug: false, }; additional_mounts.push((virtiofsd_config, tag.clone())); From d3562784bf3a62a99cb31fc0d2434cd78ba79c64 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Oct 2025 10:21:04 -0400 Subject: [PATCH 4/8] qemu: Mount virtiofs as readonly Add --readonly flag to virtiofsd and rootflags=ro to the kernel command line. Since we use systemd.volatile=overlay, the guest has a writable overlay on top, so there's no need for virtiofsd to support writes. This provides an additional layer of safety. Signed-off-by: Colin Walters --- crates/kit/src/qemu.rs | 29 +++++++++++++++++++++++++++++ crates/kit/src/run_ephemeral.rs | 1 + 2 files changed, 30 insertions(+) diff --git a/crates/kit/src/qemu.rs b/crates/kit/src/qemu.rs index 4b4724f24..c9f27a5ec 100644 --- a/crates/kit/src/qemu.rs +++ b/crates/kit/src/qemu.rs @@ -889,6 +889,23 @@ impl Default for VirtiofsConfig { } } +/// Check if virtiofsd supports the --readonly flag. +async fn virtiofsd_supports_readonly(virtiofsd_binary: &str) -> bool { + let output = tokio::process::Command::new(virtiofsd_binary) + .arg("--help") + .output() + .await; + + match output { + Ok(output) => { + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + stdout.contains("--readonly") || stderr.contains("--readonly") + } + Err(_) => false, + } +} + /// Spawn virtiofsd daemon process as tokio::process::Child. /// Searches for binary in /usr/libexec, /usr/bin, /usr/local/bin. /// Creates socket directory if needed, redirects output unless debug=true. @@ -913,6 +930,13 @@ pub async fn spawn_virtiofsd_async(config: &VirtiofsConfig) -> Result Result Date: Tue, 14 Oct 2025 11:46:17 -0400 Subject: [PATCH 5/8] DNM only test ephemeral Signed-off-by: Colin Walters --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index cab7a35d9..012e611ea 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -48,7 +48,7 @@ jobs: run: just unit - name: Run integration tests - run: just test-integration + run: just test-integration ephemeral - name: Upload junit XML if: always() From ce8494445c097b001f21cddee5b469bd927a9512 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Oct 2025 12:35:24 -0400 Subject: [PATCH 6/8] qemu: Gather virtiofsd output Signed-off-by: Colin Walters --- crates/kit/src/main.rs | 9 +++- crates/kit/src/qemu.rs | 89 +++++++++++++++++++-------------- crates/kit/src/run_ephemeral.rs | 17 +++++-- 3 files changed, 74 insertions(+), 41 deletions(-) diff --git a/crates/kit/src/main.rs b/crates/kit/src/main.rs index c573b19f7..7e88a700a 100644 --- a/crates/kit/src/main.rs +++ b/crates/kit/src/main.rs @@ -220,7 +220,12 @@ fn main() -> Result<(), Report> { } Commands::ContainerEntrypoint(opts) => { // Create a tokio runtime for async container entrypoint operations - rt.block_on(container_entrypoint::run(opts))?; + rt.block_on(async move { + let r = container_entrypoint::run(opts).await; + tracing::debug!("Container entrypoint done"); + r + })?; + tracing::trace!("Exiting runtime"); } Commands::DebugInternals(opts) => match opts.command { DebugInternalsCmds::OpenTree { path } => { @@ -243,5 +248,7 @@ fn main() -> Result<(), Report> { }, } tracing::debug!("exiting"); + // Ensure we don't block on any spawned tasks + rt.shutdown_background(); std::process::exit(0) } diff --git a/crates/kit/src/qemu.rs b/crates/kit/src/qemu.rs index c9f27a5ec..b7ceed4e1 100644 --- a/crates/kit/src/qemu.rs +++ b/crates/kit/src/qemu.rs @@ -4,10 +4,12 @@ //! automatic process cleanup, and SMBIOS credential injection. use std::fs::{File, OpenOptions}; +use std::future::Future; use std::io::ErrorKind; use std::os::fd::{AsRawFd as _, OwnedFd}; use std::os::unix::process::CommandExt as _; -use std::process::{Child, Command, Stdio}; +use std::pin::Pin; +use std::process::{Child, Command, Output, Stdio}; use std::sync::Arc; use std::time::Duration; @@ -684,7 +686,7 @@ struct VsockCopier { pub struct RunningQemu { pub qemu_process: Child, - pub virtiofsd_processes: Vec, + pub virtiofsd_processes: Vec>>>>, sd_notification: Option, } @@ -792,30 +794,54 @@ impl RunningQemu { .unwrap_or_default(); // Spawn all virtiofsd processes first - let mut virtiofsd_processes = Vec::new(); - - // Spawn main virtiofsd if configured - if let Some(ref main_config) = config.main_virtiofs_config { - debug!("Spawning main virtiofsd for: {:?}", main_config.socket_path); - let process = spawn_virtiofsd_async(main_config).await?; - virtiofsd_processes.push(process); - // Wait for socket to be ready before proceeding - wait_for_virtiofsd_socket(main_config.socket_path.as_str(), Duration::from_secs(10)) - .await?; + let mut awaiting_virtiofsd = Vec::new(); + let virtiofsd_configs = config + .main_virtiofs_config + .iter() + .chain(config.virtiofs_configs.iter()); + for config in virtiofsd_configs { + let process = spawn_virtiofsd_async(config).await?; + awaiting_virtiofsd.push((process, config.socket_path.clone())); } - // Spawn additional virtiofsd processes - for virtiofs_config in &config.virtiofs_configs { - debug!("Spawning virtiofsd for: {:?}", virtiofs_config.socket_path); - let process = spawn_virtiofsd_async(virtiofs_config).await?; - virtiofsd_processes.push(process); - - // Wait for socket to be ready before proceeding - wait_for_virtiofsd_socket( - virtiofs_config.socket_path.as_str(), - Duration::from_secs(10), - ) - .await?; + // Wait for all virtiofsd to be ready + let mut virtiofsd_processes = Vec::new(); + while let Some((proc, socket_path)) = awaiting_virtiofsd.pop() { + let socket_path = &socket_path; + let query_exists = async move { + loop { + if socket_path.exists() { + break; + } + tokio::time::sleep(Duration::from_millis(100)).await; + } + }; + tokio::pin!(query_exists); + let timeout_val = Duration::from_secs(60); + let timeout = tokio::time::sleep(timeout_val); + tokio::pin!(timeout); + debug!("Waiting for socket at {socket_path}"); + let mut output: Pin>>> = + Box::pin(proc.wait_with_output()); + tokio::select! { + output = &mut output => { + tracing::trace!("virtiofsd exited"); + let output = output?; + let status = output.status; + let stderr = String::from_utf8_lossy(&output.stderr); + tracing::trace!("returnign spawn error"); + return Err(eyre!( + "virtiofsd failed to start for socket {socket_path}\nExit status: {status:?}\nOutput: {stderr}" + )); + } + _ = timeout => { + return Err(eyre!("timed out waiting for virtiofsd socket {} to be created (waited {timeout_val:?})", socket_path)); + } + _ = query_exists => { + } + } + virtiofsd_processes.push(output); + tracing::debug!("virtiofsd socket created: {socket_path}"); } // Spawn QEMU process with additional VSOCK credential if needed let qemu_process = spawn(&config, &creds, vsockdata)?; @@ -827,11 +853,6 @@ impl RunningQemu { }) } - /// Add a virtiofsd process to be managed by this QEMU instance - pub fn add_virtiofsd_process(&mut self, process: tokio::process::Child) { - self.virtiofsd_processes.push(process); - } - /// Wait for QEMU process to exit pub async fn wait(&mut self) -> Result { let r = self.qemu_process.wait()?; @@ -965,14 +986,8 @@ pub async fn spawn_virtiofsd_async(config: &VirtiofsConfig) -> Result r, + Err(e) => { + tracing::trace!("Aborting status writer"); + if let Some(writer) = status_writer_task { + writer.abort(); + } + return Err(e); + } + }; // Handle execute command output streaming if needed if let Some((exec_pipefd, status_pipefd)) = exec_pipes { @@ -1208,6 +1218,7 @@ Options= } } else { // Wait for QEMU to complete + tracing::debug!("Waiting for qemu exit"); let exit_status = qemu.wait().await?; if !exit_status.success() { return Err(eyre!("QEMU exited with non-zero status: {}", exit_status)); From ef62b6e7fda39c999e7cdd2d8c98657a3df7244a Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Oct 2025 13:35:09 -0400 Subject: [PATCH 7/8] qemu: Move vsock listening after virtiofsd Signed-off-by: Colin Walters --- crates/kit/src/qemu.rs | 100 ++++++++++++++++++++--------------------- 1 file changed, 50 insertions(+), 50 deletions(-) diff --git a/crates/kit/src/qemu.rs b/crates/kit/src/qemu.rs index b7ceed4e1..3221c5232 100644 --- a/crates/kit/src/qemu.rs +++ b/crates/kit/src/qemu.rs @@ -693,6 +693,56 @@ pub struct RunningQemu { impl RunningQemu { /// Spawn QEMU pub async fn spawn(mut config: QemuConfig) -> Result { + // Spawn all virtiofsd processes first + let mut awaiting_virtiofsd = Vec::new(); + let virtiofsd_configs = config + .main_virtiofs_config + .iter() + .chain(config.virtiofs_configs.iter()); + for config in virtiofsd_configs { + let process = spawn_virtiofsd_async(config).await?; + awaiting_virtiofsd.push((process, config.socket_path.clone())); + } + + // Wait for all virtiofsd to be ready + let mut virtiofsd_processes = Vec::new(); + while let Some((proc, socket_path)) = awaiting_virtiofsd.pop() { + let socket_path = &socket_path; + let query_exists = async move { + loop { + if socket_path.exists() { + break; + } + tokio::time::sleep(Duration::from_millis(100)).await; + } + }; + tokio::pin!(query_exists); + let timeout_val = Duration::from_secs(60); + let timeout = tokio::time::sleep(timeout_val); + tokio::pin!(timeout); + debug!("Waiting for socket at {socket_path}"); + let mut output: Pin>>> = + Box::pin(proc.wait_with_output()); + tokio::select! { + output = &mut output => { + tracing::trace!("virtiofsd exited"); + let output = output?; + let status = output.status; + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(eyre!( + "virtiofsd failed to start for socket {socket_path}\nExit status: {status:?}\nOutput: {stderr}" + )); + } + _ = timeout => { + return Err(eyre!("timed out waiting for virtiofsd socket {} to be created (waited {timeout_val:?})", socket_path)); + } + _ = query_exists => { + } + } + virtiofsd_processes.push(output); + tracing::debug!("virtiofsd socket created: {socket_path}"); + } + let vsockdata = if let Some(vhost_fd) = config.vhost_fd.take() { // Get a unique guest CID using dynamic allocation // If /dev/vhost-vsock is not available, fall back to disabled vsock @@ -793,56 +843,6 @@ impl RunningQemu { }) .unwrap_or_default(); - // Spawn all virtiofsd processes first - let mut awaiting_virtiofsd = Vec::new(); - let virtiofsd_configs = config - .main_virtiofs_config - .iter() - .chain(config.virtiofs_configs.iter()); - for config in virtiofsd_configs { - let process = spawn_virtiofsd_async(config).await?; - awaiting_virtiofsd.push((process, config.socket_path.clone())); - } - - // Wait for all virtiofsd to be ready - let mut virtiofsd_processes = Vec::new(); - while let Some((proc, socket_path)) = awaiting_virtiofsd.pop() { - let socket_path = &socket_path; - let query_exists = async move { - loop { - if socket_path.exists() { - break; - } - tokio::time::sleep(Duration::from_millis(100)).await; - } - }; - tokio::pin!(query_exists); - let timeout_val = Duration::from_secs(60); - let timeout = tokio::time::sleep(timeout_val); - tokio::pin!(timeout); - debug!("Waiting for socket at {socket_path}"); - let mut output: Pin>>> = - Box::pin(proc.wait_with_output()); - tokio::select! { - output = &mut output => { - tracing::trace!("virtiofsd exited"); - let output = output?; - let status = output.status; - let stderr = String::from_utf8_lossy(&output.stderr); - tracing::trace!("returnign spawn error"); - return Err(eyre!( - "virtiofsd failed to start for socket {socket_path}\nExit status: {status:?}\nOutput: {stderr}" - )); - } - _ = timeout => { - return Err(eyre!("timed out waiting for virtiofsd socket {} to be created (waited {timeout_val:?})", socket_path)); - } - _ = query_exists => { - } - } - virtiofsd_processes.push(output); - tracing::debug!("virtiofsd socket created: {socket_path}"); - } // Spawn QEMU process with additional VSOCK credential if needed let qemu_process = spawn(&config, &creds, vsockdata)?; From 7ccf47ed46da983c10d53697dda460fe19e335d2 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Oct 2025 14:21:43 -0400 Subject: [PATCH 8/8] DNM upterm --- .github/workflows/main.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 012e611ea..aa199cf2b 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -47,6 +47,11 @@ jobs: - name: Run unit tests run: just unit + - name: Setup upterm session + uses: owenthereal/action-upterm@v1 + with: + limit-access-to-actor: true # Restrict to the user who triggered the workflow + - name: Run integration tests run: just test-integration ephemeral