Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

sysroot: Support /boot on root or as seperate filesystem for syslinux and u-boot #2149

Merged
merged 2 commits into from
Aug 18, 2020
Merged

sysroot: Support /boot on root or as seperate filesystem for syslinux and u-boot #2149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4deb426
Refactor tests/bootloader-entries-crosscheck.py
wmanley Jul 15, 2020
0ced9fd
sysroot: Support /boot on root or as seperate filesystem for syslinux…
wmanley Jul 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions src/libostree/ostree-bootloader-syslinux.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,15 @@ append_config_from_loader_entries (OstreeBootloaderSyslinux *self,
val = ostree_bootconfig_parser_get (config, "linux");
if (!val)
return glnx_throw (error, "No \"linux\" key in bootloader config");
g_ptr_array_add (new_lines, g_strdup_printf ("\tKERNEL %s", val));
g_ptr_array_add (new_lines, g_strdup_printf ("\tKERNEL /boot%s", val));

val = ostree_bootconfig_parser_get (config, "initrd");
if (val)
g_ptr_array_add (new_lines, g_strdup_printf ("\tINITRD %s", val));
g_ptr_array_add (new_lines, g_strdup_printf ("\tINITRD /boot%s", val));

val = ostree_bootconfig_parser_get (config, "devicetree");
if (val)
g_ptr_array_add (new_lines, g_strdup_printf ("\tDEVICETREE %s", val));
g_ptr_array_add (new_lines, g_strdup_printf ("\tDEVICETREE /boot%s", val));

val = ostree_bootconfig_parser_get (config, "options");
if (val)
Expand Down Expand Up @@ -150,10 +150,13 @@ _ostree_bootloader_syslinux_write_config (OstreeBootloader *bootloader,
if (kernel_arg == NULL)
return glnx_throw (error, "No KERNEL argument found after LABEL");

/* If this is a non-ostree kernel, just emit the lines
* we saw.
/* If this is a non-ostree kernel, just emit the lines we saw.
*
* We check for /ostree (without /boot prefix) as well to support
* upgrading ostree from <v2020.4.
*/
if (!g_str_has_prefix (kernel_arg, "/ostree/"))
if (!g_str_has_prefix (kernel_arg, "/ostree/") &&
!g_str_has_prefix (kernel_arg, "/boot/ostree/"))
{
for (guint i = 0; i < tmp_lines->len; i++)
{
Expand Down
8 changes: 4 additions & 4 deletions src/libostree/ostree-bootloader-uboot.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,19 +134,19 @@ create_config_from_boot_loader_entries (OstreeBootloaderUboot *self,
"No \"linux\" key in bootloader config");
return FALSE;
}
g_ptr_array_add (new_lines, g_strdup_printf ("kernel_image%s=%s", index_suffix, val));
g_ptr_array_add (new_lines, g_strdup_printf ("kernel_image%s=/boot%s", index_suffix, val));

val = ostree_bootconfig_parser_get (config, "initrd");
if (val)
g_ptr_array_add (new_lines, g_strdup_printf ("ramdisk_image%s=%s", index_suffix, val));
g_ptr_array_add (new_lines, g_strdup_printf ("ramdisk_image%s=/boot%s", index_suffix, val));

val = ostree_bootconfig_parser_get (config, "devicetree");
if (val)
g_ptr_array_add (new_lines, g_strdup_printf ("fdt_file%s=%s", index_suffix, val));
g_ptr_array_add (new_lines, g_strdup_printf ("fdt_file%s=/boot%s", index_suffix, val));

val = ostree_bootconfig_parser_get (config, "fdtdir");
if (val)
g_ptr_array_add (new_lines, g_strdup_printf ("fdtdir%s=%s", index_suffix, val));
g_ptr_array_add (new_lines, g_strdup_printf ("fdtdir%s=/boot%s", index_suffix, val));

val = ostree_bootconfig_parser_get (config, "options");
if (val)
Expand Down
6 changes: 6 additions & 0 deletions src/libostree/ostree-sysroot-deploy.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1998,6 +1998,12 @@ prepare_new_bootloader_link (OstreeSysroot *sysroot,
g_assert ((current_bootversion == 0 && new_bootversion == 1) ||
(current_bootversion == 1 && new_bootversion == 0));

/* This allows us to support both /boot on a seperate filesystem to / as well
* as on the same filesystem. */
if (TEMP_FAILURE_RETRY (symlinkat (".", sysroot->sysroot_fd, "boot/boot")) < 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, the simplicity of this is clearly tempting. I think what I'm pausing on here though is that so far libostree doesn't really create files except in a few places.

But...I just want to revisit again, were we overthinking this previously around detecting /boot being a mount point? Can't we just statvfs(/) and statvfs(/boot) and check the device IDs?

Passing down a boolean for that to the bootloader generation shouldn't be too hard.

Copy link

@cmurf cmurf Sep 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RE: Btrfs

statfs() f_fsid will be different per subvolume on the same file system. The first half (8 nibbles?) of the ID reported by stat -f seems to be consistent regardless of subvolume, but I'm not certain if the 2nd half of the ID can "roll over" once subvol ID's get high enough.

inode 256 is reserved for Btrfs subvolumes, i.e. if the file system is btrfs and the inode is 256, then it is a subvolume (or snapshot).

mountpoint -d returns the same value for each mountpoint that's the same Btrfs file system, even when backed by different subvolumes. This value looks like MAG:MIN but it doesn't match the MAG:MIN if the actual physical device partition.

Maybe more challenging is that a subvolume could be named anything and exist anywhere on a Btrfs file system, and yet mounted at /boot. This is relevant because this path to subvolume is what's needed for the bootloader to find it. I see BTRFS_IOC_INO_LOOKUP and BTRFS_IOC_TREE_SEARCH ioctl's used by btrfs subvolume list command.

Updated: There are other combinations. /boot could be a directory on a subvolume 'fedora33root'. It could also be a nested subvolume inside 'fedora33root'. Neither of these /boot are mounted. mount does show the "path to subvolume" for any mountpoint using Btrfs. Simplistically, if statfs() says vmlinuz/initramfs are on Btrfs, and somehow have f_fsid turned into a path-to-subvolume for the bootloader config. The mount command shows this path to subvolume for any mountpoint using btrfs - so path to kernel+initrd could be inferred.

if (errno != EEXIST)
return glnx_throw_errno_prefix (error, "symlinkat");

g_autofree char *new_target = g_strdup_printf ("loader.%d", new_bootversion);

/* We shouldn't actually need to replace but it's easier to reuse
Expand Down
52 changes: 36 additions & 16 deletions tests/bootloader-entries-crosscheck.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,36 +73,56 @@ def get_ostree_option(optionstring):
syslinux_entry = None
syslinux_default = None
for line in f:
line = line.strip()
if line.startswith('DEFAULT '):
try:
k, v = line.strip().split(" ", 1)
except ValueError:
continue
if k == 'DEFAULT':
if syslinux_entry is not None:
syslinux_default = line.split(' ', 1)[1]
elif line.startswith('LABEL '):
syslinux_default = v
elif k == 'LABEL':
if syslinux_entry is not None:
syslinux_entries.append(syslinux_entry)
syslinux_entry = {}
syslinux_entry['title'] = line.split(' ', 1)[1]
elif line.startswith('KERNEL '):
syslinux_entry['linux'] = line.split(' ', 1)[1]
elif line.startswith('INITRD '):
syslinux_entry['initrd'] = line.split(' ', 1)[1]
elif line.startswith('APPEND '):
syslinux_entry['options'] = line.split(' ', 1)[1]
syslinux_entry['title'] = v
elif k == 'KERNEL':
syslinux_entry['linux'] = v
elif k == 'INITRD':
syslinux_entry['initrd'] = v
elif k == 'APPEND':
syslinux_entry['options'] = v
if syslinux_entry is not None:
syslinux_entries.append(syslinux_entry)

if len(entries) != len(syslinux_entries):
fatal("Found {0} loader entries, but {1} SYSLINUX entries\n".format(len(entries), len(syslinux_entries)))

def assert_matches_key(a, b, key):

def assert_eq(a, b):
assert a == b, "%r == %r" % (a, b)


def assert_key_same_file(a, b, key):
aval = a[key]
bval = b[key]
if aval != bval:
fatal("Mismatch on {0}: {1} != {2}".format(key, aval, bval))
sys.stderr.write("aval: %r\nbval: %r\n" % (aval, bval))

# Paths in entries are always relative to /boot
entry = os.stat(sysroot + "/boot" + aval)

# Syslinux entries can be relative to /boot (if it's on another filesystem)
# or relative to / if /boot is on /.
s1 = os.stat(sysroot + bval)
s2 = os.stat(sysroot + "/boot" + bval)

# A symlink ensures that no matter what they point at the same file
assert_eq(entry, s1)
assert_eq(entry, s2)


for i,(entry,syslinuxentry) in enumerate(zip(entries, syslinux_entries)):
assert_matches_key(entry, syslinuxentry, 'linux')
assert_matches_key(entry, syslinuxentry, 'initrd')
assert_key_same_file(entry, syslinuxentry, 'linux')
assert_key_same_file(entry, syslinuxentry, 'initrd')
entry_ostree = get_ostree_option(entry['options'])
syslinux_ostree = get_ostree_option(syslinuxentry['options'])
if entry_ostree != syslinux_ostree:
Expand Down