install/composefs: Use atomic writes

Use `atomic_write` from `cap_std` crate
Get rid of `opeanat` crate
Call `fsync` after write to disk

Signed-off-by: Johan-Liebert1 <[email protected]>

Author: Johan-Liebert1

crates/lib/Cargo.toml

@@ -56,8 +56,6 @@ tini = "1.3.0"
 comfy-table = "7.1.1"
 thiserror = { workspace = true }
 canon-json = { workspace = true }
-openat = "0.1.21"
-openat-ext = "0.2.3"
 
 [dev-dependencies]
 similar-asserts = { workspace = true }

crates/lib/src/deploy.rs

@@ -22,18 +22,17 @@ use ostree_ext::ostree::Deployment;
 use ostree_ext::ostree::{self, Sysroot};
 use ostree_ext::sysroot::SysrootLock;
 use ostree_ext::tokio_util::spawn_blocking_cancellable_flatten;
+use rustix::fs::{fsync, renameat_with, AtFlags, RenameFlags};
 
 use crate::bls_config::{parse_bls_config, BLSConfig};
 use crate::install::{get_efi_uuid_source, get_user_config, BootType};
 use crate::progress_jsonl::{Event, ProgressWriter, SubTaskBytes, SubTaskStep};
 use crate::spec::ImageReference;
-use crate::spec::{BootOrder, HostSpec, BootEntry};
+use crate::spec::{BootEntry, BootOrder, HostSpec};
 use crate::status::{composefs_deployment_status, labels_of_config};
 use crate::store::Storage;
 use crate::utils::async_task_with_spinner;
 
-use openat_ext::OpenatDirExt;
-
 // TODO use https://github.com/ostreedev/ostree-rs-ext/pull/493/commits/afc1837ff383681b947de30c0cefc70080a4f87a
 const BASE_IMAGE_PREFIX: &str = "ostree/container/baseimage/bootc";
 
@@ -743,7 +742,6 @@ pub(crate) async fn stage(
     Ok(())
 }
 
-
 #[context("Rolling back UKI")]
 pub(crate) fn rollback_composefs_uki(current: &BootEntry, rollback: &BootEntry) -> Result<()> {
     let user_cfg_name = "grub2/user.cfg.staged";
@@ -782,7 +780,8 @@ pub(crate) fn rollback_composefs_uki(current: &BootEntry, rollback: &BootEntry)
 
 /// Filename for `loader/entries`
 const CURRENT_ENTRIES: &str = "entries";
-const ROLLBACK_ENTRIES: &str = "entries.staged";
+const STAGED_ENTRIES: &str = "entries.staged";
+const ROLLBACK_ENTRIES: &str = STAGED_ENTRIES;
 
 #[context("Getting boot entries")]
 pub(crate) fn get_sorted_boot_entries(ascending: bool) -> Result<Vec<BLSConfig>> {
@@ -833,33 +832,50 @@ pub(crate) fn rollback_composefs_bls() -> Result<()> {
     let dir_path = PathBuf::from(format!("/sysroot/boot/loader/{ROLLBACK_ENTRIES}"));
     create_dir_all(&dir_path).with_context(|| format!("Failed to create dir: {dir_path:?}"))?;
 
+    let rollback_entries_dir =
+        cap_std::fs::Dir::open_ambient_dir(&dir_path, cap_std::ambient_authority())
+            .with_context(|| format!("Opening {dir_path:?}"))?;
+
     // Write the BLS configs in there
     for cfg in all_configs {
         let file_name = format!("bootc-composefs-{}.conf", cfg.version);
 
-        let mut file = std::fs::OpenOptions::new()
-            .create(true)
-            .write(true)
-            .open(dir_path.join(&file_name))
-            .with_context(|| format!("Opening {file_name}"))?;
-
-        file.write_all(cfg.to_string().as_bytes())
+        rollback_entries_dir
+            .atomic_write(&file_name, cfg.to_string().as_bytes())
             .with_context(|| format!("Writing to {file_name}"))?;
     }
 
+    // Should we sync after every write?
+    fsync(
+        rollback_entries_dir
+            .reopen_as_ownedfd()
+            .with_context(|| format!("Reopening {dir_path:?} as owned fd"))?,
+    )
+    .with_context(|| format!("fsync {dir_path:?}"))?;
+
     // Atomically exchange "entries" <-> "entries.rollback"
-    let dir = openat::Dir::open("/sysroot/boot/loader").context("Opening loader dir")?;
+    let dir = Dir::open_ambient_dir("/sysroot/boot/loader", cap_std::ambient_authority())
+        .context("Opening loader dir")?;
 
     tracing::debug!("Atomically exchanging for {ROLLBACK_ENTRIES} and {CURRENT_ENTRIES}");
-    dir.local_exchange(ROLLBACK_ENTRIES, CURRENT_ENTRIES)
-        .context("local exchange")?;
+    renameat_with(
+        &dir,
+        ROLLBACK_ENTRIES,
+        &dir,
+        CURRENT_ENTRIES,
+        RenameFlags::EXCHANGE,
+    )
+    .context("renameat")?;
 
     tracing::debug!("Removing {ROLLBACK_ENTRIES}");
-    dir.remove_all(ROLLBACK_ENTRIES)
-        .context("Removing entries.rollback")?;
+    rustix::fs::unlinkat(&dir, ROLLBACK_ENTRIES, AtFlags::REMOVEDIR).context("unlinkat")?;
 
     tracing::debug!("Syncing to disk");
-    dir.syncfs().context("syncfs")?;
+    fsync(
+        dir.reopen_as_ownedfd()
+            .with_context(|| format!("Reopening /sysroot/boot/loader as owned fd"))?,
+    )
+    .context("fsync")?;
 
     Ok(())
 }

crates/lib/src/install.rs

@@ -14,7 +14,6 @@ mod osbuild;
 pub(crate) mod osconfig;
 
 use std::collections::HashMap;
-use std::fmt::write;
 use std::fs::create_dir_all;
 use std::io::{Read, Write};
 use std::os::fd::{AsFd, AsRawFd};
@@ -1640,25 +1639,36 @@ pub(crate) fn setup_composefs_bls_boot(
             let path = boot_dir.join(&id_hex);
             create_dir_all(&path)?;
 
-            let vmlinuz_path = path.join("vmlinuz");
-            let initrd_path = path.join("initrd");
+            let entries_dir =
+                cap_std::fs::Dir::open_ambient_dir(&path, cap_std::ambient_authority())
+                    .with_context(|| format!("Opening {path}"))?;
 
-            std::fs::write(
-                &vmlinuz_path,
-                read_file(&usr_lib_modules_vmlinuz.vmlinuz, &repo).context("Reading vmlinuz")?,
-            )
-            .context("Writing vmlinuz to path")?;
+            entries_dir
+                .atomic_write(
+                    "vmlinuz",
+                    read_file(&usr_lib_modules_vmlinuz.vmlinuz, &repo)
+                        .context("Reading vmlinuz")?,
+                )
+                .context("Writing vmlinuz to path")?;
 
             if let Some(initramfs) = &usr_lib_modules_vmlinuz.initramfs {
-                std::fs::write(
-                    &initrd_path,
-                    read_file(initramfs, &repo).context("Reading initramfs")?,
-                )
-                .context("Writing initrd to path")?;
+                entries_dir
+                    .atomic_write(
+                        "initrd",
+                        read_file(initramfs, &repo).context("Reading initrd")?,
+                    )
+                    .context("Writing initrd to path")?;
             } else {
                 anyhow::bail!("initramfs not found");
             };
 
+            // Can't call fsync on O_PATH fds, so re-open it as a non O_PATH fd
+            let owned_fd = entries_dir
+                .reopen_as_ownedfd()
+                .context("Reopen as owned fd")?;
+
+            rustix::fs::fsync(owned_fd).context("fsync")?;
+
             BLSConfig {
                 title: Some(id_hex.clone()),
                 version: 1,
@@ -1682,18 +1692,27 @@ pub(crate) fn setup_composefs_bls_boot(
 
     create_dir_all(&entries_path).with_context(|| format!("Creating {:?}", entries_path))?;
 
-    std::fs::write(
-        entries_path.join(format!("bootc-composefs-{}.conf", bls_config.version)),
+    let loader_entries_dir =
+        cap_std::fs::Dir::open_ambient_dir(&entries_path, cap_std::ambient_authority())
+            .with_context(|| format!("Opening {entries_path}"))?;
+
+    loader_entries_dir.atomic_write(
+        format!("bootc-composefs-{}.conf", bls_config.version),
         bls_config.to_string().as_bytes(),
     )?;
 
     if let Some(booted_bls) = booted_bls {
-        std::fs::write(
-            entries_path.join(format!("bootc-composefs-{}.conf", booted_bls.version)),
+        loader_entries_dir.atomic_write(
+            format!("bootc-composefs-{}.conf", booted_bls.version),
             booted_bls.to_string().as_bytes(),
         )?;
     }
 
+    let owned_loader_entries_fd = loader_entries_dir
+        .reopen_as_ownedfd()
+        .context("Reopening as owned fd")?;
+    rustix::fs::fsync(owned_loader_entries_fd).context("fsync")?;
+
     Ok(())
 }
 
@@ -1801,11 +1820,23 @@ pub(crate) fn setup_composefs_uki_boot(
             }
 
             // Write the UKI to ESP
-            let efi_linux = mounted_esp.join("EFI/Linux");
-            create_dir_all(&efi_linux).context("Creating EFI/Linux")?;
+            let efi_linux_path = mounted_esp.join("EFI/Linux");
+            create_dir_all(&efi_linux_path).context("Creating EFI/Linux")?;
+
+            let efi_linux =
+                cap_std::fs::Dir::open_ambient_dir(&efi_linux_path, cap_std::ambient_authority())
+                    .with_context(|| format!("Opening {efi_linux_path:?}"))?;
 
-            let final_uki_path = efi_linux.join(format!("{}.efi", id.to_hex()));
-            std::fs::write(final_uki_path, uki).context("Writing UKI to final path")?;
+            efi_linux
+                .atomic_write(format!("{}.efi", id.to_hex()), uki)
+                .context("Writing UKI")?;
+
+            rustix::fs::fsync(
+                efi_linux
+                    .reopen_as_ownedfd()
+                    .context("Reopening as owned fd")?,
+            )
+            .context("fsync")?;
 
             boot_label
         }
@@ -1830,24 +1861,24 @@ pub(crate) fn setup_composefs_uki_boot(
     let efi_uuid_source = get_efi_uuid_source();
 
     let user_cfg_name = if is_upgrade {
-        "grub2/user.cfg.staged"
+        "user.cfg.staged"
     } else {
-        "grub2/user.cfg"
+        "user.cfg"
     };
-    let user_cfg_path = boot_dir.join(user_cfg_name);
+
+    let grub_dir =
+        cap_std::fs::Dir::open_ambient_dir(boot_dir.join("grub2"), cap_std::ambient_authority())
+            .context("opening boot/grub2")?;
 
     // Iterate over all available deployments, and generate a menuentry for each
     //
     // TODO: We might find a staged deployment here
     if is_upgrade {
-        let mut usr_cfg = std::fs::OpenOptions::new()
-            .write(true)
-            .create(true)
-            .open(user_cfg_path)
-            .with_context(|| format!("Opening {user_cfg_name}"))?;
+        let mut buffer = vec![];
 
-        usr_cfg.write_all(efi_uuid_source.as_bytes())?;
-        usr_cfg.write_all(get_user_config(&boot_label, &id.to_hex()).as_bytes())?;
+        // Shouldn't really fail so no context here
+        buffer.write_all(efi_uuid_source.as_bytes())?;
+        buffer.write_all(get_user_config(&boot_label, &id.to_hex()).as_bytes())?;
 
         // root_path here will be /sysroot
         for entry in std::fs::read_dir(root_path.join(STATE_DIR_RELATIVE))? {
@@ -1857,39 +1888,41 @@ pub(crate) fn setup_composefs_uki_boot(
             // SAFETY: Deployment file name shouldn't containg non UTF-8 chars
             let depl_file_name = depl_file_name.to_string_lossy();
 
-            usr_cfg.write_all(get_user_config(&boot_label, &depl_file_name).as_bytes())?;
+            buffer.write_all(get_user_config(&boot_label, &depl_file_name).as_bytes())?;
         }
 
+        grub_dir
+            .atomic_write(user_cfg_name, buffer)
+            .with_context(|| format!("Writing to {user_cfg_name}"))?;
+
+        rustix::fs::fsync(grub_dir.reopen_as_ownedfd()?).context("fsync")?;
+
         return Ok(());
     }
 
-    let efi_uuid_file_path = format!("grub2/{EFI_UUID_FILE}");
-
     // Open grub2/efiuuid.cfg and write the EFI partition fs-UUID in there
     // This will be sourced by grub2/user.cfg to be used for `--fs-uuid`
-    let mut efi_uuid_file = std::fs::OpenOptions::new()
-        .write(true)
-        .create(true)
-        .open(boot_dir.join(&efi_uuid_file_path))
-        .with_context(|| format!("Opening {efi_uuid_file_path}"))?;
-
     let esp_uuid = Task::new("blkid for ESP UUID", "blkid")
         .args(["-s", "UUID", "-o", "value", &esp_device])
         .read()?;
 
-    efi_uuid_file
-        .write_all(format!("set EFI_PART_UUID=\"{}\"", esp_uuid.trim()).as_bytes())
-        .with_context(|| format!("Writing to {efi_uuid_file_path}"))?;
+    grub_dir.atomic_write(
+        EFI_UUID_FILE,
+        format!("set EFI_PART_UUID=\"{}\"", esp_uuid.trim()).as_bytes(),
+    )?;
 
     // Write to grub2/user.cfg
-    let mut usr_cfg = std::fs::OpenOptions::new()
-        .write(true)
-        .create(true)
-        .open(user_cfg_path)
-        .with_context(|| format!("Opening {user_cfg_name}"))?;
-
-    usr_cfg.write_all(efi_uuid_source.as_bytes())?;
-    usr_cfg.write_all(get_user_config(&boot_label, &id.to_hex()).as_bytes())?;
+    let mut buffer = vec![];
+
+    // Shouldn't really fail so no context here
+    buffer.write_all(efi_uuid_source.as_bytes())?;
+    buffer.write_all(get_user_config(&boot_label, &id.to_hex()).as_bytes())?;
+
+    grub_dir
+        .atomic_write(user_cfg_name, buffer)
+        .with_context(|| format!("Writing to {user_cfg_name}"))?;
+
+    rustix::fs::fsync(grub_dir.reopen_as_ownedfd()?).context("fsync")?;
 
     Ok(())
 }