Skip to content

Commit c36ee32

Browse files
valentindavidvalentindavid
committed
Use rename of directories instead of symbolic links in boot partition.
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.
1 parent 44988e6 commit c36ee32

4 files changed

Lines changed: 165 additions & 69 deletions

File tree

‎src/libostree/ostree-sysroot-cleanup.c‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,15 @@ cleanup_other_bootversions (OstreeSysroot *self,
216216
const int cleanup_subbootversion = self->subbootversion == 0 ? 1 : 0;
217217
/* Reusable buffer for path */
218218
g_autoptr(GString) buf = g_string_new ("");
219+
struct stat stbuf;
220+
221+
if ((glnx_fstatat_allow_noent (self->sysroot_fd, "boot/loader", &stbuf, AT_SYMLINK_NOFOLLOW, error))
222+
&& (!S_ISLNK (stbuf.st_mode)))
223+
{
224+
g_string_truncate (buf, 0); g_string_append_printf (buf, "boot/loader.%d", self->bootversion);
225+
if (!glnx_shutil_rm_rf_at (self->sysroot_fd, buf->str, cancellable, error))
226+
return FALSE;
227+
}
219228

220229
/* These directories are for the other major version */
221230
g_string_truncate (buf, 0); g_string_append_printf (buf, "boot/loader.%d", cleanup_bootversion);

‎src/libostree/ostree-sysroot-deploy.c‎

Lines changed: 75 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1829,38 +1829,89 @@ install_deployment_kernel (OstreeSysroot *sysroot,
18291829
return TRUE;
18301830
}
18311831

1832-
/* We generate the symlink on disk, then potentially do a syncfs() to ensure
1833-
* that it (and everything else we wrote) has hit disk. Only after that do we
1834-
* rename it into place.
1835-
*/
18361832
static gboolean
1837-
prepare_new_bootloader_link (OstreeSysroot *sysroot,
1838-
int current_bootversion,
1839-
int new_bootversion,
1840-
GCancellable *cancellable,
1841-
GError **error)
1833+
prepare_new_bootloader_dir (OstreeSysroot *sysroot,
1834+
int current_bootversion,
1835+
int new_bootversion,
1836+
GCancellable *cancellable,
1837+
GError **error)
18421838
{
1843-
GLNX_AUTO_PREFIX_ERROR ("Preparing final bootloader swap", error);
1839+
glnx_autofd int boot_dfd = -1;
1840+
1841+
GLNX_AUTO_PREFIX_ERROR ("Preparing new bootloader directory", error);
18441842
g_assert ((current_bootversion == 0 && new_bootversion == 1) ||
18451843
(current_bootversion == 1 && new_bootversion == 0));
18461844

1847-
g_autofree char *new_target = g_strdup_printf ("loader.%d", new_bootversion);
1845+
if (!glnx_opendirat (sysroot->sysroot_fd, "boot", TRUE, &boot_dfd, error))
1846+
return FALSE;
18481847

1849-
/* We shouldn't actually need to replace but it's easier to reuse
1850-
that code */
1851-
if (!symlink_at_replace (new_target, sysroot->sysroot_fd, "boot/loader.tmp",
1852-
cancellable, error))
1848+
g_autofree char *loader_dir_name = g_strdup_printf ("loader.%d", new_bootversion);
1849+
1850+
if (!glnx_shutil_mkdir_p_at (boot_dfd, loader_dir_name, 0755,
1851+
cancellable, error))
1852+
return FALSE;
1853+
1854+
g_autofree char *version_name = g_strdup_printf ("%s/version", loader_dir_name);
1855+
1856+
if (!glnx_file_replace_contents_at (boot_dfd, version_name,
1857+
(guint8*)loader_dir_name, strlen(loader_dir_name),
1858+
0, cancellable, error))
18531859
return FALSE;
18541860

18551861
return TRUE;
18561862
}
18571863

1864+
static gboolean
1865+
renameat2_exchange (int olddirfd,
1866+
const char *oldpath,
1867+
int newdirfd,
1868+
const char *newpath,
1869+
gboolean *is_atomic,
1870+
GError **error)
1871+
{
1872+
if (renameat2(olddirfd, oldpath, newdirfd, newpath, RENAME_EXCHANGE) == 0)
1873+
return TRUE;
1874+
else
1875+
{
1876+
if ((errno == EINVAL)
1877+
|| (errno == ENOSYS))
1878+
{
1879+
if (glnx_renameat2_exchange (olddirfd, oldpath, newdirfd, newpath) == 0)
1880+
{
1881+
is_atomic = FALSE;
1882+
return TRUE;
1883+
}
1884+
}
1885+
}
1886+
1887+
if (errno != ENOENT)
1888+
return glnx_throw_errno_prefix (error, "renameat2");
1889+
1890+
if (renameat2(olddirfd, oldpath, newdirfd, newpath, RENAME_NOREPLACE) == 0)
1891+
return TRUE;
1892+
else
1893+
{
1894+
if ((errno == EINVAL)
1895+
|| (errno == ENOSYS))
1896+
{
1897+
if (glnx_renameat2_noreplace (olddirfd, oldpath, newdirfd, newpath) == 0)
1898+
{
1899+
is_atomic = FALSE;
1900+
return TRUE;
1901+
}
1902+
}
1903+
}
1904+
1905+
return glnx_throw_errno_prefix (error, "renameat2");
1906+
}
1907+
18581908
/* Update the /boot/loader symlink to point to /boot/loader.$new_bootversion */
18591909
static gboolean
18601910
swap_bootloader (OstreeSysroot *sysroot,
18611911
OstreeBootloader *bootloader,
18621912
int current_bootversion,
18631913
int new_bootversion,
1914+
gboolean *is_atomic,
18641915
GCancellable *cancellable,
18651916
GError **error)
18661917
{
@@ -1873,11 +1924,8 @@ swap_bootloader (OstreeSysroot *sysroot,
18731924
if (!glnx_opendirat (sysroot->sysroot_fd, "boot", TRUE, &boot_dfd, error))
18741925
return FALSE;
18751926

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

18831931
/* Now we explicitly fsync this directory, even though it
@@ -2098,6 +2146,7 @@ write_deployments_bootswap (OstreeSysroot *self,
20982146
OstreeSysrootWriteDeploymentsOpts *opts,
20992147
OstreeBootloader *bootloader,
21002148
SyncStats *out_syncstats,
2149+
gboolean *is_atomic,
21012150
GCancellable *cancellable,
21022151
GError **error)
21032152
{
@@ -2160,15 +2209,16 @@ write_deployments_bootswap (OstreeSysroot *self,
21602209
return glnx_prefix_error (error, "Bootloader write config");
21612210
}
21622211

2163-
if (!prepare_new_bootloader_link (self, self->bootversion, new_bootversion,
2164-
cancellable, error))
2212+
if (!prepare_new_bootloader_dir (self, self->bootversion, new_bootversion,
2213+
cancellable, error))
21652214
return FALSE;
21662215

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

21702219
if (!swap_bootloader (self, bootloader, self->bootversion, new_bootversion,
2171-
cancellable, error))
2220+
is_atomic, cancellable, error))
2221+
21722222
return FALSE;
21732223

21742224
return TRUE;
@@ -2407,7 +2457,8 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self,
24072457

24082458
/* Note equivalent of try/finally here */
24092459
gboolean success = write_deployments_bootswap (self, new_deployments, opts, bootloader,
2410-
&syncstats, cancellable, error);
2460+
&syncstats, &bootloader_is_atomic,
2461+
cancellable, error);
24112462
/* Below here don't set GError until the if (!success) check */
24122463
if (boot_was_ro_mount)
24132464
{

‎src/libostree/ostree-sysroot.c‎

Lines changed: 65 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,60 @@ compare_loader_configs_for_sorting (gconstpointer a_pp,
432432
return compare_boot_loader_configs (a, b);
433433
}
434434

435+
static gboolean
436+
read_current_bootversion (OstreeSysroot *self,
437+
int *out_bootversion,
438+
GCancellable *cancellable,
439+
GError **error)
440+
{
441+
int ret_bootversion;
442+
struct stat stbuf;
443+
444+
if (!glnx_fstatat_allow_noent (self->sysroot_fd, "boot/loader", &stbuf, AT_SYMLINK_NOFOLLOW, error))
445+
return FALSE;
446+
if (errno == ENOENT)
447+
{
448+
ret_bootversion = 0;
449+
}
450+
else
451+
{
452+
if (!S_ISLNK (stbuf.st_mode))
453+
{
454+
gsize len;
455+
g_autofree char* version_content = glnx_file_get_contents_utf8_at(self->sysroot_fd, "boot/loader/version",
456+
&len, cancellable, error);
457+
if (version_content == NULL) {
458+
return FALSE;
459+
}
460+
if (len != 8)
461+
return glnx_throw (error, "Invalid version in boot/loader/version");
462+
else if (g_strcmp0 (version_content, "loader.0") == 0)
463+
ret_bootversion = 0;
464+
else if (g_strcmp0 (version_content, "loader.1") == 0)
465+
ret_bootversion = 1;
466+
else
467+
return glnx_throw (error, "Invalid version in boot/loader/version");
468+
}
469+
else
470+
{
471+
/* Backward compatibility with boot symbolic links */
472+
g_autofree char *target =
473+
glnx_readlinkat_malloc (self->sysroot_fd, "boot/loader", cancellable, error);
474+
if (!target)
475+
return FALSE;
476+
if (g_strcmp0 (target, "loader.0") == 0)
477+
ret_bootversion = 0;
478+
else if (g_strcmp0 (target, "loader.1") == 0)
479+
ret_bootversion = 1;
480+
else
481+
return glnx_throw (error, "Invalid target '%s' in boot/loader", target);
482+
}
483+
}
484+
485+
*out_bootversion = ret_bootversion;
486+
return TRUE;
487+
}
488+
435489
gboolean
436490
_ostree_sysroot_read_boot_loader_configs (OstreeSysroot *self,
437491
int bootversion,
@@ -445,12 +499,22 @@ _ostree_sysroot_read_boot_loader_configs (OstreeSysroot *self,
445499
g_autoptr(GPtrArray) ret_loader_configs =
446500
g_ptr_array_new_with_free_func ((GDestroyNotify)g_object_unref);
447501

448-
g_autofree char *entries_path = g_strdup_printf ("boot/loader.%d/entries", bootversion);
502+
g_autofree char *entries_path = NULL;
503+
int current_version;
504+
if (!read_current_bootversion (self, &current_version, cancellable, error))
505+
return FALSE;
506+
507+
if (current_version == bootversion)
508+
entries_path = g_strdup ("boot/loader/entries");
509+
else
510+
entries_path = g_strdup_printf ("boot/loader.%d/entries", bootversion);
511+
449512
gboolean entries_exists;
450513
g_auto(GLnxDirFdIterator) dfd_iter = { 0, };
451514
if (!ot_dfd_iter_init_allow_noent (self->sysroot_fd, entries_path,
452515
&dfd_iter, &entries_exists, error))
453516
return FALSE;
517+
454518
if (!entries_exists)
455519
{
456520
/* Note early return */
@@ -490,42 +554,6 @@ _ostree_sysroot_read_boot_loader_configs (OstreeSysroot *self,
490554
return TRUE;
491555
}
492556

493-
static gboolean
494-
read_current_bootversion (OstreeSysroot *self,
495-
int *out_bootversion,
496-
GCancellable *cancellable,
497-
GError **error)
498-
{
499-
int ret_bootversion;
500-
struct stat stbuf;
501-
502-
if (!glnx_fstatat_allow_noent (self->sysroot_fd, "boot/loader", &stbuf, AT_SYMLINK_NOFOLLOW, error))
503-
return FALSE;
504-
if (errno == ENOENT)
505-
{
506-
ret_bootversion = 0;
507-
}
508-
else
509-
{
510-
if (!S_ISLNK (stbuf.st_mode))
511-
return glnx_throw (error, "Not a symbolic link: boot/loader");
512-
513-
g_autofree char *target =
514-
glnx_readlinkat_malloc (self->sysroot_fd, "boot/loader", cancellable, error);
515-
if (!target)
516-
return FALSE;
517-
if (g_strcmp0 (target, "loader.0") == 0)
518-
ret_bootversion = 0;
519-
else if (g_strcmp0 (target, "loader.1") == 0)
520-
ret_bootversion = 1;
521-
else
522-
return glnx_throw (error, "Invalid target '%s' in boot/loader", target);
523-
}
524-
525-
*out_bootversion = ret_bootversion;
526-
return TRUE;
527-
}
528-
529557
static gboolean
530558
load_origin (OstreeSysroot *self,
531559
OstreeDeployment *deployment,

‎tests/admin-test.sh‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ echo "ok --print-current-dir"
7474

7575
# Test layout of bootloader config and refs
7676
assert_not_has_dir sysroot/boot/loader.0
77-
assert_has_dir sysroot/boot/loader.1
77+
assert_not_has_dir sysroot/boot/loader.1
78+
assert_file_has_content sysroot/boot/loader/version 'loader.1'
7879
assert_has_dir sysroot/ostree/boot.1.1
7980
assert_has_file sysroot/boot/loader/entries/ostree-1-testos.conf
8081
assert_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'options.* root=LABEL=MOO'
@@ -97,8 +98,9 @@ ${CMD_PREFIX} ostree admin deploy --os=testos testos:testos/buildmaster/x86_64-r
9798
new_mtime=$(stat -c '%.Y' sysroot/ostree/deploy)
9899
assert_not_streq "${orig_mtime}" "${new_mtime}"
99100
# Need a new bootversion, sine we now have two deployments
100-
assert_has_dir sysroot/boot/loader.0
101+
assert_not_has_dir sysroot/boot/loader.0
101102
assert_not_has_dir sysroot/boot/loader.1
103+
assert_file_has_content sysroot/boot/loader/version 'loader.0'
102104
assert_has_dir sysroot/ostree/boot.0.1
103105
assert_not_has_dir sysroot/ostree/boot.0.0
104106
assert_not_has_dir sysroot/ostree/boot.1.0
@@ -115,8 +117,9 @@ echo "ok second deploy"
115117

116118
${CMD_PREFIX} ostree admin deploy --os=testos testos:testos/buildmaster/x86_64-runtime
117119
# Keep the same bootversion
118-
assert_has_dir sysroot/boot/loader.0
120+
assert_not_has_dir sysroot/boot/loader.0
119121
assert_not_has_dir sysroot/boot/loader.1
122+
assert_file_has_content sysroot/boot/loader/version 'loader.0'
120123
# But swap subbootversion
121124
assert_has_dir sysroot/ostree/boot.0.0
122125
assert_not_has_dir sysroot/ostree/boot.0.1
@@ -130,7 +133,8 @@ ${CMD_PREFIX} ostree admin os-init otheros
130133

131134
${CMD_PREFIX} ostree admin deploy --os=otheros testos/buildmaster/x86_64-runtime
132135
assert_not_has_dir sysroot/boot/loader.0
133-
assert_has_dir sysroot/boot/loader.1
136+
assert_not_has_dir sysroot/boot/loader.1
137+
assert_file_has_content sysroot/boot/loader/version 'loader.1'
134138
assert_has_file sysroot/boot/loader/entries/ostree-2-testos.conf
135139
assert_has_file sysroot/boot/loader/entries/ostree-3-otheros.conf
136140
assert_file_has_content sysroot/ostree/deploy/testos/deploy/${rev}.1/etc/os-release 'NAME=TestOS'
@@ -142,8 +146,9 @@ validate_bootloader
142146
echo "ok independent deploy"
143147

144148
${CMD_PREFIX} ostree admin deploy --retain --os=testos testos:testos/buildmaster/x86_64-runtime
145-
assert_has_dir sysroot/boot/loader.0
149+
assert_not_has_dir sysroot/boot/loader.0
146150
assert_not_has_dir sysroot/boot/loader.1
151+
assert_file_has_content sysroot/boot/loader/version 'loader.0'
147152
assert_has_file sysroot/boot/loader/entries/ostree-4-testos.conf
148153
assert_file_has_content sysroot/ostree/deploy/testos/deploy/${rev}.2/etc/os-release 'NAME=TestOS'
149154
assert_has_file sysroot/boot/loader/entries/ostree-2-testos.conf
@@ -160,7 +165,8 @@ rm sysroot/ostree/deploy/testos/deploy/${rev}.3/etc/aconfigfile
160165
ln -s /ENOENT sysroot/ostree/deploy/testos/deploy/${rev}.3/etc/a-new-broken-symlink
161166
${CMD_PREFIX} ostree admin deploy --retain --os=testos testos:testos/buildmaster/x86_64-runtime
162167
assert_not_has_dir sysroot/boot/loader.0
163-
assert_has_dir sysroot/boot/loader.1
168+
assert_not_has_dir sysroot/boot/loader.1
169+
assert_file_has_content sysroot/boot/loader/version 'loader.1'
164170
link=sysroot/ostree/deploy/testos/deploy/${rev}.4/etc/a-new-broken-symlink
165171
if ! test -L ${link}; then
166172
ls -al ${link}
@@ -184,8 +190,9 @@ assert_has_file sysroot/boot/loader/entries/ostree-1-testos.conf
184190
assert_not_has_file sysroot/boot/loader/entries/ostree-2-testos.conf
185191
assert_not_has_file sysroot/boot/loader/entries/ostree-3-otheros.conf
186192
${CMD_PREFIX} ostree admin deploy --not-as-default --os=otheros testos:testos/buildmaster/x86_64-runtime
187-
assert_has_dir sysroot/boot/loader.0
193+
assert_not_has_dir sysroot/boot/loader.0
188194
assert_not_has_dir sysroot/boot/loader.1
195+
assert_file_has_content sysroot/boot/loader/version 'loader.0'
189196
assert_has_file sysroot/boot/loader/entries/ostree-2-testos.conf
190197
assert_has_file sysroot/boot/loader/entries/ostree-1-otheros.conf
191198
${CMD_PREFIX} ostree admin status
@@ -195,7 +202,8 @@ echo "ok deploy --not-as-default"
195202

196203
${CMD_PREFIX} ostree admin deploy --retain-rollback --os=otheros testos:testos/buildmaster/x86_64-runtime
197204
assert_not_has_dir sysroot/boot/loader.0
198-
assert_has_dir sysroot/boot/loader.1
205+
assert_not_has_dir sysroot/boot/loader.1
206+
assert_file_has_content sysroot/boot/loader/version 'loader.1'
199207
assert_has_file sysroot/boot/loader/entries/ostree-3-otheros.conf
200208
assert_has_file sysroot/boot/loader/entries/ostree-2-testos.conf
201209
assert_has_file sysroot/boot/loader/entries/ostree-1-otheros.conf

0 commit comments

Comments
 (0)