repository: use OnceCell for objects_dir

Rename our `Repository::object_dir()` accessor to `.objects_dir()` (the
directory is called `"objects"`) and change its implementation to use a
OnceCell.  ostree uses this "initialize on first use" pattern for some
of its directories as well, and I like it.

We use the external `once_cell::` crate (which we already had as a
-devel dependency) because the .try() version of the API is not yet
stable in the standard library.

Clean up our ensure_object() implementation to use `.objects_dir()` and
generally make things a bit more robust:
 - we avoid unnecessary mkdir() calls for directories which almost
   certainly already exist
 - instead of checking merely for the existence of an object file with
   the correct name, we actually measure it now
 - we play around less with joining pathnames (and we can drop a
   now-unused trait helper method on FsVerityHashValue)

Signed-off-by: Allison Karlitskaya <[email protected]>

Author: allisonkarlitskaya

Cargo.toml

@@ -26,6 +26,7 @@ hex = "0.4.0"
 indicatif = { version = "0.17.0", features = ["tokio"] }
 log = "0.4.8"
 oci-spec = "0.7.0"
+once_cell = { version = "1.21.3", default-features = false }
 regex-automata = { version = "0.4.4", default-features = false }
 rustix = { version = "1.0.0", features = ["fs", "mount", "process"] }
 serde = "1.0.145"
@@ -41,7 +42,6 @@ zstd = "0.13.0"
 
 [dev-dependencies]
 insta = "1.42.2"
-once_cell = "1.21.3"
 similar-asserts = "1.7.0"
 test-with = { version = "0.14", default-features = false, features = ["executable", "runtime"] }
 tokio-test = "0.4.4"

src/bin/composefs-setup-root.rs

@@ -169,7 +169,7 @@ fn open_root_fs(path: &Path) -> Result<OwnedFd> {
 fn mount_composefs_image(sysroot: &OwnedFd, name: &str) -> Result<OwnedFd> {
     let repo = Repository::<Sha256HashValue>::open_path(sysroot, "composefs")?;
     let image = repo.open_image(name)?;
-    composefs_fsmount(image, name, repo.object_dir()?).context("Failed to mount composefs image")
+    composefs_fsmount(image, name, repo.objects_dir()?).context("Failed to mount composefs image")
 }
 
 fn mount_subdir(

src/fsverity/hashvalue.rs

@@ -60,17 +60,17 @@ where
     }
 
     fn to_object_pathname(&self) -> String {
-        format!("{:02x}/{}", self.as_bytes()[0], self.to_object_basename())
+        format!(
+            "{:02x}/{}",
+            self.as_bytes()[0],
+            hex::encode(&self.as_bytes()[1..])
+        )
     }
 
     fn to_object_dir(&self) -> String {
         format!("{:02x}", self.as_bytes()[0])
     }
 
-    fn to_object_basename(&self) -> String {
-        hex::encode(&self.as_bytes()[1..])
-    }
-
     fn to_hex(&self) -> String {
         hex::encode(self.as_bytes())
     }

src/repository.rs

@@ -2,16 +2,17 @@ use std::{
     collections::HashSet,
     ffi::CStr,
     fs::File,
-    io::{ErrorKind, Read, Write},
+    io::{Read, Write},
     os::fd::{AsFd, OwnedFd},
     path::{Path, PathBuf},
 };
 
 use anyhow::{bail, ensure, Context, Result};
+use once_cell::sync::OnceCell;
 use rustix::{
     fs::{
-        accessat, fdatasync, flock, linkat, mkdirat, open, openat, readlinkat, symlinkat, Access,
-        AtFlags, Dir, FileType, FlockOperation, Mode, OFlags, CWD,
+        fdatasync, flock, linkat, mkdirat, open, openat, readlinkat, symlinkat, AtFlags, Dir,
+        FileType, FlockOperation, Mode, OFlags, CWD,
     },
     io::{Errno, Result as ErrnoResult},
 };
@@ -26,9 +27,35 @@ use crate::{
     util::{proc_self_fd, Sha256Digest},
 };
 
+/// Call openat() on the named subdirectory of "dirfd", possibly creating it first.
+///
+/// We assume that the directory will probably exist (ie: we try the open first), and on ENOENT, we
+/// mkdirat() and retry.
+fn ensure_dir_and_openat(dirfd: impl AsFd, filename: &str, flags: OFlags) -> ErrnoResult<OwnedFd> {
+    match openat(
+        &dirfd,
+        filename,
+        flags | OFlags::CLOEXEC | OFlags::DIRECTORY,
+        0o666.into(),
+    ) {
+        Ok(file) => Ok(file),
+        Err(Errno::NOENT) => match mkdirat(&dirfd, filename, 0o777.into()) {
+            Ok(()) | Err(Errno::EXIST) => openat(
+                dirfd,
+                filename,
+                flags | OFlags::CLOEXEC | OFlags::DIRECTORY,
+                0o666.into(),
+            ),
+            Err(other) => Err(other),
+        },
+        Err(other) => Err(other),
+    }
+}
+
 #[derive(Debug)]
 pub struct Repository<ObjectID: FsVerityHashValue> {
     repository: OwnedFd,
+    objects: OnceCell<OwnedFd>,
     _data: std::marker::PhantomData<ObjectID>,
 }
 
@@ -39,11 +66,9 @@ impl<ObjectID: FsVerityHashValue> Drop for Repository<ObjectID> {
 }
 
 impl<ObjectID: FsVerityHashValue> Repository<ObjectID> {
-    pub fn object_dir(&self) -> ErrnoResult<OwnedFd> {
-        self.openat(
-            "objects",
-            OFlags::PATH | OFlags::DIRECTORY | OFlags::CLOEXEC,
-        )
+    pub fn objects_dir(&self) -> ErrnoResult<&OwnedFd> {
+        self.objects
+            .get_or_try_init(|| ensure_dir_and_openat(&self.repository, "objects", OFlags::PATH))
     }
 
     pub fn open_path(dirfd: impl AsFd, path: impl AsRef<Path>) -> Result<Self> {
@@ -58,6 +83,7 @@ impl<ObjectID: FsVerityHashValue> Repository<ObjectID> {
 
         Ok(Self {
             repository,
+            objects: OnceCell::new(),
             _data: std::marker::PhantomData,
         })
     }
@@ -80,48 +106,61 @@ impl<ObjectID: FsVerityHashValue> Repository<ObjectID> {
     }
 
     pub fn ensure_object(&self, data: &[u8]) -> Result<ObjectID> {
-        let digest: ObjectID = compute_verity(data);
-        let dir = PathBuf::from(format!("objects/{}", digest.to_object_dir()));
-        let file = dir.join(digest.to_object_basename());
+        let objects = self.objects_dir()?;
+        let id: ObjectID = compute_verity(data);
 
-        // fairly common...
-        if accessat(&self.repository, &file, Access::READ_OK, AtFlags::empty()) == Ok(()) {
-            return Ok(digest);
-        }
+        let path = id.to_object_pathname();
 
-        self.ensure_dir("objects")?;
-        self.ensure_dir(&dir)?;
+        // the usual case is that the file will already exist
+        match openat(
+            objects,
+            &path,
+            OFlags::RDONLY | OFlags::CLOEXEC,
+            Mode::empty(),
+        ) {
+            Ok(fd) => {
+                // measure the existing file to ensure that it's correct
+                // TODO: try to replace file if it's broken?
+                ensure_verity_equal(fd, &id)?;
+                return Ok(id);
+            }
+            Err(Errno::NOENT) => {
+                // in this case we'll create the file
+            }
+            Err(other) => {
+                return Err(other).context("Checking for existing object in repository")?;
+            }
+        }
 
-        let fd = openat(
-            &self.repository,
-            &dir,
-            OFlags::RDWR | OFlags::CLOEXEC | OFlags::TMPFILE,
-            0o666.into(),
-        )?;
-        rustix::io::write(&fd, data)?; // TODO: no write_all() here...
-        fdatasync(&fd)?;
+        let fd = ensure_dir_and_openat(objects, &id.to_object_dir(), OFlags::RDWR | OFlags::TMPFILE)?;
+        let mut file = File::from(fd);
+        file.write_all(data)?;
+        fdatasync(&file)?;
 
         // We can't enable verity with an open writable fd, so re-open and close the old one.
-        let ro_fd = open(proc_self_fd(&fd), OFlags::RDONLY, Mode::empty())?;
-        drop(fd);
+        let ro_fd = open(proc_self_fd(&file), OFlags::RDONLY, Mode::empty())?;
+        drop(file);
 
         enable_verity::<ObjectID>(&ro_fd).context("Enabling verity digest")?;
-        ensure_verity_equal(&ro_fd, &digest).context("Double-checking verity digest")?;
+        ensure_verity_equal(&ro_fd, &id).context("Double-checking verity digest")?;
 
-        if let Err(err) = linkat(
+        match linkat(
             CWD,
             proc_self_fd(&ro_fd),
-            &self.repository,
-            file,
+            objects,
+            path,
             AtFlags::SYMLINK_FOLLOW,
         ) {
-            if err.kind() != ErrorKind::AlreadyExists {
-                return Err(err.into());
+            Ok(()) => {}
+            Err(Errno::EXIST) => {
+                // TODO: strictly, we should measure the newly-appeared file
+            }
+            Err(other) => {
+                return Err(other).context("Linking created object file");
             }
         }
 
-        drop(ro_fd);
-        Ok(digest)
+        Ok(id)
     }
 
     fn open_with_verity(&self, filename: &str, expected_verity: &ObjectID) -> Result<OwnedFd> {
@@ -350,7 +389,7 @@ impl<ObjectID: FsVerityHashValue> Repository<ObjectID> {
         Ok(mount_composefs_at(
             image,
             name,
-            &self.object_dir()?,
+            self.objects_dir()?,
             mountpoint,
         )?)
     }