From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 7843C75335 for ; Wed, 21 Apr 2021 15:10:27 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 6683FF805 for ; Wed, 21 Apr 2021 15:10:27 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 064A1F7F9 for ; Wed, 21 Apr 2021 15:10:26 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id CA09B44D34 for ; Wed, 21 Apr 2021 15:10:25 +0200 (CEST) Date: Wed, 21 Apr 2021 15:10:24 +0200 From: Wolfgang Bumiller To: Fabian Ebner Cc: pbs-devel@lists.proxmox.com Message-ID: <20210421131024.3gysewl5e33u25kd@wobu-vie.proxmox.com> References: <20210420095345.7775-1-f.ebner@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210420095345.7775-1-f.ebner@proxmox.com> User-Agent: NeoMutt/20180716 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.417 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [disks.rs, lvm.rs, zfs.rs] Subject: Re: [pbs-devel] [PATCH/RFC proxmox-backup 1/2] disks: refactor partition type handling X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 Apr 2021 13:10:27 -0000 Looks fine, though I wonder if we should shorten this? On Tue, Apr 20, 2021 at 11:53:44AM +0200, Fabian Ebner wrote: > in preparation to also get the file system type from lsblk. > > Signed-off-by: Fabian Ebner > --- > src/tools/disks.rs | 35 ++++++++++++++++++++++------------- > src/tools/disks/lvm.rs | 18 ++++++++++-------- > src/tools/disks/zfs.rs | 17 ++++++++--------- > 3 files changed, 40 insertions(+), 30 deletions(-) > > diff --git a/src/tools/disks.rs b/src/tools/disks.rs > index 8573695d..d374df0e 100644 > --- a/src/tools/disks.rs > +++ b/src/tools/disks.rs > @@ -46,6 +46,14 @@ pub struct DiskManage { > mounted_devices: OnceCell>, > } > > +/// Information for a device as returned by lsblk. #[derive(Deserialize)] > +pub struct LsblkInfo { > + /// Path to the device. > + path: String, > + /// Partition type GUID. #[serde(rename = "parttype")] > + partition_type: Option, > +} > + > impl DiskManage { > /// Create a new disk management context. > pub fn new() -> Arc { > @@ -556,30 +564,31 @@ pub struct BlockDevStat { > } > > /// Use lsblk to read partition type uuids. > -pub fn get_partition_type_info() -> Result>, Error> { > +pub fn get_lsblk_info() -> Result, Error> { > > let mut command = std::process::Command::new("lsblk"); > command.args(&["--json", "-o", "path,parttype"]); > > let output = crate::tools::run_command(command, None)?; > > - let mut res: HashMap> = HashMap::new(); > - > let output: serde_json::Value = output.parse()?; let mut output: serde_json::Value = output.parse()?; Ok(serde_json::from_value(output["blockdevices"].take())?) } And pretty much skip the rest of the function ;-) > + > + let mut res = vec![]; > + > if let Some(list) = output["blockdevices"].as_array() { > for info in list { > let path = match info["path"].as_str() { > - Some(p) => p, > - None => continue, > - }; > - let partition_type = match info["parttype"].as_str() { > - Some(t) => t.to_owned(), > + Some(p) => p.to_string(), > None => continue, > }; > - let devices = res.entry(partition_type).or_insert(Vec::new()); > - devices.push(path.to_string()); > + let partition_type = info["parttype"].as_str().map(|t| t.to_string()); > + res.push(LsblkInfo { > + path, > + partition_type, > + }); > } > } > + > Ok(res) > } > > @@ -736,14 +745,14 @@ pub fn get_disks( > > let disk_manager = DiskManage::new(); > > - let partition_type_map = get_partition_type_info()?; > + let lsblk_info = get_lsblk_info()?; > > - let zfs_devices = zfs_devices(&partition_type_map, None).or_else(|err| -> Result, Error> { > + 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(&partition_type_map)?; > + let lvm_devices = get_lvm_devices(&lsblk_info)?; > > // fixme: ceph journals/volumes > > diff --git a/src/tools/disks/lvm.rs b/src/tools/disks/lvm.rs > index 1e8f825d..1a81cea0 100644 > --- a/src/tools/disks/lvm.rs > +++ b/src/tools/disks/lvm.rs > @@ -1,10 +1,12 @@ > -use std::collections::{HashSet, HashMap}; > +use std::collections::HashSet; > use std::os::unix::fs::MetadataExt; > > use anyhow::{Error}; > use serde_json::Value; > use lazy_static::lazy_static; > > +use super::LsblkInfo; > + > lazy_static!{ > static ref LVM_UUIDS: HashSet<&'static str> = { > let mut set = HashSet::new(); > @@ -17,7 +19,7 @@ lazy_static!{ > /// > /// The set is indexed by using the unix raw device number (dev_t is u64) > pub fn get_lvm_devices( > - partition_type_map: &HashMap>, > + lsblk_info: &[LsblkInfo], > ) -> Result, Error> { > > const PVS_BIN_PATH: &str = "pvs"; > @@ -29,12 +31,12 @@ pub fn get_lvm_devices( > > let mut device_set: HashSet = HashSet::new(); > > - for device_list in partition_type_map.iter() > - .filter_map(|(uuid, list)| if LVM_UUIDS.contains(uuid.as_str()) { Some(list) } else { None }) > - { > - for device in device_list { > - let meta = std::fs::metadata(device)?; > - device_set.insert(meta.rdev()); > + for info in lsblk_info.iter() { > + if let Some(partition_type) = &info.partition_type { > + if LVM_UUIDS.contains(partition_type.as_str()) { > + let meta = std::fs::metadata(&info.path)?; > + device_set.insert(meta.rdev()); > + } > } > } > > diff --git a/src/tools/disks/zfs.rs b/src/tools/disks/zfs.rs > index e0084939..55e0aa30 100644 > --- a/src/tools/disks/zfs.rs > +++ b/src/tools/disks/zfs.rs > @@ -1,5 +1,5 @@ > use std::path::PathBuf; > -use std::collections::{HashMap, HashSet}; > +use std::collections::HashSet; > use std::os::unix::fs::MetadataExt; > > use anyhow::{bail, Error}; > @@ -67,12 +67,11 @@ pub fn zfs_pool_stats(pool: &OsStr) -> Result, Error> { > Ok(Some(stat)) > } > > - > /// Get set of devices used by zfs (or a specific zfs pool) > /// > /// The set is indexed by using the unix raw device number (dev_t is u64) > pub fn zfs_devices( > - partition_type_map: &HashMap>, > + lsblk_info: &[LsblkInfo], > pool: Option, > ) -> Result, Error> { > > @@ -86,12 +85,12 @@ pub fn zfs_devices( > } > } > > - for device_list in partition_type_map.iter() > - .filter_map(|(uuid, list)| if ZFS_UUIDS.contains(uuid.as_str()) { Some(list) } else { None }) > - { > - for device in device_list { > - let meta = std::fs::metadata(device)?; > - 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()); > + } > } > } > > -- > 2.20.1