Merge pull request #43 from containers/rootfs-fixes

--rootfs fixes

src/commands/create/mod.rs

@@ -5,6 +5,7 @@ mod domain;
 mod first_boot;
 mod runtime_env;
 
+use std::ffi::OsStr;
 use std::fs::{self, File, Permissions};
 use std::io::ErrorKind;
 use std::os::unix::fs::{FileTypeExt, MetadataExt, PermissionsExt};
@@ -20,38 +21,39 @@ use crate::commands::create::custom_opts::CustomOptions;
 use crate::commands::create::domain::set_up_libvirt_domain_xml;
 use crate::commands::create::first_boot::FirstBootConfig;
 use crate::commands::create::runtime_env::RuntimeEnv;
-use crate::crun::crun_create;
 use crate::util::{
-    bind_mount_dir_read_only_with_different_context, bind_mount_dir_with_different_context,
-    bind_mount_file, create_overlay_vm_image, find_single_file_in_dirs, is_mountpoint,
-    set_file_context, SpecExt, VmImageInfo,
+    bind_mount_dir_with_different_context, bind_mount_file, create_overlay_vm_image, crun,
+    find_single_file_in_dirs, is_mountpoint, set_file_context, SpecExt, VmImageInfo,
 };
 
-pub fn create(global_args: &liboci_cli::GlobalOpts, args: &liboci_cli::Create) -> Result<()> {
+pub fn create(args: &liboci_cli::Create, raw_args: &[impl AsRef<OsStr>]) -> Result<()> {
     let bundle_path: &Utf8Path = args.bundle.as_path().try_into()?;
     let config_path = bundle_path.join("config.json");
 
     let mut spec = oci_spec::runtime::Spec::load(&config_path)?;
-
-    // We include container_id in the path to ensure no overlap with the user container's contents.
-    let original_root_path = spec.root_path()?.to_path_buf();
-
-    let new_root_path = original_root_path.join(format!("crun-vm-root-{}", args.container_id));
-    fs::create_dir_all(&new_root_path)?;
-
-    let private_root_path = original_root_path.join(format!("crun-vm-priv-{}", args.container_id));
-    fs::create_dir_all(&private_root_path)?;
+    let original_root_path: Utf8PathBuf = spec.root_path()?.canonicalize()?.try_into()?; // ensure absolute
 
     let runtime_env = RuntimeEnv::current(&spec, &original_root_path)?;
     let custom_options = CustomOptions::from_spec(&spec, runtime_env)?;
 
-    set_up_container_root(&mut spec, &new_root_path, &custom_options)?;
+    // We include container_id in our paths to ensure no overlap with the user container's contents.
+    let priv_dir_path = original_root_path.join(format!("crun-vm-{}", args.container_id));
+    fs::create_dir_all(&priv_dir_path)?;
+
+    if let Some(context) = spec.mount_label() {
+        // the directory we're using as the root for the container is not the one that podman
+        // prepared for us, so we need to set its context ourselves to prevent SELinux from getting
+        // angry at us
+        set_file_context(&priv_dir_path, context)?;
+    }
+
+    set_up_container_root(&mut spec, &priv_dir_path, &custom_options)?;
     let is_first_create = is_first_create(&spec)?;
 
     let base_vm_image_info = set_up_vm_image(
         &spec,
         &original_root_path,
-        &private_root_path,
+        &priv_dir_path,
         &custom_options,
         is_first_create,
     )?;
@@ -74,7 +76,7 @@ pub fn create(global_args: &liboci_cli::GlobalOpts, args: &liboci_cli::Create) -
     spec.save(&config_path)?;
     spec.save(spec.root_path()?.join("crun-vm/config.json"))?; // to aid debugging
 
-    crun_create(global_args, args)?; // actually create container
+    crun(raw_args)?; // actually create container
 
     Ok(())
 }
@@ -98,26 +100,22 @@ fn is_first_create(spec: &oci_spec::runtime::Spec) -> Result<bool> {
 
 fn set_up_container_root(
     spec: &mut oci_spec::runtime::Spec,
-    new_root_path: &Utf8Path,
+    priv_dir_path: &Utf8Path,
     custom_options: &CustomOptions,
 ) -> Result<()> {
+    let new_root_path = priv_dir_path.join("root");
+    fs::create_dir_all(&new_root_path)?;
+
     // create root directory
 
     spec.set_root(Some(
         oci_spec::runtime::RootBuilder::default()
-            .path(new_root_path)
+            .path(&new_root_path)
             .readonly(false)
             .build()
             .unwrap(),
     ));
 
-    if let Some(context) = spec.mount_label() {
-        // the directory we're using as the root for the container is not the one that podman
-        // prepared for us, so we need to set its context ourselves to prevent SELinux from getting
-        // angry at us
-        set_file_context(new_root_path, context)?;
-    }
-
     // set up container scripts
 
     #[derive(RustEmbed)]
@@ -130,7 +128,7 @@ fn set_up_container_root(
 
         let file = Scripts::get(&path).unwrap();
         fs::write(&path_in_host, file.data)?;
-        fs::set_permissions(&path_in_host, Permissions::from_mode(0o555))?;
+        fs::set_permissions(&path_in_host, Permissions::from_mode(0o755))?;
     }
 
     // configure container entrypoint
@@ -154,7 +152,7 @@ fn set_up_container_root(
 fn set_up_vm_image(
     spec: &oci_spec::runtime::Spec,
     original_root_path: &Utf8Path,
-    private_root_path: &Utf8Path,
+    priv_dir_path: &Utf8Path,
     custom_options: &CustomOptions,
     is_first_create: bool,
 ) -> Result<VmImageInfo> {
@@ -171,35 +169,31 @@ fn set_up_vm_image(
 
     // mount user-provided VM image file into container
 
-    // TODO: Can we assume the container engine will always clean up all our mounts, since they're
-    // under the container's root?
+    // Make VM image file available in a subtree that doesn't overlap our internal container root so
+    // overlayfs works.
 
-    let mirror_vm_image_path_in_container =
-        Utf8Path::new("crun-vm/image").join(vm_image_path_in_host.file_name().unwrap());
-    let mirror_vm_image_path_in_host = spec.root_path()?.join(&mirror_vm_image_path_in_container);
-    let mirror_vm_image_path_in_container =
-        Utf8Path::new("/").join(mirror_vm_image_path_in_container);
+    let image_dir_path = priv_dir_path.join("image");
+    fs::create_dir_all(&image_dir_path)?;
 
-    if custom_options.persistent {
-        let vm_image_dir_path = vm_image_path_in_host.parent().unwrap();
-        let vm_image_dir_name = vm_image_dir_path.file_name().unwrap();
+    if !image_dir_path.join("image").exists() {
+        fs::hard_link(vm_image_path_in_host, image_dir_path.join("image"))?;
+    }
 
+    let mirror_vm_image_path_in_container = Utf8PathBuf::from("/crun-vm/image/image");
+    let mirror_vm_image_path_in_host = spec.root_path()?.join("crun-vm/image/image");
+
+    if custom_options.persistent {
         // Mount overlayfs to expose the user's VM image file with a different SELinux context so we
         // can always access it, using the file's parent as the upperdir so that writes still
         // propagate to it.
 
         if !is_mountpoint(mirror_vm_image_path_in_host.parent().unwrap())? {
-            let scratch_dir_path = vm_image_dir_path
-                .parent()
-                .unwrap()
-                .join(format!(".crun-vm.{}.tmp", vm_image_dir_name));
-
             bind_mount_dir_with_different_context(
-                vm_image_path_in_host.parent().unwrap(),
+                image_dir_path,
                 mirror_vm_image_path_in_host.parent().unwrap(),
+                priv_dir_path.join("scratch"),
                 spec.mount_label(),
-                true,
-                scratch_dir_path,
+                false,
             )?;
         }
 
@@ -217,11 +211,12 @@ fn set_up_vm_image(
         // can always access it.
 
         if !is_mountpoint(mirror_vm_image_path_in_host.parent().unwrap())? {
-            bind_mount_dir_read_only_with_different_context(
-                vm_image_path_in_host.parent().unwrap(),
+            bind_mount_dir_with_different_context(
+                image_dir_path,
                 mirror_vm_image_path_in_host.parent().unwrap(),
+                priv_dir_path.join("scratch"),
                 spec.mount_label(),
-                private_root_path.join("scratch"),
+                true,
             )?;
         }
 

src/commands/delete.rs

@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+use std::ffi::OsStr;
+use std::path::PathBuf;
+use std::process::{Command, Stdio};
+
+use anyhow::{ensure, Result};
+use camino::Utf8PathBuf;
+use serde::Deserialize;
+
+use crate::util::{crun, ensure_unmounted};
+
+pub fn delete(args: &liboci_cli::Delete, raw_args: &[impl AsRef<OsStr>]) -> Result<()> {
+    // get container root path
+
+    // the container might not exist because creation failed midway through, so we ignore errors
+    let root_path = get_root_path(&args.container_id).ok();
+
+    // actually delete the container
+
+    crun(raw_args)?;
+
+    // clean up crun-vm mounts so that user doesn't have to deal with them when they decide to
+    // delete crun-vm's state/private directory
+
+    if let Some(root_path) = root_path {
+        let private_dir_path: Utf8PathBuf = root_path
+            .canonicalize()?
+            .parent()
+            .unwrap()
+            .to_path_buf()
+            .try_into()?;
+
+        let image_dir_path = private_dir_path.join("root/crun-vm/image");
+        let image_file_path = image_dir_path.join("image");
+
+        ensure_unmounted(image_file_path)?;
+        ensure_unmounted(image_dir_path)?;
+    }
+
+    Ok(())
+}
+
+fn get_root_path(container_id: &str) -> Result<Utf8PathBuf> {
+    let output = Command::new("crun")
+        .arg("state")
+        .arg(container_id)
+        .stderr(Stdio::inherit())
+        .output()?;
+
+    ensure!(output.status.success());
+
+    #[derive(Deserialize)]
+    struct ContainerState {
+        rootfs: PathBuf,
+    }
+
+    let state: ContainerState = serde_json::from_slice(&output.stdout)?;
+
+    Ok(state.rootfs.try_into()?)
+}

src/commands/exec.rs

@@ -1,17 +1,16 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-use std::{
-    env,
-    fs::File,
-    io::{BufReader, BufWriter},
-};
+use std::env;
+use std::ffi::OsStr;
+use std::fs::File;
+use std::io::{BufReader, BufWriter};
 
 use anyhow::{bail, Result};
 use clap::Parser;
 
-use crate::crun::crun_exec;
+use crate::util::crun;
 
-pub fn exec(global_args: &liboci_cli::GlobalOpts, args: &liboci_cli::Exec) -> Result<()> {
+pub fn exec(args: &liboci_cli::Exec, raw_args: &[impl AsRef<OsStr>]) -> Result<()> {
     assert!(args.command.is_empty());
 
     let process_config_path = args.process.as_ref().expect("process config");
@@ -28,7 +27,7 @@ pub fn exec(global_args: &liboci_cli::GlobalOpts, args: &liboci_cli::Exec) -> Re
         &process,
     )?;
 
-    crun_exec(global_args, args)?;
+    crun(raw_args)?;
 
     Ok(())
 }

src/commands/mod.rs

@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 
 pub mod create;
+pub mod delete;
 pub mod exec;