From 6389f56a459dda2d8590726cc24bfd08041ff0bf Mon Sep 17 00:00:00 2001 From: HuijingHei Date: Fri, 24 Nov 2023 20:38:10 +0800 Subject: [PATCH] utils.rs: remove duplicate tmpfiles entries rpm-ostree should scan the tmpfiles.d snippets at install time and skip synthesizing entries if they are already specified. As we may have confilicted tmpfile entries, like it will prevent to cleanup of `/var/tmp` by `systemd-tmpfiles-clean.service`: `pkg-filesystem.conf` has `d /var/tmp 1777 root root - -` `systemd's tmp.conf` has `q /var/tmp 1777 root root 30d` It is not as simple because the auto-conversion we do at package import time may be duplicating a tmpfiles.d dropin that lives in a separate package. With Jonathan's suggestion: from rpmostree_context_assemble(), we call a function like `rpmostreecxx::deduplicate_tmpfiles_entries(tmprootfs_dfd)`, which scans the dropins, sorts them into two `HashMaps` and then unlink all the `pkg-*` files and create a single `pkg-rpm-ostree-autovar.conf` (re-using the same prefix ensures that this same logic works client-side). See https://github.com/coreos/rpm-ostree/issues/26#issuecomment-1823109683 Fixes https://github.com/coreos/rpm-ostree/issues/26 --- rpmostree-cxxrs.cxx | 14 ++++ rpmostree-cxxrs.h | 2 + rust/src/composepost.rs | 5 ++ rust/src/lib.rs | 1 + rust/src/utils.rs | 107 ++++++++++++++++++++++++++ src/app/rpm-ostree-0-integration.conf | 1 - src/libpriv/rpmostree-core.cxx | 5 ++ 7 files changed, 134 insertions(+), 1 deletion(-) diff --git a/rpmostree-cxxrs.cxx b/rpmostree-cxxrs.cxx index 4dd46f6923..338980008f 100644 --- a/rpmostree-cxxrs.cxx +++ b/rpmostree-cxxrs.cxx @@ -2921,6 +2921,9 @@ extern "C" void rpmostreecxx$cxxbridge1$cache_branch_to_nevra (::rust::Str nevra, ::rust::String *return$) noexcept; + ::rust::repr::PtrLen + rpmostreecxx$cxxbridge1$deduplicate_tmpfiles_entries (::std::int32_t rootfs) noexcept; + ::std::uint32_t rpmostreecxx$cxxbridge1$CxxGObjectArray$length (::rpmostreecxx::CxxGObjectArray &self) noexcept { @@ -6055,6 +6058,16 @@ cache_branch_to_nevra (::rust::Str nevra) noexcept rpmostreecxx$cxxbridge1$cache_branch_to_nevra (nevra, &return$.value); return ::std::move (return$.value); } + +void +deduplicate_tmpfiles_entries (::std::int32_t rootfs) +{ + ::rust::repr::PtrLen error$ = rpmostreecxx$cxxbridge1$deduplicate_tmpfiles_entries (rootfs); + if (error$.ptr) + { + throw ::rust::impl< ::rust::Error>::error (error$); + } +} } // namespace rpmostreecxx extern "C" @@ -6769,5 +6782,6 @@ Vec< ::rpmostreecxx::LockedPackage>::truncate (::std::size_t len) { return cxxbridge1$rust_vec$rpmostreecxx$LockedPackage$truncate (this, len); } + } // namespace cxxbridge1 } // namespace rust diff --git a/rpmostree-cxxrs.h b/rpmostree-cxxrs.h index ce564475a4..5b8320eff3 100644 --- a/rpmostree-cxxrs.h +++ b/rpmostree-cxxrs.h @@ -2043,4 +2043,6 @@ ::rpmostreecxx::GKeyFile *treefile_to_origin (::rpmostreecxx::Treefile const &tf void origin_validate_roundtrip (::rpmostreecxx::GKeyFile const &kf) noexcept; ::rust::String cache_branch_to_nevra (::rust::Str nevra) noexcept; + +void deduplicate_tmpfiles_entries (::std::int32_t rootfs); } // namespace rpmostreecxx diff --git a/rust/src/composepost.rs b/rust/src/composepost.rs index cd8e6dab01..5b9d106972 100644 --- a/rust/src/composepost.rs +++ b/rust/src/composepost.rs @@ -734,6 +734,11 @@ fn var_to_tmpfiles(rootfs: &Dir, cancellable: Option<&gio::Cancellable>) -> Resu // These two are part of systemd's var.tmp "var/log/wtmp", "var/log/btmp", + // dup in pkg-rpm-ostree-autovar.conf + "var/lib/alternatives", + // dup in rpm-ostree-0-integration.conf + "var/lib/rpm", + "var/lib/vagrant", ]; let pwdb = PasswdDB::populate_new(rootfs)?; diff --git a/rust/src/lib.rs b/rust/src/lib.rs index b18cd89122..2ada4c66ad 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -710,6 +710,7 @@ pub mod ffi { extern "Rust" { fn prepare_rpm_layering(rootfs: i32, merge_passwd_dir: &str) -> Result; fn complete_rpm_layering(rootfs: i32) -> Result<()>; + fn deduplicate_tmpfiles_entries(rootfs: i32) -> Result<()>; fn passwd_cleanup(rootfs: i32) -> Result<()>; fn migrate_group_except_root(rootfs: i32, preserved_groups: &Vec) -> Result<()>; fn migrate_passwd_except_root(rootfs: i32) -> Result<()>; diff --git a/rust/src/utils.rs b/rust/src/utils.rs index 380527c038..4b6f5e11cf 100644 --- a/rust/src/utils.rs +++ b/rust/src/utils.rs @@ -8,18 +8,23 @@ */ use crate::cxxrsutil::*; +use crate::ffiutil; use crate::variant_utils; use anyhow::{bail, Context, Result}; use camino::Utf8Path; +use cap_std::fs::{Dir, OpenOptions}; use cap_std::io_lifetimes::AsFilelike; +use fn_error_context::context; use glib::Variant; use once_cell::sync::Lazy; use ostree_ext::prelude::*; use ostree_ext::{glib, ostree}; use regex::Regex; +use rustix::fs::OpenOptionsExt; use std::borrow::Cow; use std::collections::{HashMap, HashSet}; use std::io::prelude::*; +use std::io::BufReader; use std::os::fd::OwnedFd; use std::os::unix::io::IntoRawFd; use std::path::Path; @@ -304,6 +309,108 @@ pub(crate) fn translate_path_for_ostree_impl(path: &str) -> Option { None } +#[context("Deduplicate tmpfiles entries")] +pub fn deduplicate_tmpfiles_entries(tmprootfs_dfd: i32) -> CxxResult<()> { + let tmprootfs_dfd = unsafe { ffiutil::ffi_dirfd(tmprootfs_dfd)? }; + static TMPFILESD: &str = "usr/lib/tmpfiles.d"; + static AUTOVAR_PATH: &str = "pkg-rpm-ostree-autovar.conf"; + let tmpfiles_dir = tmprootfs_dfd + .open_dir(TMPFILESD) + .context("readdir {TMPFILESD}")?; + + let mut auto_tmpfiles_list = Vec::new(); + let mut system_tmpfiles_list = Vec::new(); + + for entry in tmpfiles_dir.entries()? { + let entry = entry?; + let name = entry.file_name(); + let name = name.to_str().unwrap(); + // skip README + if name == "README" { + continue; + } + if name.starts_with("pkg-") { + auto_tmpfiles_list.push(format!("{name}")); + } else { + system_tmpfiles_list.push(format!("{name}")); + } + } + + if auto_tmpfiles_list.is_empty() || system_tmpfiles_list.is_empty() { + println!("Not found any auto-generated or system tmpfiles.d config"); + return Ok(()); + } + + // save all entries to hashmaps + let mut auto_tmpfiles_entries = + save_tmpfile_entries(&tmpfiles_dir, &auto_tmpfiles_list).unwrap(); + let system_tmpfiles_entries = + save_tmpfile_entries(&tmpfiles_dir, &system_tmpfiles_list).unwrap(); + + // remove duplicated entries in auto-generated tmpfiles.d, + // which are already in system tmpfiles + for key in system_tmpfiles_entries.keys() { + if auto_tmpfiles_entries.contains_key(key) { + auto_tmpfiles_entries.remove(key); + } + } + + { + // save the noduplicated entries + let auto_tmpfiles_entries_nodup: Vec = + auto_tmpfiles_entries.into_values().collect(); + + let mut openopts = OpenOptions::new(); + openopts.create(true).write(true).mode(0o644); + let mut path = tmpfiles_dir + .open_with(AUTOVAR_PATH, &openopts) + .with_context(|| format!("Creating {AUTOVAR_PATH}"))?; + + for entry in auto_tmpfiles_entries_nodup { + path.write_all(entry.as_bytes())?; + path.write_all(b"\n")?; + } + } + + // unlink all the auto-generated files + for path in &auto_tmpfiles_list { + // skip pkg-rpm-ostree-autovar.conf + if path == AUTOVAR_PATH { + continue; + } + tmpfiles_dir + .remove_file(path) + .with_context(|| format!("unlinkat({})", path))?; + } + Ok(()) +} + +#[context("Save tmpfiles entries to hashmap")] +pub(crate) fn save_tmpfile_entries( + tmpfiles_dir: &Dir, + var_list: &Vec, +) -> Result> { + let mut tmpflies_entries: HashMap = HashMap::new(); + for tmpflies in var_list.iter() { + let contents = tmpfiles_dir.open(tmpflies).map(BufReader::new)?; + + for (line_num, line) in contents.lines().enumerate() { + let input = line + .with_context(|| format!("failed to read tmpfiles entry at line {}", line_num))?; + + // Skip empty and comment lines + if input.is_empty() || input.starts_with('#') { + continue; + } + + let parts: Vec<&str> = input.split_whitespace().collect(); + let entry = parts.get(1).unwrap(); + tmpflies_entries.insert(entry.to_string(), input.to_string()); + } + } + Ok(tmpflies_entries) +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/app/rpm-ostree-0-integration.conf b/src/app/rpm-ostree-0-integration.conf index c5fd009a0b..1e9822ef29 100644 --- a/src/app/rpm-ostree-0-integration.conf +++ b/src/app/rpm-ostree-0-integration.conf @@ -14,5 +14,4 @@ d /var/usrlocal/share 0755 root root - d /var/usrlocal/src 0755 root root - d /var/mnt 0755 root root - d /run/media 0755 root root - -d /var/lib 0755 root root - L /var/lib/rpm - - - - ../../usr/share/rpm diff --git a/src/libpriv/rpmostree-core.cxx b/src/libpriv/rpmostree-core.cxx index 0c52c70e3f..908f238793 100644 --- a/src/libpriv/rpmostree-core.cxx +++ b/src/libpriv/rpmostree-core.cxx @@ -4442,6 +4442,11 @@ rpmostree_context_assemble (RpmOstreeContext *self, GCancellable *cancellable, G task->end (msg); } + /* Remove duplicated tmpfiles entries; + * see https://github.com/coreos/rpm-ostree/issues/26 + */ + ROSCXX_TRY (deduplicate_tmpfiles_entries (tmprootfs_dfd), error); + /* We want this to be the first error message if something went wrong * with a script; see https://github.com/projectatomic/rpm-ostree/pull/888 * (otherwise, on a script that did `rm -rf`, we'd fail first on the renameat below)