Merge pull request #1048 from cgwalters/blockdev-no-task

install/blockdev: Break cyclic build dependency

Author: cgwalters

lib/src/blockdev.rs

@@ -14,9 +14,6 @@ use fn_error_context::context;
 use regex::Regex;
 use serde::Deserialize;
 
-#[cfg(feature = "install-to-disk")]
-use crate::install::run_in_host_mountns;
-use crate::task::Task;
 use bootc_utils::CommandRunExt;
 
 #[derive(Debug, Deserialize)]
@@ -91,16 +88,6 @@ impl Device {
     }
 }
 
-#[context("Failed to wipe {dev}")]
-#[cfg(feature = "install-to-disk")]
-pub(crate) fn wipefs(dev: &Utf8Path) -> Result<()> {
-    Task::new_and_run(
-        format!("Wiping device {dev}"),
-        "wipefs",
-        ["-a", dev.as_str()],
-    )
-}
-
 #[context("Listing device {dev}")]
 pub(crate) fn list_dev(dev: &Utf8Path) -> Result<Device> {
     let mut devs: DevicesOutput = Command::new("lsblk")
@@ -187,10 +174,9 @@ impl Partition {
 
 #[context("Listing partitions of {dev}")]
 pub(crate) fn partitions_of(dev: &Utf8Path) -> Result<PartitionTable> {
-    let o = Task::new_quiet("sfdisk")
+    let o: SfDiskOutput = Command::new("sfdisk")
         .args(["-J", dev.as_str()])
-        .read()?;
-    let o: SfDiskOutput = serde_json::from_str(&o).context("Parsing sfdisk output")?;
+        .run_and_parse_json()?;
     Ok(o.partitiontable)
 }
 
@@ -214,16 +200,15 @@ impl LoopbackDevice {
             Err(_e) => "off",
         };
 
-        let dev = Task::new("losetup", "losetup")
+        let dev = Command::new("losetup")
             .args([
                 "--show",
                 format!("--direct-io={direct_io}").as_str(),
                 "-P",
                 "--find",
             ])
             .arg(path)
-            .quiet()
-            .read()?;
+            .run_get_string()?;
         let dev = Utf8PathBuf::from(dev.trim());
         tracing::debug!("Allocated loopback {dev}");
         Ok(Self { dev: Some(dev) })
@@ -242,10 +227,7 @@ impl LoopbackDevice {
             tracing::trace!("loopback device already deallocated");
             return Ok(());
         };
-        Task::new("losetup", "losetup")
-            .args(["-d", dev.as_str()])
-            .quiet()
-            .run()
+        Command::new("losetup").args(["-d", dev.as_str()]).run()
     }
 
     /// Consume this device, unmounting it.
@@ -262,21 +244,6 @@ impl Drop for LoopbackDevice {
     }
 }
 
-#[cfg(feature = "install-to-disk")]
-pub(crate) fn udev_settle() -> Result<()> {
-    // There's a potential window after rereading the partition table where
-    // udevd hasn't yet received updates from the kernel, settle will return
-    // immediately, and lsblk won't pick up partition labels.  Try to sleep
-    // our way out of this.
-    std::thread::sleep(std::time::Duration::from_millis(200));
-
-    let st = run_in_host_mountns("udevadm").arg("settle").status()?;
-    if !st.success() {
-        anyhow::bail!("Failed to run udevadm settle: {st:?}");
-    }
-    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> {
@@ -293,15 +260,15 @@ fn split_lsblk_line(line: &str) -> HashMap<String, String> {
 /// hierarchy of `device` capable of containing other partitions. So e.g. parent devices of type
 /// "part" doesn't match, but "disk" and "mpath" does.
 pub(crate) fn find_parent_devices(device: &str) -> Result<Vec<String>> {
-    let output = Task::new_quiet("lsblk")
+    let output = Command::new("lsblk")
         // Older lsblk, e.g. in CentOS 7.6, doesn't support PATH, but --paths option
         .arg("--pairs")
         .arg("--paths")
         .arg("--inverse")
         .arg("--output")
         .arg("NAME,TYPE")
         .arg(device)
-        .read()?;
+        .run_get_string()?;
     let mut parents = Vec::new();
     // skip first line, which is the device itself
     for line in output.lines().skip(1) {

lib/src/install/baseline.rs

@@ -131,6 +131,31 @@ fn mkfs<'a>(
     Ok(u)
 }
 
+#[context("Failed to wipe {dev}")]
+pub(crate) fn wipefs(dev: &Utf8Path) -> Result<()> {
+    Task::new_and_run(
+        format!("Wiping device {dev}"),
+        "wipefs",
+        ["-a", dev.as_str()],
+    )
+}
+
+pub(crate) fn udev_settle() -> Result<()> {
+    // There's a potential window after rereading the partition table where
+    // udevd hasn't yet received updates from the kernel, settle will return
+    // immediately, and lsblk won't pick up partition labels.  Try to sleep
+    // our way out of this.
+    std::thread::sleep(std::time::Duration::from_millis(200));
+
+    let st = super::run_in_host_mountns("udevadm")
+        .arg("settle")
+        .status()?;
+    if !st.success() {
+        anyhow::bail!("Failed to run udevadm settle: {st:?}");
+    }
+    Ok(())
+}
+
 #[context("Creating rootfs")]
 #[cfg(feature = "install-to-disk")]
 pub(crate) fn install_create_rootfs(
@@ -164,10 +189,10 @@ pub(crate) fn install_create_rootfs(
         for child in device.children.iter().flatten() {
             let child = child.path();
             println!("Wiping {child}");
-            crate::blockdev::wipefs(Utf8Path::new(&child))?;
+            wipefs(Utf8Path::new(&child))?;
         }
         println!("Wiping {dev}");
-        crate::blockdev::wipefs(dev)?;
+        wipefs(dev)?;
     } else if device.has_children() {
         anyhow::bail!(
             "Detected existing partitions on {}; use e.g. `wipefs` or --wipe if you intend to overwrite",
@@ -289,7 +314,7 @@ pub(crate) fn install_create_rootfs(
 
     // Full udev sync; it'd obviously be better to await just the devices
     // we're targeting, but this is a simple coarse hammer.
-    crate::blockdev::udev_settle()?;
+    udev_settle()?;
 
     // Re-read what we wrote into structured information
     let base_partitions = &crate::blockdev::partitions_of(&devpath)?;

utils/src/command.rs

@@ -23,6 +23,9 @@ pub trait CommandRunExt {
     /// and will return an error if the child process exits abnormally.
     fn run_get_output(&mut self) -> Result<Box<dyn std::io::BufRead>>;
 
+    /// Execute the child process and capture its output as a string.
+    fn run_get_string(&mut self) -> Result<String>;
+
     /// Execute the child process, parsing its stdout as JSON. This uses `run` internally
     /// and will return an error if the child process exits abnormally.
     fn run_and_parse_json<T: serde::de::DeserializeOwned>(&mut self) -> Result<T>;
@@ -118,6 +121,13 @@ impl CommandRunExt for Command {
         Ok(Box::new(std::io::BufReader::new(stdout)))
     }
 
+    fn run_get_string(&mut self) -> Result<String> {
+        let mut s = String::new();
+        let mut o = self.run_get_output()?;
+        o.read_to_string(&mut s)?;
+        Ok(s)
+    }
+
     /// Synchronously execute the child, and parse its stdout as JSON.
     fn run_and_parse_json<T: serde::de::DeserializeOwned>(&mut self) -> Result<T> {
         let output = self.run_get_output()?;