use PathBuf/&Path to represent file/directory paths (openzfs#431)

In general, `PathBuf`/`&Path` should be used instead of `String`/`&str`
because unix paths can be any nul-terminated byte array, which is more
general than Rust's String (which must be valid UTF-8).  By using
String, we are unable to operate on some files (those whose names are
not valid UTF-8).  However, for the agent, this isn't that big of a deal
in practice, since this isn't a general file-processing utility.

However, we can still benefit from using Path:
* The more specific type makes it more clear what variables are used for
* We can take advantage of the directory-path-specific helper methods

This commit changes our codebase to use PathBuf/Path.

cmd/zfs_object_agent/object_perf/src/main.rs

@@ -2,6 +2,7 @@
 #![allow(clippy::print_stderr)]
 #![allow(clippy::print_stdout)]
 
+use std::path::PathBuf;
 use std::time::Duration;
 
 use clap::Parser;
@@ -69,11 +70,11 @@ struct Cli {
         value_name = "FILE",
         default_value = "/var/log/perflog"
     )]
-    output_file: String,
+    output_file: PathBuf,
 
     /// Configuration file to set tunables (toml/json/yaml)
     #[clap(short = 't', long, value_name = "FILE")]
-    config_file: Option<String>,
+    config_file: Option<PathBuf>,
 
     #[clap(subcommand)]
     command: Commands,

cmd/zfs_object_agent/server/src/main.rs

@@ -1,3 +1,4 @@
+use std::path::PathBuf;
 use std::time::Duration;
 
 use clap::Args;
@@ -38,7 +39,7 @@ struct LoggingArgs {
 
     /// File to log output to
     #[clap(short = 'o', long, value_name = "FILE")]
-    output_file: Option<String>,
+    output_file: Option<PathBuf>,
 
     /// Logging configuration yaml file
     #[clap(
@@ -48,7 +49,7 @@ struct LoggingArgs {
         conflicts_with = "output-file",
         conflicts_with = "verbosity"
     )]
-    log_config: Option<String>,
+    log_config: Option<PathBuf>,
 }
 
 #[derive(Parser)]
@@ -62,11 +63,11 @@ struct Cli {
 
     /// Configuration file to set tunables (toml/json/yaml)
     #[clap(short = 't', long, value_name = "FILE")]
-    config_file: Option<String>,
+    config_file: Option<PathBuf>,
 
     /// Directory for unix-domain sockets
     #[clap(short = 'k', long, value_name = "DIR", default_value = "/etc/zfs")]
-    socket_dir: String,
+    socket_dir: PathBuf,
 
     /// File/device to use for ZettaCache
     #[clap(
@@ -75,12 +76,12 @@ struct Cli {
         value_name = "PATH",
         conflicts_with = "cache-device-dir"
     )]
-    cache_device: Option<Vec<String>>,
+    cache_device: Option<Vec<PathBuf>>,
 
     /// Directory path to use for importing devices that are part of the
     /// ZettaCache
     #[clap(short = 'd', long, value_name = "DIR")]
-    cache_device_dir: Option<String>,
+    cache_device_dir: Option<PathBuf>,
 
     /// Clear the cache when it has incompatible features
     #[clap(long)]
@@ -269,8 +270,8 @@ fn main() {
             });
 
             let cache_mode = match cli.cache_device {
-                Some(paths) => Some(CacheOpenMode::new_device_list(paths)),
-                None => cli.cache_device_dir.map(CacheOpenMode::new_device_dir),
+                Some(paths) => Some(CacheOpenMode::DeviceList(paths)),
+                None => cli.cache_device_dir.map(CacheOpenMode::DiscoveryDirectory),
             };
 
             match zettaobject::init::start(

cmd/zfs_object_agent/util/src/logging.rs

@@ -5,6 +5,7 @@ use std::io::BufWriter;
 use std::io::Write;
 use std::panic;
 use std::panic::PanicInfo;
+use std::path::Path;
 use std::process;
 use std::sync::atomic::AtomicBool;
 use std::sync::atomic::Ordering;
@@ -241,7 +242,7 @@ fn setup_console_logging(verbosity: u64) {
     log4rs::init_config(config).unwrap();
 }
 
-fn setup_logfile(verbosity: u64, logfile: &str) {
+fn setup_logfile(verbosity: u64, logfile: &Path) {
     let config = Config::builder()
         .appender(
             Appender::builder()
@@ -277,8 +278,8 @@ fn setup_logfile(verbosity: u64, logfile: &str) {
 
 pub fn setup_logging(
     verbosity: u64,
-    file_name: Option<&str>,
-    log_config: Option<&str>,
+    file_name: Option<&Path>,
+    log_config: Option<&Path>,
     quiet_start: bool,
 ) {
     /*

cmd/zfs_object_agent/util/src/message.rs

@@ -1,5 +1,6 @@
 use std::fmt::Debug;
 use std::mem::size_of;
+use std::path::PathBuf;
 use std::ptr;
 use std::slice;
 
@@ -141,5 +142,5 @@ pub const TYPE_INITIATE_MERGE: &str = "initiate merge";
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct AddDiskRequest {
-    pub path: String,
+    pub path: PathBuf,
 }

cmd/zfs_object_agent/util/src/tunable.rs

@@ -1,5 +1,6 @@
 use std::any::type_name;
 use std::fmt::Debug;
+use std::path::Path;
 use std::sync::atomic::*;
 use std::sync::RwLock;
 use std::time::Duration;
@@ -289,10 +290,14 @@ impl Percent {
     }
 }
 
-pub fn read_config(file_name: &str) -> Result<()> {
+pub fn read_config(file_name: &Path) -> Result<()> {
     let mut config = CONFIG.write().unwrap();
     *config = Config::builder()
-        .add_source(config::File::with_name(file_name))
+        .add_source(config::File::with_name(
+            file_name
+                .to_str()
+                .ok_or_else(|| anyhow!("file name not utf8"))?,
+        ))
         .build()?;
     Ok(())
 }

cmd/zfs_object_agent/util/src/zcache_devices.rs

@@ -3,12 +3,14 @@
 //! These structures on the zettacache side are serialized and then deserialized
 //! by the zettacache subcommands.
 
+use std::path::PathBuf;
+
 use serde::Deserialize;
 use serde::Serialize;
 
 #[derive(Serialize, Deserialize)]
 pub struct DeviceEntry {
-    pub name: String,
+    pub name: PathBuf,
     pub size: u64,
 }
 

cmd/zfs_object_agent/zcache/src/add.rs

@@ -1,5 +1,7 @@
 //! `zcache add` subcommand
 
+use std::path::PathBuf;
+
 use anyhow::anyhow;
 use anyhow::Result;
 use async_trait::async_trait;
@@ -15,7 +17,7 @@ use crate::subcommand::ZcacheSubCommand;
 #[derive(Parser)]
 #[clap(about = "Add a disk to the ZettaCache.")]
 pub struct Add {
-    path: String,
+    path: PathBuf,
 }
 
 #[async_trait]
@@ -32,7 +34,7 @@ impl ZcacheSubCommand for Add {
             .await
         {
             Ok(_) => {
-                writeln_stdout!("Disk {} added", self.path);
+                writeln_stdout!("Disk {:?} added", self.path);
             }
             Err(RemoteError::ResultError(_)) => return Err(anyhow!("No cache found")),
             Err(RemoteError::Other(e)) => return Err(e),

cmd/zfs_object_agent/zcache/src/list.rs

@@ -2,7 +2,6 @@
 
 use std::fs;
 use std::path::Path;
-use std::path::PathBuf;
 
 use anyhow::anyhow;
 use anyhow::Context;
@@ -50,24 +49,24 @@ pub struct List {
 
 impl List {
     /// Derive the device name to display based on command input flags.
-    fn derive_name(&self, path: &str) -> String {
-        let path_buf: PathBuf;
-
+    fn derive_name(&self, path: &Path) -> String {
         let device_path = if self.real_paths {
             // Follow any symlinks to get the underlying device
             // e.g. "/dev/xvdz1" -> "/dev/nvme1n1p1"
-            path_buf = fs::canonicalize(path).unwrap();
-            path_buf.as_path()
+            fs::canonicalize(path).unwrap_or_else(|_| path.to_owned())
         } else {
-            Path::new(path)
+            path.to_owned()
         };
 
         if self.full_paths {
-            device_path.to_str().unwrap()
+            device_path.to_string_lossy().into()
         } else {
-            device_path.file_name().unwrap().to_str().unwrap()
+            device_path
+                .file_name()
+                .unwrap_or(device_path.as_os_str())
+                .to_string_lossy()
+                .into()
         }
-        .to_string()
     }
 
     fn max_name_length(&self, devices: &[DeviceEntry]) -> usize {

cmd/zfs_object_agent/zcache/src/main.rs

@@ -14,6 +14,8 @@ mod stats;
 mod subcommand;
 mod sync;
 
+use std::path::PathBuf;
+
 use anyhow::Result;
 use clap::Parser;
 use clap::Subcommand;
@@ -38,7 +40,7 @@ struct Cli {
 
     /// File to log debugging output to
     #[clap(long, requires = "verbose", value_name = "FILE", global = true)]
-    log_file: Option<String>,
+    log_file: Option<PathBuf>,
 
     #[clap(subcommand)]
     command: Commands,

cmd/zfs_object_agent/zcdb/src/main.rs

@@ -1,3 +1,5 @@
+use std::path::PathBuf;
+
 use clap::Parser;
 use clap::Subcommand;
 use git_version::git_version;
@@ -21,7 +23,7 @@ static GIT_VERSION: &str = git_version!(
 struct Cli {
     /// File/device to use for ZettaCache
     #[clap(short = 'c', long, value_name = "PATH")]
-    cache_device: Option<Vec<String>>,
+    cache_device: Option<Vec<PathBuf>>,
 
     /// Directory path to use for importing devices that are part of the
     /// ZettaCache
@@ -32,15 +34,15 @@ struct Cli {
         conflicts_with = "cache-device",
         default_value = "/dev/disk/by-id/"
     )]
-    cache_device_dir: String,
+    cache_device_dir: PathBuf,
 
     /// Sets the verbosity level for logging and debugging
     #[clap(short = 'v', long, parse(from_occurrences), global = true)]
     verbose: u64,
 
     /// File to log debugging output to
     #[clap(long, requires = "verbose", value_name = "FILE", global = true)]
-    log_file: Option<String>,
+    log_file: Option<PathBuf>,
 
     #[clap(subcommand)]
     command: Commands,
@@ -111,8 +113,8 @@ async fn main() -> Result<(), anyhow::Error> {
 
     // Set up cache paths
     let cache_mode = match cli.cache_device {
-        Some(paths) => CacheOpenMode::new_device_list(paths),
-        None => CacheOpenMode::new_device_dir(cli.cache_device_dir),
+        Some(paths) => CacheOpenMode::DeviceList(paths),
+        None => CacheOpenMode::DiscoveryDirectory(cli.cache_device_dir),
     };
 
     match cli.command {

cmd/zfs_object_agent/zettacache/src/block_access.rs

@@ -7,6 +7,7 @@ use std::io::Write;
 use std::os::unix::prelude::AsRawFd;
 use std::os::unix::prelude::OpenOptionsExt;
 use std::path::Path;
+use std::path::PathBuf;
 use std::sync::atomic::Ordering;
 use std::sync::RwLock;
 use std::thread::sleep;
@@ -146,7 +147,8 @@ pub struct Disk {
     #[allow(dead_code)]
     file: &'static File,
 
-    device_path: String,
+    path: PathBuf,
+    canonical_path: PathBuf,
     size: u64,
     sector_size: usize,
     #[derivative(Debug = "ignore")]
@@ -201,7 +203,7 @@ struct WriteMessage {
 }
 
 impl Disk {
-    pub fn new(disk_path: &str, readonly: bool) -> Result<Disk> {
+    pub fn new(path: &Path, readonly: bool) -> Result<Disk> {
         // Note: using std file open so that this func can be non-async.
         // Although this is blocking from a tokio thread, it's used
         // infrequently, and we're already blocking from the ioctls to get the
@@ -210,8 +212,8 @@ impl Disk {
             .read(true)
             .write(!readonly)
             .custom_flags(CUSTOM_OFLAGS)
-            .open(disk_path)
-            .with_context(|| format!("opening disk '{}'", disk_path))?;
+            .open(path)
+            .with_context(|| format!("opening disk {path:?}"))?;
         // see comment in `struct Disk`
         let file = &*Box::leak(Box::new(file));
         let stat = nix::sys::stat::fstat(file.as_raw_fd())?;
@@ -226,15 +228,11 @@ impl Disk {
             size = u64::try_from(stat.st_size)?;
             sector_size = *MIN_SECTOR_SIZE;
         } else {
-            panic!("{}: invalid file type {:?}", disk_path, mode);
+            panic!("{path:?}: invalid file type {mode:?}");
         }
 
-        let device = Path::new(disk_path)
-            .file_name()
-            .unwrap()
-            .to_str()
-            .unwrap()
-            .to_owned();
+        let short_name = path.file_name().unwrap().to_string_lossy().to_string();
+        let canonical_path = Path::new(path).canonicalize()?;
 
         let (reader_tx, reader_rx) = flume::unbounded();
 
@@ -254,11 +252,12 @@ impl Disk {
             metadata_writer_rxs.push(rx);
         }
 
-        let io_stats = &*Box::leak(Box::new(DiskIoStats::new(device)));
+        let io_stats = &*Box::leak(Box::new(DiskIoStats::new(short_name)));
 
         let this = Disk {
             file,
-            device_path: disk_path.to_string(),
+            path: path.to_owned(),
+            canonical_path,
             size,
             sector_size,
             io_stats,
@@ -298,7 +297,7 @@ impl Disk {
                 });
             }
         }
-        info!("opening cache file {}: {:?}", disk_path, this);
+        info!("opening cache file {path:?}: {this:?}");
 
         Ok(this)
     }
@@ -567,7 +566,7 @@ impl BlockAccess {
             .unwrap()
             .iter()
             .map(|d| DeviceEntry {
-                name: d.device_path.to_string(),
+                name: d.path.clone(),
                 size: d.size,
             })
             .collect();
@@ -585,10 +584,8 @@ impl BlockAccess {
         }
     }
 
-    pub fn disk_path(&self, disk: DiskId) -> String {
-        self.disks.read().unwrap()[disk.index()]
-            .device_path
-            .to_string()
+    pub fn disk_path(&self, disk: DiskId) -> PathBuf {
+        self.disks.read().unwrap()[disk.index()].path.clone()
     }
 
     pub fn total_capacity(&self) -> u64 {