public inbox for git-commits@fedoraproject.org
help / color / mirror / Atom feed
* [rpms/ostree] rawhide: Backport changes for ostree min-free-space accounting
@ 2026-07-27 16:51 Dusty Mabe
0 siblings, 0 replies; only message in thread
From: Dusty Mabe @ 2026-07-27 16:51 UTC (permalink / raw)
To: git-commits
A new commit has been pushed.
Repo : rpms/ostree
Branch : rawhide
Commit : 029cd0b3ef04f81d4113ecf2fce04cd51e62b1f6
Author : Dusty Mabe <dusty@dustymabe.com>
Date : 2026-07-27T12:06:33-04:00
Stats : +385/-0 in 4 file(s)
URL : https://src.fedoraproject.org/rpms/ostree/c/029cd0b3ef04f81d4113ecf2fce04cd51e62b1f6?branch=rawhide
Log:
Backport changes for ostree min-free-space accounting
- https://github.com/ostreedev/ostree/pull/3614
- https://github.com/ostreedev/ostree/pull/3615
Assisted-By: <anthropic/claude-opus-4.6>
---
diff --git a/0001-lib-commit-Report-whether-committed-object-already-existed.patch b/0001-lib-commit-Report-whether-committed-object-already-existed.patch
new file mode 100644
index 0000000..788d8a4
--- /dev/null
+++ b/0001-lib-commit-Report-whether-committed-object-already-existed.patch
@@ -0,0 +1,152 @@
+From 974b5184cbd4b83e0423452acbb61b86d19ee160 Mon Sep 17 00:00:00 2001
+From: Jonathan Lebon <jlebon@redhat.com>
+Date: Wed, 22 Jul 2026 22:08:29 -0400
+Subject: [PATCH 1/3] lib/commit: Report whether committed object already
+ existed
+
+Add an `out_existed` output parameter to
+`_ostree_repo_commit_tmpf_final()` and `commit_loose_regfile_object()`.
+This requires switching from `GLNX_LINK_TMPFILE_NOREPLACE_IGNORE_EXIST`
+to `GLNX_LINK_TMPFILE_NOREPLACE` so we can detect `G_IO_ERROR_EXISTS`
+ourselves.
+
+No users for now. Prep for a follow-up patch using this.
+
+Assisted-by: AI
+---
+ src/libostree/ostree-repo-commit.c | 39 ++++++++++++++++++-----------
+ src/libostree/ostree-repo-private.h | 3 ++-
+ src/libostree/ostree-repo-pull.c | 4 +--
+ 3 files changed, 29 insertions(+), 17 deletions(-)
+
+diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c
+index b7773e1db1..498376090a 100644
+--- a/src/libostree/ostree-repo-commit.c
++++ b/src/libostree/ostree-repo-commit.c
+@@ -181,7 +181,8 @@ ot_security_smack_reset_fd (int fd)
+ /* Given an O_TMPFILE regular file, link it into place. */
+ gboolean
+ _ostree_repo_commit_tmpf_final (OstreeRepo *self, const char *checksum, OstreeObjectType objtype,
+- GLnxTmpfile *tmpf, GCancellable *cancellable, GError **error)
++ GLnxTmpfile *tmpf, gboolean *out_existed, GCancellable *cancellable,
++ GError **error)
+ {
+ char tmpbuf[_OSTREE_LOOSE_PATH_MAX];
+ _ostree_loose_path (tmpbuf, checksum, objtype, self->mode);
+@@ -193,10 +194,19 @@ _ostree_repo_commit_tmpf_final (OstreeRepo *self, const char *checksum, OstreeOb
+ if (!_ostree_tmpf_fsverity (self, tmpf, NULL, error))
+ return FALSE;
+
+- if (!glnx_link_tmpfile_at (tmpf, GLNX_LINK_TMPFILE_NOREPLACE_IGNORE_EXIST, dest_dfd, tmpbuf,
+- error))
+- return FALSE;
+- /* We're done with the fd */
++ gboolean existed = FALSE;
++ g_autoptr (GError) local_error = NULL;
++ if (!glnx_link_tmpfile_at (tmpf, GLNX_LINK_TMPFILE_NOREPLACE, dest_dfd, tmpbuf, &local_error))
++ {
++ if (!g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_EXISTS))
++ {
++ g_propagate_error (error, g_steal_pointer (&local_error));
++ return FALSE;
++ }
++ existed = TRUE;
++ }
++ if (out_existed)
++ *out_existed = existed;
+ glnx_tmpfile_clear (tmpf);
+ return TRUE;
+ }
+@@ -237,8 +247,8 @@ commit_path_final (OstreeRepo *self, const char *checksum, OstreeObjectType objt
+ */
+ static gboolean
+ commit_loose_regfile_object (OstreeRepo *self, const char *checksum, GLnxTmpfile *tmpf, guint32 uid,
+- guint32 gid, guint32 mode, GVariant *xattrs, GCancellable *cancellable,
+- GError **error)
++ guint32 gid, guint32 mode, GVariant *xattrs, gboolean *out_existed,
++ GCancellable *cancellable, GError **error)
+ {
+ if (self->mode == OSTREE_REPO_MODE_BARE)
+ {
+@@ -308,8 +318,8 @@ commit_loose_regfile_object (OstreeRepo *self, const char *checksum, GLnxTmpfile
+ return glnx_throw_errno_prefix (error, "fsync");
+ }
+
+- if (!_ostree_repo_commit_tmpf_final (self, checksum, OSTREE_OBJECT_TYPE_FILE, tmpf, cancellable,
+- error))
++ if (!_ostree_repo_commit_tmpf_final (self, checksum, OSTREE_OBJECT_TYPE_FILE, tmpf, out_existed,
++ cancellable, error))
+ return FALSE;
+
+ return TRUE;
+@@ -569,7 +579,7 @@ _ostree_repo_bare_content_commit (OstreeRepo *self, OstreeRepoBareContent *barew
+ return FALSE;
+
+ if (!commit_loose_regfile_object (self, checksum_buf, &real->tmpf, real->uid, real->gid,
+- real->mode, real->xattrs, cancellable, error))
++ real->mode, real->xattrs, NULL, cancellable, error))
+ return FALSE;
+
+ /* Let's have a guarantee that after commit the object is cleaned up */
+@@ -1168,7 +1178,7 @@ write_content_object (OstreeRepo *self, const char *expected_checksum, GInputStr
+ return FALSE;
+
+ /* This path is for regular files */
+- if (!commit_loose_regfile_object (self, actual_checksum, &tmpf, uid, gid, mode, xattrs,
++ if (!commit_loose_regfile_object (self, actual_checksum, &tmpf, uid, gid, mode, xattrs, NULL,
+ cancellable, error))
+ return FALSE;
+
+@@ -1389,7 +1399,8 @@ write_metadata_object (OstreeRepo *self, OstreeObjectType objtype, const char *e
+ return FALSE;
+
+ /* And commit it into place */
+- if (!_ostree_repo_commit_tmpf_final (self, actual_checksum, objtype, &tmpf, cancellable, error))
++ if (!_ostree_repo_commit_tmpf_final (self, actual_checksum, objtype, &tmpf, NULL, cancellable,
++ error))
+ return FALSE;
+
+ if (objtype == OSTREE_OBJECT_TYPE_COMMIT)
+@@ -4466,8 +4477,8 @@ import_one_object_direct (OstreeRepo *dest_repo, OstreeRepo *src_repo, const cha
+ (void)futimens (tmp_dest.fd, ts);
+ }
+
+- if (!_ostree_repo_commit_tmpf_final (dest_repo, checksum, objtype, &tmp_dest, cancellable,
+- error))
++ if (!_ostree_repo_commit_tmpf_final (dest_repo, checksum, objtype, &tmp_dest, NULL,
++ cancellable, error))
+ return FALSE;
+ }
+
+diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h
+index 77ea2a6bcc..ddaf5e7888 100644
+--- a/src/libostree/ostree-repo-private.h
++++ b/src/libostree/ostree-repo-private.h
+@@ -360,7 +360,8 @@ gboolean _ostree_repo_import_object (OstreeRepo *self, OstreeRepo *source, Ostre
+
+ gboolean _ostree_repo_commit_tmpf_final (OstreeRepo *self, const char *checksum,
+ OstreeObjectType objtype, GLnxTmpfile *tmpf,
+- GCancellable *cancellable, GError **error);
++ gboolean *out_existed, GCancellable *cancellable,
++ GError **error);
+
+ typedef struct
+ {
+diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
+index 9989415fa2..527ec2cb97 100644
+--- a/src/libostree/ostree-repo-pull.c
++++ b/src/libostree/ostree-repo-pull.c
+@@ -952,8 +952,8 @@ content_fetch_on_complete (GObject *object, GAsyncResult *result, gpointer user_
+ if (pull_data->trusted_http_direct)
+ {
+ g_assert (!verifying_bareuseronly);
+- if (!_ostree_repo_commit_tmpf_final (pull_data->repo, checksum, objtype, &tmpf, cancellable,
+- error))
++ if (!_ostree_repo_commit_tmpf_final (pull_data->repo, checksum, objtype, &tmpf, NULL,
++ cancellable, error))
+ goto out;
+ pull_data->n_fetched_content++;
+ }
+--
+2.49.0
+
diff --git a/0002-lib-commit-Fix-min-free-space-accounting-for-duplicate-content-objects.patch b/0002-lib-commit-Fix-min-free-space-accounting-for-duplicate-content-objects.patch
new file mode 100644
index 0000000..598c2b4
--- /dev/null
+++ b/0002-lib-commit-Fix-min-free-space-accounting-for-duplicate-content-objects.patch
@@ -0,0 +1,142 @@
+From 16e537cb8a9b69d8f254dfbf4abf4f0358251f56 Mon Sep 17 00:00:00 2001
+From: Jonathan Lebon <jlebon@redhat.com>
+Date: Wed, 22 Jul 2026 22:09:15 -0400
+Subject: [PATCH 2/3] lib/commit: Fix min-free-space accounting for duplicate
+ content objects
+
+First, in the _ostree_repo_bare_content_commit(), we never actually
+checked whether the file was already present in the repo before linking
+it in place. Do this so that we can no-op up front.
+
+Second, and the actual bug this patch is fixing: the min-free-space
+accounting in write_content_object() and
+_ostree_repo_bare_content_commit() optimistically reserves space from
+the running `txn.max_blocks` counter _before_ knowing whether we'll
+actually no-op or not based on the object already existing.
+
+Over many duplicate writes within a single transaction, the counter
+diverges from the actual free space and eventually hits zero, causing a
+spurious "min-free-space would be exceeded" error even with plenty of
+disk space available.
+
+This may be the source of the CI issue FCOS is hitting in
+https://github.com/coreos/fedora-coreos-config/pull/4265 due to the
+bootc SELinux relabeling done since
+https://github.com/bootc-dev/bootc/pull/2088.
+
+This patch saves the number of blocks reserved and credits them back in
+both code paths when a duplicate object is detected.
+
+This was heavily AI-guided, finding the bug, and then doing red-green
+testing towards the fix.
+
+Assisted-by: AI
+---
+ src/libostree/ostree-repo-commit.c | 41 ++++++++++++++++++++++--------
+ 1 file changed, 31 insertions(+), 10 deletions(-)
+
+diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c
+index 498376090a..f0fdbd57aa 100644
+--- a/src/libostree/ostree-repo-commit.c
++++ b/src/libostree/ostree-repo-commit.c
+@@ -550,6 +550,7 @@ _ostree_repo_bare_content_commit (OstreeRepo *self, OstreeRepoBareContent *barew
+ OstreeRealRepoBareContent *real = (OstreeRealRepoBareContent *)barewrite;
+ g_assert (real->initialized);
+
++ fsblkcnt_t object_blocks_reserved = 0;
+ if ((self->min_free_space_percent > 0 || self->min_free_space_mb > 0) && self->in_transaction)
+ {
+ struct stat st_buf;
+@@ -559,15 +560,15 @@ _ostree_repo_bare_content_commit (OstreeRepo *self, OstreeRepoBareContent *barew
+ g_mutex_lock (&self->txn_lock);
+ g_assert_cmpint (self->txn.blocksize, >, 0);
+
+- const fsblkcnt_t object_blocks = (st_buf.st_size / self->txn.blocksize) + 1;
+- if (object_blocks > self->txn.max_blocks)
++ object_blocks_reserved = (st_buf.st_size / self->txn.blocksize) + 1;
++ if (object_blocks_reserved > self->txn.max_blocks)
+ {
+ self->cleanup_stagedir = TRUE;
+ g_mutex_unlock (&self->txn_lock);
+ return throw_min_free_space_error (self, st_buf.st_size, error);
+ }
+ /* This is the main bit that needs mutex protection */
+- self->txn.max_blocks -= object_blocks;
++ self->txn.max_blocks -= object_blocks_reserved;
+ g_mutex_unlock (&self->txn_lock);
+ }
+
+@@ -578,9 +579,19 @@ _ostree_repo_bare_content_commit (OstreeRepo *self, OstreeRepoBareContent *barew
+ checksum_buf, error))
+ return FALSE;
+
++ gboolean obj_existed;
+ if (!commit_loose_regfile_object (self, checksum_buf, &real->tmpf, real->uid, real->gid,
+- real->mode, real->xattrs, NULL, cancellable, error))
++ real->mode, real->xattrs, &obj_existed, cancellable, error))
+ return FALSE;
++ /* If the object already existed, credit back the space reservation we
++ * made above — no new disk space was consumed.
++ */
++ if (obj_existed)
++ {
++ g_mutex_lock (&self->txn_lock);
++ self->txn.max_blocks += object_blocks_reserved;
++ g_mutex_unlock (&self->txn_lock);
++ }
+
+ /* Let's have a guarantee that after commit the object is cleaned up */
+ _ostree_repo_bare_content_cleanup (barewrite);
+@@ -969,21 +980,28 @@ write_content_object (OstreeRepo *self, const char *expected_checksum, GInputStr
+
+ (void)file_input_owned; // Conditionally owned
+
+- /* Free space check; only applies during transactions */
++ /* Free space check; only applies during transactions.
++ *
++ * We optimistically reserve space here before we know the object's
++ * checksum. If we later discover the object already exists in the repo
++ * (i.e. it is a duplicate), we credit the reservation back — see the
++ * have_obj block below.
++ */
++ fsblkcnt_t object_blocks_reserved = 0;
+ if ((self->min_free_space_percent > 0 || self->min_free_space_mb > 0) && self->in_transaction)
+ {
+ g_mutex_lock (&self->txn_lock);
+ g_assert_cmpint (self->txn.blocksize, >, 0);
+- const fsblkcnt_t object_blocks = (size / self->txn.blocksize) + 1;
+- if (object_blocks > self->txn.max_blocks)
++ object_blocks_reserved = (size / self->txn.blocksize) + 1;
++ if (object_blocks_reserved > self->txn.max_blocks)
+ {
+- guint64 bytes_required = (guint64)object_blocks * self->txn.blocksize;
++ guint64 bytes_required = (guint64)object_blocks_reserved * self->txn.blocksize;
+ self->cleanup_stagedir = TRUE;
+ g_mutex_unlock (&self->txn_lock);
+ return throw_min_free_space_error (self, bytes_required, error);
+ }
+ /* This is the main bit that needs mutex protection */
+- self->txn.max_blocks -= object_blocks;
++ self->txn.max_blocks -= object_blocks_reserved;
+ g_mutex_unlock (&self->txn_lock);
+ }
+
+@@ -1110,11 +1128,14 @@ write_content_object (OstreeRepo *self, const char *expected_checksum, GInputStr
+ if (!_ostree_repo_has_loose_object (self, actual_checksum, OSTREE_OBJECT_TYPE_FILE, &have_obj,
+ cancellable, error))
+ return FALSE;
+- /* If we already have it, just update the stats. */
++ /* If we already have it, just update the stats. Also credit back the
++ * free-space reservation we made above — no new disk space was consumed.
++ */
+ if (have_obj)
+ {
+ g_mutex_lock (&self->txn_lock);
+ self->txn.stats.content_objects_total++;
++ self->txn.max_blocks += object_blocks_reserved;
+ g_mutex_unlock (&self->txn_lock);
+
+ if (!_create_payload_link (self, actual_checksum, actual_payload_checksum, file_info,
+--
+2.49.0
+
diff --git a/0003-lib-commit-Fix-min-free-space-accounting-for-reflinked-content-objects.patch b/0003-lib-commit-Fix-min-free-space-accounting-for-reflinked-content-objects.patch
new file mode 100644
index 0000000..d9ee113
--- /dev/null
+++ b/0003-lib-commit-Fix-min-free-space-accounting-for-reflinked-content-objects.patch
@@ -0,0 +1,85 @@
+From 07547d29e05a7e6c321a8cc7101d0d7fd361f5a6 Mon Sep 17 00:00:00 2001
+From: Jonathan Lebon <jlebon@redhat.com>
+Date: Fri, 24 Jul 2026 13:02:58 -0400
+Subject: [PATCH 3/3] lib/commit: Fix min-free-space accounting for reflinked
+ content objects
+
+When writing a content object,
+create_regular_tmpfile_linkable_with_content() opportunistically tries
+FICLONE to reflink data from the input fd. The reflinked file shares its
+data blocks via copy-on-write, so the actual disk cost is just metadata.
+
+However, the min-free-space accounting in write_content_object()
+reserves the full file size in max_blocks upfront and never credits it
+back after a successful reflink. Over many files this causes max_blocks
+to diverge significantly from actual free space.
+
+Propagate the reflink status from
+create_regular_tmpfile_linkable_with_content back to
+write_content_object so it can credit back the reserved blocks,
+following the same pattern used for duplicate objects in 16e537cb
+("lib/commit: Fix min-free-space accounting for duplicate content
+objects").
+
+This should fix the CI issues in
+https://github.com/coreos/fedora-coreos-config/pull/4265.
+
+Heavily AI-guided investigation and resolution and red-green testing.
+
+Assisted-by: AI
+---
+ src/libostree/ostree-repo-commit.c | 22 +++++++++++++++++++---
+ 1 file changed, 19 insertions(+), 3 deletions(-)
+
+diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c
+index f0fdbd57aa..a634563905 100644
+--- a/src/libostree/ostree-repo-commit.c
++++ b/src/libostree/ostree-repo-commit.c
+@@ -617,8 +617,8 @@ _ostree_repo_bare_content_cleanup (OstreeRepoBareContent *regwrite)
+ static gboolean
+ create_regular_tmpfile_linkable_with_content (OstreeRepo *self, guint64 length,
+ GInputStream *original_input, GInputStream *input,
+- GLnxTmpfile *out_tmpf, GCancellable *cancellable,
+- GError **error)
++ GLnxTmpfile *out_tmpf, gboolean *out_was_reflinked,
++ GCancellable *cancellable, GError **error)
+ {
+ g_auto (GLnxTmpfile) tmpf = {
+ 0,
+@@ -670,6 +670,8 @@ create_regular_tmpfile_linkable_with_content (OstreeRepo *self, guint64 length,
+ if (!glnx_fchmod (tmpf.fd, 0644, error))
+ return FALSE;
+
++ if (out_was_reflinked)
++ *out_was_reflinked = did_clone;
+ *out_tmpf = tmpf;
+ tmpf.initialized = FALSE;
+ return TRUE;
+@@ -1032,9 +1034,23 @@ write_content_object (OstreeRepo *self, const char *expected_checksum, GInputStr
+ }
+ else if (repo_mode != OSTREE_REPO_MODE_ARCHIVE)
+ {
++ gboolean was_reflinked = FALSE;
+ if (!create_regular_tmpfile_linkable_with_content (self, size, input, file_input, &tmpf,
+- cancellable, error))
++ &was_reflinked, cancellable, error))
+ return FALSE;
++ /* We got a reflink, so we're not actually paying for data here. Credit back max_block
++ * reservation. In theory, we're still paying for inode metadata, but that's a variable cost
++ * depending on the actual filesystem. E.g. xfs allocates from a separate pool, not from data
++ * blocks, so it truly doesn't consume f_bavail. btrfs OTOH would but it wouldn't even cost a
++ * whole block. Trying to get this exactly right is probably not worth it since the actual
++ * amount is still trivial.
++ */
++ if (was_reflinked && object_blocks_reserved > 0)
++ {
++ g_mutex_lock (&self->txn_lock);
++ self->txn.max_blocks += object_blocks_reserved;
++ g_mutex_unlock (&self->txn_lock);
++ }
+ }
+ else
+ {
+--
+2.49.0
+
diff --git a/ostree.spec b/ostree.spec
index f2305a3..e8e7f10 100644
--- a/ostree.spec
+++ b/ostree.spec
@@ -19,6 +19,12 @@ URL: https://ostreedev.github.io/ostree/
ExcludeArch: %{ix86}
%endif
+# Backport https://github.com/ostreedev/ostree/pull/3614
+Patch0: 0001-lib-commit-Report-whether-committed-object-already-existed.patch
+Patch1: 0002-lib-commit-Fix-min-free-space-accounting-for-duplicate-content-objects.patch
+# Backport https://github.com/ostreedev/ostree/pull/3615
+Patch2: 0003-lib-commit-Fix-min-free-space-accounting-for-reflinked-content-objects.patch
+
BuildRequires: make
BuildRequires: git
# We always run autogen.sh
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2026-07-27 16:51 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-27 16:51 [rpms/ostree] rawhide: Backport changes for ostree min-free-space accounting Dusty Mabe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox