Skip to content

Commit

Permalink
install: Fork blockdev --rereadpt instead of internal ioctl
Browse files Browse the repository at this point in the history
The `ioctl` to reread the partition table was the last thing
pulling in `nix`. Honestly especially in the install path
I have no problem forking external binaries, and since we're
already depending on util-linux-core for other things like
`sfdisk` and `lsblk`, it just makes sense to so here with
the command whose entire role in life is to just issue
that `ioctl`.

This drops out an older verison of `nix` (which is a fine crate, but again
I think `rustix` is nicer). There's a different version
pulled in via libsystemd, but we can get to that later.

Signed-off-by: Colin Walters <[email protected]>
  • Loading branch information
cgwalters committed Jul 17, 2024
1 parent d91c00a commit 0002d6e
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 67 deletions.
21 changes: 1 addition & 20 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions lib/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ libc = { workspace = true }
liboverdrop = "0.1.0"
libsystemd = "0.7"
openssl = "^0.10.64"
# TODO drop this in favor of rustix
nix = { version = "0.29", features = ["ioctl", "sched" ] }
regex = "1.10.4"
rustix = { "version" = "0.38.34", features = ["thread", "fs", "system", "process"] }
schemars = { version = "0.8.17", features = ["chrono"] }
Expand Down
37 changes: 0 additions & 37 deletions lib/src/blockdev.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
use std::collections::HashMap;
use std::env;
use std::fs::File;
use std::os::unix::io::AsRawFd;
use std::path::Path;
use std::process::Command;
use std::sync::OnceLock;

use anyhow::{anyhow, Context, Result};
use camino::{Utf8Path, Utf8PathBuf};
use fn_error_context::context;
use nix::errno::Errno;
use regex::Regex;
use serde::Deserialize;

Expand Down Expand Up @@ -270,30 +267,6 @@ pub(crate) fn udev_settle() -> Result<()> {
Ok(())
}

#[allow(unsafe_code)]
pub(crate) fn reread_partition_table(file: &mut File, retry: bool) -> Result<()> {
let fd = file.as_raw_fd();
// Reread sometimes fails inexplicably. Retry several times before
// giving up.
let max_tries = if retry { 20 } else { 1 };
for retries in (0..max_tries).rev() {
let result = unsafe { ioctl::blkrrpart(fd) };
match result {
Ok(_) => break,
Err(err) if retries == 0 && err == Errno::EINVAL => {
return Err(err)
.context("couldn't reread partition table: device may not support partitions")
}
Err(err) if retries == 0 && err == Errno::EBUSY => {
return Err(err).context("couldn't reread partition table: device is in use")
}
Err(err) if retries == 0 => return Err(err).context("couldn't reread partition table"),
Err(_) => std::thread::sleep(std::time::Duration::from_millis(100)),
}
}
Ok(())
}

/// Parse key-value pairs from lsblk --pairs.
/// Newer versions of lsblk support JSON but the one in CentOS 7 doesn't.
fn split_lsblk_line(line: &str) -> HashMap<String, String> {
Expand Down Expand Up @@ -340,16 +313,6 @@ pub(crate) fn find_parent_devices(device: &str) -> Result<Vec<String>> {
Ok(parents)
}

// create unsafe ioctl wrappers
#[allow(clippy::missing_safety_doc)]
mod ioctl {
use libc::c_int;
use nix::{ioctl_none, ioctl_read, ioctl_read_bad, libc, request_code_none};
ioctl_none!(blkrrpart, 0x12, 95);
ioctl_read_bad!(blksszget, request_code_none!(0x12, 104), c_int);
ioctl_read!(blkgetsize64, 0x12, 114, libc::size_t);
}

/// Parse a string into mibibytes
pub(crate) fn parse_size_mib(mut s: &str) -> Result<u64> {
let suffixes = [
Expand Down
12 changes: 4 additions & 8 deletions lib/src/install/baseline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,14 +315,10 @@ pub(crate) fn install_create_rootfs(
tracing::debug!("Created partition table");

// Reread the partition table
{
let mut f = std::fs::OpenOptions::new()
.write(true)
.open(&devpath)
.with_context(|| format!("opening {devpath}"))?;
crate::blockdev::reread_partition_table(&mut f, true)
.context("Rereading partition table")?;
}
Task::new("Reread partition table", "blockdev")
.arg("--rereadpt")
.arg(devpath.as_str())
.run()?;

// Full udev sync; it'd obviously be better to await just the devices
// we're targeting, but this is a simple coarse hammer.
Expand Down

0 comments on commit 0002d6e

Please sign in to comment.