Colin Walters 2024-01-21 11:03:23 -05:00
parent b6a78f3a65
commit de30b9d6bc
6 changed files with 3 additions and 317 deletions

1
.gitignore vendored
View File

@ -104,3 +104,4 @@
/libostree-2023.6.tar.xz
/libostree-2023.7.tar.xz
/libostree-2023.8.tar.xz
/libostree-2024.1.tar.xz

View File

@ -1,211 +0,0 @@
From 529ab041d887b4e21be7a30d1187ac9bab92b55f Mon Sep 17 00:00:00 2001
From: Jonathan Lebon <jonathan@jlebon.com>
Date: Thu, 4 Jan 2024 11:14:39 -0500
Subject: [PATCH 1/2] lib/deploy: Round to block size in early prune space
check
When we estimate how much space a new bootcsum dir will use, we
weren't accounting for the space overhead from files not using the
last filesystem block completely. This doesn't matter much if counting
a few files, but e.g. on FCOS aarch64, we include lots of small
devicetree blobs in the bootfs. That loss can add up to enough for the
`fallocate()` check to pass but copying still hitting `ENOSPC` later on.
I think a better fix here is to change approach entirely and instead
refactor `install_deployment_kernel()` so that we can call just the
copying bits of it as part of the early prune logic. We'll get a more
accurate assessment and it's not lost work since we won't need to
recopy later on. Also this would not require having to keep in sync the
estimator and the install bits.
That said, this is blocking FCOS releases, so I went with a more tactical
fix for now.
Fixes: https://github.com/coreos/fedora-coreos-tracker/issues/1637
---
src/libostree/ostree-sysroot-deploy.c | 42 +++++++++++++++++----------
src/libotutil/ot-fs-utils.c | 18 ++++++++----
src/libotutil/ot-fs-utils.h | 4 +--
3 files changed, 42 insertions(+), 22 deletions(-)
diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c
index a809d560..78c47e76 100644
--- a/src/libostree/ostree-sysroot-deploy.c
+++ b/src/libostree/ostree-sysroot-deploy.c
@@ -2450,7 +2450,8 @@ write_deployments_finish (OstreeSysroot *self, GCancellable *cancellable, GError
}
static gboolean
-add_file_size_if_nonnull (int dfd, const char *path, guint64 *inout_size, GError **error)
+add_file_size_if_nonnull (int dfd, const char *path, guint64 blocksize, guint64 *inout_size,
+ GError **error)
{
if (path == NULL)
return TRUE;
@@ -2460,14 +2461,21 @@ add_file_size_if_nonnull (int dfd, const char *path, guint64 *inout_size, GError
return FALSE;
*inout_size += stbuf.st_size;
+ if (blocksize > 0)
+ {
+ off_t rem = stbuf.st_size % blocksize;
+ if (rem > 0)
+ *inout_size += blocksize - rem;
+ }
+
return TRUE;
}
/* calculates the total size of the bootcsum dir in /boot after we would copy
* it. This reflects the logic in install_deployment_kernel(). */
static gboolean
-get_kernel_layout_size (OstreeSysroot *self, OstreeDeployment *deployment, guint64 *out_size,
- GCancellable *cancellable, GError **error)
+get_kernel_layout_size (OstreeSysroot *self, OstreeDeployment *deployment, guint64 blocksize,
+ guint64 *out_size, GCancellable *cancellable, GError **error)
{
g_autofree char *deployment_dirpath = ostree_sysroot_get_deployment_dirpath (self, deployment);
glnx_autofd int deployment_dfd = -1;
@@ -2479,11 +2487,11 @@ get_kernel_layout_size (OstreeSysroot *self, OstreeDeployment *deployment, guint
return FALSE;
guint64 bootdir_size = 0;
- if (!add_file_size_if_nonnull (kernel_layout->boot_dfd, kernel_layout->kernel_srcpath,
+ if (!add_file_size_if_nonnull (kernel_layout->boot_dfd, kernel_layout->kernel_srcpath, blocksize,
&bootdir_size, error))
return FALSE;
if (!add_file_size_if_nonnull (kernel_layout->boot_dfd, kernel_layout->initramfs_srcpath,
- &bootdir_size, error))
+ blocksize, &bootdir_size, error))
return FALSE;
if (kernel_layout->devicetree_srcpath)
{
@@ -2491,22 +2499,22 @@ get_kernel_layout_size (OstreeSysroot *self, OstreeDeployment *deployment, guint
if (kernel_layout->devicetree_namever)
{
if (!add_file_size_if_nonnull (kernel_layout->boot_dfd, kernel_layout->devicetree_srcpath,
- &bootdir_size, error))
+ blocksize, &bootdir_size, error))
return FALSE;
}
else
{
guint64 dirsize = 0;
if (!ot_get_dir_size (kernel_layout->boot_dfd, kernel_layout->devicetree_srcpath,
- &dirsize, cancellable, error))
+ blocksize, &dirsize, cancellable, error))
return FALSE;
bootdir_size += dirsize;
}
}
if (!add_file_size_if_nonnull (kernel_layout->boot_dfd, kernel_layout->kernel_hmac_srcpath,
- &bootdir_size, error))
+ blocksize, &bootdir_size, error))
return FALSE;
- if (!add_file_size_if_nonnull (kernel_layout->boot_dfd, kernel_layout->aboot_srcpath,
+ if (!add_file_size_if_nonnull (kernel_layout->boot_dfd, kernel_layout->aboot_srcpath, blocksize,
&bootdir_size, error))
return FALSE;
@@ -2583,6 +2591,11 @@ auto_early_prune_old_deployments (OstreeSysroot *self, GPtrArray *new_deployment
g_autoptr (GHashTable) new_bootcsums
= g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
+ /* get bootfs block size */
+ struct statvfs stvfsbuf;
+ if (TEMP_FAILURE_RETRY (fstatvfs (self->boot_fd, &stvfsbuf)) < 0)
+ return glnx_throw_errno_prefix (error, "fstatvfs(boot)");
+
g_auto (GStrv) bootdirs = NULL;
if (!_ostree_sysroot_list_all_boot_directories (self, &bootdirs, cancellable, error))
return glnx_prefix_error (error, "listing bootcsum directories in bootfs");
@@ -2597,7 +2610,8 @@ auto_early_prune_old_deployments (OstreeSysroot *self, GPtrArray *new_deployment
guint64 bootdir_size;
g_autofree char *ostree_bootdir = g_build_filename ("ostree", bootdir, NULL);
- if (!ot_get_dir_size (self->boot_fd, ostree_bootdir, &bootdir_size, cancellable, error))
+ if (!ot_get_dir_size (self->boot_fd, ostree_bootdir, stvfsbuf.f_bsize, &bootdir_size,
+ cancellable, error))
return FALSE;
/* for our purposes of sizing bootcsums, it's highly unlikely we need a
@@ -2609,10 +2623,7 @@ auto_early_prune_old_deployments (OstreeSysroot *self, GPtrArray *new_deployment
* that users report it and we tweak this code to handle this.
*
* An alternative is working with the block size instead, which would
- * be easier to handle. But ideally, `ot_get_dir_size` would be block
- * size aware too for better accuracy, which is awkward since the
- * function itself is generic over directories and doesn't consider
- * e.g. mount points from different filesystems. */
+ * be easier to handle. */
g_printerr ("bootcsum %s size exceeds %u; disabling auto-prune optimization\n", bootdir,
G_MAXUINT);
return TRUE;
@@ -2640,7 +2651,8 @@ auto_early_prune_old_deployments (OstreeSysroot *self, GPtrArray *new_deployment
}
guint64 bootdir_size = 0;
- if (!get_kernel_layout_size (self, deployment, &bootdir_size, cancellable, error))
+ if (!get_kernel_layout_size (self, deployment, stvfsbuf.f_bsize, &bootdir_size, cancellable,
+ error))
return FALSE;
/* see similar logic in previous loop */
diff --git a/src/libotutil/ot-fs-utils.c b/src/libotutil/ot-fs-utils.c
index efbb2f20..1e961a98 100644
--- a/src/libotutil/ot-fs-utils.c
+++ b/src/libotutil/ot-fs-utils.c
@@ -227,11 +227,12 @@ ot_parse_file_by_line (const char *path, gboolean (*cb) (const char *, void *, G
return TRUE;
}
-/* Calculate the size of the files contained in a directory. Symlinks are not
- * followed. */
+/* Calculate the size of the files contained in a directory. Symlinks are
+ * not followed. If `blocksize` is nonzero, all sizes are rounded to its next
+ * multiple. */
gboolean
-ot_get_dir_size (int dfd, const char *path, guint64 *out_size, GCancellable *cancellable,
- GError **error)
+ot_get_dir_size (int dfd, const char *path, guint64 blocksize, guint64 *out_size,
+ GCancellable *cancellable, GError **error)
{
g_auto (GLnxDirFdIterator) dfd_iter = {
0,
@@ -256,11 +257,18 @@ ot_get_dir_size (int dfd, const char *path, guint64 *out_size, GCancellable *can
return FALSE;
*out_size += stbuf.st_size;
+ if (blocksize > 0)
+ {
+ off_t rem = stbuf.st_size % blocksize;
+ if (rem > 0)
+ *out_size += blocksize - rem;
+ }
}
else if (dent->d_type == DT_DIR)
{
guint64 subdir_size;
- if (!ot_get_dir_size (dfd_iter.fd, dent->d_name, &subdir_size, cancellable, error))
+ if (!ot_get_dir_size (dfd_iter.fd, dent->d_name, blocksize, &subdir_size, cancellable,
+ error))
return FALSE;
*out_size += subdir_size;
diff --git a/src/libotutil/ot-fs-utils.h b/src/libotutil/ot-fs-utils.h
index b64adce2..7df79ba2 100644
--- a/src/libotutil/ot-fs-utils.h
+++ b/src/libotutil/ot-fs-utils.h
@@ -75,7 +75,7 @@ GBytes *ot_fd_readall_or_mmap (int fd, goffset offset, GError **error);
gboolean ot_parse_file_by_line (const char *path, gboolean (*cb) (const char *, void *, GError **),
void *cbdata, GCancellable *cancellable, GError **error);
-gboolean ot_get_dir_size (int dfd, const char *path, guint64 *out_size, GCancellable *cancellable,
- GError **error);
+gboolean ot_get_dir_size (int dfd, const char *path, guint64 blocksize, guint64 *out_size,
+ GCancellable *cancellable, GError **error);
G_END_DECLS
--
2.43.0

View File

@ -1,53 +0,0 @@
From a1c1c0b500d23ff129adbfe9486a067788b24969 Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Wed, 3 Jan 2024 14:01:38 -0500
Subject: [PATCH] prepare-root: Fix composefs + ostree admin unlock --hotfix
compat
There's a test case for `ostree admin unlock --hotfix` that
runs in FCOS, not here; it breaks when enabling composefs.
The reason is because the composefs is mounted readonly, and
we tried to remount it writable. Instead of trying to remount
the rootfs writable at this point forcibly, honor the
*real* sysroot readonly state flag from the underlying FS before
we mounted the composefs.
Note that in FCOS derivatives we always have the root mounted
writable via `rw` on the kernel cmdline and this is the default
general expectation now with ostree usage.
---
src/switchroot/ostree-prepare-root.c | 17 +++++------------
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/src/switchroot/ostree-prepare-root.c b/src/switchroot/ostree-prepare-root.c
index 1a0539e1..d7c44e97 100644
--- a/src/switchroot/ostree-prepare-root.c
+++ b/src/switchroot/ostree-prepare-root.c
@@ -639,18 +639,11 @@ main (int argc, char *argv[])
const char usr_ovl_options[]
= "lowerdir=" TMP_SYSROOT "/usr,upperdir=.usr-ovl-upper,workdir=.usr-ovl-work";
- /* Except overlayfs barfs if we try to mount it on a read-only
- * filesystem. For this use case I think admins are going to be
- * okay if we remount the rootfs here, rather than waiting until
- * later boot and `systemd-remount-fs.service`.
- */
- if (path_is_on_readonly_fs (TMP_SYSROOT))
- {
- if (mount (TMP_SYSROOT, TMP_SYSROOT, NULL, MS_REMOUNT | MS_SILENT, NULL) < 0)
- err (EXIT_FAILURE, "failed to remount rootfs writable (for overlayfs)");
- }
-
- if (mount ("overlay", TMP_SYSROOT "/usr", "overlay", MS_SILENT, usr_ovl_options) < 0)
+ unsigned long mflags = MS_SILENT;
+ // Propagate readonly state
+ if (!sysroot_currently_writable)
+ mflags |= MS_RDONLY;
+ if (mount ("overlay", TMP_SYSROOT "/usr", "overlay", mflags, usr_ovl_options) < 0)
err (EXIT_FAILURE, "failed to mount /usr overlayfs");
}
else if (!using_composefs)
--
2.41.0

View File

@ -1,43 +0,0 @@
From 261287be37b7af046c31c98b3ca36741b1b421eb Mon Sep 17 00:00:00 2001
From: Jonathan Lebon <jonathan@jlebon.com>
Date: Thu, 4 Jan 2024 11:14:40 -0500
Subject: [PATCH 2/2] lib/deploy: Add safety margin in early prune space check
There are a few things the estimator doesn't account for, e.g. writing
the new BLS entries. Rather than trying to perfect it (since I think we
should change approach entirely -- see previous commit message), just
add a 1M margin to the space check.
---
src/libostree/ostree-sysroot-deploy.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c
index 78c47e76..7f45ccac 100644
--- a/src/libostree/ostree-sysroot-deploy.c
+++ b/src/libostree/ostree-sysroot-deploy.c
@@ -59,6 +59,12 @@
SD_ID128_MAKE (e8, 64, 6c, d6, 3d, ff, 46, 25, b7, 79, 09, a8, e7, a4, 09, 94)
#endif
+/* How much additional space we require available on top of what we accounted
+ * during the early prune fallocate space check. This accounts for anything not
+ * captured directly by `get_kernel_layout_size()` like writing new BLS entries.
+ */
+#define EARLY_PRUNE_SAFETY_MARGIN_SIZE (1 << 20) /* 1 MB */
+
/*
* Like symlinkat() but overwrites (atomically) an existing
* symlink.
@@ -2541,6 +2547,9 @@ dfd_fallocate_check (int dfd, off_t len, gboolean *out_passed, GError **error)
if (!glnx_open_tmpfile_linkable_at (dfd, ".", O_WRONLY | O_CLOEXEC, &tmpf, error))
return FALSE;
+ /* add the safety margin */
+ len += EARLY_PRUNE_SAFETY_MARGIN_SIZE;
+
*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)
--
2.43.0

View File

@ -12,20 +12,12 @@
Summary: Tool for managing bootable, immutable filesystem trees
Name: ostree
Version: 2023.8
Version: 2024.1
Release: %autorelease
Source0: https://github.com/ostreedev/%{name}/releases/download/v%{version}/libostree-%{version}.tar.xz
License: LGPL-2.0-or-later
URL: https://ostree.readthedocs.io/en/latest/
# Backport https://github.com/ostreedev/ostree/pull/3129/commits/a1c1c0b500d23ff129adbfe9486a067788b24969
# To aid https://github.com/coreos/fedora-coreos-config/pull/2783
Patch0: 0001-prepare-root-Fix-composefs-ostree-admin-unlock-hotfi.patch
# Backport https://github.com/ostreedev/ostree/pull/3130
Patch1: 0001-lib-deploy-Round-to-block-size-in-early-prune-space-.patch
Patch2: 0002-lib-deploy-Add-safety-margin-in-early-prune-space-ch.patch
# Conditional to ELN right now to reduce blast radius; xref
# https://github.com/containers/composefs/pull/229#issuecomment-1838735764
%if 0%{?rhel} >= 10

View File

@ -1 +1 @@
SHA512 (libostree-2023.8.tar.xz) = 9b09cdd74ed3a6385ca43e34801113ad59331c26fbb6f63330becae5a6f11b562e61246fd7bdb1094efc2f57557e89b462aac1a34967656b49e982a7db8add89
SHA512 (libostree-2024.1.tar.xz) = edbb2146ff4d4e0b60df3c02a046044b710b903c61843eab2daf63f75a1031cec8827cb01da85179ce9e5f693419c6e250e4a2a6aaf4b003ffb5efc81b4b8e02