Add `blockdev` and update bios.rs #820

HuijingHei · 2025-01-20T05:43:54Z

Prep for #132

Add bootc-blockdev and local crate blockdev
See containers/bootc#1050

blockdev.rs: Add #[allow(dead_code)] for the unused functions

bios.rs: update functions to use blockdev

openshift-ci · 2025-01-20T05:43:58Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

cgwalters · 2025-01-20T15:47:53Z

src/bios.rs

-        let device = self.get_device()?;
-        let device = device.trim();
-        self.run_grub_install("/", device)?;
+        let sysroot = sysroot.recover_path()?;


This is taking us back from being file-descriptor based to path based, which is OK here, but it's usually better to find a way to continue using the fd.

Make sense, update to use format!("/proc/self/fd/{}", sysroot.as_raw_fd()), is this that you meant?

cgwalters · 2025-01-20T15:51:19Z

src/bios.rs

-        self.run_grub_install("/", device)?;
+        let sysroot = sysroot.recover_path()?;
+        let dest_root = sysroot.to_str().unwrap_or("/");
+        let devices = blockdev::get_backing_devices(&dest_root)?


In this case, one technique is doing e.g. findmnt -nv -T . -o SOURCE and passing the file descriptor as the working directory. This would extend our bindings in findmnt.rs.

Sorry, seems this is a little confusing, findmnt find the source of mount point, for example on RAID1 is /dev/md127, what we want is call find_parent_devices() to get the parent device.

cgwalters · 2025-01-20T15:51:45Z

src/bios.rs

+        let devices = blockdev::get_backing_devices(&dest_root)?
+            .into_iter()
+            .next();
+        let dev = devices.unwrap();


Let's use .ok_or_else(|| anyhow::anyhow!("Failed to find backing device for {dest_root}")) or so

cgwalters · 2025-01-20T16:15:46Z

src/blockdev.rs

+}
+
+/// Find all bios_boot partitions on the backing devices with mountpoint boot
+#[allow(dead_code)]


I'm fine with this as is but I think it'd be slightly cleaner if this was part of the second patch

cgwalters · 2025-01-20T16:17:05Z

src/blockdev.rs

@@ -0,0 +1,208 @@
+use std::collections::HashMap;


Not a blocker here but maybe we should try to sync this code with https://github.com/containers/bootc/blob/main/lib/src/blockdev.rs ?

I could try to refactor that into a separate internal crate that we sync back into bootupd and coreos-installer?

Or of course we could even try to publish it.

Not a blocker here but maybe we should try to sync this code with https://github.com/containers/bootc/blob/main/lib/src/blockdev.rs ?

I could try to refactor that into a separate internal crate that we sync back into bootupd and coreos-installer?

Or of course we could even try to publish it.

Thanks, that would be better and make this easier.
I tried to copy the bootc/blockdev.rs, but seems there are a lot of dependencies that also need to sync, for example Task and bootc_utils::CommandRunExt, so I give up.

The idea of #820 (comment) is that the Task dependency is gone. However yes, we'd need to pull in bootc-utils too. We could even do that now.

OK I did just that in #821

Cool, thanks!

Can we import https://github.com/containers/bootc/blob/main/lib/src/blockdev.rs like what you did in #821 ? Or wait until it is published?

containers/bootc#1050

cgwalters · 2025-01-20T16:21:00Z

src/bios.rs

-        let device = device.trim();
-        self.run_grub_install("/", device)?;
+        let target_root = "/";
+        let devices = blockdev::get_backing_devices(&target_root)?


How about adding a helper function like blockdev::get_single_backing_device() that errors out if it finds > 1 (and also errors out if we somehow get zero).

Update to use blockdev::get_single_device().

The install code and the blockdev code call each other. Clean up blockdev to only use `bootc-utils`. Prep for splitting out the blockdev stuff into at least a separate internal crate; ref coreos/bootupd#820 Signed-off-by: Colin Walters <[email protected]>

cgwalters · 2025-01-21T01:45:39Z

FYI I did containers/bootc#1048 to try to prepare bootc's blockdev.rs for reuse, but again nonblocking

HuijingHei · 2025-01-24T09:56:55Z

Close this as did in #824

openshift-ci bot added the do-not-merge/work-in-progress label Jan 20, 2025

HuijingHei mentioned this pull request Jan 20, 2025

Support updating multiple EFIs in mirrored setups (RAID1) #813

Draft

HuijingHei marked this pull request as ready for review January 20, 2025 05:49

openshift-ci bot removed the do-not-merge/work-in-progress label Jan 20, 2025

cgwalters reviewed Jan 20, 2025

View reviewed changes

cgwalters mentioned this pull request Jan 20, 2025

install/blockdev: Break cyclic build dependency containers/bootc#1048

Merged

HuijingHei force-pushed the add-blockdev branch 3 times, most recently from 45bc80f to 74f86fa Compare January 22, 2025 13:17

HuijingHei marked this pull request as draft January 22, 2025 13:29

openshift-ci bot added the do-not-merge/work-in-progress label Jan 22, 2025

HuijingHei added 2 commits January 22, 2025 23:05

Add blockdev.rs to parse block device

b204c76

blockdev.rs: add #[allow(dead_code)] for the unused functions

25cb329

HuijingHei force-pushed the add-blockdev branch from 74f86fa to 3bbfd45 Compare January 22, 2025 15:12

bios.rs: update functions to use blockdev

aeec489

HuijingHei force-pushed the add-blockdev branch from 3bbfd45 to aeec489 Compare January 22, 2025 15:13

HuijingHei closed this Jan 24, 2025

HuijingHei mentioned this pull request Jan 26, 2025

Add bootc-blockdev and local mod blockdev #824

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `blockdev` and update bios.rs #820

Add `blockdev` and update bios.rs #820

HuijingHei commented Jan 20, 2025 •

edited

Loading

openshift-ci bot commented Jan 20, 2025

cgwalters Jan 20, 2025

HuijingHei Jan 22, 2025

cgwalters Jan 20, 2025

HuijingHei Jan 21, 2025

cgwalters Jan 20, 2025

cgwalters Jan 20, 2025

cgwalters Jan 20, 2025

HuijingHei Jan 21, 2025

cgwalters Jan 21, 2025

HuijingHei Jan 21, 2025

HuijingHei Jan 21, 2025 •

edited

Loading

cgwalters Jan 22, 2025

cgwalters Jan 20, 2025

HuijingHei Jan 22, 2025

cgwalters commented Jan 21, 2025

HuijingHei commented Jan 24, 2025

Add blockdev and update bios.rs #820

Add blockdev and update bios.rs #820

Conversation

HuijingHei commented Jan 20, 2025 • edited Loading

openshift-ci bot commented Jan 20, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HuijingHei Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cgwalters commented Jan 21, 2025

HuijingHei commented Jan 24, 2025

Add `blockdev` and update bios.rs #820

Add `blockdev` and update bios.rs #820

HuijingHei commented Jan 20, 2025 •

edited

Loading

HuijingHei Jan 21, 2025 •

edited

Loading