state: Handle state file for GrubCC

Store the sysroot_path in StateLockGuard which we require for mounting
the ESP. Also, prevent mounting ESP at a tempdir, instead just mount it
at `/boot` which is what the BLS spec suggest

Fix issue with composefs systems where lsblk would fail to find the
backing device for "/" due to it being mounted as a virtual filesystem.
Instead we now use `/sysroot` or `/boot` to find the baking device

Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>

Author: Johan-Liebert1

src/backend/statefile.rs

@@ -1,36 +1,85 @@
 //! On-disk saved state.
 
 use crate::bootloader::Bootloader;
+use crate::bootupd::list_dev_current_root;
 use crate::efi::Efi;
 use crate::freezethaw::fsfreeze_thaw_cycle;
 use crate::model::SavedState;
 use crate::util::SignalTerminationGuard;
 use anyhow::{bail, Context, Result};
+use bootc_internal_blockdev::Device;
+use camino::Utf8PathBuf;
 use cap_std::ambient_authority;
 use cap_std::fs::{Dir, Permissions, PermissionsExt};
 use cap_std_ext::dirext::CapStdExtDirExt;
 use fn_error_context::context;
 use fs2::FileExt;
 use std::fs::File;
 use std::io::prelude::*;
-use std::os::fd::{AsRawFd, FromRawFd};
 use std::path::Path;
-use tempfile::tempdir;
+
+fn parse_statefile(statusf: cap_std::fs::File) -> Result<Option<SavedState>> {
+    let mut bufr = std::io::BufReader::new(statusf);
+    let mut s = String::new();
+    bufr.read_to_string(&mut s)?;
+    let state: serde_json::Result<SavedState> = serde_json::from_str(s.as_str());
+
+    let r = match state {
+        Ok(s) => s,
+        Err(orig_err) => {
+            let state: serde_json::Result<crate::model_legacy::SavedState01> =
+                serde_json::from_str(s.as_str());
+            match state {
+                Ok(s) => s.upconvert(),
+                Err(_) => {
+                    return Err(orig_err.into());
+                }
+            }
+        }
+    };
+
+    Ok(Some(r))
+}
+
+/// lsblk: composefs:abc123..: not a block device
+/// is what lsblk throws on composefs booted systems if we try to
+/// get block devices using "/"
+///
+/// First, try to get the device from the `root` which is necessary
+/// during installs as we don't want (or can't) to open up /sysroot or /boot
+///
+/// If that fails, it means we're not on the install path so we get the
+/// device from checking mount point from /boot or /sysroot
+#[context("Getting parent device")]
+fn get_parent_device(root: &Dir) -> Result<Device> {
+    match bootc_internal_blockdev::list_dev_by_dir(&root) {
+        Ok(d) => Ok(d),
+        Err(e) => {
+            // Not really an error just yet
+            log::debug!("{e:?}");
+            list_dev_current_root()
+        }
+    }
+}
 
 impl SavedState {
     /// System-wide bootupd write lock (relative to sysroot).
     const WRITE_LOCK_PATH: &'static str = "run/bootupd-lock";
     /// Top-level directory for statefile (relative to sysroot).
     pub(crate) const STATEFILE_DIR: &'static str = "boot";
-    /// On-disk bootloader statefile, akin to a tiny rpm/dpkg database, stored in `/boot`.
+    /// On-disk bootloader statefile, akin to a tiny rpm/dpkg database,
+    /// stored in `/boot` for Grub and in `ESP` for GrubCC
     pub(crate) const STATEFILE_NAME: &'static str = "bootupd-state.json";
 
     /// Try to acquire a system-wide lock to ensure non-conflicting state updates.
     ///
     /// While ordinarily the daemon runs as a systemd unit (which implicitly
     /// ensures a single instance) this is a double check against other
     /// execution paths.
-    pub(crate) fn acquire_write_lock(sysroot: Dir) -> Result<StateLockGuard> {
+    pub(crate) fn acquire_write_lock(
+        sysroot_path: Utf8PathBuf,
+        sysroot: Dir,
+    ) -> Result<StateLockGuard> {
         sysroot
             .atomic_write_with_perms(Self::WRITE_LOCK_PATH, "", Permissions::from_mode(0o644))
             .context("Creating lock file")?;
@@ -43,6 +92,7 @@ impl SavedState {
         lockfile.lock_exclusive().context("Acquiring lock")?;
 
         let guard = StateLockGuard {
+            sysroot_path,
             sysroot,
             termguard: Some(SignalTerminationGuard::new()?),
             lockfile: Some(lockfile),
@@ -52,8 +102,9 @@ impl SavedState {
 
     /// Use this for cases when the target root isn't booted, which is
     /// offline installs.
-    pub(crate) fn unlocked(sysroot: Dir) -> Result<StateLockGuard> {
+    pub(crate) fn unlocked(sysroot_path: Utf8PathBuf, sysroot: Dir) -> Result<StateLockGuard> {
         Ok(StateLockGuard {
+            sysroot_path,
             sysroot,
             termguard: None,
             lockfile: None,
@@ -67,70 +118,49 @@ impl SavedState {
         bootloader: Option<Bootloader>,
     ) -> Result<Option<SavedState>> {
         let root_path = root_path.as_ref();
-        let sysroot = Dir::open_ambient_dir(root_path, ambient_authority())
+
+        let root = Dir::open_ambient_dir(root_path, ambient_authority())
             .with_context(|| format!("opening sysroot '{}'", root_path.display()))?;
 
-        let (statefile, _esp_guard) = match bootloader {
+        match bootloader {
             Some(b) => match b {
                 Bootloader::Grub => {
                     let path = Path::new(Self::STATEFILE_DIR).join(Self::STATEFILE_NAME);
-                    (sysroot.open_optional(&path)?, None)
+
+                    match root.open_optional(&path)? {
+                        Some(f) => parse_statefile(f),
+                        None => Ok(None),
+                    }
                 }
 
                 Bootloader::GrubCC => {
                     let efi = Efi::default();
 
-                    let dir = Dir::open_ambient_dir(&root_path, ambient_authority())
-                        .with_context(|| format!("Opening filesystem path {root_path:?}"))?;
-                    let device = bootc_internal_blockdev::list_dev_by_dir(&dir)?;
+                    let device = get_parent_device(&root)?;
 
                     // Since we write the state file to all ESPs, it should be enough to get it
                     // from the first one. Though, we could check the integrity by getting from
                     // all the ESPs and making sure they're all the same...
                     let esp = device.find_first_colocated_esp()?;
 
-                    let tmpdir = tempdir()?;
-                    std::fs::create_dir_all(tmpdir.path().join("efi"))
-                        .context("Creating efi inside tmpdir")?;
-
+                    // According to BLS, the ESP should be mounted at /boot or /boot/efi
+                    // which the following method already checks
                     let mounted = efi
-                        .ensure_mounted_esp(tmpdir.path(), &Path::new(&esp.path()))
+                        .ensure_mounted_esp(&Path::new("/"), &Path::new(&esp.path()))
                         .context("Mounting ESP")?;
 
                     let dir = Dir::open_ambient_dir(&mounted, ambient_authority())?;
 
-                    (dir.open_optional(Self::STATEFILE_NAME)?, Some(efi))
+                    match dir.open_optional(Self::STATEFILE_NAME)? {
+                        Some(f) => parse_statefile(f),
+                        None => Ok(None),
+                    }
                 }
             },
 
             // No bootloader, we're probably running inside a container
-            None => (None, None),
-        };
-
-        let saved_state = if let Some(statusf) = statefile {
-            let mut bufr = std::io::BufReader::new(statusf);
-            let mut s = String::new();
-            bufr.read_to_string(&mut s)?;
-            let state: serde_json::Result<SavedState> = serde_json::from_str(s.as_str());
-            let r = match state {
-                Ok(s) => s,
-                Err(orig_err) => {
-                    let state: serde_json::Result<crate::model_legacy::SavedState01> =
-                        serde_json::from_str(s.as_str());
-                    match state {
-                        Ok(s) => s.upconvert(),
-                        Err(_) => {
-                            return Err(orig_err.into());
-                        }
-                    }
-                }
-            };
-            Some(r)
-        } else {
-            None
-        };
-
-        Ok(saved_state)
+            None => Ok(None),
+        }
     }
 
     /// Check whether statefile exists.
@@ -163,6 +193,7 @@ impl SavedState {
 /// Write-lock guard for statefile, protecting against concurrent state updates.
 #[derive(Debug)]
 pub(crate) struct StateLockGuard {
+    pub(crate) sysroot_path: Utf8PathBuf,
     pub(crate) sysroot: Dir,
     #[allow(dead_code)]
     termguard: Option<SignalTerminationGuard>,
@@ -192,33 +223,29 @@ impl StateLockGuard {
             return Ok(());
         }
 
-        let dir = unsafe { Dir::from_raw_fd(self.sysroot.as_raw_fd()) };
-        let device = bootc_internal_blockdev::list_dev_by_dir(&dir)?;
+        let device = get_parent_device(&self.sysroot)?;
         let all_esps = device
             .find_colocated_esps()
             .context("Searching for ESP")?
             .ok_or_else(|| anyhow::anyhow!("ESP not found"))?;
 
         let efi = Efi::default();
 
-        let tmpdir = tempdir()?;
-
-        // [`ensure_mounted_esp`] needs this
-        std::fs::create_dir_all(tmpdir.path().join("efi")).context("Creating efi inside tmpdir")?;
+        let serialized_state = serde_json::to_vec(state).context("Serializing state")?;
 
         for esp in all_esps {
             let mounted = efi
-                .ensure_mounted_esp(tmpdir.path(), &Path::new(&esp.path()))
+                .ensure_mounted_esp(&self.sysroot_path.as_std_path(), &Path::new(&esp.path()))
                 .context("Mounting ESP")?;
 
             let dir = Dir::open_ambient_dir(&mounted, ambient_authority())?;
 
-            dir.atomic_replace_with(SavedState::STATEFILE_NAME, |w| -> std::io::Result<()> {
-                serde_json::to_writer(w, state)?;
-                Ok(())
-            })?;
-
-            // dir.set_permissions(SavedState::STATEFILE_NAME, 0o644)?;
+            dir.atomic_write_with_perms(
+                SavedState::STATEFILE_NAME,
+                &serialized_state,
+                Permissions::from_mode(0o644),
+            )
+            .context("Writing state file")?;
 
             // Do the sync before unmount
             fsfreeze_thaw_cycle(dir.reopen_as_ownedfd()?)?;

src/bootupd.rs

@@ -216,8 +216,8 @@ pub(crate) fn install(
     // Unmount the ESP, etc.
     drop(target_components);
 
-    let mut state_guard =
-        SavedState::unlocked(sysroot.try_clone()?).context("failed to acquire write lock")?;
+    let mut state_guard = SavedState::unlocked(dest_root.into(), sysroot.try_clone()?)
+        .context("failed to acquire write lock")?;
     state_guard
         .update_state(&state, bootloader)
         .context("failed to update state")?;
@@ -374,8 +374,8 @@ pub(crate) fn update(name: &str, rootcxt: &RootContext) -> Result<ComponentUpdat
     let interrupted = pending_container.get(component.name()).cloned();
     pending_container.insert(component.name().into(), update.clone());
     let sysroot = sysroot.try_clone()?;
-    let mut state_guard =
-        SavedState::acquire_write_lock(sysroot).context("Failed to acquire write lock")?;
+    let mut state_guard = SavedState::acquire_write_lock(rootcxt.path.clone(), sysroot)
+        .context("Failed to acquire write lock")?;
     state_guard
         .update_state(&state, bootloader)
         .context("Failed to update state")?;
@@ -428,8 +428,8 @@ pub(crate) fn adopt_and_update(
     };
 
     let sysroot = sysroot.try_clone()?;
-    let mut state_guard =
-        SavedState::acquire_write_lock(sysroot).context("Failed to acquire write lock")?;
+    let mut state_guard = SavedState::acquire_write_lock(rootcxt.path.clone(), sysroot)
+        .context("Failed to acquire write lock")?;
 
     let inst = component
         .adopt_update(&rootcxt, &update, with_static_config)
@@ -458,7 +458,7 @@ pub(crate) fn adopt_and_update(
 /// then falling back to `/sysroot`. This avoids issues with virtual
 /// filesystems like composefs that are mounted on `/`.
 #[context("Finding block device from boot or sysroot")]
-fn list_dev_current_root() -> Result<Device> {
+pub(crate) fn list_dev_current_root() -> Result<Device> {
     let auth = cap_std::ambient_authority();
     for path in ["/boot", "/sysroot"] {
         if let Ok(dir) = Dir::open_ambient_dir(path, auth) {
@@ -488,13 +488,9 @@ pub(crate) fn status() -> Result<Status> {
     let mut known_components = get_components();
     let sysroot = Dir::open_ambient_dir("/", ambient_authority())?;
 
-    let bootloader = if running_in_container() {
-        None
-    } else {
-        Some(get_bootloader()?)
-    };
+    let bootloader = get_bootloader()?;
+    let state = SavedState::load_from_disk("/", Some(bootloader))?;
 
-    let state = SavedState::load_from_disk("/", bootloader)?;
     if let Some(state) = state {
         for (name, ic) in state.installed.iter() {
             log::trace!("Gathering status for installed component: {}", name);