Merge pull request #1525 from cgwalters/store-no-deref

Some more refactoring of storage

Author: Johan-Liebert1

crates/lib/src/bootc_kargs.rs

@@ -116,7 +116,8 @@ pub(crate) fn get_kargs(
     fetched: &ImageState,
 ) -> Result<Vec<String>> {
     let cancellable = gio::Cancellable::NONE;
-    let repo = &sysroot.repo();
+    let ostree = sysroot.get_ostree()?;
+    let repo = &ostree.repo();
     let mut kargs = vec![];
     let sys_arch = std::env::consts::ARCH;
 
@@ -129,7 +130,7 @@ pub(crate) fn get_kargs(
     };
 
     // Get the kargs in kargs.d of the merge
-    let merge_root = &crate::utils::deployment_fd(sysroot, merge_deployment)?;
+    let merge_root = &crate::utils::deployment_fd(ostree, merge_deployment)?;
     let existing_kargs = get_kargs_in_root(merge_root, sys_arch)?;
 
     // Get the kargs in kargs.d of the pending image

crates/lib/src/boundimage.rs

@@ -13,7 +13,7 @@ use fn_error_context::context;
 use ostree_ext::containers_image_proxy;
 use ostree_ext::ostree::Deployment;
 
-use crate::imgstorage::PullMode;
+use crate::podstorage::{CStorage, PullMode};
 use crate::store::Storage;
 
 /// The path in a root for bound images; this directory should only contain
@@ -39,7 +39,7 @@ pub(crate) struct ResolvedBoundImage {
 
 /// Given a deployment, pull all container images it references.
 pub(crate) async fn pull_bound_images(sysroot: &Storage, deployment: &Deployment) -> Result<()> {
-    let bound_images = query_bound_images_for_deployment(sysroot, deployment)?;
+    let bound_images = query_bound_images_for_deployment(sysroot.get_ostree()?, deployment)?;
     pull_images(sysroot, bound_images).await
 }
 
@@ -158,7 +158,7 @@ pub(crate) async fn pull_images(
 
 #[context("Pulling bound images")]
 pub(crate) async fn pull_images_impl(
-    imgstore: &crate::imgstorage::Storage,
+    imgstore: &CStorage,
     bound_images: Vec<crate::boundimage::BoundImage>,
 ) -> Result<()> {
     let n = bound_images.len();

crates/lib/src/cli.rs

@@ -24,6 +24,7 @@ use ostree_ext::container as ostree_container;
 use ostree_ext::container_utils::ostree_booted;
 use ostree_ext::keyfileext::KeyFileExt;
 use ostree_ext::ostree;
+use ostree_ext::sysroot::SysrootLock;
 use schemars::schema_for;
 use serde::{Deserialize, Serialize};
 
@@ -778,13 +779,9 @@ fn has_soft_reboot_capability(deployment: Option<&crate::spec::BootEntry>) -> bo
 
 /// Prepare a soft reboot for the given deployment
 #[context("Preparing soft reboot")]
-fn prepare_soft_reboot(
-    sysroot: &crate::store::Storage,
-    deployment: &ostree::Deployment,
-) -> Result<()> {
+fn prepare_soft_reboot(sysroot: &SysrootLock, deployment: &ostree::Deployment) -> Result<()> {
     let cancellable = ostree::gio::Cancellable::NONE;
     sysroot
-        .sysroot
         .deployment_set_soft_reboot(deployment, false, cancellable)
         .context("Failed to prepare soft-reboot")?;
     Ok(())
@@ -829,7 +826,7 @@ where
 /// Handle soft reboot for staged deployments (used by upgrade and switch)
 #[context("Handling staged soft reboot")]
 fn handle_staged_soft_reboot(
-    sysroot: &crate::store::Storage,
+    sysroot: &SysrootLock,
     soft_reboot_mode: Option<SoftRebootMode>,
     host: &crate::spec::Host,
 ) -> Result<()> {
@@ -843,7 +840,7 @@ fn handle_staged_soft_reboot(
 
 /// Perform a soft reboot for a staged deployment
 #[context("Soft reboot staged deployment")]
-fn soft_reboot_staged(sysroot: &crate::store::Storage) -> Result<()> {
+fn soft_reboot_staged(sysroot: &SysrootLock) -> Result<()> {
     println!("Staged deployment is soft-reboot capable, preparing for soft-reboot...");
 
     let deployments_list = sysroot.deployments();
@@ -858,7 +855,7 @@ fn soft_reboot_staged(sysroot: &crate::store::Storage) -> Result<()> {
 
 /// Perform a soft reboot for a rollback deployment
 #[context("Soft reboot rollback deployment")]
-fn soft_reboot_rollback(sysroot: &crate::store::Storage) -> Result<()> {
+fn soft_reboot_rollback(sysroot: &SysrootLock) -> Result<()> {
     println!("Rollback deployment is soft-reboot capable, preparing for soft-reboot...");
 
     let deployments_list = sysroot.deployments();
@@ -910,9 +907,9 @@ fn prepare_for_write() -> Result<()> {
 #[context("Upgrading")]
 async fn upgrade(opts: UpgradeOpts) -> Result<()> {
     let sysroot = &get_storage().await?;
-    let repo = &sysroot.repo();
-    let (booted_deployment, _deployments, host) =
-        crate::status::get_status_require_booted(sysroot)?;
+    let ostree = sysroot.get_ostree()?;
+    let repo = &ostree.repo();
+    let (booted_deployment, _deployments, host) = crate::status::get_status_require_booted(ostree)?;
     let imgref = host.spec.image.as_ref();
     let prog: ProgressWriter = opts.progress.try_into()?;
 
@@ -988,7 +985,7 @@ async fn upgrade(opts: UpgradeOpts) -> Result<()> {
             .unwrap_or_default();
         if staged_unchanged {
             println!("Staged update present, not changed.");
-            handle_staged_soft_reboot(sysroot, opts.soft_reboot, &host)?;
+            handle_staged_soft_reboot(ostree, opts.soft_reboot, &host)?;
             if opts.apply {
                 crate::reboot::reboot()?;
             }
@@ -1013,8 +1010,8 @@ async fn upgrade(opts: UpgradeOpts) -> Result<()> {
         if opts.soft_reboot.is_some() {
             // At this point we have new staged deployment and the host definition has changed.
             // We need the updated host status before we check if we can prepare the soft-reboot.
-            let updated_host = crate::status::get_status(sysroot, Some(&booted_deployment))?.1;
-            handle_staged_soft_reboot(sysroot, opts.soft_reboot, &updated_host)?;
+            let updated_host = crate::status::get_status(ostree, Some(&booted_deployment))?.1;
+            handle_staged_soft_reboot(ostree, opts.soft_reboot, &updated_host)?;
         }
 
         if opts.apply {
@@ -1058,9 +1055,9 @@ async fn switch(opts: SwitchOpts) -> Result<()> {
     let cancellable = gio::Cancellable::NONE;
 
     let sysroot = &get_storage().await?;
-    let repo = &sysroot.repo();
-    let (booted_deployment, _deployments, host) =
-        crate::status::get_status_require_booted(sysroot)?;
+    let ostree = sysroot.get_ostree()?;
+    let repo = &ostree.repo();
+    let (booted_deployment, _deployments, host) = crate::status::get_status_require_booted(ostree)?;
 
     let new_spec = {
         let mut new_spec = host.spec.clone();
@@ -1095,8 +1092,8 @@ async fn switch(opts: SwitchOpts) -> Result<()> {
     if opts.soft_reboot.is_some() {
         // At this point we have staged the deployment and the host definition has changed.
         // We need the updated host status before we check if we can prepare the soft-reboot.
-        let updated_host = crate::status::get_status(sysroot, Some(&booted_deployment))?.1;
-        handle_staged_soft_reboot(sysroot, opts.soft_reboot, &updated_host)?;
+        let updated_host = crate::status::get_status(ostree, Some(&booted_deployment))?.1;
+        handle_staged_soft_reboot(ostree, opts.soft_reboot, &updated_host)?;
     }
 
     if opts.apply {
@@ -1110,17 +1107,18 @@ async fn switch(opts: SwitchOpts) -> Result<()> {
 #[context("Rollback")]
 async fn rollback(opts: RollbackOpts) -> Result<()> {
     let sysroot = &get_storage().await?;
+    let ostree = sysroot.get_ostree()?;
     crate::deploy::rollback(sysroot).await?;
 
     if opts.soft_reboot.is_some() {
         // Get status of rollback deployment to check soft-reboot capability
-        let host = crate::status::get_status_require_booted(sysroot)?.2;
+        let host = crate::status::get_status_require_booted(ostree)?.2;
 
         handle_soft_reboot(
             opts.soft_reboot,
             host.status.rollback.as_ref(),
             "rollback",
-            || soft_reboot_rollback(sysroot),
+            || soft_reboot_rollback(ostree),
         )?;
     }
 
@@ -1135,10 +1133,10 @@ async fn rollback(opts: RollbackOpts) -> Result<()> {
 #[context("Editing spec")]
 async fn edit(opts: EditOpts) -> Result<()> {
     let sysroot = &get_storage().await?;
-    let repo = &sysroot.repo();
+    let ostree = sysroot.get_ostree()?;
+    let repo = &ostree.repo();
 
-    let (booted_deployment, _deployments, host) =
-        crate::status::get_status_require_booted(sysroot)?;
+    let (booted_deployment, _deployments, host) = crate::status::get_status_require_booted(ostree)?;
     let new_host: Host = if let Some(filename) = opts.filename {
         let mut r = std::io::BufReader::new(std::fs::File::open(filename)?);
         serde_yaml::from_reader(&mut r)?

crates/lib/src/deploy.rs

@@ -304,10 +304,11 @@ async fn handle_layer_progress_print(
 /// Gather all bound images in all deployments, then prune the image store,
 /// using the gathered images as the roots (that will not be GC'd).
 pub(crate) async fn prune_container_store(sysroot: &Storage) -> Result<()> {
-    let deployments = sysroot.deployments();
+    let ostree = sysroot.get_ostree()?;
+    let deployments = ostree.deployments();
     let mut all_bound_images = Vec::new();
     for deployment in deployments {
-        let bound = crate::boundimage::query_bound_images_for_deployment(sysroot, &deployment)?;
+        let bound = crate::boundimage::query_bound_images_for_deployment(ostree, &deployment)?;
         all_bound_images.extend(bound.into_iter());
     }
     // Convert to a hashset of just the image names
@@ -463,11 +464,11 @@ pub(crate) async fn cleanup(sysroot: &Storage) -> Result<()> {
     let bound_prune = prune_container_store(sysroot);
 
     // We create clones (just atomic reference bumps) here to move to the thread.
-    let repo = sysroot.repo();
-    let sysroot = sysroot.sysroot.clone();
+    let ostree = sysroot.get_ostree_cloned()?;
+    let repo = ostree.repo();
     let repo_prune =
         ostree_ext::tokio_util::spawn_blocking_cancellable_flatten(move |cancellable| {
-            let locked_sysroot = &SysrootLock::from_assumed_locked(&sysroot);
+            let locked_sysroot = &SysrootLock::from_assumed_locked(&ostree);
             let cancellable = Some(cancellable);
             let repo = &repo;
             let txn = repo.auto_transaction(cancellable)?;
@@ -488,7 +489,7 @@ pub(crate) async fn cleanup(sysroot: &Storage) -> Result<()> {
 
             // Then, for each deployment which is derived (e.g. has configmaps) we synthesize
             // a base ref to ensure that it's not GC'd.
-            for (i, deployment) in sysroot.deployments().into_iter().enumerate() {
+            for (i, deployment) in ostree.deployments().into_iter().enumerate() {
                 let commit = deployment.csum();
                 if let Some(base) = get_base_commit(repo, &commit)? {
                     repo.transaction_set_refspec(&format!("{BASE_IMAGE_PREFIX}/{i}"), Some(&base));
@@ -543,7 +544,7 @@ async fn deploy(
         None
     };
     // Clone all the things to move to worker thread
-    let sysroot_clone = sysroot.sysroot.clone();
+    let ostree = sysroot.get_ostree_cloned()?;
     // ostree::Deployment is incorrectly !Send 😢 so convert it to an integer
     let merge_deployment = merge_deployment.map(|d| d.index() as usize);
     let stateroot = stateroot.to_string();
@@ -553,7 +554,7 @@ async fn deploy(
     let r = async_task_with_spinner(
         "Deploying",
         spawn_blocking_cancellable_flatten(move |cancellable| -> Result<_> {
-            let sysroot = sysroot_clone;
+            let ostree = ostree;
             let stateroot = Some(stateroot);
             let mut opts = ostree::SysrootDeployTreeOpts::default();
 
@@ -565,11 +566,11 @@ async fn deploy(
             if let Some(kargs) = override_kargs.as_deref() {
                 opts.override_kernel_argv = Some(&kargs);
             }
-            let deployments = sysroot.deployments();
+            let deployments = ostree.deployments();
             let merge_deployment = merge_deployment.map(|m| &deployments[m]);
             let origin = glib::KeyFile::new();
             origin.load_from_data(&origin_data, glib::KeyFileFlags::NONE)?;
-            let d = sysroot.stage_tree_with_options(
+            let d = ostree.stage_tree_with_options(
                 stateroot.as_deref(),
                 &ostree_commit,
                 Some(&origin),
@@ -582,7 +583,8 @@ async fn deploy(
     )
     .await?;
     // SAFETY: We must have a staged deployment
-    let staged = sysroot.staged_deployment().unwrap();
+    let ostree = sysroot.get_ostree()?;
+    let staged = ostree.staged_deployment().unwrap();
     assert_eq!(staged.index(), r);
     Ok(staged)
 }
@@ -608,6 +610,7 @@ pub(crate) async fn stage(
     spec: &RequiredHostSpec<'_>,
     prog: ProgressWriter,
 ) -> Result<()> {
+    let ostree = sysroot.get_ostree()?;
     let mut subtask = SubTaskStep {
         subtask: "merging".into(),
         description: "Merging Image".into(),
@@ -629,7 +632,7 @@ pub(crate) async fn stage(
             .collect(),
     })
     .await;
-    let merge_deployment = sysroot.merge_deployment(Some(stateroot));
+    let merge_deployment = ostree.merge_deployment(Some(stateroot));
 
     subtask.completed = true;
     subtasks.push(subtask.clone());
@@ -740,15 +743,17 @@ pub(crate) async fn stage(
 /// Implementation of rollback functionality
 pub(crate) async fn rollback(sysroot: &Storage) -> Result<()> {
     const ROLLBACK_JOURNAL_ID: &str = "26f3b1eb24464d12aa5e7b544a6b5468";
-    let repo = &sysroot.repo();
-    let (booted_deployment, deployments, host) = crate::status::get_status_require_booted(sysroot)?;
+    let ostree = sysroot.get_ostree()?;
+    let (booted_deployment, deployments, host) = crate::status::get_status_require_booted(ostree)?;
 
     let new_spec = {
         let mut new_spec = host.spec.clone();
         new_spec.boot_order = new_spec.boot_order.swap();
         new_spec
     };
 
+    let repo = &ostree.repo();
+
     // Just to be sure
     host.spec.verify_transition(&new_spec)?;
 
@@ -788,7 +793,7 @@ pub(crate) async fn rollback(sysroot: &Storage) -> Result<()> {
         .chain(deployments.other)
         .collect::<Vec<_>>();
     tracing::debug!("Writing new deployments: {new_deployments:?}");
-    sysroot.write_deployments(&new_deployments, gio::Cancellable::NONE)?;
+    ostree.write_deployments(&new_deployments, gio::Cancellable::NONE)?;
     if reverting {
         println!("Next boot: current deployment");
     } else {

crates/lib/src/fsck.rs

@@ -106,8 +106,9 @@ static CHECK_RESOLVCONF: FsckCheck =
 /// But at the current time fsck is an experimental feature that we should only be running
 /// in our CI.
 fn check_resolvconf(storage: &Storage) -> FsckResult {
+    let ostree = storage.get_ostree()?;
     // For now we only check the booted deployment.
-    if storage.booted_deployment().is_none() {
+    if ostree.booted_deployment().is_none() {
         return fsck_ok();
     }
     // Read usr/etc/resolv.conf directly.
@@ -240,7 +241,8 @@ fn check_fsverity(storage: &Storage) -> Pin<Box<dyn Future<Output = FsckResult>
 }
 
 async fn check_fsverity_inner(storage: &Storage) -> FsckResult {
-    let repo = &storage.repo();
+    let ostree = storage.get_ostree()?;
+    let repo = &ostree.repo();
     let verity_state = ostree_ext::fsverity::is_verity_enabled(repo)?;
     tracing::debug!(
         "verity: expected={:?} found={:?}",
@@ -249,7 +251,7 @@ async fn check_fsverity_inner(storage: &Storage) -> FsckResult {
     );
 
     let verity_found_state =
-        verity_state_of_all_objects(&storage.repo(), verity_state.desired == Tristate::Enabled)
+        verity_state_of_all_objects(&ostree.repo(), verity_state.desired == Tristate::Enabled)
             .await?;
     let Some((missing, rest)) = collect_until(
         verity_found_state.missing.iter(),

crates/lib/src/image.rs

@@ -14,7 +14,7 @@ use serde::Serialize;
 use crate::{
     boundimage::query_bound_images,
     cli::{ImageListFormat, ImageListType},
-    imgstorage::ensure_floating_c_storage_initialized,
+    podstorage::{ensure_floating_c_storage_initialized, CStorage},
 };
 
 /// The name of the image we push to containers-storage if nothing is specified.
@@ -42,7 +42,8 @@ struct ImageOutput {
 
 #[context("Listing host images")]
 fn list_host_images(sysroot: &crate::store::Storage) -> Result<Vec<ImageOutput>> {
-    let repo = sysroot.repo();
+    let ostree = sysroot.get_ostree()?;
+    let repo = ostree.repo();
     let images = ostree_ext::container::store::list_images(&repo).context("Querying images")?;
 
     Ok(images
@@ -129,8 +130,8 @@ pub(crate) async fn list_entrypoint(
 pub(crate) async fn push_entrypoint(source: Option<&str>, target: Option<&str>) -> Result<()> {
     let transport = Transport::ContainerStorage;
     let sysroot = crate::cli::get_storage().await?;
-
-    let repo = &sysroot.repo();
+    let ostree = sysroot.get_ostree()?;
+    let repo = &ostree.repo();
 
     // If the target isn't specified, push to containers-storage + our default image
     let target = if let Some(target) = target {
@@ -150,7 +151,7 @@ pub(crate) async fn push_entrypoint(source: Option<&str>, target: Option<&str>)
     let source = if let Some(source) = source {
         ImageReference::try_from(source).context("Parsing source image")?
     } else {
-        let status = crate::status::get_status_require_booted(&sysroot)?;
+        let status = crate::status::get_status_require_booted(&ostree)?;
         // SAFETY: We know it's booted
         let booted = status.2.status.booted.unwrap();
         let booted_image = booted.image.unwrap().image;
@@ -171,7 +172,7 @@ pub(crate) async fn push_entrypoint(source: Option<&str>, target: Option<&str>)
 /// Thin wrapper for invoking `podman image <X>` but set up for our internal
 /// image store (as distinct from /var/lib/containers default).
 pub(crate) async fn imgcmd_entrypoint(
-    storage: &crate::imgstorage::Storage,
+    storage: &CStorage,
     arg: &str,
     args: &[std::ffi::OsString],
 ) -> std::result::Result<(), anyhow::Error> {