Various repository fixes

crates/composefs/Cargo.toml

@@ -29,6 +29,7 @@ tempfile = { version = "3.8.0", optional = true, default-features = false }
 xxhash-rust = { version = "0.8.2", default-features = false, features = ["xxh32"] }
 zerocopy = { version = "0.8.0", default-features = false, features = ["derive", "std"] }
 zstd = { version = "0.13.0", default-features = false }
+rand = { version = "0.9.1", default-features = true }
 
 [dev-dependencies]
 insta = "1.42.2"

crates/composefs/src/repository.rs

@@ -12,8 +12,8 @@ use anyhow::{bail, ensure, Context, Result};
 use once_cell::sync::OnceCell;
 use rustix::{
     fs::{
-        fdatasync, flock, linkat, mkdirat, open, openat, readlinkat, symlinkat, AtFlags, Dir,
-        FileType, FlockOperation, Mode, OFlags, CWD,
+        fdatasync, flock, linkat, mkdirat, open, openat, readlinkat, AtFlags, Dir, FileType,
+        FlockOperation, Mode, OFlags, CWD,
     },
     io::{Errno, Result as ErrnoResult},
 };
@@ -25,7 +25,7 @@ use crate::{
     },
     mount::mount_composefs_at,
     splitstream::{DigestMap, SplitStreamReader, SplitStreamWriter},
-    util::{proc_self_fd, Sha256Digest},
+    util::{filter_errno, proc_self_fd, replace_symlinkat, Sha256Digest},
 };
 
 /// Call openat() on the named subdirectory of "dirfd", possibly creating it first.
@@ -261,7 +261,7 @@ impl<ObjectID: FsVerityHashValue> Repository<ObjectID> {
         let stream_path = format!("streams/{}", hex::encode(sha256));
         let object_id = writer.done()?;
         let object_path = Self::format_object_path(&object_id);
-        self.ensure_symlink(&stream_path, &object_path)?;
+        self.symlink(&stream_path, &object_path)?;
 
         if let Some(name) = reference {
             let reference_path = format!("streams/refs/{name}");
@@ -310,7 +310,7 @@ impl<ObjectID: FsVerityHashValue> Repository<ObjectID> {
                 let object_id = writer.done()?;
 
                 let object_path = Self::format_object_path(&object_id);
-                self.ensure_symlink(&stream_path, &object_path)?;
+                self.symlink(&stream_path, &object_path)?;
                 object_id
             }
         };
@@ -331,9 +331,11 @@ impl<ObjectID: FsVerityHashValue> Repository<ObjectID> {
         let filename = format!("streams/{name}");
 
         let file = File::from(if let Some(verity_hash) = verity {
-            self.open_with_verity(&filename, verity_hash)?
+            self.open_with_verity(&filename, verity_hash)
+                .with_context(|| format!("Opening ref 'streams/{name}'"))?
         } else {
-            self.openat(&filename, OFlags::RDONLY)?
+            self.openat(&filename, OFlags::RDONLY)
+                .with_context(|| format!("Opening ref 'streams/{name}'"))?
         });
 
         SplitStreamReader::new(file)
@@ -366,7 +368,7 @@ impl<ObjectID: FsVerityHashValue> Repository<ObjectID> {
         let object_path = Self::format_object_path(&object_id);
         let image_path = format!("images/{}", object_id.to_hex());
 
-        self.ensure_symlink(&image_path, &object_path)?;
+        self.symlink(&image_path, &object_path)?;
 
         if let Some(reference) = name {
             let ref_path = format!("images/refs/{reference}");
@@ -384,7 +386,9 @@ impl<ObjectID: FsVerityHashValue> Repository<ObjectID> {
     }
 
     pub fn open_image(&self, name: &str) -> Result<OwnedFd> {
-        let image = self.openat(&format!("images/{name}"), OFlags::RDONLY)?;
+        let image = self
+            .openat(&format!("images/{name}"), OFlags::RDONLY)
+            .with_context(|| format!("Opening ref 'images/{name}'"))?;
 
         if !name.contains("/") {
             // A name with no slashes in it is taken to be a sha256 fs-verity digest
@@ -432,14 +436,8 @@ impl<ObjectID: FsVerityHashValue> Repository<ObjectID> {
             relative.push(target_component);
         }
 
-        symlinkat(relative, &self.repository, name)
-    }
-
-    pub fn ensure_symlink<P: AsRef<Path>>(&self, name: P, target: &str) -> ErrnoResult<()> {
-        self.symlink(name, target).or_else(|e| match e {
-            Errno::EXIST => Ok(()),
-            _ => Err(e),
-        })
+        // Atomically replace existing symlink
+        replace_symlinkat(&relative, &self.repository, name)
     }
 
     fn read_symlink_hashvalue(dirfd: &OwnedFd, name: &CStr) -> Result<ObjectID> {
@@ -485,15 +483,28 @@ impl<ObjectID: FsVerityHashValue> Repository<ObjectID> {
     fn gc_category(&self, category: &str) -> Result<HashSet<ObjectID>> {
         let mut objects = HashSet::new();
 
-        let category_fd = self.openat(category, OFlags::RDONLY | OFlags::DIRECTORY)?;
+        let Some(category_fd) = filter_errno(
+            self.openat(category, OFlags::RDONLY | OFlags::DIRECTORY),
+            Errno::NOENT,
+        )
+        .context("Opening {category} dir in repository")?
+        else {
+            return Ok(objects);
+        };
 
-        let refs = openat(
-            &category_fd,
-            "refs",
-            OFlags::RDONLY | OFlags::DIRECTORY,
-            Mode::empty(),
-        )?;
-        Self::walk_symlinkdir(refs, &mut objects)?;
+        if let Some(refs) = filter_errno(
+            openat(
+                &category_fd,
+                "refs",
+                OFlags::RDONLY | OFlags::DIRECTORY,
+                Mode::empty(),
+            ),
+            Errno::NOENT,
+        )
+        .context("Opening {category}/refs dir in repository")?
+        {
+            Self::walk_symlinkdir(refs, &mut objects)?;
+        }
 
         for item in Dir::read_from(&category_fd)? {
             let entry = item?;

crates/composefs/src/util.rs

@@ -1,8 +1,17 @@
+use rand::{distr::Alphanumeric, Rng};
 use std::{
     io::{Error, ErrorKind, Read, Result},
-    os::fd::{AsFd, AsRawFd},
+    os::{
+        fd::{AsFd, AsRawFd, OwnedFd},
+        unix::ffi::OsStrExt,
+    },
+    path::Path,
 };
 
+use rustix::{
+    fs::{readlinkat, renameat, symlinkat, unlinkat, AtFlags},
+    io::{Errno, Result as ErrnoResult},
+};
 use tokio::io::{AsyncRead, AsyncReadExt};
 
 /// Formats a string like "/proc/self/fd/3" for the given fd.  This can be used to work with kernel
@@ -97,6 +106,66 @@ pub fn parse_sha256(string: impl AsRef<str>) -> Result<Sha256Digest> {
     Ok(value)
 }
 
+pub(crate) fn filter_errno<T>(
+    result: rustix::io::Result<T>,
+    ignored: Errno,
+) -> ErrnoResult<Option<T>> {
+    match result {
+        Ok(result) => Ok(Some(result)),
+        Err(err) if err == ignored => Ok(None),
+        Err(err) => Err(err),
+    }
+}
+
+fn generate_tmpname(prefix: &str) -> String {
+    let rand_string: String = rand::rng()
+        .sample_iter(&Alphanumeric)
+        .take(12)
+        .map(char::from)
+        .collect();
+    format!("{}{}", prefix, rand_string)
+}
+
+pub(crate) fn replace_symlinkat(
+    target: impl AsRef<Path>,
+    dirfd: &OwnedFd,
+    name: impl AsRef<Path>,
+) -> ErrnoResult<()> {
+    let name = name.as_ref();
+    let target = target.as_ref();
+
+    // Step 1: try to create the symlink
+    if filter_errno(symlinkat(target, dirfd, name), Errno::EXIST)?.is_some() {
+        return Ok(());
+    };
+
+    // Step 2: the symlink already exists.  Maybe it already has the correct target?
+    if let Some(current_target) = filter_errno(readlinkat(dirfd, name, []), Errno::NOENT)? {
+        if current_target.into_bytes() == target.as_os_str().as_bytes() {
+            return Ok(());
+        }
+    }
+
+    // Step 3: full atomic replace path
+    for _ in 0..16 {
+        let tmp_name = generate_tmpname(".symlink-");
+        if filter_errno(symlinkat(target, dirfd, &tmp_name), Errno::EXIST)?.is_none() {
+            // This temporary filename already exists, try another
+            continue;
+        }
+
+        match renameat(dirfd, &tmp_name, dirfd, name) {
+            Ok(_) => return Ok(()),
+            Err(e) => {
+                let _ = unlinkat(dirfd, tmp_name, AtFlags::empty());
+                return Err(e);
+            }
+        }
+    }
+
+    Err(Errno::EXIST)
+}
+
 #[cfg(test)]
 mod test {
     use similar_asserts::assert_eq;