From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 0D1C61FF140 for ; Fri, 27 Mar 2026 22:12:45 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5252B1F23B; Fri, 27 Mar 2026 22:13:08 +0100 (CET) Message-ID: <8fe38e72-9f7b-4df2-9434-0ca9ea8c4f0d@proxmox.com> Date: Fri, 27 Mar 2026 22:13:00 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH proxmox v2 05/25] disks: import from Proxmox Backup Server To: Lukas Wagner , pdm-devel@lists.proxmox.com References: <20260319094617.169594-1-l.wagner@proxmox.com> <20260319094617.169594-6-l.wagner@proxmox.com> Content-Language: en-US From: Thomas Lamprecht In-Reply-To: <20260319094617.169594-6-l.wagner@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1774645930242 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.011 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: 5BTPZMNCOD5V42CRFL3TC4ZXIRCIMRGA X-Message-ID-Hash: 5BTPZMNCOD5V42CRFL3TC4ZXIRCIMRGA X-MailFrom: t.lamprecht@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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/`) > + 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, > + /// Vendor > + pub vendor: Option, > + /// Model > + pub model: Option, > + /// WWN > + pub wwn: Option, > + /// Disk size > + pub size: u64, > + /// Serisal number > + pub serial: Option, > + /// Partitions on the device > + pub partitions: Option>, > + /// Linux device path (/dev/xxx) > + pub devpath: Option, > + /// Set if disk contains a GPT partition table > + pub gpt: bool, > + /// RPM > + pub rpm: Option, > +} > + [...] > +/// Get disk usage information for multiple disks > +fn get_disks( > + // filter - list of device names (without leading /dev) > + disks: Option>, > + // do no include data from smartctl > + no_smart: bool, > + // include partitions > + include_partitions: bool, > +) -> Result, Error> { > + let disk_manager = DiskManage::new(); > + > + let lsblk_info = get_lsblk_info()?; > + > + let zfs_devices = > + zfs_devices(&lsblk_info, None).or_else(|err| -> Result, 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> = 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) > +} > +