From 0002d6e3c34aeac3f91bb219639eef0cbccee851 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 17 Jul 2024 17:42:18 -0400 Subject: [PATCH] install: Fork `blockdev --rereadpt` instead of internal ioctl 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 --- Cargo.lock | 21 +-------------------- lib/Cargo.toml | 2 -- lib/src/blockdev.rs | 37 ------------------------------------- lib/src/install/baseline.rs | 12 ++++-------- 4 files changed, 5 insertions(+), 67 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b6a548c22..ca7e74101 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -183,7 +183,6 @@ dependencies = [ "libc", "liboverdrop", "libsystemd", - "nix 0.29.0", "openssl", "ostree-ext", "regex", @@ -316,12 +315,6 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" -[[package]] -name = "cfg_aliases" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" - [[package]] name = "chrono" version = "0.4.38" @@ -1066,7 +1059,7 @@ dependencies = [ "hmac", "libc", "log", - "nix 0.27.1", + "nix", "nom", "once_cell", "serde", @@ -1188,18 +1181,6 @@ dependencies = [ "memoffset", ] -[[package]] -name = "nix" -version = "0.29.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "71e2746dc3a24dd78b3cfcb7be93368c6de9963d30f43a6a73998a9cf4b17b46" -dependencies = [ - "bitflags 2.4.2", - "cfg-if", - "cfg_aliases", - "libc", -] - [[package]] name = "nom" version = "7.1.3" diff --git a/lib/Cargo.toml b/lib/Cargo.toml index 62fb4f00b..6c3638161 100644 --- a/lib/Cargo.toml +++ b/lib/Cargo.toml @@ -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"] } diff --git a/lib/src/blockdev.rs b/lib/src/blockdev.rs index f0d08e117..9be1d3d65 100644 --- a/lib/src/blockdev.rs +++ b/lib/src/blockdev.rs @@ -1,7 +1,5 @@ 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; @@ -9,7 +7,6 @@ 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; @@ -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 { @@ -340,16 +313,6 @@ pub(crate) fn find_parent_devices(device: &str) -> Result> { 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 { let suffixes = [ diff --git a/lib/src/install/baseline.rs b/lib/src/install/baseline.rs index e8d4c0237..d43042c14 100644 --- a/lib/src/install/baseline.rs +++ b/lib/src/install/baseline.rs @@ -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.