ext2: Make Builder take a root directory path

Instead of having Builder and a path to the root directory separately,
the Builder struct should contain the root directory so we can contain
all the file system configuration inside of the builder.

BUG=b:329359333
TEST=presubmit

Change-Id: I4e97c170ffa3a7fc720063b4f97c813f2fde3863
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5889313
Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Takaya Saeki <takayas@chromium.org>

ext2/examples/mkfs.rs

@@ -8,7 +8,7 @@
 mod linux {
     use std::fs::OpenOptions;
     use std::io::Write;
-    use std::path::Path;
+    use std::path::PathBuf;
 
     use argh::FromArgs;
     use base::MappedRegion;
@@ -44,16 +44,14 @@ mod linux {
 
     pub fn main() -> anyhow::Result<()> {
         let args: Args = argh::from_env();
-        let src_dir = args.src.as_ref().map(|s| Path::new(s.as_str()));
+        let src_dir = args.src.as_ref().map(|s| PathBuf::new().join(s));
         let builder = ext2::Builder {
             blocks_per_group: args.blocks_per_group,
             inodes_per_group: args.inodes_per_group,
             size: args.size,
+            root_dir: src_dir,
         };
-        let mem = builder
+        let mem = builder.allocate_memory()?.build_mmap_info()?.do_mmap()?;
-            .allocate_memory()?
-            .build_mmap_info(src_dir)?
-            .do_mmap()?;
         if args.dry_run {
             println!("Done!");
             return Ok(());

ext2/src/blockgroup.rs

@@ -147,6 +147,7 @@ mod test {
                 inodes_per_group: 1024,
                 blocks_per_group,
                 size,
+                root_dir: None,
             },
         )
         .unwrap();
@@ -200,6 +201,7 @@ mod test {
                 inodes_per_group: 512,
                 blocks_per_group,
                 size: mem_size,
+                root_dir: None,
             },
         )
         .unwrap();

ext2/src/builder.rs

@@ -4,7 +4,7 @@
 
 //! Provides structs and logic to build ext2 file system with configurations.
 
-use std::path::Path;
+use std::path::PathBuf;
 
 use anyhow::bail;
 use anyhow::Context;
@@ -29,6 +29,8 @@ pub struct Builder {
     pub inodes_per_group: u32,
     /// The size of the memory region.
     pub size: u32,
+    /// The roof directory to be copied to the file system.
+    pub root_dir: Option<PathBuf>,
 }
 
 impl Default for Builder {
@@ -37,6 +39,7 @@ impl Default for Builder {
             blocks_per_group: 4096,
             inodes_per_group: 4096,
             size: 4096 * 4096,
+            root_dir: None,
         }
     }
 }
@@ -88,10 +91,10 @@ pub struct MemRegion {
 
 impl MemRegion {
     /// Constructs an ext2 metadata by traversing `src_dir`.
-    pub fn build_mmap_info(mut self, src_dir: Option<&Path>) -> Result<MemRegionWithMappingInfo> {
+    pub fn build_mmap_info(mut self) -> Result<MemRegionWithMappingInfo> {
         let arena = Arena::new(BLOCK_SIZE, &mut self.mem).context("failed to allocate arena")?;
         let mut ext2 = Ext2::new(&self.cfg, &arena).context("failed to create Ext2 struct")?;
-        if let Some(dir) = src_dir {
+        if let Some(dir) = self.cfg.root_dir {
             ext2.copy_dirtree(&arena, dir)
                 .context("failed to copy directory tree")?;
         }

ext2/src/fs.rs

@@ -132,7 +132,7 @@ impl DirEntryBlock<'_> {
 }
 
 /// A struct to represent an ext2 filesystem.
-pub struct Ext2<'a> {
+pub(crate) struct Ext2<'a> {
     sb: &'a mut SuperBlock,
     cur_block_group: usize,
     cur_inode_table: usize,
@@ -143,10 +143,8 @@ pub struct Ext2<'a> {
 }
 
 impl<'a> Ext2<'a> {
-    /// Create a new ext2 filesystem.
+    pub(crate) fn new(builder: &Builder, arena: &'a Arena<'a>) -> Result<Self> {
-    pub(crate) fn new(cfg: &Builder, arena: &'a Arena<'a>) -> Result<Self> {
+        let sb = SuperBlock::new(arena, builder)?;
-        let sb = SuperBlock::new(arena, cfg)?;
-
         let mut group_metadata = vec![];
         for i in 0..sb.num_groups() {
             group_metadata.push(GroupMetaData::new(arena, sb, i)?);

ext2/tests/tests.rs

@@ -63,12 +63,12 @@ fn run_debugfs_cmd(args: &[&str], disk: &PathBuf) -> String {
     stdout.trim_start().trim_end().to_string()
 }
 
-fn mkfs(td: &TempDir, builder: Builder, src_dir: Option<&Path>) -> PathBuf {
+fn mkfs(td: &TempDir, builder: Builder) -> PathBuf {
     let path = td.path().join("empty.ext2");
     let mem = builder
         .allocate_memory()
         .unwrap()
-        .build_mmap_info(src_dir)
+        .build_mmap_info()
         .unwrap()
         .do_mmap()
         .unwrap();
@@ -97,7 +97,6 @@ fn test_mkfs_empty() {
             inodes_per_group: 1024,
             ..Default::default()
         },
-        None,
     );
 
     // Ensure the content of the generated disk image with `debugfs`.
@@ -122,8 +121,8 @@ fn test_mkfs_empty_multi_block_groups() {
             blocks_per_group,
             inodes_per_group: 4096,
             size: 4096 * blocks_per_group * num_groups,
+            ..Default::default()
         },
-        None,
     );
     assert_eq!(
         run_debugfs_cmd(&["ls"], &disk),
@@ -235,9 +234,9 @@ fn test_simple_dir() {
         Builder {
             blocks_per_group: 2048,
             inodes_per_group: 4096,
+            root_dir: Some(dir.clone()),
             ..Default::default()
         },
-        Some(&dir),
     );
 
     assert_eq_dirs(&td, &dir, &disk);
@@ -269,9 +268,9 @@ fn test_nested_dirs() {
         Builder {
             blocks_per_group: 2048,
             inodes_per_group: 4096,
+            root_dir: Some(dir.clone()),
             ..Default::default()
         },
-        Some(&dir),
     );
 
     assert_eq_dirs(&td, &dir, &disk);
@@ -298,9 +297,9 @@ fn test_file_contents() {
         Builder {
             blocks_per_group: 2048,
             inodes_per_group: 4096,
+            root_dir: Some(dir.clone()),
             ..Default::default()
         },
-        Some(&dir),
     );
 
     assert_eq_dirs(&td, &dir, &disk);
@@ -321,9 +320,9 @@ fn test_max_file_name() {
         Builder {
             blocks_per_group: 2048,
             inodes_per_group: 4096,
+            root_dir: Some(dir.clone()),
             ..Default::default()
         },
-        Some(&dir),
     );
 
     assert_eq_dirs(&td, &dir, &disk);
@@ -350,9 +349,9 @@ fn test_mkfs_indirect_block() {
         Builder {
             blocks_per_group: 4096,
             inodes_per_group: 4096,
+            root_dir: Some(dir.clone()),
             ..Default::default()
         },
-        Some(&dir),
     );
 
     assert_eq_dirs(&td, &dir, &disk);
@@ -387,9 +386,9 @@ fn test_mkfs_symlink() {
         Builder {
             blocks_per_group: 2048,
             inodes_per_group: 4096,
+            root_dir: Some(dir.clone()),
             ..Default::default()
         },
-        Some(&dir),
     );
 
     assert_eq_dirs(&td, &dir, &disk);
@@ -418,9 +417,9 @@ fn test_mkfs_abs_symlink() {
         Builder {
             blocks_per_group: 2048,
             inodes_per_group: 4096,
+            root_dir: Some(dir.clone()),
             ..Default::default()
         },
-        Some(&dir),
     );
 
     assert_eq_dirs(&td, &dir, &disk);
@@ -444,9 +443,9 @@ fn test_mkfs_symlink_to_deleted() {
         Builder {
             blocks_per_group: 2048,
             inodes_per_group: 4096,
+            root_dir: Some(dir.clone()),
             ..Default::default()
         },
-        Some(&dir),
     );
 
     assert_eq_dirs(&td, &dir, &disk);
@@ -485,9 +484,9 @@ fn test_mkfs_long_symlink() {
         Builder {
             blocks_per_group: 2048,
             inodes_per_group: 4096,
+            root_dir: Some(dir.clone()),
             ..Default::default()
         },
-        Some(&dir),
     );
 
     assert_eq_dirs(&td, &dir, &disk);
@@ -519,9 +518,9 @@ fn test_ignore_lost_found() {
         Builder {
             blocks_per_group: 2048,
             inodes_per_group: 4096,
+            root_dir: Some(dir.clone()),
             ..Default::default()
         },
-        Some(&dir),
     );
 
     // dump the disk contents to `dump_dir`.
@@ -577,9 +576,9 @@ fn test_multiple_block_directory_entry() {
         Builder {
             blocks_per_group: 2048,
             inodes_per_group: 4096,
+            root_dir: Some(dir.clone()),
             ..Default::default()
         },
-        Some(&dir),
     );
 
     assert_eq_dirs(&td, &dir, &disk);
@@ -617,8 +616,8 @@ fn test_multiple_bg_multi_inode_bitmap() {
             blocks_per_group,
             inodes_per_group,
             size: BLOCK_SIZE * blocks_per_group * num_groups,
+            root_dir: Some(dir.clone()),
         },
-        Some(&dir),
     );
 
     assert_eq_dirs(&td, &dir, &disk);
@@ -656,8 +655,8 @@ fn test_multiple_bg_multi_block_bitmap() {
             blocks_per_group,
             inodes_per_group,
             size: BLOCK_SIZE * blocks_per_group * num_groups,
+            root_dir: Some(dir.clone()),
         },
-        Some(&dir),
     );
 
     assert_eq_dirs(&td, &dir, &disk);
@@ -694,8 +693,8 @@ fn test_multiple_bg_big_files() {
             blocks_per_group,
             inodes_per_group: 1024,
             size: BLOCK_SIZE * blocks_per_group * num_groups,
+            root_dir: Some(dir.clone()),
         },
-        Some(&dir),
     );
 
     assert_eq_dirs(&td, &dir, &disk);

src/crosvm/sys/linux/device_helpers.rs

@@ -1308,6 +1308,7 @@ pub fn create_pmem_ext2_device(
         inodes_per_group: opts.inodes_per_group,
         blocks_per_group: opts.blocks_per_group,
         size: mapping_size as u32,
+        ..Default::default()
     };
 
     let max_open_files = base::linux::max_open_files()

src/crosvm/sys/linux/ext2.rs

@@ -49,7 +49,7 @@ pub fn launch(
     path: &Path,
     ugid: &(Option<u32>, Option<u32>),
     ugid_map: (&str, &str),
-    builder: ext2::Builder,
+    mut builder: ext2::Builder,
     jail_config: &Option<JailConfig>,
 ) -> Result<Pid> {
     let max_open_files = base::linux::max_open_files()
@@ -72,6 +72,9 @@ pub fn launch(
         create_base_minijail(path, max_open_files)?
     };
 
+    // Use "/" in the new mount namespace as the root for mkfs.
+    builder.root_dir = Some(std::path::PathBuf::from("/"));
+
     let shm = SharedMemory::new("pmem_ext2_shm", builder.size as u64)
         .context("failed to create shared memory")?;
     let mut keep_rds = vec![
@@ -102,11 +105,10 @@ fn mkfs_callback(
     builder: ext2::Builder,
     shm: SharedMemory,
 ) -> Result<()> {
-    let jailed_root = Some(std::path::Path::new("/"));
     let file_mappings = builder
         .build_on_shm(&shm)
         .context("failed to build memory region")?
-        .build_mmap_info(jailed_root)
+        .build_mmap_info()
         .context("failed to build ext2")?
         .mapping_info;