[pbs-devel] [PATCH proxmox-backup 1/2] api: avoid retrieving lsblk result twice
Gabriel Goller
g.goller at proxmox.com
Mon Sep 16 16:56:26 CEST 2024
Avoid running `lsblk` twice when executing the `list_disk`
endpoint/command. This and the various other small nits improve the
performance of the endpoint.
Does not really fix, but is related to: #4961.
Signed-off-by: Gabriel Goller <g.goller at proxmox.com>
---
src/api2/node/disks/mod.rs | 6 +++++-
src/tools/disks/mod.rs | 10 +++++++---
src/tools/disks/smart.rs | 10 +++-------
src/tools/disks/zfs.rs | 15 ++++++++-------
src/tools/disks/zpool_list.rs | 2 +-
5 files changed, 24 insertions(+), 19 deletions(-)
diff --git a/src/api2/node/disks/mod.rs b/src/api2/node/disks/mod.rs
index 4ef4ee2b8ac7..abcb8ee40ef1 100644
--- a/src/api2/node/disks/mod.rs
+++ b/src/api2/node/disks/mod.rs
@@ -115,7 +115,11 @@ pub fn smart_status(disk: String, healthonly: Option<bool>) -> Result<SmartData,
let manager = DiskManage::new();
let disk = manager.disk_by_name(&disk)?;
- get_smart_data(&disk, healthonly)
+ if let Some(path) = disk.device_path() {
+ get_smart_data(path, healthonly)
+ } else {
+ bail!("disk {:?} has no node in /dev", disk.syspath());
+ }
}
#[api(
diff --git a/src/tools/disks/mod.rs b/src/tools/disks/mod.rs
index c729c26a20a9..04f62b818238 100644
--- a/src/tools/disks/mod.rs
+++ b/src/tools/disks/mod.rs
@@ -863,8 +863,8 @@ fn get_partitions_info(
lvm_devices: &HashSet<u64>,
zfs_devices: &HashSet<u64>,
file_system_devices: &HashSet<u64>,
+ lsblk_infos: &[LsblkInfo],
) -> Vec<PartitionInfo> {
- let lsblk_infos = get_lsblk_info().ok();
partitions
.values()
.map(|disk| {
@@ -887,8 +887,8 @@ fn get_partitions_info(
let mounted = disk.is_mounted().unwrap_or(false);
let mut filesystem = None;
- if let (Some(devpath), Some(infos)) = (devpath.as_ref(), lsblk_infos.as_ref()) {
- for info in infos.iter().filter(|i| i.path.eq(devpath)) {
+ if let Some(devpath) = devpath.as_ref() {
+ for info in lsblk_infos.iter().filter(|i| i.path.eq(devpath)) {
used = match info.partition_type.as_deref() {
Some("21686148-6449-6e6f-744e-656564454649") => PartitionUsageType::BIOS,
Some("c12a7328-f81f-11d2-ba4b-00a0c93ec93b") => PartitionUsageType::EFI,
@@ -942,6 +942,7 @@ fn get_disks(
// fixme: ceph journals/volumes
let mut result = HashMap::new();
+ let mut device_paths = Vec::new();
for item in proxmox_sys::fs::scan_subdir(libc::AT_FDCWD, "/sys/block", &BLOCKDEVICE_NAME_REGEX)?
{
@@ -1009,6 +1010,8 @@ fn get_disks(
.map(|p| p.to_owned())
.map(|p| p.to_string_lossy().to_string());
+ device_paths.push((name.clone(), devpath.clone()));
+
let wwn = disk.wwn().map(|s| s.to_string_lossy().into_owned());
let partitions: Option<Vec<PartitionInfo>> = if include_partitions {
@@ -1018,6 +1021,7 @@ fn get_disks(
&lvm_devices,
&zfs_devices,
&file_system_devices,
+ &lsblk_info,
))
})
} else {
diff --git a/src/tools/disks/smart.rs b/src/tools/disks/smart.rs
index 3ad782b7b248..4868815f6f32 100644
--- a/src/tools/disks/smart.rs
+++ b/src/tools/disks/smart.rs
@@ -1,8 +1,8 @@
-use std::collections::{HashMap, HashSet};
use std::sync::LazyLock;
+use std::{collections::{HashMap, HashSet}, path::Path};
use ::serde::{Deserialize, Serialize};
-use anyhow::{bail, Error};
+use anyhow::Error;
use proxmox_schema::api;
@@ -76,7 +76,7 @@ pub struct SmartData {
}
/// Read smartctl data for a disk (/dev/XXX).
-pub fn get_smart_data(disk: &super::Disk, health_only: bool) -> Result<SmartData, Error> {
+pub fn get_smart_data(disk_path: &Path, health_only: bool) -> Result<SmartData, Error> {
const SMARTCTL_BIN_PATH: &str = "smartctl";
let mut command = std::process::Command::new(SMARTCTL_BIN_PATH);
@@ -85,10 +85,6 @@ pub fn get_smart_data(disk: &super::Disk, health_only: bool) -> Result<SmartData
command.args(["-A", "-j"]);
}
- let disk_path = match disk.device_path() {
- Some(path) => path,
- None => bail!("disk {:?} has no node in /dev", disk.syspath()),
- };
command.arg(disk_path);
let output = proxmox_sys::command::run_command(
diff --git a/src/tools/disks/zfs.rs b/src/tools/disks/zfs.rs
index 2abb5176c1e6..3b7da1540835 100644
--- a/src/tools/disks/zfs.rs
+++ b/src/tools/disks/zfs.rs
@@ -70,7 +70,7 @@ pub fn zfs_pool_stats(pool: &OsStr) -> Result<Option<BlockDevStat>, Error> {
///
/// The set is indexed by using the unix raw device number (dev_t is u64)
pub fn zfs_devices(lsblk_info: &[LsblkInfo], pool: Option<String>) -> Result<HashSet<u64>, Error> {
- let list = zpool_list(pool, true)?;
+ let list = zpool_list(pool.as_ref(), true)?;
let mut device_set = HashSet::new();
for entry in list {
@@ -79,12 +79,13 @@ pub fn zfs_devices(lsblk_info: &[LsblkInfo], pool: Option<String>) -> Result<Has
device_set.insert(meta.rdev());
}
}
-
- for info in lsblk_info.iter() {
- if let Some(partition_type) = &info.partition_type {
- if ZFS_UUIDS.contains(partition_type.as_str()) {
- let meta = std::fs::metadata(&info.path)?;
- device_set.insert(meta.rdev());
+ if pool.is_none() {
+ for info in lsblk_info.iter() {
+ if let Some(partition_type) = &info.partition_type {
+ if ZFS_UUIDS.contains(partition_type.as_str()) {
+ let meta = std::fs::metadata(&info.path)?;
+ device_set.insert(meta.rdev());
+ }
}
}
}
diff --git a/src/tools/disks/zpool_list.rs b/src/tools/disks/zpool_list.rs
index 83ce063dfe16..485a721b9865 100644
--- a/src/tools/disks/zpool_list.rs
+++ b/src/tools/disks/zpool_list.rs
@@ -136,7 +136,7 @@ fn parse_zpool_list(i: &str) -> Result<Vec<ZFSPoolInfo>, Error> {
///
/// Devices are only included when run with verbose flags
/// set. Without, device lists are empty.
-pub fn zpool_list(pool: Option<String>, verbose: bool) -> Result<Vec<ZFSPoolInfo>, Error> {
+pub fn zpool_list(pool: Option<&String>, verbose: bool) -> Result<Vec<ZFSPoolInfo>, Error> {
// Note: zpools list verbose output can include entries for 'special', 'cache' and 'logs'
// and maybe other things.
--
2.39.5
More information about the pbs-devel
mailing list