Skip to content

Commit

Permalink
feat(zpool): Change ZpoolEngine::{status,status_all,all} functions (#196
Browse files Browse the repository at this point in the history
)

- `all` is removed to avoid being confused with `status_all`.
 - `status` is now taking `StatusOptions` just like `status_all`

BREAKING CHANGE: ZpoolEngine::all removed. ZpoolEngine::status requires
an additional argument.
  • Loading branch information
andoriyu authored May 13, 2023
1 parent 5d2c847 commit e0be797
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 37 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

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

2 changes: 1 addition & 1 deletion src/log.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use std::ops::Deref;
use once_cell::sync::OnceCell;
use slog::{Drain, Logger as SlogLogger};
use slog_stdlog::StdLog;
use std::borrow::Borrow;
use std::ops::Deref;

static GLOBAL_LOGGER: OnceCell<GlobalLogger> = OnceCell::new();

Expand Down
5 changes: 1 addition & 4 deletions src/zpool/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,10 +416,7 @@ pub trait ZpoolEngine {
fn import_from_dir<N: AsRef<str>>(&self, name: N, dir: PathBuf) -> ZpoolResult<()>;

/// Get the detailed status of the given pools.
fn status<N: AsRef<str>>(&self, name: N) -> ZpoolResult<Zpool>;

/// Get a status of each active (imported) pool in the system
fn all(&self) -> ZpoolResult<Vec<Zpool>>;
fn status<N: AsRef<str>>(&self, name: N, opts: StatusOptions) -> ZpoolResult<Zpool>;

/// Query status with options
fn status_all(&self, opts: StatusOptions) -> ZpoolResult<Vec<Zpool>>;
Expand Down
33 changes: 19 additions & 14 deletions src/zpool/open3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,10 +267,15 @@ impl ZpoolEngine for ZpoolOpen3 {
}
}

fn status<N: AsRef<str>>(&self, name: N) -> ZpoolResult<Zpool> {
fn status<N: AsRef<str>>(&self, name: N, opts: StatusOptions) -> ZpoolResult<Zpool> {
let mut z = self.zpool();
z.arg("status");
z.arg("-P");
if opts.full_paths {
z.arg("-P");
}
if opts.resolve_links {
z.arg("-L");
}
z.arg(name.as_ref());
debug!(self.logger, "executing"; "cmd" => format_args!("{:?}", z));
let out = z.output()?;
Expand All @@ -287,14 +292,6 @@ impl ZpoolEngine for ZpoolOpen3 {
Ok(zpool)
}

fn all(&self) -> ZpoolResult<Vec<Zpool>> {
let mut z = self.zpool();
z.arg("status");
debug!(self.logger, "executing"; "cmd" => format_args!("{:?}", z));
let out = z.output()?;
self.zpools_from_import(out)
}

fn status_all(&self, opts: StatusOptions) -> ZpoolResult<Vec<Zpool>> {
let mut z = self.zpool();
z.arg("status");
Expand Down Expand Up @@ -565,8 +562,14 @@ mod test {
let stdout = include_str!("fixtures/status_with_block_device_nested");
let zpools: Vec<Zpool> = StdoutParser::parse(Rule::zpools, stdout.as_ref())
.map_err(|_| ZpoolError::ParseError)
.map(|pairs| pairs.map(Zpool::from_pest_pair).collect()).unwrap();
let drives = &zpools[0].vdevs().iter().flat_map(|vdev| vdev.disks().iter()).map(|drive| drive.path().display().to_string()).collect::<Vec<String>>();
.map(|pairs| pairs.map(Zpool::from_pest_pair).collect())
.unwrap();
let drives = &zpools[0]
.vdevs()
.iter()
.flat_map(|vdev| vdev.disks().iter())
.map(|drive| drive.path().display().to_string())
.collect::<Vec<String>>();

let expected: Vec<String> = [
"/dev/diskid/DISK-ZCT2K2R6",
Expand All @@ -575,8 +578,10 @@ mod test {
"/dev/diskid/DISK-ZCT2QWL9",
"/dev/diskid/DISK-ZCT2QXEL",
"/dev/diskid/DISK-ZCT2RH0W",
].iter().map(|d| d.to_string()).collect();
]
.iter()
.map(|d| d.to_string())
.collect();
assert_eq!(&expected, drives);
}

}
52 changes: 35 additions & 17 deletions tests/test_zpool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use rand::Rng;
use libzetta::{
slog::*,
zpool::{
open3::{StatusOptions, StatusOptionsBuilder},
CreateMode, CreateVdevRequest, CreateZpoolRequestBuilder, DestroyMode, ExportMode,
FailMode, Health, OfflineMode, OnlineMode, Zpool, ZpoolEngine, ZpoolError, ZpoolErrorKind,
ZpoolOpen3, ZpoolPropertiesWriteBuilder,
Expand Down Expand Up @@ -436,7 +437,7 @@ fn test_status() {
.unwrap();
zpool.create(topo.clone()).unwrap();

let result = zpool.status(&name).unwrap();
let result = zpool.status(&name, StatusOptions::default()).unwrap();
assert_eq!(&name, result.name());
assert_eq!(&result, &topo);
});
Expand All @@ -454,7 +455,7 @@ fn test_all() {
zpool.create(topo.clone()).unwrap();

let result: Vec<Zpool> = zpool
.all()
.status_all(StatusOptions::default())
.unwrap()
.iter()
.cloned()
Expand All @@ -473,7 +474,7 @@ fn test_all_empty() {
let zpool = ZpoolOpen3::default();

let result: Vec<Zpool> = zpool
.all()
.status_all(StatusOptions::default())
.unwrap()
.iter()
.cloned()
Expand Down Expand Up @@ -556,13 +557,13 @@ fn test_zpool_take_device_from_mirror_offline() {
let result = zpool.take_offline(&name, &vdev0_path, OfflineMode::UntilReboot);
assert!(result.is_ok());

let z = zpool.status(&name).unwrap();
let z = zpool.status(&name, StatusOptions::default()).unwrap();
assert_eq!(&Health::Degraded, z.health());

let result = zpool.bring_online(&name, &vdev0_path, OnlineMode::Simple);
assert!(result.is_ok());

let z = zpool.status(&name).unwrap();
let z = zpool.status(&name, StatusOptions::default()).unwrap();
assert_eq!(&Health::Online, z.health());
});
}
Expand All @@ -586,13 +587,13 @@ fn test_zpool_take_device_from_mirror_offline_expand() {
let result = zpool.take_offline(&name, &vdev0_path, OfflineMode::UntilReboot);
assert!(result.is_ok());

let z = zpool.status(&name).unwrap();
let z = zpool.status(&name, StatusOptions::default()).unwrap();
assert_eq!(&Health::Degraded, z.health());

let result = zpool.bring_online(&name, &vdev0_path, OnlineMode::Expand);
assert!(result.is_ok());

let z = zpool.status(&name).unwrap();
let z = zpool.status(&name, StatusOptions::default()).unwrap();
assert_eq!(&Health::Online, z.health());
});
}
Expand All @@ -613,7 +614,7 @@ fn test_zpool_attach_then_detach_single() {

zpool.attach(&name, &vdev0_path, &vdev1_path).unwrap();

let z = zpool.status(&name).unwrap();
let z = zpool.status(&name, StatusOptions::default()).unwrap();
let topo_actual = CreateZpoolRequestBuilder::default()
.name(name.clone())
.create_mode(CreateMode::Force)
Expand All @@ -626,7 +627,7 @@ fn test_zpool_attach_then_detach_single() {
assert_eq!(&z, &topo_actual);

zpool.detach(&name, &vdev1_path).unwrap();
let z = zpool.status(&name).unwrap();
let z = zpool.status(&name, StatusOptions::default()).unwrap();
assert_eq!(&z, &topo);

let err = zpool.detach(&name, &vdev0_path).unwrap_err();
Expand Down Expand Up @@ -662,7 +663,7 @@ fn test_zpool_add_naked() {

assert!(result.is_ok());

let z = zpool.status(&name).unwrap();
let z = zpool.status(&name, StatusOptions::default()).unwrap();

assert_eq!(topo_expected, z);
});
Expand Down Expand Up @@ -695,7 +696,7 @@ fn test_zpool_add_naked_force() {

assert!(result.is_ok());

let z = zpool.status(&name).unwrap();
let z = zpool.status(&name, StatusOptions::default()).unwrap();

assert_eq!(topo_expected, z);
});
Expand Down Expand Up @@ -739,7 +740,7 @@ fn test_zpool_add_mirror() {

assert!(result.is_ok());

let z = zpool.status(&name).unwrap();
let z = zpool.status(&name, StatusOptions::default()).unwrap();

assert_eq!(topo_expected, z);
});
Expand Down Expand Up @@ -806,7 +807,7 @@ fn test_zpool_remove_zil() {
.build()
.unwrap();

let result = zpool.status(&name).unwrap();
let result = zpool.status(&name, StatusOptions::default()).unwrap();

assert_eq!(topo, result);
});
Expand Down Expand Up @@ -838,7 +839,11 @@ fn test_zpool_add_cache() {

assert!(result.is_ok());

let z = zpool.status(&name).unwrap();
let opts = StatusOptionsBuilder::default()
.full_paths(true)
.build()
.unwrap();
let z = zpool.status(&name, opts).unwrap();
assert_eq!(topo_expected, z);
});
}
Expand All @@ -858,7 +863,12 @@ fn test_create_with_spare() {
.build()
.unwrap();
zpool.create(topo.clone()).unwrap();
let z = zpool.status(&name).unwrap();

let opts = StatusOptionsBuilder::default()
.full_paths(true)
.build()
.unwrap();
let z = zpool.status(&name, opts).unwrap();
assert_eq!(topo, z);
});
}
Expand Down Expand Up @@ -889,7 +899,11 @@ fn test_zpool_add_spare() {

assert!(result.is_ok());

let z = zpool.status(&name).unwrap();
let opts = StatusOptionsBuilder::default()
.full_paths(true)
.build()
.unwrap();
let z = zpool.status(&name, opts).unwrap();
assert_eq!(topo_expected, z);
});
}
Expand Down Expand Up @@ -931,7 +945,11 @@ fn test_zpool_replace_disk() {
let wait_time = time::Duration::from_secs(13);
thread::sleep(wait_time);

let z = zpool.status(&name).unwrap();
let opts = StatusOptionsBuilder::default()
.full_paths(true)
.build()
.unwrap();
let z = zpool.status(&name, opts).unwrap();
assert_eq!(topo_expected, z);
});
}

0 comments on commit e0be797

Please sign in to comment.