This commit is contained in:
Dusty Mabe 2023-05-30 17:22:29 -04:00
parent 686cfc7fbc
commit 8eb35485b4
No known key found for this signature in database
GPG Key ID: 3302DBD73952E671
6 changed files with 252 additions and 1 deletions

View File

@ -0,0 +1,27 @@
From 632ffa430205473e6727e3402016ad8077164dea Mon Sep 17 00:00:00 2001
From: Jonathan Lebon <jonathan@jlebon.com>
Date: Sat, 27 May 2023 10:27:55 -0400
Subject: [PATCH 1/5] lib/deploy: Initialize var to pacify gcc static analysis
Classic case of analysis getting confused by variables initialized by
a function.
---
src/libostree/ostree-sysroot-deploy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c
index b87866ff..610fde5e 100644
--- a/src/libostree/ostree-sysroot-deploy.c
+++ b/src/libostree/ostree-sysroot-deploy.c
@@ -2534,7 +2534,7 @@ auto_early_prune_old_deployments (OstreeSysroot *self, GPtrArray *new_deployment
continue;
}
- guint64 bootdir_size;
+ guint64 bootdir_size = 0;
if (!get_kernel_layout_size (self, deployment, &bootdir_size, cancellable, error))
return FALSE;
--
2.40.1

View File

@ -0,0 +1,25 @@
From 115d5cf0730cd39b3a27799bd15dfb17ba97c9b4 Mon Sep 17 00:00:00 2001
From: Jonathan Lebon <jonathan@jlebon.com>
Date: Sat, 27 May 2023 10:33:39 -0400
Subject: [PATCH 2/5] lib/deploy: Drop unused variable
Noticed this diagnostic in my editor with clangd hooked up.
---
src/libostree/ostree-sysroot-deploy.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c
index 610fde5e..fbaf83c5 100644
--- a/src/libostree/ostree-sysroot-deploy.c
+++ b/src/libostree/ostree-sysroot-deploy.c
@@ -2910,7 +2910,6 @@ lint_deployment_fs (OstreeSysroot *self, OstreeDeployment *deployment, int deplo
g_auto (GLnxDirFdIterator) dfd_iter = {
0,
};
- glnx_autofd int dest_dfd = -1;
gboolean exists;
if (!ot_dfd_iter_init_allow_noent (deployment_dfd, "var", &dfd_iter, &exists, error))
--
2.40.1

View File

@ -0,0 +1,25 @@
From a3c0d6a3fee7958ebfb6f8bc3c70f053be5c3085 Mon Sep 17 00:00:00 2001
From: Jonathan Lebon <jonathan@jlebon.com>
Date: Sat, 27 May 2023 10:35:12 -0400
Subject: [PATCH 3/5] lib/deploy: Log case when auto-pruning is hopeless
For easier diagnostics.
---
src/libostree/ostree-sysroot-deploy.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c
index fbaf83c5..7de6fdf3 100644
--- a/src/libostree/ostree-sysroot-deploy.c
+++ b/src/libostree/ostree-sysroot-deploy.c
@@ -2578,6 +2578,7 @@ auto_early_prune_old_deployments (OstreeSysroot *self, GPtrArray *new_deployment
{
/* Even if we auto-pruned, the new bootdirs wouldn't fit. Just let the
* code continue and let it hit ENOSPC. */
+ g_printerr ("Disabling auto-prune optimization; insufficient space left in bootfs\n");
return TRUE;
}
--
2.40.1

View File

@ -0,0 +1,37 @@
From 76649127d1f415abdfd2e8de62ce8a2beef5f412 Mon Sep 17 00:00:00 2001
From: Jonathan Lebon <jonathan@jlebon.com>
Date: Sun, 28 May 2023 18:37:48 -0400
Subject: [PATCH 4/5] lib/deploy: Rename variable for clarity
`size_to_remove` looks cryptic in contrast to
`new_new_bootcsum_dirs_total_size`. Rename it in the style of the latter
for easier reading.
---
src/libostree/ostree-sysroot-deploy.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c
index 7de6fdf3..450f593e 100644
--- a/src/libostree/ostree-sysroot-deploy.c
+++ b/src/libostree/ostree-sysroot-deploy.c
@@ -2567,14 +2567,14 @@ auto_early_prune_old_deployments (OstreeSysroot *self, GPtrArray *new_deployment
/* OK, we would fail if we tried to write the new bootdirs. Is it salvageable?
* First, calculate how much space we could save with the bootcsums scheduled
* for removal. */
- guint64 size_to_remove = 0;
+ guint64 bootcsum_dirs_to_remove_total_size = 0;
GLNX_HASH_TABLE_FOREACH_KV (current_bootcsums, const char *, bootcsum, gpointer, sizep)
{
if (!g_hash_table_contains (new_bootcsums, bootcsum))
- size_to_remove += GPOINTER_TO_UINT (sizep);
+ bootcsum_dirs_to_remove_total_size += GPOINTER_TO_UINT (sizep);
}
- if (net_new_bootcsum_dirs_total_size > (available_size + size_to_remove))
+ if (net_new_bootcsum_dirs_total_size > (available_size + bootcsum_dirs_to_remove_total_size))
{
/* Even if we auto-pruned, the new bootdirs wouldn't fit. Just let the
* code continue and let it hit ENOSPC. */
--
2.40.1

View File

@ -0,0 +1,128 @@
From 193ef29f3f0886e3d59e6580e394f8817f2f123e Mon Sep 17 00:00:00 2001
From: Jonathan Lebon <jonathan@jlebon.com>
Date: Sat, 27 May 2023 10:37:30 -0400
Subject: [PATCH 5/5] lib/deploy: Use `fallocate` for early prune space check
The `f_bfree` member of the `statvfs` struct is documented as the
"number of free blocks". However, different filesystems have different
interpretations of this. E.g. on XFS, this is truly the number of blocks
free for allocating data. On ext4 however, it includes blocks that
are actually reserved by the filesystem and cannot be used for file
data. (Note this is separate from the distinction between `f_bfree` and
`f_bavail` which isn't relevant to us here since we're privileged.)
If a kernel and initrd is sized just right so that it's still within the
`f_bfree` limit but above what we can actually allocate, the early prune
code won't kick in since it'll think that there is enough space. So we
end up hitting `ENOSPC` when we actually copy the files in.
Rework the early prune code to instead use `fallocate` which guarantees
us that a file of a certain size can fit on the filesystem. `fallocate`
requires filesystem support, but all the filesystems we care about for
the bootfs support it (including even FAT).
(There's technically a TOCTOU race here that existed also with the
`statvfs` code where free space could change between when we check
and when we copy. Ideally we'd be able to pass down that fd to the
copying bits, but anyway in practice the bootfs is pretty much owned by
libostree and one doesn't expect concurrent writes during a finalization
operation.)
---
src/libostree/ostree-sysroot-deploy.c | 66 +++++++++++++++++++++------
1 file changed, 51 insertions(+), 15 deletions(-)
diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c
index 450f593e..425abe8b 100644
--- a/src/libostree/ostree-sysroot-deploy.c
+++ b/src/libostree/ostree-sysroot-deploy.c
@@ -2441,6 +2441,30 @@ get_kernel_layout_size (OstreeSysroot *self, OstreeDeployment *deployment, guint
return TRUE;
}
+/* This is a roundabout but more trustworthy way of doing a space check than
+ * relying on statvfs's f_bfree when you know the size of the objects. */
+static gboolean
+dfd_fallocate_check (int dfd, __off_t len, gboolean *out_passed, GError **error)
+{
+ g_auto (GLnxTmpfile) tmpf = {
+ 0,
+ };
+ if (!glnx_open_tmpfile_linkable_at (dfd, ".", O_WRONLY | O_CLOEXEC, &tmpf, error))
+ return FALSE;
+
+ *out_passed = TRUE;
+ /* There's glnx_try_fallocate, but not with the same error semantics. */
+ if (TEMP_FAILURE_RETRY (fallocate (tmpf.fd, 0, 0, len)) < 0)
+ {
+ if (G_IN_SET (errno, ENOSYS, EOPNOTSUPP))
+ return TRUE;
+ else if (errno != ENOSPC)
+ return glnx_throw_errno_prefix (error, "fallocate");
+ *out_passed = FALSE;
+ }
+ return TRUE;
+}
+
/* Analyze /boot and figure out if the new deployments won't fit in the
* remaining space. If they won't, check if deleting the deployments that are
* getting rotated out (e.g. the current rollback) would free up sufficient
@@ -2553,16 +2577,17 @@ auto_early_prune_old_deployments (OstreeSysroot *self, GPtrArray *new_deployment
net_new_bootcsum_dirs_total_size += bootdir_size;
}
- /* get bootfs free space */
- struct statvfs stvfsbuf;
- if (TEMP_FAILURE_RETRY (fstatvfs (self->boot_fd, &stvfsbuf)) < 0)
- return glnx_throw_errno_prefix (error, "fstatvfs(boot)");
-
- guint64 available_size = stvfsbuf.f_bsize * stvfsbuf.f_bfree;
-
- /* does the bootfs have enough free space for net-new bootdirs? */
- if (net_new_bootcsum_dirs_total_size <= available_size)
- return TRUE; /* nothing to do! */
+ {
+ gboolean bootfs_has_space = FALSE;
+ if (!dfd_fallocate_check (self->boot_fd, net_new_bootcsum_dirs_total_size, &bootfs_has_space,
+ error))
+ return glnx_prefix_error (error, "Checking if bootfs has space");
+
+ /* does the bootfs have enough free space for temporarily holding both the new
+ * and old bootdirs? */
+ if (bootfs_has_space)
+ return TRUE; /* nothing to do! */
+ }
/* OK, we would fail if we tried to write the new bootdirs. Is it salvageable?
* First, calculate how much space we could save with the bootcsums scheduled
@@ -2574,12 +2599,23 @@ auto_early_prune_old_deployments (OstreeSysroot *self, GPtrArray *new_deployment
bootcsum_dirs_to_remove_total_size += GPOINTER_TO_UINT (sizep);
}
- if (net_new_bootcsum_dirs_total_size > (available_size + bootcsum_dirs_to_remove_total_size))
+ if (net_new_bootcsum_dirs_total_size > bootcsum_dirs_to_remove_total_size)
{
- /* Even if we auto-pruned, the new bootdirs wouldn't fit. Just let the
- * code continue and let it hit ENOSPC. */
- g_printerr ("Disabling auto-prune optimization; insufficient space left in bootfs\n");
- return TRUE;
+ /* Check whether if we did early prune, we'd have enough space to write
+ * the new bootcsum dirs. */
+ gboolean bootfs_has_space = FALSE;
+ if (!dfd_fallocate_check (
+ self->boot_fd, net_new_bootcsum_dirs_total_size - bootcsum_dirs_to_remove_total_size,
+ &bootfs_has_space, error))
+ return glnx_prefix_error (error, "Checking if bootfs has space");
+
+ if (!bootfs_has_space)
+ {
+ /* Even if we auto-pruned, the new bootdirs wouldn't fit. Just let the
+ * code continue and let it hit ENOSPC. */
+ g_printerr ("Disabling auto-prune optimization; insufficient space left in bootfs\n");
+ return TRUE;
+ }
}
g_printerr ("Insufficient space left in bootfs; updating bootloader in two steps\n");
--
2.40.1

View File

@ -8,11 +8,17 @@
Summary: Tool for managing bootable, immutable filesystem trees
Name: ostree
Version: 2023.3
Release: 1%{?dist}
Release: 2%{?dist}
Source0: https://github.com/ostreedev/%{name}/releases/download/v%{version}/libostree-%{version}.tar.xz
License: LGPLv2+
URL: https://ostree.readthedocs.io/en/latest/
Patch1: 0001-lib-deploy-Initialize-var-to-pacify-gcc-static-analy.patch
Patch2: 0002-lib-deploy-Drop-unused-variable.patch
Patch3: 0003-lib-deploy-Log-case-when-auto-pruning-is-hopeless.patch
Patch4: 0004-lib-deploy-Rename-variable-for-clarity.patch
Patch5: 0005-lib-deploy-Use-fallocate-for-early-prune-space-check.patch
BuildRequires: make
BuildRequires: git
# We always run autogen.sh
@ -169,6 +175,9 @@ find %{buildroot} -name '*.la' -delete
%endif
%changelog
* Tue May 30 2023 Dusty Mabe <dusty@dustymabe.com> - 2023.3-2
- Backport OSTree Autoprune fixes in https://github.com/ostreedev/ostree/pull/2866
* Thu May 18 2023 Joseph Marrero <jmarrero@fedoraproject.org> - 2023.3-1
- https://github.com/ostreedev/ostree/releases/tag/v2023.3