Skip to content

Commit

Permalink
postprocess: Oxidize directory size counting
Browse files Browse the repository at this point in the history
We're getting a weird OOM issue in RHCOS builds:
openshift/os#594
which looks like:
`fstatat(92/4682a3540a7d5f834f5159c024f412ff528600f8a683116ce99a1832251b67.filez): Cannot allocate memory`

Reading some code here I noticed that the "open directory" call
could follow symlinks when it shouldn't.  Let's just RIIR and
make this code more explicit - e.g. we ignore symlinks.

I'm pretty sure this is a no-op, but let's harden the code.
  • Loading branch information
cgwalters committed Jul 26, 2021
1 parent ff4be57 commit 085c135
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 44 deletions.
29 changes: 29 additions & 0 deletions rust/src/composepost.rs
Original file line number Diff line number Diff line change
Expand Up @@ -883,6 +883,35 @@ pub fn prepare_rpmdb_base_location(
Ok(())
}

/// Recurse into this directory and return the total size of all regular files.
pub fn directory_size(
dfd: i32,
mut cancellable: Pin<&mut crate::FFIGCancellable>,
) -> CxxResult<u64> {
let cancellable = &cancellable.gobj_wrap();
let dfd = crate::ffiutil::ffi_view_openat_dir(dfd);
fn directory_size_recurse(d: &openat::Dir, cancellable: &gio::Cancellable) -> Result<u64> {
let mut r = 0;
for ent in d.list_self()? {
cancellable.set_error_if_cancelled()?;
let ent = ent?;
let meta = d.metadata(ent.file_name())?;
match meta.simple_type() {
openat::SimpleType::Dir => {
let child = d.sub_dir(ent.file_name())?;
r += directory_size_recurse(&child, cancellable)?;
}
openat::SimpleType::File => {
r += meta.stat().st_size as u64;
}
_ => {},
}
}
Ok(r)
}
Ok(directory_size_recurse(&dfd, cancellable)?)
}

#[context("Hardlinking rpmdb to base location")]
fn hardlink_rpmdb_base_location(
rootfs: &openat::Dir,
Expand Down
3 changes: 3 additions & 0 deletions rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,9 @@ pub mod ffi {
) -> Result<()>;
fn compose_postprocess_rpm_macro(rootfs_dfd: i32) -> Result<()>;
fn rewrite_rpmdb_for_target(rootfs_dfd: i32) -> Result<()>;
fn directory_size(
dfd: i32,
mut cancellable: Pin<&mut GCancellable>) -> Result<u64>;
}

// A grab-bag of metadata from the deployment's ostree commit
Expand Down
45 changes: 1 addition & 44 deletions src/libpriv/rpmostree-postprocess.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -763,47 +763,6 @@ filter_xattrs_cb (OstreeRepo *repo,
}
}

static gboolean
count_filesizes (int dfd,
const char *path,
off_t *out_n_bytes,
GCancellable *cancellable,
GError **error)
{
g_auto(GLnxDirFdIterator) dfd_iter = { 0, };

if (!glnx_dirfd_iterator_init_at (dfd, path, TRUE, &dfd_iter, error))
return FALSE;

while (TRUE)
{
struct dirent *dent = NULL;

if (!glnx_dirfd_iterator_next_dent_ensure_dtype (&dfd_iter, &dent, cancellable, error))
return FALSE;
if (!dent)
break;

if (dent->d_type == DT_DIR)
{
if (!count_filesizes (dfd_iter.fd, dent->d_name, out_n_bytes,
cancellable, error))
return FALSE;
}
else
{
struct stat stbuf;

if (!glnx_fstatat (dfd_iter.fd, dent->d_name, &stbuf, AT_SYMLINK_NOFOLLOW, error))
return FALSE;

(*out_n_bytes) += stbuf.st_size;
}
}

return TRUE;
}

static gpointer
write_dfd_thread (gpointer datap)
{
Expand Down Expand Up @@ -884,9 +843,7 @@ rpmostree_compose_commit (int rootfs_fd,
if (devino_cache)
ostree_repo_commit_modifier_set_devino_cache (commit_modifier, devino_cache);

off_t n_bytes = 0;
if (!count_filesizes (rootfs_fd, ".", &n_bytes, cancellable, error))
return FALSE;
uint64_t n_bytes = rpmostreecxx::directory_size(rootfs_fd, *cancellable);

tdata.n_bytes = n_bytes;
tdata.repo = repo;
Expand Down

0 comments on commit 085c135

Please sign in to comment.