install: Use zipl to install bootloader on s390x #665

yoheiueda · 2024-07-03T07:36:28Z

This is a part of s390x enablement #569

Currently, the zipl command is called via libostree to install bootloader on s390x, but it does not work correctly on a loop device.

This patch changes bootc to execute the zipl command directly with appropriate command line options for installation on a loop device.

cgwalters

Thanks so much for working on this! I have some style/structural nits but I think we're heading in the right direction.

lib/src/bootloader.rs

cgwalters · 2024-07-03T12:53:05Z

lib/src/bootloader.rs

+        }
+    };
+
+    let image_path   = boot_dir.join(image.trim_start_matches('/')).canonicalize_utf8()?;


Did something specifically motivate the .canonicalize_utf8() here out of curiosity?

canonicalize_utf8 is just a cosmetic, nice-to-have thing here.

When the target filesystem is /boot, image_path becomes something like /run/bootc/mounts/rootfs/boot/boot/ostree/.... boot directory appears twice in the path, and the second boot directory is actually a symlink to ..

This path representation is OK for zipl to work correctly, but looks a little bit strange. So I use canonicalize_utf8 to remove the duplicated boot from the path.

cgwalters · 2024-07-03T12:54:32Z

lib/src/install.rs

@@ -1229,7 +1223,12 @@ async fn install_to_filesystem_impl(state: &State, rootfs: &mut RootSetup) -> Re
            .context("Writing aleph version")?;
    }

-    crate::bootloader::install_via_bootupd(&rootfs.device, &rootfs.rootfs, &state.config_opts)?;
+    if cfg!(target_arch = "s390x") {
+        // TODO: Integrate s390x support into install_via_bootupd


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

cgwalters · 2024-07-03T13:24:06Z

lib/src/install.rs

@@ -572,15 +572,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") {


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

cgwalters · 2024-07-10T14:57:56Z

@yoheiueda let me know if you want me to take a pass at updating this PR

yoheiueda · 2024-07-10T15:08:56Z

@cgwalters I am working on this. I will update the patch this week.

BTW, lsblk does not show an partition offset in a disk in my environment. Do you have any idea how to get partition offset info with lsblk?

cgwalters · 2024-07-10T15:18:52Z

BTW, lsblk does not show an partition offset in a disk in my environment. Do you have any idea how to get partition offset info with lsblk?

Just briefly looking at lsblk -J -O /dev/vda in a virt environment I see a start attribute; does that not exist for dasd?

yoheiueda · 2024-07-10T15:31:29Z

lsblk from util-linux 2.37.4 (RHEL 9.4) does not have start attribute, but lsblk from util-linux 2.40.1 (Fedora 40) does have the attribute.

util-linux/util-linux@856457b

It looks like the start attribute has been introduced from 2.38.

@cgwalters do you think it is safe to assume that the newer lsblk? I think the commit needs to be backported to RHEL eventually if we use this command.

jlebon · 2024-07-10T15:38:17Z

It might be in sfdisk -J since that one actually reads the GPT header.

You can also look at the start file in sysfs directly too.

cgwalters · 2024-07-10T15:43:35Z

@cgwalters do you think it is safe to assume that the newer lsblk? I think the commit needs to be backported to RHEL eventually if we use this command.

We need to support whatever is in C9S for code we land today as a baseline. So we could ask for that to be backported.

But, I think it's easiest for now to go with what jlebon suggested, and maybe also queue a backport request.

yoheiueda · 2024-07-11T08:56:56Z

@cgwalters @jlebon thank you for your suggestions. I will use /sys/dev/<maj:min>/start to get a partition offset.

lib/src/bootloader.rs

cgwalters

This is overall looking good!

lib/src/bootloader.rs

cgwalters · 2024-07-13T17:41:48Z

xref #667 (comment)

Specifically on this PR, we all the preparatory changes should be there now in the blockdev bits - do you want to rebase this one on #680 ?

Note specifically for example 61feba4 I think is cleaner.

As of now, if we land #680 first then this PR should no longer textually conflict with #667 at least and they can land in either order.

yoheiueda · 2024-07-14T14:33:03Z

@cgwalters thank you for the new PR. It looks good.

I'll rebase this PR on #680.

yoheiueda · 2024-07-17T07:55:45Z

@cgwalters Ive rebased this PR on #688

cgwalters · 2024-07-17T13:28:21Z

lib/src/bootloader.rs

+    // 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;


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.

This patch adds a new function that identifies a filesystem by partition UUID. Signed-off-by: Yohei Ueda <[email protected]> Signed-off-by: Colin Walters <[email protected]>

Running zipl via ostree does not work correctly on a loop deivce. This patch changes bootc to execute zipl directly with appropriate command line options for installation on a loop device. Signed-off-by: Yohei Ueda <[email protected]>

cgwalters · 2024-07-17T14:08:02Z

(I resolved the trivial conflict with #667 )

efi: change `--update-firmware` to match current Anaconda logic

Release 0.15.0

github-actions bot added the area/install Issues related to `bootc install` label Jul 3, 2024

yoheiueda mentioned this pull request Jul 3, 2024

s390x enablement tracker #569

Closed

cgwalters reviewed Jul 3, 2024

View reviewed changes

yoheiueda force-pushed the zipl branch 5 times, most recently from 1b1ffa0 to 054ea4b Compare July 11, 2024 15:15

yoheiueda mentioned this pull request Jul 11, 2024

lib/bootloader: Write to PReP partition on ppc64le #667

Merged

yoheiueda force-pushed the zipl branch 2 times, most recently from 1b88ae8 to fefca69 Compare July 11, 2024 15:36

cgwalters reviewed Jul 11, 2024

View reviewed changes

lib/src/bootloader.rs Outdated Show resolved Hide resolved

yoheiueda force-pushed the zipl branch from fefca69 to a7510c6 Compare July 12, 2024 07:24

cgwalters reviewed Jul 12, 2024

View reviewed changes

lib/src/bootloader.rs Outdated Show resolved Hide resolved

lib/src/bootloader.rs Outdated Show resolved Hide resolved

lib/src/bootloader.rs Outdated Show resolved Hide resolved

cgwalters reviewed Jul 12, 2024

View reviewed changes

lib/src/bootloader.rs Outdated Show resolved Hide resolved

lib/src/bootloader.rs Outdated Show resolved Hide resolved

lib/src/bootloader.rs Outdated Show resolved Hide resolved

cgwalters mentioned this pull request Jul 13, 2024

blockdev+install updates #680

Merged

yoheiueda force-pushed the zipl branch from a7510c6 to 52a90bd Compare July 17, 2024 07:51

cgwalters approved these changes Jul 17, 2024

View reviewed changes

mount: Add inspect_filesystem_by_uuid

ccbc503

This patch adds a new function that identifies a filesystem by partition UUID. Signed-off-by: Yohei Ueda <[email protected]> Signed-off-by: Colin Walters <[email protected]>

install: Use zipl to install bootloader on s390x

03812c1

Running zipl via ostree does not work correctly on a loop deivce. This patch changes bootc to execute zipl directly with appropriate command line options for installation on a loop device. Signed-off-by: Yohei Ueda <[email protected]>

cgwalters force-pushed the zipl branch from 52a90bd to 03812c1 Compare July 17, 2024 14:07

cgwalters enabled auto-merge July 17, 2024 14:13

cgwalters merged commit 80d102d into containers:main Jul 17, 2024
17 of 27 checks passed

cgwalters added a commit to cgwalters/bootc that referenced this pull request Nov 5, 2024

Merge pull request containers#665 from HuijingHei/sync-anaconda

6e37e52

efi: change `--update-firmware` to match current Anaconda logic

cgwalters pushed a commit to cgwalters/bootc that referenced this pull request Nov 6, 2024

Merge pull request containers#665 from cgwalters/release

db500d3

Release 0.15.0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

install: Use zipl to install bootloader on s390x #665

install: Use zipl to install bootloader on s390x #665

yoheiueda commented Jul 3, 2024

cgwalters left a comment

cgwalters Jul 3, 2024

yoheiueda Jul 3, 2024

cgwalters Jul 3, 2024

cgwalters Jul 3, 2024

cgwalters commented Jul 10, 2024

yoheiueda commented Jul 10, 2024

cgwalters commented Jul 10, 2024

yoheiueda commented Jul 10, 2024

jlebon commented Jul 10, 2024

cgwalters commented Jul 10, 2024

yoheiueda commented Jul 11, 2024

cgwalters left a comment

cgwalters commented Jul 13, 2024 •

edited

Loading

yoheiueda commented Jul 14, 2024

yoheiueda commented Jul 17, 2024

cgwalters Jul 17, 2024

cgwalters commented Jul 17, 2024

install: Use zipl to install bootloader on s390x #665

install: Use zipl to install bootloader on s390x #665

Conversation

yoheiueda commented Jul 3, 2024

cgwalters left a comment

Choose a reason for hiding this comment

cgwalters Jul 3, 2024

Choose a reason for hiding this comment

yoheiueda Jul 3, 2024

Choose a reason for hiding this comment

cgwalters Jul 3, 2024

Choose a reason for hiding this comment

cgwalters Jul 3, 2024

Choose a reason for hiding this comment

cgwalters commented Jul 10, 2024

yoheiueda commented Jul 10, 2024

cgwalters commented Jul 10, 2024

yoheiueda commented Jul 10, 2024

jlebon commented Jul 10, 2024

cgwalters commented Jul 10, 2024

yoheiueda commented Jul 11, 2024

cgwalters left a comment

Choose a reason for hiding this comment

cgwalters commented Jul 13, 2024 • edited Loading

yoheiueda commented Jul 14, 2024

yoheiueda commented Jul 17, 2024

cgwalters Jul 17, 2024

Choose a reason for hiding this comment

cgwalters commented Jul 17, 2024

cgwalters commented Jul 13, 2024 •

edited

Loading