More virtiofs

.github/workflows/main.yml

@@ -47,8 +47,13 @@ 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
+        run: just test-integration ephemeral
 
       - name: Upload junit XML
         if: always()

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)
 }

crates/kit/src/qemu.rs

@@ -4,13 +4,16 @@
 //! 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;
 
+use camino::Utf8PathBuf;
 use cap_std_ext::cmdext::CapStdExtCommandExt;
 use color_eyre::eyre::{eyre, Context};
 use color_eyre::Result;
@@ -117,7 +120,7 @@ pub enum BootMode {
         initramfs_path: String,
         kernel_cmdline: Vec<String>,
         /// VirtIO-FS socket for root filesystem
-        virtiofs_socket: String,
+        virtiofs_socket: Utf8PathBuf,
     },
     #[allow(dead_code)]
     DiskBoot {
@@ -170,7 +173,7 @@ impl QemuConfig {
         vcpus: u32,
         kernel_path: String,
         initramfs_path: String,
-        virtiofs_socket: String,
+        virtiofs_socket: Utf8PathBuf,
     ) -> Self {
         Self {
             memory_mb,
@@ -290,7 +293,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);
@@ -683,13 +686,63 @@ struct VsockCopier {
 
 pub struct RunningQemu {
     pub qemu_process: Child,
-    pub virtiofsd_processes: Vec<tokio::process::Child>,
+    pub virtiofsd_processes: Vec<Pin<Box<dyn Future<Output = std::io::Result<Output>>>>>,
     sd_notification: Option<VsockCopier>,
 }
 
 impl RunningQemu {
     /// Spawn QEMU
     pub async fn spawn(mut config: QemuConfig) -> Result<Self> {
+        // 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<dyn Future<Output = std::io::Result<Output>>>> =
+                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
@@ -790,28 +843,6 @@ 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, Duration::from_secs(10)).await?;
-        }
-
-        // 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, Duration::from_secs(10))
-                .await?;
-        }
         // Spawn QEMU process with additional VSOCK credential if needed
         let qemu_process = spawn(&config, &creds, vsockdata)?;
 
@@ -822,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<std::process::ExitStatus> {
         let r = self.qemu_process.wait()?;
@@ -847,7 +873,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)
@@ -865,32 +891,42 @@ 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,
+    pub socket_path: Utf8PathBuf,
     /// 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 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(),
-            cache_mode: "always".to_string(),
-            sandbox: "none".to_string(),
+            socket_path: "/run/inner-shared/virtiofs.sock".into(),
+            shared_dir: "/run/source-image".into(),
             debug: false,
         }
     }
 }
 
+/// 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.
@@ -915,6 +951,13 @@ pub async fn spawn_virtiofsd_async(config: &VirtiofsConfig) -> Result<tokio::pro
             )
         })?;
 
+    // Check if virtiofsd supports --readonly flag
+    let supports_readonly = virtiofsd_supports_readonly(virtiofsd_binary).await;
+    debug!(
+        "virtiofsd at {} supports --readonly: {}",
+        virtiofsd_binary, supports_readonly
+    );
+
     let mut cmd = tokio::process::Command::new(virtiofsd_binary);
     // SAFETY: This API is safe to call in a forked child.
     unsafe {
@@ -925,27 +968,26 @@ pub async fn spawn_virtiofsd_async(config: &VirtiofsConfig) -> Result<tokio::pro
     }
     cmd.args([
         "--socket-path",
-        &config.socket_path,
+        config.socket_path.as_str(),
         "--shared-dir",
-        &config.shared_dir,
-        "--cache",
-        &config.cache_mode,
-        "--sandbox",
-        &config.sandbox,
+        config.shared_dir.as_str(),
+        // Ensure we don't hit fd exhaustion
+        "--cache=never",
+        // We always run in a container
+        "--sandbox=none",
     ]);
 
+    // Only add --readonly if supported
+    if supports_readonly {
+        cmd.arg("--readonly");
+    }
+
     // https://gitlab.com/virtio-fs/virtiofsd/-/issues/17 - this is the new default,
     // but we want to be compatible with older virtiofsd too.
     cmd.arg("--inode-file-handles=fallback");
 
-    // Redirect stdout/stderr to /dev/null unless debug mode is enabled
-    if !config.debug {
-        cmd.stdout(std::process::Stdio::null())
-            .stderr(std::process::Stdio::null());
-    } else {
-        cmd.stdout(std::process::Stdio::piped())
-            .stderr(std::process::Stdio::piped());
-    }
+    cmd.stdout(std::process::Stdio::piped());
+    cmd.stderr(std::process::Stdio::piped());
 
     let child = cmd.spawn().with_context(|| {
         format!(
@@ -1014,7 +1056,7 @@ pub fn validate_virtiofsd_config(config: &VirtiofsConfig) -> 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"));
     }
 
@@ -1030,25 +1072,5 @@ pub fn validate_virtiofsd_config(config: &VirtiofsConfig) -> 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()) {
-        return Err(eyre!(
-            "Invalid virtiofsd sandbox mode: '{}'. Valid options: {}",
-            config.sandbox,
-            valid_sandbox_modes.join(", ")
-        ));
-    }
-
     Ok(())
 }