bug: Fix bugs with disk widget and disk encryption (#423)

Two issues were highlighted as a result of using either Void Linux with disk encryption, or just disk encryption on Linux in general:

Two fixes:
1. Fixes a failed `usage()` call in the `get_disk_usage` function from failing the entire result.  Now it only returns an entry with N/A results.  This occurred in some distros and disk encryption setups, for example, the one for Void Linux here: https://docs.voidlinux.org/installation/guides/fde.html.

2. Fixes a potential mapping issue with disk encryption on Linux in general.  Since the disk might map to `/dev/mapper/whatever`, but the I/O harvester was using another name, the mappings would not match.  As such, we now also check if a symlink exists; if it does, then we take it and work out the correct path.  This also fixes the disk name being wrong.
This commit is contained in:
Clement Tsang 2021-02-24 20:23:35 -05:00 committed by GitHub
parent 25a0a7b1d0
commit fe74328647
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 88 additions and 28 deletions

View File

@ -18,6 +18,7 @@
"GIGA",
"Hamberg",
"KIBI",
"LUKS",
"Lukas",
"MEBI",
"MEBIBYTE",

View File

@ -41,6 +41,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- [#417](https://github.com/ClementTsang/bottom/pull/417): Fixes the sort menu and sort shortcuts not syncing up.
- [#423](https://github.com/ClementTsang/bottom/pull/423): Fixes disk encryption causing the disk widget to fail or not properly map I/O statistics.
## [0.5.7] - 2021-01-30
## Bug Fixes

View File

@ -4,9 +4,9 @@ use crate::app::Filter;
pub struct DiskHarvest {
pub name: String,
pub mount_point: String,
pub free_space: u64,
pub used_space: u64,
pub total_space: u64,
pub free_space: Option<u64>,
pub used_space: Option<u64>,
pub total_space: Option<u64>,
}
#[derive(Clone, Debug)]
@ -35,7 +35,6 @@ pub async fn get_io_usage(actually_get: bool) -> crate::utils::error::Result<Opt
let mount_point = io.device_name().to_str().unwrap_or("Name Unavailable");
// FIXME: [MOUNT POINT] Add the filter here I guess?
io_hash.insert(
mount_point.to_string(),
Some(IOData {
@ -64,11 +63,43 @@ pub async fn get_disk_usage(
while let Some(part) = partitions_stream.next().await {
if let Ok(partition) = part {
let name = (partition
.device()
.unwrap_or_else(|| std::ffi::OsStr::new("Name Unavailable"))
.to_str()
.unwrap_or("Name Unavailable"))
let symlink: std::ffi::OsString;
let name = (if let Some(device) = partition.device() {
// See if this disk is actually mounted elsewhere on Linux...
// This is a workaround to properly map I/O in some cases (i.e. disk encryption), see
// https://github.com/ClementTsang/bottom/issues/419
if cfg!(target_os = "linux") {
if let Ok(path) = std::fs::read_link(device) {
if path.is_absolute() {
symlink = path.into_os_string();
symlink.as_os_str()
} else {
let mut combined_path = std::path::PathBuf::new();
combined_path.push(device);
combined_path.pop(); // Pop the current file...
combined_path.push(path.clone());
if let Ok(path) = std::fs::canonicalize(combined_path) {
// Resolve the local path into an absolute one...
symlink = path.into_os_string();
symlink.as_os_str()
} else {
symlink = path.into_os_string();
symlink.as_os_str()
}
}
} else {
device
}
} else {
device
}
} else {
std::ffi::OsStr::new("Name Unavailable")
}
.to_str()
.unwrap_or("Name Unavailable"))
.to_string();
let mount_point = (partition
@ -91,14 +122,24 @@ pub async fn get_disk_usage(
};
if to_keep {
let usage = heim::disk::usage(partition.mount_point().to_path_buf()).await?;
vec_disks.push(DiskHarvest {
free_space: usage.free().get::<heim::units::information::byte>(),
used_space: usage.used().get::<heim::units::information::byte>(),
total_space: usage.total().get::<heim::units::information::byte>(),
mount_point,
name,
});
// The usage line fails in some cases (Void linux + LUKS, see https://github.com/ClementTsang/bottom/issues/419)
if let Ok(usage) = heim::disk::usage(partition.mount_point().to_path_buf()).await {
vec_disks.push(DiskHarvest {
free_space: Some(usage.free().get::<heim::units::information::byte>()),
used_space: Some(usage.used().get::<heim::units::information::byte>()),
total_space: Some(usage.total().get::<heim::units::information::byte>()),
mount_point,
name,
});
} else {
vec_disks.push(DiskHarvest {
free_space: None,
used_space: None,
total_space: None,
mount_point,
name,
});
}
}
}
}

View File

@ -116,20 +116,36 @@ pub fn convert_disk_row(current_data: &data_farmer::DataCollection) -> Vec<Vec<S
.iter()
.zip(&current_data.io_labels)
.for_each(|(disk, (io_read, io_write))| {
let converted_free_space = get_simple_byte_values(disk.free_space, false);
let converted_total_space = get_simple_byte_values(disk.total_space, false);
disk_vector.push(vec![
disk.name.to_string(),
disk.mount_point.to_string(),
format!(
"{:.0}%",
disk.used_space as f64 / disk.total_space as f64 * 100_f64
),
format!("{:.*}{}", 0, converted_free_space.0, converted_free_space.1),
let free_space_fmt = if let Some(free_space) = disk.free_space {
let converted_free_space = get_simple_byte_values(free_space, false);
format!("{:.*}{}", 0, converted_free_space.0, converted_free_space.1)
} else {
"N/A".to_string()
};
let total_space_fmt = if let Some(total_space) = disk.total_space {
let converted_total_space = get_simple_byte_values(total_space, false);
format!(
"{:.*}{}",
0, converted_total_space.0, converted_total_space.1
),
)
} else {
"N/A".to_string()
};
let usage_fmt = if let (Some(used_space), Some(total_space)) =
(disk.used_space, disk.total_space)
{
format!("{:.0}%", used_space as f64 / total_space as f64 * 100_f64)
} else {
"N/A".to_string()
};
disk_vector.push(vec![
disk.name.to_string(),
disk.mount_point.to_string(),
usage_fmt,
free_space_fmt,
total_space_fmt,
io_read.to_string(),
io_write.to_string(),
]);