Skip to content

Commit

Permalink
Use rename of directories instead of symbolic links in boot partition.
Browse files Browse the repository at this point in the history 
This uses `renameat2` to do atomic swap of the loader directory in the
boot partition. It fallsback to non-atomic rename. This stays atomic
on filesystems supporting links but also provide a non-atomic behavior
when filesystem does not provide any atomic alternative.

This is working with SystemD boot on EFI using boot loader
specifications.

There is still the issue of losing `/loader/loader.conf` with SystemD
boot.
  • Loading branch information
@valentindavid
valentindavid committed Nov 15, 2021
1 parent 800289a commit 5825205
Show file tree
Hide file tree
Showing 4 changed files with 166 additions and 68 deletions.
9 changes: 9 additions & 0 deletions src/libostree/ostree-sysroot-cleanup.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,15 @@ cleanup_other_bootversions (OstreeSysroot *self,
const int cleanup_subbootversion = self->subbootversion == 0 ? 1 : 0;
/* Reusable buffer for path */
g_autoptr(GString) buf = g_string_new ("");
struct stat stbuf;

if ((glnx_fstatat_allow_noent (self->sysroot_fd, "boot/loader", &stbuf, AT_SYMLINK_NOFOLLOW, error))
&& (!S_ISLNK (stbuf.st_mode)))
{
g_string_truncate (buf, 0); g_string_append_printf (buf, "boot/loader.%d", self->bootversion);
if (!glnx_shutil_rm_rf_at (self->sysroot_fd, buf->str, cancellable, error))
return FALSE;
}

/* These directories are for the other major version */
g_string_truncate (buf, 0); g_string_append_printf (buf, "boot/loader.%d", cleanup_bootversion);
Expand Down
95 changes: 74 additions & 21 deletions src/libostree/ostree-sysroot-deploy.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2026,18 +2026,18 @@ install_deployment_kernel (OstreeSysroot *sysroot,
return TRUE;
}

/* We generate the symlink on disk, then potentially do a syncfs() to ensure
/* We generate the directory on disk, then potentially do a syncfs() to ensure
* that it (and everything else we wrote) has hit disk. Only after that do we
* rename it into place.
*/
static gboolean
prepare_new_bootloader_link (OstreeSysroot *sysroot,
int current_bootversion,
int new_bootversion,
GCancellable *cancellable,
GError **error)
prepare_new_bootloader_dir (OstreeSysroot *sysroot,
int current_bootversion,
int new_bootversion,
GCancellable *cancellable,
GError **error)
{
GLNX_AUTO_PREFIX_ERROR ("Preparing final bootloader swap", error);
GLNX_AUTO_PREFIX_ERROR ("Preparing new bootloader directory", error);
g_assert ((current_bootversion == 0 && new_bootversion == 1) ||
(current_bootversion == 1 && new_bootversion == 0));

Expand All @@ -2047,23 +2047,76 @@ prepare_new_bootloader_link (OstreeSysroot *sysroot,
if (errno != EEXIST)
return glnx_throw_errno_prefix (error, "symlinkat");

g_autofree char *new_target = g_strdup_printf ("loader.%d", new_bootversion);
if (!_ostree_sysroot_ensure_boot_fd (sysroot, error))
return FALSE;

/* We shouldn't actually need to replace but it's easier to reuse
that code */
if (!symlink_at_replace (new_target, sysroot->sysroot_fd, "boot/loader.tmp",
cancellable, error))
g_autofree char *loader_dir_name = g_strdup_printf ("loader.%d", new_bootversion);

if (!glnx_shutil_mkdir_p_at (sysroot->boot_fd, loader_dir_name, 0755,
cancellable, error))
return FALSE;

g_autofree char *version_name = g_strdup_printf ("%s/version", loader_dir_name);

if (!glnx_file_replace_contents_at (sysroot->boot_fd, version_name,
(guint8*)loader_dir_name, strlen(loader_dir_name),
0, cancellable, error))
return FALSE;

return TRUE;
}

static gboolean
renameat2_exchange (int olddirfd,
const char *oldpath,
int newdirfd,
const char *newpath,
gboolean *is_atomic,
GError **error)
{
if (renameat2 (olddirfd, oldpath, newdirfd, newpath, RENAME_EXCHANGE) == 0)
return TRUE;
else
{
if ((errno == EINVAL)
|| (errno == ENOSYS))
{
if (glnx_renameat2_exchange (olddirfd, oldpath, newdirfd, newpath) == 0)
{
is_atomic = FALSE;
return TRUE;
}
}
}

if (errno != ENOENT)
return glnx_throw_errno_prefix (error, "renameat2");

if (renameat2 (olddirfd, oldpath, newdirfd, newpath, RENAME_NOREPLACE) == 0)
return TRUE;
else
{
if ((errno == EINVAL)
|| (errno == ENOSYS))
{
if (glnx_renameat2_noreplace (olddirfd, oldpath, newdirfd, newpath) == 0)
{
is_atomic = FALSE;
return TRUE;
}
}
}

return glnx_throw_errno_prefix (error, "renameat2");
}

/* Update the /boot/loader symlink to point to /boot/loader.$new_bootversion */
static gboolean
swap_bootloader (OstreeSysroot *sysroot,
OstreeBootloader *bootloader,
int current_bootversion,
int new_bootversion,
gboolean *is_atomic,
GCancellable *cancellable,
GError **error)
{
Expand All @@ -2075,11 +2128,8 @@ swap_bootloader (OstreeSysroot *sysroot,
if (!_ostree_sysroot_ensure_boot_fd (sysroot, error))
return FALSE;

/* The symlink was already written, and we used syncfs() to ensure
* its data is in place. Renaming now should give us atomic semantics;
* see https://bugzilla.gnome.org/show_bug.cgi?id=755595
*/
if (!glnx_renameat (sysroot->boot_fd, "loader.tmp", sysroot->boot_fd, "loader", error))
g_autofree char *new_target = g_strdup_printf ("loader.%d", new_bootversion);
if (!renameat2_exchange(sysroot->boot_fd, new_target, sysroot->boot_fd, "loader", is_atomic, error))
return FALSE;

/* Now we explicitly fsync this directory, even though it
Expand Down Expand Up @@ -2254,6 +2304,7 @@ write_deployments_bootswap (OstreeSysroot *self,
OstreeBootloader *bootloader,
SyncStats *out_syncstats,
char **out_subbootdir,
gboolean *is_atomic,
GCancellable *cancellable,
GError **error)
{
Expand Down Expand Up @@ -2317,15 +2368,16 @@ write_deployments_bootswap (OstreeSysroot *self,
return glnx_prefix_error (error, "Bootloader write config");
}

if (!prepare_new_bootloader_link (self, self->bootversion, new_bootversion,
cancellable, error))
if (!prepare_new_bootloader_dir (self, self->bootversion, new_bootversion,
cancellable, error))
return FALSE;

if (!full_system_sync (self, out_syncstats, cancellable, error))
return FALSE;

if (!swap_bootloader (self, bootloader, self->bootversion, new_bootversion,
cancellable, error))
is_atomic, cancellable, error))

return FALSE;

if (out_subbootdir)
Expand Down Expand Up @@ -2534,7 +2586,8 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self,
bootloader_is_atomic = bootloader != NULL && _ostree_bootloader_is_atomic (bootloader);

if (!write_deployments_bootswap (self, new_deployments, opts, bootloader,
&syncstats, &new_subbootdir, cancellable, error))
&syncstats, &new_subbootdir, &bootloader_is_atomic,
cancellable, error))
return FALSE;
}

Expand Down
106 changes: 67 additions & 39 deletions src/libostree/ostree-sysroot.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,62 @@ compare_loader_configs_for_sorting (gconstpointer a_pp,
return compare_boot_loader_configs (a, b);
}

/* Get the bootversion from the `/boot/loader` directory or symlink. */
static gboolean
read_current_bootversion (OstreeSysroot *self,
int *out_bootversion,
GCancellable *cancellable,
GError **error)
{
int ret_bootversion;
struct stat stbuf;

if (!glnx_fstatat_allow_noent (self->sysroot_fd, "boot/loader", &stbuf, AT_SYMLINK_NOFOLLOW, error))
return FALSE;
if (errno == ENOENT)
{
g_debug ("Didn't find $sysroot/boot/loader directory or symlink; assuming bootversion 0");
ret_bootversion = 0;
}
else
{
if (!S_ISLNK (stbuf.st_mode))
{
gsize len;
g_autofree char* version_content = glnx_file_get_contents_utf8_at(self->sysroot_fd, "boot/loader/version",
&len, cancellable, error);
if (version_content == NULL) {
return FALSE;
}
if (len != 8)
return glnx_throw (error, "Invalid version in boot/loader/version");
else if (g_strcmp0 (version_content, "loader.0") == 0)
ret_bootversion = 0;
else if (g_strcmp0 (version_content, "loader.1") == 0)
ret_bootversion = 1;
else
return glnx_throw (error, "Invalid version in boot/loader/version");
}
else
{
/* Backward compatibility with boot symbolic links */
g_autofree char *target =
glnx_readlinkat_malloc (self->sysroot_fd, "boot/loader", cancellable, error);
if (!target)
return FALSE;
if (g_strcmp0 (target, "loader.0") == 0)
ret_bootversion = 0;
else if (g_strcmp0 (target, "loader.1") == 0)
ret_bootversion = 1;
else
return glnx_throw (error, "Invalid target '%s' in boot/loader", target);
}
}

*out_bootversion = ret_bootversion;
return TRUE;
}

/* Read all the bootconfigs from `/boot/loader/`. */
gboolean
_ostree_sysroot_read_boot_loader_configs (OstreeSysroot *self,
Expand All @@ -571,12 +627,22 @@ _ostree_sysroot_read_boot_loader_configs (OstreeSysroot *self,
g_autoptr(GPtrArray) ret_loader_configs =
g_ptr_array_new_with_free_func ((GDestroyNotify)g_object_unref);

g_autofree char *entries_path = g_strdup_printf ("boot/loader.%d/entries", bootversion);
g_autofree char *entries_path = NULL;
int current_version;
if (!read_current_bootversion (self, &current_version, cancellable, error))
return FALSE;

if (current_version == bootversion)
entries_path = g_strdup ("boot/loader/entries");
else
entries_path = g_strdup_printf ("boot/loader.%d/entries", bootversion);

gboolean entries_exists;
g_auto(GLnxDirFdIterator) dfd_iter = { 0, };
if (!ot_dfd_iter_init_allow_noent (self->sysroot_fd, entries_path,
&dfd_iter, &entries_exists, error))
return FALSE;

if (!entries_exists)
{
/* Note early return */
Expand Down Expand Up @@ -616,44 +682,6 @@ _ostree_sysroot_read_boot_loader_configs (OstreeSysroot *self,
return TRUE;
}

/* Get the bootversion from the `/boot/loader` symlink. */
static gboolean
read_current_bootversion (OstreeSysroot *self,
int *out_bootversion,
GCancellable *cancellable,
GError **error)
{
int ret_bootversion;
struct stat stbuf;

if (!glnx_fstatat_allow_noent (self->sysroot_fd, "boot/loader", &stbuf, AT_SYMLINK_NOFOLLOW, error))
return FALSE;
if (errno == ENOENT)
{
g_debug ("Didn't find $sysroot/boot/loader symlink; assuming bootversion 0");
ret_bootversion = 0;
}
else
{
if (!S_ISLNK (stbuf.st_mode))
return glnx_throw (error, "Not a symbolic link: boot/loader");

g_autofree char *target =
glnx_readlinkat_malloc (self->sysroot_fd, "boot/loader", cancellable, error);
if (!target)
return FALSE;
if (g_strcmp0 (target, "loader.0") == 0)
ret_bootversion = 0;
else if (g_strcmp0 (target, "loader.1") == 0)
ret_bootversion = 1;
else
return glnx_throw (error, "Invalid target '%s' in boot/loader", target);
}

*out_bootversion = ret_bootversion;
return TRUE;
}

static gboolean
load_origin (OstreeSysroot *self,
OstreeDeployment *deployment,
Expand Down
24 changes: 16 additions & 8 deletions tests/admin-test.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ echo "ok --print-current-dir"

# Test layout of bootloader config and refs
assert_not_has_dir sysroot/boot/loader.0
assert_has_dir sysroot/boot/loader.1
assert_not_has_dir sysroot/boot/loader.1
assert_file_has_content sysroot/boot/loader/version 'loader.1'
assert_has_dir sysroot/ostree/boot.1.1
assert_has_file sysroot/boot/loader/entries/ostree-1-testos.conf
assert_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'options.* root=LABEL=MOO'
Expand All @@ -103,8 +104,9 @@ ${CMD_PREFIX} ostree admin deploy --os=testos testos:testos/buildmain/x86_64-run
new_mtime=$(stat -c '%.Y' sysroot/ostree/deploy)
assert_not_streq "${orig_mtime}" "${new_mtime}"
# Need a new bootversion, sine we now have two deployments
assert_has_dir sysroot/boot/loader.0
assert_not_has_dir sysroot/boot/loader.0
assert_not_has_dir sysroot/boot/loader.1
assert_file_has_content sysroot/boot/loader/version 'loader.0'
assert_has_dir sysroot/ostree/boot.0.1
assert_not_has_dir sysroot/ostree/boot.0.0
assert_not_has_dir sysroot/ostree/boot.1.0
Expand All @@ -121,8 +123,9 @@ echo "ok second deploy"

${CMD_PREFIX} ostree admin deploy --os=testos testos:testos/buildmain/x86_64-runtime
# Keep the same bootversion
assert_has_dir sysroot/boot/loader.0
assert_not_has_dir sysroot/boot/loader.0
assert_not_has_dir sysroot/boot/loader.1
assert_file_has_content sysroot/boot/loader/version 'loader.0'
# But swap subbootversion
assert_has_dir sysroot/ostree/boot.0.0
assert_not_has_dir sysroot/ostree/boot.0.1
Expand All @@ -136,7 +139,8 @@ ${CMD_PREFIX} ostree admin os-init otheros

${CMD_PREFIX} ostree admin deploy --os=otheros testos/buildmain/x86_64-runtime
assert_not_has_dir sysroot/boot/loader.0
assert_has_dir sysroot/boot/loader.1
assert_not_has_dir sysroot/boot/loader.1
assert_file_has_content sysroot/boot/loader/version 'loader.1'
assert_has_file sysroot/boot/loader/entries/ostree-2-testos.conf
assert_has_file sysroot/boot/loader/entries/ostree-3-otheros.conf
assert_file_has_content sysroot/ostree/deploy/testos/deploy/${rev}.1/etc/os-release 'NAME=TestOS'
Expand All @@ -148,8 +152,9 @@ validate_bootloader
echo "ok independent deploy"

${CMD_PREFIX} ostree admin deploy --retain --os=testos testos:testos/buildmain/x86_64-runtime
assert_has_dir sysroot/boot/loader.0
assert_not_has_dir sysroot/boot/loader.0
assert_not_has_dir sysroot/boot/loader.1
assert_file_has_content sysroot/boot/loader/version 'loader.0'
assert_has_file sysroot/boot/loader/entries/ostree-4-testos.conf
assert_file_has_content sysroot/ostree/deploy/testos/deploy/${rev}.2/etc/os-release 'NAME=TestOS'
assert_has_file sysroot/boot/loader/entries/ostree-2-testos.conf
Expand All @@ -166,7 +171,8 @@ rm sysroot/ostree/deploy/testos/deploy/${rev}.3/etc/aconfigfile
ln -s /ENOENT sysroot/ostree/deploy/testos/deploy/${rev}.3/etc/a-new-broken-symlink
${CMD_PREFIX} ostree admin deploy --retain --os=testos testos:testos/buildmain/x86_64-runtime
assert_not_has_dir sysroot/boot/loader.0
assert_has_dir sysroot/boot/loader.1
assert_not_has_dir sysroot/boot/loader.1
assert_file_has_content sysroot/boot/loader/version 'loader.1'
link=sysroot/ostree/deploy/testos/deploy/${rev}.4/etc/a-new-broken-symlink
if ! test -L ${link}; then
ls -al ${link}
Expand All @@ -190,8 +196,9 @@ assert_has_file sysroot/boot/loader/entries/ostree-1-testos.conf
assert_not_has_file sysroot/boot/loader/entries/ostree-2-testos.conf
assert_not_has_file sysroot/boot/loader/entries/ostree-3-otheros.conf
${CMD_PREFIX} ostree admin deploy --not-as-default --os=otheros testos:testos/buildmain/x86_64-runtime
assert_has_dir sysroot/boot/loader.0
assert_not_has_dir sysroot/boot/loader.0
assert_not_has_dir sysroot/boot/loader.1
assert_file_has_content sysroot/boot/loader/version 'loader.0'
assert_has_file sysroot/boot/loader/entries/ostree-2-testos.conf
assert_has_file sysroot/boot/loader/entries/ostree-1-otheros.conf
${CMD_PREFIX} ostree admin status
Expand All @@ -201,7 +208,8 @@ echo "ok deploy --not-as-default"

${CMD_PREFIX} ostree admin deploy --retain-rollback --os=otheros testos:testos/buildmain/x86_64-runtime
assert_not_has_dir sysroot/boot/loader.0
assert_has_dir sysroot/boot/loader.1
assert_not_has_dir sysroot/boot/loader.1
assert_file_has_content sysroot/boot/loader/version 'loader.1'
assert_has_file sysroot/boot/loader/entries/ostree-3-otheros.conf
assert_has_file sysroot/boot/loader/entries/ostree-2-testos.conf
assert_has_file sysroot/boot/loader/entries/ostree-1-otheros.conf
Expand Down

0 comments on commit 5825205

Please sign in to comment.