From f16bc8a21f5c59e0f638c890290e7d15d57564c2 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Oct 2025 09:51:20 -0400 Subject: [PATCH 01/11] 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 6a5dc3ad71d03f03cb6301ff33d162eaa9ace129 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Oct 2025 10:28:14 -0400 Subject: [PATCH 02/11] qemu: Just hardcode virtiofs 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 180398abffdd2619e48a43af4a747efcd7acdc80 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Oct 2025 10:13:49 -0400 Subject: [PATCH 03/11] 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 0ec53f17be97e72a6afed607751ec947a42391bd Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Oct 2025 10:21:04 -0400 Subject: [PATCH 04/11] 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 12:35:24 -0400 Subject: [PATCH 05/11] 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 366f4e3a2006e8b333d2868b25e804f6f7b73dd6 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Oct 2025 13:35:09 -0400 Subject: [PATCH 06/11] qemu: Move vsock listening after virtiofsd Otherwise we can hang at runtime shutdown because we don't terminate the vsock listener thread. 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 ed8cafb7259d6516f561eb8acb23127f86346605 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Oct 2025 14:55:58 -0400 Subject: [PATCH 07/11] Remove useless debugging option Noticed this when doing to add a new one. Signed-off-by: Colin Walters --- crates/kit/src/run_ephemeral.rs | 15 --------------- crates/kit/src/to_disk.rs | 1 - docs/src/man/bcvk-ephemeral-run-ssh.md | 4 ---- docs/src/man/bcvk-ephemeral-run.md | 4 ---- 4 files changed, 24 deletions(-) diff --git a/crates/kit/src/run_ephemeral.rs b/crates/kit/src/run_ephemeral.rs index eebdc435b..fef46b137 100644 --- a/crates/kit/src/run_ephemeral.rs +++ b/crates/kit/src/run_ephemeral.rs @@ -250,12 +250,6 @@ pub struct RunEphemeralOpts { )] pub systemd_units_dir: Option, - #[clap( - long = "log-cmdline", - help = "Log full podman command before execution" - )] - pub log_cmdline: bool, - #[clap( long = "bind-storage-ro", help = "Mount host container storage (RO) at /run/virtiofs-mnt-hoststorage" @@ -508,15 +502,6 @@ fn prepare_run_command_with_temp( cmd.args([&opts.image, ENTRYPOINT]); - // Log the full command line if requested - if opts.log_cmdline { - let args: Vec = cmd - .get_args() - .map(|arg| arg.to_string_lossy().to_string()) - .collect(); - debug!("Executing: podman {}", args.join(" ")); - } - Ok((cmd, td)) } diff --git a/crates/kit/src/to_disk.rs b/crates/kit/src/to_disk.rs index c18aa2c45..e0614f762 100644 --- a/crates/kit/src/to_disk.rs +++ b/crates/kit/src/to_disk.rs @@ -414,7 +414,6 @@ pub fn run(opts: ToDiskOpts) -> Result<()> { bind_mounts: Vec::new(), // No additional bind mounts needed ro_bind_mounts: Vec::new(), // No additional ro bind mounts needed systemd_units_dir: None, // No custom systemd units - log_cmdline: opts.additional.common.debug, // Log kernel command line if debug bind_storage_ro: true, // Mount host container storage read-only mount_disk_files: vec![format!( "{}:output:{}", diff --git a/docs/src/man/bcvk-ephemeral-run-ssh.md b/docs/src/man/bcvk-ephemeral-run-ssh.md index 15fb4e275..2ae548d51 100644 --- a/docs/src/man/bcvk-ephemeral-run-ssh.md +++ b/docs/src/man/bcvk-ephemeral-run-ssh.md @@ -97,10 +97,6 @@ Run ephemeral VM and SSH into it Directory with systemd units to inject (expects system/ subdirectory) -**--log-cmdline** - - Log full podman command before execution - **--bind-storage-ro** Mount host container storage (RO) at /run/virtiofs-mnt-hoststorage diff --git a/docs/src/man/bcvk-ephemeral-run.md b/docs/src/man/bcvk-ephemeral-run.md index f87d6e7bb..97540fd46 100644 --- a/docs/src/man/bcvk-ephemeral-run.md +++ b/docs/src/man/bcvk-ephemeral-run.md @@ -123,10 +123,6 @@ This design allows bcvk to provide VM-like isolation and boot behavior while lev Directory with systemd units to inject (expects system/ subdirectory) -**--log-cmdline** - - Log full podman command before execution - **--bind-storage-ro** Mount host container storage (RO) at /run/virtiofs-mnt-hoststorage From 58b5bcc737ed9092fd2c9f7598a91db714637d53 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Oct 2025 14:55:58 -0400 Subject: [PATCH 08/11] ci: Consistently set libvirt session Signed-off-by: Colin Walters --- .github/workflows/main.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index cab7a35d9..6b48e2422 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -7,6 +7,12 @@ on: branches: [ main ] workflow_dispatch: +env: + # Something seems to be setting this in the default GHA runners, which breaks bcvk + # as the default runner user doesn't have access + LIBVIRT_DEFAULT_URI: "qemu:///session" + + jobs: build-and-test: runs-on: ubuntu-24.04 From d0893e5887ce710389dbd19d7200ab4c900e915b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Oct 2025 15:55:57 -0400 Subject: [PATCH 09/11] Increase and centralize ssh timeout 60s may be too short for heavily loaded GHA runners. Signed-off-by: Colin Walters --- crates/kit/src/ephemeral.rs | 6 +----- crates/kit/src/run_ephemeral_ssh.rs | 19 +++++++++++-------- crates/kit/src/to_disk.rs | 6 +----- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/crates/kit/src/ephemeral.rs b/crates/kit/src/ephemeral.rs index c386eef6c..3bd58def2 100644 --- a/crates/kit/src/ephemeral.rs +++ b/crates/kit/src/ephemeral.rs @@ -103,11 +103,7 @@ impl EphemeralCommands { // Create progress bar if stderr is a terminal let progress_bar = crate::boot_progress::create_boot_progress_bar(); - run_ephemeral_ssh::wait_for_ssh_ready( - &opts.container_name, - std::time::Duration::from_secs(60), - progress_bar, - )?; + run_ephemeral_ssh::wait_for_ssh_ready(&opts.container_name, None, progress_bar)?; ssh::connect_via_container(&opts.container_name, opts.args) } diff --git a/crates/kit/src/run_ephemeral_ssh.rs b/crates/kit/src/run_ephemeral_ssh.rs index ce2280c9e..295c67e4c 100644 --- a/crates/kit/src/run_ephemeral_ssh.rs +++ b/crates/kit/src/run_ephemeral_ssh.rs @@ -11,6 +11,9 @@ use crate::run_ephemeral::{run_detached, RunEphemeralOpts}; use crate::ssh; use crate::supervisor_status::{SupervisorState, SupervisorStatus}; +/// Timeout waiting for connection +pub(crate) const SSH_TIMEOUT: std::time::Duration = const { Duration::from_secs(240) }; + #[derive(Debug, clap::Parser, serde::Serialize, serde::Deserialize)] pub struct RunEphemeralSshOpts { #[command(flatten)] @@ -28,9 +31,11 @@ pub struct RunEphemeralSshOpts { /// Returns Ok(false) if we don't support systemd status notifications. pub fn wait_for_vm_ssh( container_name: &str, - timeout: Duration, + timeout: Option, progress: ProgressBar, ) -> Result<(bool, ProgressBar)> { + let timeout = timeout.unwrap_or(SSH_TIMEOUT); + debug!( "Waiting for VM readiness via supervisor status file (timeout: {}s)...", timeout.as_secs() @@ -103,15 +108,13 @@ pub fn wait_for_vm_ssh( /// This is used as a fallback when systemd notification is not available. pub fn wait_for_ssh_ready( container_name: &str, - timeout: Duration, + timeout: Option, progress: ProgressBar, ) -> Result { - let (_, progress) = wait_for_vm_ssh(container_name, timeout, progress)?; + let timeout = timeout.unwrap_or(SSH_TIMEOUT); + let (_, progress) = wait_for_vm_ssh(container_name, Some(timeout), progress)?; - debug!( - "Polling SSH connectivity (timeout: {}s)...", - timeout.as_secs() - ); + debug!("Polling SSH connectivity...",); let start_time = Instant::now(); // Use SSH options optimized for connectivity testing @@ -162,7 +165,7 @@ pub fn run_ephemeral_ssh(opts: RunEphemeralSshOpts) -> Result<()> { debug!("Using container ID: {}", container_name); let progress_bar = crate::boot_progress::create_boot_progress_bar(); - let progress_bar = wait_for_ssh_ready(&container_name, Duration::from_secs(60), progress_bar)?; + let progress_bar = wait_for_ssh_ready(&container_name, None, progress_bar)?; progress_bar.finish_and_clear(); // Execute SSH connection directly (no thread needed for this) diff --git a/crates/kit/src/to_disk.rs b/crates/kit/src/to_disk.rs index e0614f762..589a29ea5 100644 --- a/crates/kit/src/to_disk.rs +++ b/crates/kit/src/to_disk.rs @@ -432,11 +432,7 @@ pub fn run(opts: ToDiskOpts) -> Result<()> { let result = (|| -> Result<()> { // Wait for SSH to be ready let progress_bar = crate::boot_progress::create_boot_progress_bar(); - let progress_bar = wait_for_ssh_ready( - &container_id, - std::time::Duration::from_secs(60), - progress_bar, - )?; + let progress_bar = wait_for_ssh_ready(&container_id, None, progress_bar)?; progress_bar.finish_and_clear(); // Connect via SSH and execute the installation command From 8e64c2de30f71e4fd021f89e131fde75fb2ddf74 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Oct 2025 16:14:51 -0400 Subject: [PATCH 10/11] to-disk: Log ssh connection timeout Signed-off-by: Colin Walters --- crates/kit/src/run_ephemeral_ssh.rs | 6 +++--- crates/kit/src/to_disk.rs | 7 ++++++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/crates/kit/src/run_ephemeral_ssh.rs b/crates/kit/src/run_ephemeral_ssh.rs index 295c67e4c..85575ee30 100644 --- a/crates/kit/src/run_ephemeral_ssh.rs +++ b/crates/kit/src/run_ephemeral_ssh.rs @@ -110,7 +110,7 @@ pub fn wait_for_ssh_ready( container_name: &str, timeout: Option, progress: ProgressBar, -) -> Result { +) -> Result<(std::time::Duration, ProgressBar)> { let timeout = timeout.unwrap_or(SSH_TIMEOUT); let (_, progress) = wait_for_vm_ssh(container_name, Some(timeout), progress)?; @@ -135,7 +135,7 @@ pub fn wait_for_ssh_ready( if let Ok(exit_status) = status { if exit_status.success() { debug!("SSH connection successful, VM is ready"); - return Ok(progress); + return Ok((start_time.elapsed(), progress)); } } @@ -165,7 +165,7 @@ pub fn run_ephemeral_ssh(opts: RunEphemeralSshOpts) -> Result<()> { debug!("Using container ID: {}", container_name); let progress_bar = crate::boot_progress::create_boot_progress_bar(); - let progress_bar = wait_for_ssh_ready(&container_name, None, progress_bar)?; + let (_duration, progress_bar) = wait_for_ssh_ready(&container_name, None, progress_bar)?; progress_bar.finish_and_clear(); // Execute SSH connection directly (no thread needed for this) diff --git a/crates/kit/src/to_disk.rs b/crates/kit/src/to_disk.rs index 589a29ea5..0d11c2c25 100644 --- a/crates/kit/src/to_disk.rs +++ b/crates/kit/src/to_disk.rs @@ -84,6 +84,7 @@ use camino::Utf8PathBuf; use clap::{Parser, ValueEnum}; use color_eyre::eyre::{eyre, Context}; use color_eyre::Result; +use indicatif::HumanDuration; use indoc::indoc; use tracing::debug; @@ -432,8 +433,12 @@ pub fn run(opts: ToDiskOpts) -> Result<()> { let result = (|| -> Result<()> { // Wait for SSH to be ready let progress_bar = crate::boot_progress::create_boot_progress_bar(); - let progress_bar = wait_for_ssh_ready(&container_id, None, progress_bar)?; + let (duration, progress_bar) = wait_for_ssh_ready(&container_id, None, progress_bar)?; progress_bar.finish_and_clear(); + println!( + "Connected ({} elapsed), beginning installation...", + HumanDuration(duration) + ); // Connect via SSH and execute the installation command debug!( From ae94c2d7d65d866365211f4d94c863f1a81e8743 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Oct 2025 16:41:57 -0400 Subject: [PATCH 11/11] tests: Bump nextest timeout, remove nested timeouts Having a slower virtiofsd meant we are now more likely to hit timeouts. Drop the nested timeouts in the tests, relying fully on the nextest timeout. Signed-off-by: Colin Walters --- .config/nextest.toml | 3 +- crates/integration-tests/src/main.rs | 13 + .../src/tests/libvirt_base_disks.rs | 115 +++--- .../src/tests/libvirt_verb.rs | 335 +++++++----------- .../src/tests/mount_feature.rs | 101 +++--- 5 files changed, 235 insertions(+), 332 deletions(-) diff --git a/.config/nextest.toml b/.config/nextest.toml index 6bbde459c..66cbf4898 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -20,7 +20,8 @@ status-level = "pass" # Profile for integration tests - run with limited parallelism due to QEMU/KVM resources [profile.integration] test-threads = 2 -slow-timeout = { period = "500s", terminate-after = 60 } +# Full 20 minutes for a timeout by default, since GHA runners can be really slow +slow-timeout = { period = "1200s", terminate-after = 60 } fail-fast = false failure-output = "immediate" success-output = "never" diff --git a/crates/integration-tests/src/main.rs b/crates/integration-tests/src/main.rs index 033c0f820..55d355c6d 100644 --- a/crates/integration-tests/src/main.rs +++ b/crates/integration-tests/src/main.rs @@ -116,6 +116,19 @@ pub(crate) fn run_bcvk(args: &[&str]) -> std::io::Result { run_command(&bck, args) } +/// Run the bcvk command with inherited stdout/stderr (no capture) +/// Use this when you just need to verify the command succeeded without checking output +pub(crate) fn run_bcvk_nocapture(args: &[&str]) -> std::io::Result<()> { + let bck = get_bck_command().expect("Failed to get bcvk command"); + let status = std::process::Command::new(&bck).args(args).status()?; + assert!( + status.success(), + "bcvk command failed with args: {:?}", + args + ); + Ok(()) +} + fn test_images_list() -> Result<()> { println!("Running test: bcvk images list --json"); diff --git a/crates/integration-tests/src/tests/libvirt_base_disks.rs b/crates/integration-tests/src/tests/libvirt_base_disks.rs index 47b9173c9..c7dd9dde0 100644 --- a/crates/integration-tests/src/tests/libvirt_base_disks.rs +++ b/crates/integration-tests/src/tests/libvirt_base_disks.rs @@ -8,11 +8,10 @@ use std::process::Command; -use crate::{get_bck_command, get_test_image}; +use crate::{get_bck_command, get_test_image, run_bcvk}; /// Test that base disk is created and reused for multiple VMs pub fn test_base_disk_creation_and_reuse() { - let bck = get_bck_command().unwrap(); let test_image = get_test_image(); // Generate unique names for test VMs @@ -33,74 +32,60 @@ pub fn test_base_disk_creation_and_reuse() { // Create first VM - this should create a new base disk println!("Creating first VM (should create base disk)..."); - let vm1_output = Command::new("timeout") - .args([ - "300s", - &bck, - "libvirt", - "run", - "--name", - &vm1_name, - "--filesystem", - "ext4", - &test_image, - ]) - .output() - .expect("Failed to create first VM"); - - let vm1_stdout = String::from_utf8_lossy(&vm1_output.stdout); - let vm1_stderr = String::from_utf8_lossy(&vm1_output.stderr); - - println!("VM1 stdout: {}", vm1_stdout); - println!("VM1 stderr: {}", vm1_stderr); - - if !vm1_output.status.success() { + let vm1_output = run_bcvk(&[ + "libvirt", + "run", + "--name", + &vm1_name, + "--filesystem", + "ext4", + &test_image, + ]) + .expect("Failed to create first VM"); + + println!("VM1 stdout: {}", vm1_output.stdout); + println!("VM1 stderr: {}", vm1_output.stderr); + + if !vm1_output.success() { cleanup_domain(&vm1_name); cleanup_domain(&vm2_name); - panic!("Failed to create first VM: {}", vm1_stderr); + panic!("Failed to create first VM: {}", vm1_output.stderr); } // Verify base disk was created assert!( - vm1_stdout.contains("Using base disk") || vm1_stdout.contains("base disk"), + vm1_output.stdout.contains("Using base disk") || vm1_output.stdout.contains("base disk"), "Should mention base disk creation" ); // Create second VM - this should reuse the base disk println!("Creating second VM (should reuse base disk)..."); - let vm2_output = Command::new("timeout") - .args([ - "300s", - &bck, - "libvirt", - "run", - "--name", - &vm2_name, - "--filesystem", - "ext4", - &test_image, - ]) - .output() - .expect("Failed to create second VM"); - - let vm2_stdout = String::from_utf8_lossy(&vm2_output.stdout); - let vm2_stderr = String::from_utf8_lossy(&vm2_output.stderr); - - println!("VM2 stdout: {}", vm2_stdout); - println!("VM2 stderr: {}", vm2_stderr); + let vm2_output = run_bcvk(&[ + "libvirt", + "run", + "--name", + &vm2_name, + "--filesystem", + "ext4", + &test_image, + ]) + .expect("Failed to create second VM"); + + println!("VM2 stdout: {}", vm2_output.stdout); + println!("VM2 stderr: {}", vm2_output.stderr); // Cleanup before assertions cleanup_domain(&vm1_name); cleanup_domain(&vm2_name); - if !vm2_output.status.success() { - panic!("Failed to create second VM: {}", vm2_stderr); + if !vm2_output.success() { + panic!("Failed to create second VM: {}", vm2_output.stderr); } // Verify base disk was reused (should be faster and mention using existing) assert!( - vm2_stdout.contains("Using base disk") || vm2_stdout.contains("base disk"), + vm2_output.stdout.contains("Using base disk") || vm2_output.stdout.contains("base disk"), "Should mention using base disk" ); @@ -183,7 +168,6 @@ pub fn test_base_disks_prune_dry_run() { /// Test that VM disks reference base disks correctly pub fn test_vm_disk_references_base() { - let bck = get_bck_command().unwrap(); let test_image = get_test_image(); let timestamp = std::time::SystemTime::now() @@ -197,26 +181,21 @@ pub fn test_vm_disk_references_base() { cleanup_domain(&vm_name); // Create VM - let output = Command::new("timeout") - .args([ - "300s", - &bck, - "libvirt", - "run", - "--name", - &vm_name, - "--filesystem", - "ext4", - &test_image, - ]) - .output() - .expect("Failed to create VM"); - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); + let output = run_bcvk(&[ + "libvirt", + "run", + "--name", + &vm_name, + "--filesystem", + "ext4", + &test_image, + ]) + .expect("Failed to create VM"); + + if !output.success() { cleanup_domain(&vm_name); - panic!("Failed to create VM: {}", stderr); + panic!("Failed to create VM: {}", output.stderr); } // Get VM disk path from domain XML diff --git a/crates/integration-tests/src/tests/libvirt_verb.rs b/crates/integration-tests/src/tests/libvirt_verb.rs index 96b56df81..cf164ffe1 100644 --- a/crates/integration-tests/src/tests/libvirt_verb.rs +++ b/crates/integration-tests/src/tests/libvirt_verb.rs @@ -9,7 +9,9 @@ use std::process::Command; -use crate::{get_bck_command, get_test_image, LIBVIRT_INTEGRATION_TEST_LABEL}; +use crate::{ + get_bck_command, get_test_image, run_bcvk, run_bcvk_nocapture, LIBVIRT_INTEGRATION_TEST_LABEL, +}; /// Test libvirt list functionality (lists domains) pub fn test_libvirt_list_functionality() { @@ -189,7 +191,6 @@ pub fn test_libvirt_ssh_integration() { /// Test full libvirt run + SSH workflow like run_ephemeral SSH tests pub fn test_libvirt_run_ssh_full_workflow() { - let bck = get_bck_command().unwrap(); let test_image = get_test_image(); // Generate unique domain name for this test @@ -207,42 +208,30 @@ pub fn test_libvirt_run_ssh_full_workflow() { ); // Cleanup any existing domain with this name - let _ = Command::new("virsh") - .args(&["destroy", &domain_name]) - .output(); - let _ = Command::new(&bck) - .args(&["libvirt", "rm", &domain_name, "--force", "--stop"]) - .output(); + cleanup_domain(&domain_name); // Create domain with SSH key generation println!("Creating libvirt domain with SSH key injection..."); - let create_output = Command::new("timeout") - .args([ - "300s", // 5 minute timeout for domain creation - &bck, - "libvirt", - "run", - "--name", - &domain_name, - "--label", - LIBVIRT_INTEGRATION_TEST_LABEL, - "--filesystem", - "ext4", - &test_image, - ]) - .output() - .expect("Failed to run libvirt run with SSH"); - - let create_stdout = String::from_utf8_lossy(&create_output.stdout); - let create_stderr = String::from_utf8_lossy(&create_output.stderr); - - println!("Create stdout: {}", create_stdout); - println!("Create stderr: {}", create_stderr); - - if !create_output.status.success() { + let create_output = run_bcvk(&[ + "libvirt", + "run", + "--name", + &domain_name, + "--label", + LIBVIRT_INTEGRATION_TEST_LABEL, + "--filesystem", + "ext4", + &test_image, + ]) + .expect("Failed to run libvirt run with SSH"); + + println!("Create stdout: {}", create_output.stdout); + println!("Create stderr: {}", create_output.stderr); + + if !create_output.success() { cleanup_domain(&domain_name); - panic!("Failed to create domain with SSH: {}", create_stderr); + panic!("Failed to create domain with SSH: {}", create_output.stderr); } println!("Successfully created domain: {}", domain_name); @@ -253,39 +242,25 @@ pub fn test_libvirt_run_ssh_full_workflow() { // Test SSH connection with simple command println!("Testing SSH connection: echo 'hello world'"); - let ssh_output = Command::new("timeout") - .args([ - "60s", - &bck, - "libvirt", - "ssh", - &domain_name, - "--", - "echo", - "hello world", - ]) - .output() + let ssh_output = run_bcvk(&["libvirt", "ssh", &domain_name, "--", "echo", "hello world"]) .expect("Failed to run libvirt ssh command"); - let ssh_stdout = String::from_utf8_lossy(&ssh_output.stdout); - let ssh_stderr = String::from_utf8_lossy(&ssh_output.stderr); - - println!("SSH stdout: {}", ssh_stdout); - println!("SSH stderr: {}", ssh_stderr); + println!("SSH stdout: {}", ssh_output.stdout); + println!("SSH stderr: {}", ssh_output.stderr); // Cleanup domain before checking results cleanup_domain(&domain_name); // Check SSH results - if !ssh_output.status.success() { - panic!("SSH connection failed: {}", ssh_stderr); + if !ssh_output.success() { + panic!("SSH connection failed: {}", ssh_output.stderr); } // Verify we got the expected output assert!( - ssh_stdout.contains("hello world"), + ssh_output.stdout.contains("hello world"), "Expected 'hello world' in SSH output. Got: {}", - ssh_stdout + ssh_output.stdout ); println!("✓ Successfully executed 'echo hello world' via SSH"); @@ -319,7 +294,6 @@ fn cleanup_domain(domain_name: &str) { /// Wait for SSH to become available on a domain with a timeout fn wait_for_ssh_available( - bck: &str, domain_name: &str, timeout_secs: u64, ) -> Result<(), Box> { @@ -333,21 +307,10 @@ fn wait_for_ssh_available( loop { // Try a simple SSH command to test connectivity - let ssh_test = Command::new("timeout") - .args([ - "10s", // Short timeout for individual SSH attempts - bck, - "libvirt", - "ssh", - domain_name, - "--", - "echo", - "ssh-ready", - ]) - .output(); + let ssh_test = run_bcvk(&["libvirt", "ssh", domain_name, "--", "echo", "ssh-ready"]); match ssh_test { - Ok(output) if output.status.success() => { + Ok(output) if output.success() => { println!("✓ SSH is now available"); return Ok(()); } @@ -513,44 +476,32 @@ pub fn test_libvirt_bind_storage_ro() { println!("Testing --bind-storage-ro with domain: {}", domain_name); // Cleanup any existing domain with this name - let _ = Command::new("virsh") - .args(&["destroy", &domain_name]) - .output(); - let _ = Command::new(&bck) - .args(&["libvirt", "rm", &domain_name, "--force", "--stop"]) - .output(); + cleanup_domain(&domain_name); // Create domain with --bind-storage-ro flag println!("Creating libvirt domain with --bind-storage-ro..."); - let create_output = Command::new("timeout") - .args([ - "300s", // 5 minute timeout for domain creation - &bck, - "libvirt", - "run", - "--name", - &domain_name, - "--label", - LIBVIRT_INTEGRATION_TEST_LABEL, - "--bind-storage-ro", - "--filesystem", - "ext4", - &test_image, - ]) - .output() - .expect("Failed to run libvirt run with --bind-storage-ro"); - - let create_stdout = String::from_utf8_lossy(&create_output.stdout); - let create_stderr = String::from_utf8_lossy(&create_output.stderr); - - println!("Create stdout: {}", create_stdout); - println!("Create stderr: {}", create_stderr); - - if !create_output.status.success() { + let create_output = run_bcvk(&[ + "libvirt", + "run", + "--name", + &domain_name, + "--label", + LIBVIRT_INTEGRATION_TEST_LABEL, + "--bind-storage-ro", + "--filesystem", + "ext4", + &test_image, + ]) + .expect("Failed to run libvirt run with --bind-storage-ro"); + + println!("Create stdout: {}", create_output.stdout); + println!("Create stderr: {}", create_output.stderr); + + if !create_output.success() { cleanup_domain(&domain_name); panic!( "Failed to create domain with --bind-storage-ro: {}", - create_stderr + create_output.stderr ); } @@ -606,92 +557,75 @@ pub fn test_libvirt_bind_storage_ro() { println!("✓ hoststorage tag is present in filesystem configuration"); // Wait for VM to boot and SSH to become available - if let Err(e) = wait_for_ssh_available(&bck, &domain_name, 180) { + if let Err(e) = wait_for_ssh_available(&domain_name, 180) { cleanup_domain(&domain_name); panic!("Failed to establish SSH connection: {}", e); } // Create mount point and mount virtiofs filesystem println!("Creating mount point and mounting virtiofs filesystem..."); - let mount_setup = Command::new("timeout") - .args([ - "30s", - &bck, - "libvirt", - "ssh", - &domain_name, - "--", - "sudo", - "mkdir", - "-p", - "/run/virtiofs-mnt-hoststorage", - ]) - .output() - .expect("Failed to create mount point"); - - if !mount_setup.status.success() { - let stderr = String::from_utf8_lossy(&mount_setup.stderr); - println!("Warning: Failed to create mount point: {}", stderr); + let mount_setup = run_bcvk(&[ + "libvirt", + "ssh", + &domain_name, + "--", + "sudo", + "mkdir", + "-p", + "/run/virtiofs-mnt-hoststorage", + ]) + .expect("Failed to create mount point"); + + if !mount_setup.success() { + println!( + "Warning: Failed to create mount point: {}", + mount_setup.stderr + ); } - let mount_cmd = Command::new("timeout") - .args([ - "30s", - &bck, - "libvirt", - "ssh", - &domain_name, - "--", - "sudo", - "mount", - "-t", - "virtiofs", - "hoststorage", - "/run/virtiofs-mnt-hoststorage", - ]) - .output() - .expect("Failed to mount virtiofs"); - - if !mount_cmd.status.success() { + let mount_cmd = run_bcvk(&[ + "libvirt", + "ssh", + &domain_name, + "--", + "sudo", + "mount", + "-t", + "virtiofs", + "hoststorage", + "/run/virtiofs-mnt-hoststorage", + ]) + .expect("Failed to mount virtiofs"); + + if !mount_cmd.success() { cleanup_domain(&domain_name); - let stderr = String::from_utf8_lossy(&mount_cmd.stderr); - panic!("Failed to mount virtiofs filesystem: {}", stderr); + panic!("Failed to mount virtiofs filesystem: {}", mount_cmd.stderr); } // Test SSH connection and verify container storage mount inside VM println!("Testing SSH connection and checking container storage mount..."); - let st = Command::new("timeout") - .args([ - "60s", - &bck, - "libvirt", - "ssh", - &domain_name, - "--", - "ls", - "-la", - "/run/virtiofs-mnt-hoststorage/overlay", - ]) - .status() - .expect("Failed to run SSH command to check container storage"); - - assert!(st.success()); + run_bcvk_nocapture(&[ + "libvirt", + "ssh", + &domain_name, + "--", + "ls", + "-la", + "/run/virtiofs-mnt-hoststorage/overlay", + ]) + .expect("Failed to run SSH command to check container storage"); // Verify that the mount is read-only println!("Verifying that the mount is read-only..."); - let ro_test_st = Command::new("timeout") - .args([ - "30s", - &bck, - "libvirt", - "ssh", - &domain_name, - "--", - "touch", - "/run/virtiofs-mnt-hoststorage/test-write", - ]) - .status() - .expect("Failed to run SSH command to test read-only mount"); + let ro_test_st = run_bcvk(&[ + "libvirt", + "ssh", + &domain_name, + "--", + "touch", + "/run/virtiofs-mnt-hoststorage/test-write", + ]) + .expect("Failed to run SSH command to test read-only mount"); assert!( !ro_test_st.success(), @@ -725,45 +659,36 @@ pub fn test_libvirt_label_functionality() { ); // Cleanup any existing domain with this name - let _ = Command::new("virsh") - .args(&["destroy", &domain_name]) - .output(); - let _ = Command::new(&bck) - .args(&["libvirt", "rm", &domain_name, "--force", "--stop"]) - .output(); + cleanup_domain(&domain_name); // Create domain with multiple labels println!("Creating libvirt domain with multiple labels..."); - let create_output = Command::new("timeout") - .args([ - "300s", - &bck, - "libvirt", - "run", - "--name", - &domain_name, - "--label", - LIBVIRT_INTEGRATION_TEST_LABEL, - "--label", - "test-env", - "--label", - "temporary", - "--filesystem", - "ext4", - &test_image, - ]) - .output() - .expect("Failed to run libvirt run with labels"); - - let create_stdout = String::from_utf8_lossy(&create_output.stdout); - let create_stderr = String::from_utf8_lossy(&create_output.stderr); - - println!("Create stdout: {}", create_stdout); - println!("Create stderr: {}", create_stderr); - - if !create_output.status.success() { + let create_output = run_bcvk(&[ + "libvirt", + "run", + "--name", + &domain_name, + "--label", + LIBVIRT_INTEGRATION_TEST_LABEL, + "--label", + "test-env", + "--label", + "temporary", + "--filesystem", + "ext4", + &test_image, + ]) + .expect("Failed to run libvirt run with labels"); + + println!("Create stdout: {}", create_output.stdout); + println!("Create stderr: {}", create_output.stderr); + + if !create_output.success() { cleanup_domain(&domain_name); - panic!("Failed to create domain with labels: {}", create_stderr); + panic!( + "Failed to create domain with labels: {}", + create_output.stderr + ); } println!("Successfully created domain with labels: {}", domain_name); diff --git a/crates/integration-tests/src/tests/mount_feature.rs b/crates/integration-tests/src/tests/mount_feature.rs index 73579c332..36e4fd07c 100644 --- a/crates/integration-tests/src/tests/mount_feature.rs +++ b/crates/integration-tests/src/tests/mount_feature.rs @@ -16,10 +16,9 @@ use camino::Utf8Path; use std::fs; -use std::process::Command; use tempfile::TempDir; -use crate::{get_bck_command, get_test_image, INTEGRATION_TEST_LABEL}; +use crate::{get_test_image, run_bcvk, INTEGRATION_TEST_LABEL}; /// Create a systemd unit that verifies a mount exists and contains expected content fn create_mount_verify_unit( @@ -77,8 +76,6 @@ StandardError=journal+console } pub fn test_mount_feature_bind() { - let bck = get_bck_command().unwrap(); - // Create a temporary directory to test bind mounting let temp_dir = TempDir::new().expect("Failed to create temp directory"); let temp_dir_path = Utf8Path::from_path(temp_dir.path()).expect("temp dir path is not utf8"); @@ -99,39 +96,32 @@ pub fn test_mount_feature_bind() { println!("Testing bind mount with temp directory: {}", temp_dir_path); // Run with bind mount and verification unit - let output = Command::new("timeout") - .args([ - "60s", - &bck, - "ephemeral", - "run", - "--rm", - "--label", - INTEGRATION_TEST_LABEL, - "--console", - "-K", - "--bind", - &format!("{}:testmount", temp_dir_path), - "--systemd-units", - units_dir_path.as_str(), - "--karg", - "systemd.unit=verify-mount-testmount.service", - "--karg", - "systemd.journald.forward_to_console=1", - &get_test_image(), - ]) - .output() - .expect("Failed to run bcvk with bind mount"); - - let stdout = String::from_utf8_lossy(&output.stdout); - assert!(stdout.contains("ok mount verify")); + let output = run_bcvk(&[ + "ephemeral", + "run", + "--rm", + "--label", + INTEGRATION_TEST_LABEL, + "--console", + "-K", + "--bind", + &format!("{}:testmount", temp_dir_path), + "--systemd-units", + units_dir_path.as_str(), + "--karg", + "systemd.unit=verify-mount-testmount.service", + "--karg", + "systemd.journald.forward_to_console=1", + &get_test_image(), + ]) + .expect("Failed to run bcvk with bind mount"); + + assert!(output.stdout.contains("ok mount verify")); println!("Successfully tested and verified bind mount feature"); } pub fn test_mount_feature_ro_bind() { - let bck = get_bck_command().unwrap(); - // Create a temporary directory to test read-only bind mounting let temp_dir = TempDir::new().expect("Failed to create temp directory"); let temp_dir_path = Utf8Path::from_path(temp_dir.path()).expect("temp dir path is not utf8"); @@ -154,30 +144,25 @@ pub fn test_mount_feature_ro_bind() { ); // Run with read-only bind mount and verification unit - let output = Command::new("timeout") - .args([ - "60s", - &bck, - "ephemeral", - "run", - "--rm", - "--label", - INTEGRATION_TEST_LABEL, - "--console", - "-K", - "--ro-bind", - &format!("{}:romount", temp_dir_path), - "--systemd-units", - units_dir_path.as_str(), - "--karg", - "systemd.unit=verify-ro-mount-romount.service", - "--karg", - "systemd.journald.forward_to_console=1", - &get_test_image(), - ]) - .output() - .expect("Failed to run bcvk with ro-bind mount"); - - let stdout = String::from_utf8_lossy(&output.stdout); - assert!(stdout.contains("ok mount verify")); + let output = run_bcvk(&[ + "ephemeral", + "run", + "--rm", + "--label", + INTEGRATION_TEST_LABEL, + "--console", + "-K", + "--ro-bind", + &format!("{}:romount", temp_dir_path), + "--systemd-units", + units_dir_path.as_str(), + "--karg", + "systemd.unit=verify-ro-mount-romount.service", + "--karg", + "systemd.journald.forward_to_console=1", + &get_test_image(), + ]) + .expect("Failed to run bcvk with ro-bind mount"); + + assert!(output.stdout.contains("ok mount verify")); }