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.

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

install: Use zipl to install bootloader on s390x #665

Merged
merged 2 commits into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
79 changes: 78 additions & 1 deletion lib/src/bootloader.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use anyhow::Result;
use anyhow::{anyhow, bail, Context, Result};
use camino::{Utf8Path, Utf8PathBuf};
use fn_error_context::context;

Expand Down Expand Up @@ -51,3 +51,80 @@ pub(crate) fn install_via_bootupd(
.verbose()
.run()
}

#[context("Installing bootloader using zipl")]
pub(crate) fn install_via_zipl(device: &PartitionTable, boot_uuid: &str) -> Result<()> {
// Identify the target boot partition from UUID
let fs = crate::mount::inspect_filesystem_by_uuid(boot_uuid)?;
let boot_dir = Utf8Path::new(&fs.target);
let maj_min = fs.maj_min;
Comment on lines +57 to +60
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just looking at this a bit more closely...I think this is fine for now, but what I think we should do instead as a followup is just pass a Dir (file descriptor) for the boot as we mounted it. Then we can probably use findmnt -J . on it or so.


// Ensure that the found partition is a part of the target device
let device_path = device.path();

let partitions = crate::blockdev::list_dev(device_path)?
.children
.with_context(|| format!("no partition found on {device_path}"))?;
let boot_part = partitions
.iter()
.find(|part| part.maj_min.as_deref() == Some(maj_min.as_str()))
.with_context(|| format!("partition device {maj_min} is not on {device_path}"))?;
let boot_part_offset = boot_part.start.unwrap_or(0);

// Find exactly one BLS configuration under /boot/loader/entries
// TODO: utilize the BLS parser in ostree
let bls_dir = boot_dir.join("boot/loader/entries");
yoheiueda marked this conversation as resolved.
Show resolved Hide resolved
let bls_entry = bls_dir
.read_dir_utf8()?
.try_fold(None, |acc, e| -> Result<_> {
let e = e?;
let name = Utf8Path::new(e.file_name());
if let Some("conf") = name.extension() {
if acc.is_some() {
bail!("more than one BLS configurations under {bls_dir}");
}
Ok(Some(e.path().to_owned()))
} else {
Ok(None)
}
})?
.with_context(|| format!("no BLS configuration under {bls_dir}"))?;

let bls_path = bls_dir.join(bls_entry);
let bls_conf =
std::fs::read_to_string(&bls_path).with_context(|| format!("reading {bls_path}"))?;

let mut kernel = None;
let mut initrd = None;
let mut options = None;

for line in bls_conf.lines() {
match line.split_once(char::is_whitespace) {
Some(("linux", val)) => kernel = Some(val.trim().trim_start_matches('/')),
Some(("initrd", val)) => initrd = Some(val.trim().trim_start_matches('/')),
Some(("options", val)) => options = Some(val.trim()),
_ => (),
}
}

let kernel = kernel.ok_or_else(|| anyhow!("missing 'linux' key in default BLS config"))?;
let initrd = initrd.ok_or_else(|| anyhow!("missing 'initrd' key in default BLS config"))?;
let options = options.ok_or_else(|| anyhow!("missing 'options' key in default BLS config"))?;

let image = boot_dir.join(kernel).canonicalize_utf8()?;
let ramdisk = boot_dir.join(initrd).canonicalize_utf8()?;

// Execute the zipl command to install bootloader
let zipl_desc = format!("running zipl to install bootloader on {device_path}");
let zipl_task = Task::new(&zipl_desc, "zipl")
.args(["--target", boot_dir.as_str()])
.args(["--image", image.as_str()])
.args(["--ramdisk", ramdisk.as_str()])
.args(["--parameters", options])
.args(["--targetbase", device_path.as_str()])
.args(["--targettype", "SCSI"])
.args(["--targetblocksize", "512"])
.args(["--targetoffset", &boot_part_offset.to_string()])
.args(["--add-files", "--verbose"]);
zipl_task.verbose().run().context(zipl_desc)
}
27 changes: 14 additions & 13 deletions lib/src/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -573,15 +573,9 @@ async fn initialize_ostree_root_from_self(
crate::lsm::ensure_dir_labeled(rootfs_dir, "boot", None, 0o755.into(), sepolicy)?;
}

// Default to avoiding grub2-mkconfig etc., but we need to use zipl on s390x.
// TODO: Lower this logic into ostree proper.
let bootloader = if cfg!(target_arch = "s390x") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this means we're now relying on ostreedev/ostree@e3d93a8 which should be fine.

"zipl"
} else {
"none"
};
for (k, v) in [
("sysroot.bootloader", bootloader),
// Default to avoiding grub2-mkconfig etc.
("sysroot.bootloader", "none"),
// Always flip this one on because we need to support alongside installs
// to systems without a separate boot partition.
("sysroot.bootprefix", "true"),
Expand Down Expand Up @@ -1239,12 +1233,17 @@ async fn install_to_filesystem_impl(state: &State, rootfs: &mut RootSetup) -> Re
})
.context("Writing aleph version")?;
}
if cfg!(target_arch = "s390x") {
// TODO: Integrate s390x support into install_via_bootupd
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah...that would definitely be better in the longer term I think, but this is totally fine to do here for now. See also coreos/bootupd#432

crate::bootloader::install_via_zipl(&rootfs.device_info, boot_uuid)?;
} else {
crate::bootloader::install_via_bootupd(
&rootfs.device_info,
&rootfs.rootfs,
&state.config_opts,
)?;
}

crate::bootloader::install_via_bootupd(
&rootfs.device_info,
&rootfs.rootfs,
&state.config_opts,
)?;
tracing::debug!("Installed bootloader");

// Finalize mounted filesystems
Expand Down Expand Up @@ -1688,7 +1687,9 @@ fn test_gather_root_args() {
// A basic filesystem using a UUID
let inspect = Filesystem {
source: "/dev/vda4".into(),
target: "/".into(),
fstype: "xfs".into(),
maj_min: "252:4".into(),
options: "rw".into(),
uuid: Some("965eb3c7-5a3f-470d-aaa2-1bcf04334bc6".into()),
};
Expand Down
11 changes: 10 additions & 1 deletion lib/src/mount.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ use crate::task::Task;
pub(crate) struct Filesystem {
// Note if you add an entry to this list, you need to change the --output invocation below too
pub(crate) source: String,
pub(crate) target: String,
#[serde(rename = "maj:min")]
pub(crate) maj_min: String,
pub(crate) fstype: String,
pub(crate) options: String,
pub(crate) uuid: Option<String>,
Expand All @@ -29,7 +32,7 @@ fn run_findmnt(args: &[&str], path: &str) -> Result<Filesystem> {
"-J",
"-v",
// If you change this you probably also want to change the Filesystem struct above
"--output=SOURCE,FSTYPE,OPTIONS,UUID",
"--output=SOURCE,TARGET,MAJ:MIN,FSTYPE,OPTIONS,UUID",
])
.args(args)
.arg(path)
Expand All @@ -49,6 +52,12 @@ pub(crate) fn inspect_filesystem(path: &Utf8Path) -> Result<Filesystem> {
run_findmnt(&["--mountpoint"], path.as_str())
}

#[context("Inspecting filesystem by UUID {uuid}")]
/// Inspect a filesystem by partition UUID
pub(crate) fn inspect_filesystem_by_uuid(uuid: &str) -> Result<Filesystem> {
run_findmnt(&["--source"], &(format!("UUID={uuid}")))
}

/// Mount a device to the target path.
pub(crate) fn mount(dev: &str, target: &Utf8Path) -> Result<()> {
Task::new_and_run(
Expand Down