Re: [PATCH proxmox v2 05/25] disks: import from Proxmox Backup Server

Thomas Lamprecht
2026-03-27T21:12:45Z

Am 20.03.26 um 11:23 schrieb Lukas Wagner:
> This is based on the disks module from PBS and left unchanged.
>
> The version has not been set to 1.0 yet since it seems like this crate
> could use a bit a cleanup (custom error type instead of anyhow,
> documentation).

+1, and also some other clean up for pre-existing pain points w.r.t.
S.M.A.R.T, see inline.

> ---
> Cargo.toml | 6 +
> proxmox-disks/Cargo.toml | 30 +
> proxmox-disks/debian/changelog | 5 +
> proxmox-disks/debian/control | 94 ++
> proxmox-disks/debian/copyright | 18 +
> proxmox-disks/debian/debcargo.toml | 7 +
> proxmox-disks/src/lib.rs | 1396 ++++++++++++++++++++++++++++
> proxmox-disks/src/lvm.rs | 60 ++
> proxmox-disks/src/parse_helpers.rs | 52 ++
> proxmox-disks/src/smart.rs | 227 +++++
> proxmox-disks/src/zfs.rs | 205 ++++
> proxmox-disks/src/zpool_list.rs | 294 ++++++
> proxmox-disks/src/zpool_status.rs | 496 ++++++++++
> 13 files changed, 2890 insertions(+)
> create mode 100644 proxmox-disks/Cargo.toml
> create mode 100644 proxmox-disks/debian/changelog
> create mode 100644 proxmox-disks/debian/control
> create mode 100644 proxmox-disks/debian/copyright
> create mode 100644 proxmox-disks/debian/debcargo.toml
> create mode 100644 proxmox-disks/src/lib.rs
> create mode 100644 proxmox-disks/src/lvm.rs
> create mode 100644 proxmox-disks/src/parse_helpers.rs
> create mode 100644 proxmox-disks/src/smart.rs
> create mode 100644 proxmox-disks/src/zfs.rs
> create mode 100644 proxmox-disks/src/zpool_list.rs
> create mode 100644 proxmox-disks/src/zpool_status.rs
>

> diff --git a/proxmox-disks/src/lib.rs b/proxmox-disks/src/lib.rs
> new file mode 100644
> index 00000000..e6056c14
> --- /dev/null
> +++ b/proxmox-disks/src/lib.rs

[...]

> +#[api(
> + properties: {
> + used: {
> + type: DiskUsageType,
> + },
> + "disk-type": {
> + type: DiskType,
> + },
> + status: {
> + type: SmartStatus,
> + },
> + partitions: {
> + optional: true,
> + items: {
> + type: PartitionInfo
> + }
> + }
> + }
> +)]
> +#[derive(Debug, Serialize, Deserialize)]
> +#[serde(rename_all = "kebab-case")]
> +/// Information about how a Disk is used
> +pub struct DiskUsageInfo {
> + /// Disk name (`/sys/block/<name>`)
> + pub name: String,
> + pub used: DiskUsageType,
> + pub disk_type: DiskType,
> + pub status: SmartStatus,

pre-existing, I know, but after taking a third look I remembered the s.m.a.r.t slow
saga we had a while ago [0], and I think factoring this out would be a good time to
finally address the bigger pain points of that. One would be to make this optional
or drop this from the struct completely.

[0]: https://lore.proxmox.com/all/7b0ac740-096b-4850-a81d-5fa6bc9c347a@proxmox.com/

> + /// Disk wearout
> + pub wearout: Option<f64>,
> + /// Vendor
> + pub vendor: Option<String>,
> + /// Model
> + pub model: Option<String>,
> + /// WWN
> + pub wwn: Option<String>,
> + /// Disk size
> + pub size: u64,
> + /// Serisal number
> + pub serial: Option<String>,
> + /// Partitions on the device
> + pub partitions: Option<Vec<PartitionInfo>>,
> + /// Linux device path (/dev/xxx)
> + pub devpath: Option<String>,
> + /// Set if disk contains a GPT partition table
> + pub gpt: bool,
> + /// RPM
> + pub rpm: Option<u64>,
> +}
> +

[...]

> +/// Get disk usage information for multiple disks
> +fn get_disks(
> + // filter - list of device names (without leading /dev)
> + disks: Option<Vec<String>>,
> + // do no include data from smartctl
> + no_smart: bool,
> + // include partitions
> + include_partitions: bool,
> +) -> Result<HashMap<String, DiskUsageInfo>, Error> {
> + let disk_manager = DiskManage::new();
> +
> + let lsblk_info = get_lsblk_info()?;
> +
> + let zfs_devices =
> + zfs_devices(&lsblk_info, None).or_else(|err| -> Result<HashSet<u64>, Error> {
> + eprintln!("error getting zfs devices: {err}");
> + Ok(HashSet::new())
> + })?;
> +
> + let lvm_devices = get_lvm_devices(&lsblk_info)?;
> +
> + let file_system_devices = get_file_system_devices(&lsblk_info)?;
> +
> + // 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)?
> + {
> + let item = item?;
> +
> + let name = item.file_name().to_str().unwrap().to_string();
> +
> + if let Some(ref disks) = disks {
> + if !disks.contains(&name) {
> + continue;
> + }
> + }
> +
> + let sys_path = format!("/sys/block/{name}");
> +
> + if let Ok(target) = std::fs::read_link(&sys_path) {
> + if let Some(target) = target.to_str() {
> + if ISCSI_PATH_REGEX.is_match(target) {
> + continue;
> + } // skip iSCSI devices
> + }
> + }
> +
> + let disk = disk_manager.clone().disk_by_sys_path(&sys_path)?;
> +
> + let devnum = disk.devnum()?;
> +
> + let size = match disk.size() {
> + Ok(size) => size,
> + Err(_) => continue, // skip devices with unreadable size
> + };
> +
> + let disk_type = match disk.guess_disk_type() {
> + Ok(disk_type) => disk_type,
> + Err(_) => continue, // skip devices with undetectable type
> + };
> +
> + let mut usage = DiskUsageType::Unused;
> +
> + if lvm_devices.contains(&devnum) {
> + usage = DiskUsageType::LVM;
> + }
> +
> + match disk.is_mounted() {
> + Ok(true) => usage = DiskUsageType::Mounted,
> + Ok(false) => {}
> + Err(_) => continue, // skip devices with undetectable mount status
> + }
> +
> + if zfs_devices.contains(&devnum) {
> + usage = DiskUsageType::ZFS;
> + }
> +
> + let vendor = disk
> + .vendor()
> + .unwrap_or(None)
> + .map(|s| s.to_string_lossy().trim().to_string());
> +
> + let model = disk.model().map(|s| s.to_string_lossy().into_owned());
> +
> + let serial = disk.serial().map(|s| s.to_string_lossy().into_owned());
> +
> + let devpath = disk
> + .device_path()
> + .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 {
> + disk.partitions().map_or(None, |parts| {
> + Some(get_partitions_info(
> + parts,
> + &lvm_devices,
> + &zfs_devices,
> + &file_system_devices,
> + &lsblk_info,
> + ))
> + })
> + } else {
> + None
> + };
> +
> + if usage != DiskUsageType::Mounted {
> + match scan_partitions(disk_manager.clone(), &lvm_devices, &zfs_devices, &name) {
> + Ok(part_usage) => {
> + if part_usage != DiskUsageType::Unused {
> + usage = part_usage;
> + }
> + }
> + Err(_) => continue, // skip devices if scan_partitions fail
> + };
> + }
> +
> + if usage == DiskUsageType::Unused && file_system_devices.contains(&devnum) {
> + usage = DiskUsageType::FileSystem;
> + }
> +
> + if usage == DiskUsageType::Unused && disk.has_holders()? {
> + usage = DiskUsageType::DeviceMapper;
> + }
> +
> + let info = DiskUsageInfo {
> + name: name.clone(),
> + vendor,
> + model,
> + partitions,
> + serial,
> + devpath,
> + size,
> + wwn,
> + disk_type,
> + status: SmartStatus::Unknown,
> + wearout: None,
> + used: usage,
> + gpt: disk.has_gpt(),
> + rpm: disk.ata_rotation_rate_rpm(),
> + };
> +
> + result.insert(name, info);
> + }
> +
> + if !no_smart {

I still think such a branch should simple not exist for a get_disk (list) method, like
mentioned in (potentially now slightly outdated) [0].

> + let (tx, rx) = crossbeam_channel::bounded(result.len());
> +
> + let parallel_handler =
> + ParallelHandler::new("smartctl data", 4, move |device: (String, String)| {
> + match get_smart_data(Path::new(&device.1), false) {
> + Ok(smart_data) => tx.send((device.0, smart_data))?,
> + // do not fail the whole disk output just because smartctl couldn't query one
> + Err(err) => {
> + proxmox_log::error!("failed to gather smart data for {} – {err}", device.1)
> + }
> + }
> + Ok(())
> + });
> +
> + for (name, path) in device_paths.into_iter() {
> + if let Some(p) = path {
> + parallel_handler.send((name, p))?;
> + }
> + }
> +
> + parallel_handler.complete()?;
> + while let Ok(msg) = rx.recv() {
> + if let Some(value) = result.get_mut(&msg.0) {
> + value.wearout = msg.1.wearout;
> + value.status = msg.1.status;
> + }
> + }
> + }
> + Ok(result)
> +}
> +