From 8eb35485b4a398428443636518553b4193977bda Mon Sep 17 00:00:00 2001 From: Dusty Mabe Date: Tue, 30 May 2023 17:22:29 -0400 Subject: [PATCH] Backport OSTree Autoprune fixes in https://github.com/ostreedev/ostree/pull/2866 --- ...alize-var-to-pacify-gcc-static-analy.patch | 27 ++++ 0002-lib-deploy-Drop-unused-variable.patch | 25 ++++ ...g-case-when-auto-pruning-is-hopeless.patch | 25 ++++ ...b-deploy-Rename-variable-for-clarity.patch | 37 +++++ ...allocate-for-early-prune-space-check.patch | 128 ++++++++++++++++++ ostree.spec | 11 +- 6 files changed, 252 insertions(+), 1 deletion(-) create mode 100644 0001-lib-deploy-Initialize-var-to-pacify-gcc-static-analy.patch create mode 100644 0002-lib-deploy-Drop-unused-variable.patch create mode 100644 0003-lib-deploy-Log-case-when-auto-pruning-is-hopeless.patch create mode 100644 0004-lib-deploy-Rename-variable-for-clarity.patch create mode 100644 0005-lib-deploy-Use-fallocate-for-early-prune-space-check.patch diff --git a/0001-lib-deploy-Initialize-var-to-pacify-gcc-static-analy.patch b/0001-lib-deploy-Initialize-var-to-pacify-gcc-static-analy.patch new file mode 100644 index 0000000..8444961 --- /dev/null +++ b/0001-lib-deploy-Initialize-var-to-pacify-gcc-static-analy.patch @@ -0,0 +1,27 @@ +From 632ffa430205473e6727e3402016ad8077164dea Mon Sep 17 00:00:00 2001 +From: Jonathan Lebon +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 + diff --git a/0002-lib-deploy-Drop-unused-variable.patch b/0002-lib-deploy-Drop-unused-variable.patch new file mode 100644 index 0000000..0d9bcfa --- /dev/null +++ b/0002-lib-deploy-Drop-unused-variable.patch @@ -0,0 +1,25 @@ +From 115d5cf0730cd39b3a27799bd15dfb17ba97c9b4 Mon Sep 17 00:00:00 2001 +From: Jonathan Lebon +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 + diff --git a/0003-lib-deploy-Log-case-when-auto-pruning-is-hopeless.patch b/0003-lib-deploy-Log-case-when-auto-pruning-is-hopeless.patch new file mode 100644 index 0000000..73347fc --- /dev/null +++ b/0003-lib-deploy-Log-case-when-auto-pruning-is-hopeless.patch @@ -0,0 +1,25 @@ +From a3c0d6a3fee7958ebfb6f8bc3c70f053be5c3085 Mon Sep 17 00:00:00 2001 +From: Jonathan Lebon +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 + diff --git a/0004-lib-deploy-Rename-variable-for-clarity.patch b/0004-lib-deploy-Rename-variable-for-clarity.patch new file mode 100644 index 0000000..a9bc289 --- /dev/null +++ b/0004-lib-deploy-Rename-variable-for-clarity.patch @@ -0,0 +1,37 @@ +From 76649127d1f415abdfd2e8de62ce8a2beef5f412 Mon Sep 17 00:00:00 2001 +From: Jonathan Lebon +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 + diff --git a/0005-lib-deploy-Use-fallocate-for-early-prune-space-check.patch b/0005-lib-deploy-Use-fallocate-for-early-prune-space-check.patch new file mode 100644 index 0000000..acd8502 --- /dev/null +++ b/0005-lib-deploy-Use-fallocate-for-early-prune-space-check.patch @@ -0,0 +1,128 @@ +From 193ef29f3f0886e3d59e6580e394f8817f2f123e Mon Sep 17 00:00:00 2001 +From: Jonathan Lebon +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 + diff --git a/ostree.spec b/ostree.spec index 3da7cc0..a51a982 100644 --- a/ostree.spec +++ b/ostree.spec @@ -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 - 2023.3-2 +- Backport OSTree Autoprune fixes in https://github.com/ostreedev/ostree/pull/2866 + * Thu May 18 2023 Joseph Marrero - 2023.3-1 - https://github.com/ostreedev/ostree/releases/tag/v2023.3