Make parent devices global

[HuijingHei]

The main of the patch is to initialize parent devices once at the beginning and read them then, also remove some duplicated sysroot as openat::Dir::open("/") and pass as a parameter.

Refer to #855 (comment)

[HuijingHei]

@dustymabe I think you meant to initialize esp devices, store them and read after, this is the first step make parent devices global, initialize once and read then. Not sure if can make esp devices or bios devices like this, or other way?

[nikita-dubrovskii]

LGTM

[cgwalters]

Here's where I'd suggest moving the static instead.

However even then in theory the parent devices are scoped to a particular destination root and so it can't actually be static. Which means it's hard to cache in that way.

So if we wanted to avoid querying the devices multiple times, then what we'd need to do is query them up front and then pass them down. That may involve more code churn though.

[cgwalters]

Hmm so actually we always operated on /, and this code pattern was just a really complicated way to go from / -> file descriptor -> / effectively.

The only tricky thing here is that eventually we actually do want to operate on the non-booted root! See #108

[cgwalters]

Per above I think we can just for now hardcode something like
let devices = crate::blockdev::get_devices("/") here.

It actually may make sense to move the static to this function as it's the one that's also hardcoding /.

[HuijingHei]

Updated, seems will read the static devices later, move the static to crate::blockdev::get_parent_devices() with option, add get_parent_devices(None) to verify it works. WDYT?

[dustymabe]

IANA rust expert, but LGTM

[HuijingHei]

Thank you all for the review! Does it make sense to do the same thing for esp devices and bios devices, make all of them global, initialize at the beginning and reuse them? Like

struct DeviceParts {
    parent_devices: Vec<String>,
    efi_devices: Option<Vec<String>>,
    bios_devices: Option<Vec<String>>,
}

Or just parent_devices is enough?

[HuijingHei]

Edit:
If there is no objection, will merge it tomorrow.

[HuijingHei]

Sorry that click the wrong button, reopen it.

[cgwalters]

I think Dusty's original comment in #855 (comment) was correct in that we're clearly parsing the devices a lot. However, this is turning into a nontrivial architectural change and it's not clear to me that we have any real performance concerns here. (Really, most of this data is cached in the kernel or filesystem, we're just re-parsing files into JSON and then back into data structures, but computers are fast)

Looking at this more I don't think we want a global cache, we just want to gather the context up front and pass it down. So instead of passing just plain "sysroot" we'd pass e.g.

struct RootContext {
  sysroot: openat::Dir,
  devices: Vec<String>
}

or so? (And actually given all the pain that using openat has caused here, we might as well add path: Utf8PathBuf to that struct, so anything that wants to can just access it as an absolute path instead. I have something similar in bootc's install code. To make that work when we eventually do #108 we'd just need to also mount the target root)

[dustymabe]

Looking at this more I don't think we want a global cache, we just want to gather the context up front and pass it down. So instead of passing just plain "sysroot" we'd pass e.g...

This SGTM.