[pbs-devel] [PATCH v3 proxmox-backup 1/3] disks: refactor partition type handling

[pbs-devel] [PATCH v3 proxmox-backup 1/3] disks: refactor partition type handling

[Fabian Ebner]

in preparation to also get the file system type from lsblk.

Co-developed-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v2:
    * rebase because of moved file

 src/tools/disks/lvm.rs | 18 ++++++++++--------
 src/tools/disks/mod.rs | 37 ++++++++++++++++---------------------
 src/tools/disks/zfs.rs | 17 ++++++++---------
 3 files changed, 34 insertions(+), 38 deletions(-)

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<String, Vec<String>>,
+    lsblk_info: &[LsblkInfo],
 ) -> Result<HashSet<u64>, Error> {
 
     const PVS_BIN_PATH: &str = "pvs";
@@ -29,12 +31,12 @@ pub fn get_lvm_devices(
 
     let mut device_set: HashSet<u64> = 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/mod.rs b/src/tools/disks/mod.rs
index 1a72a6c8..6e68954c 100644
--- a/src/tools/disks/mod.rs
+++ b/src/tools/disks/mod.rs
@@ -46,6 +46,16 @@ pub struct DiskManage {
     mounted_devices: OnceCell<HashSet<dev_t>>,
 }
 
+/// 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<String>,
+}
+
 impl DiskManage {
     /// Create a new disk management context.
     pub fn new() -> Arc<Self> {
@@ -556,31 +566,16 @@ pub struct BlockDevStat {
 }
 
 /// Use lsblk to read partition type uuids.
-pub fn get_partition_type_info() -> Result<HashMap<String, Vec<String>>, Error> {
+pub fn get_lsblk_info() -> Result<Vec<LsblkInfo>, 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<String, Vec<String>> = HashMap::new();
+    let mut output: serde_json::Value = output.parse()?;
 
-    let output: serde_json::Value = output.parse()?;
-    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(),
-                None => continue,
-            };
-            let devices = res.entry(partition_type).or_insert(Vec::new());
-            devices.push(path.to_string());
-        }
-    }
-    Ok(res)
+    Ok(serde_json::from_value(output["blockdevices"].take())?)
 }
 
 #[api()]
@@ -736,14 +731,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<HashSet<u64>, Error> {
+    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(&partition_type_map)?;
+    let lvm_devices = get_lvm_devices(&lsblk_info)?;
 
     // fixme: ceph journals/volumes
 
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<Option<BlockDevStat>, 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<String, Vec<String>>,
+    lsblk_info: &[LsblkInfo],
     pool: Option<String>,
 ) -> Result<HashSet<u64>, 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.30.2

[pbs-devel] [PATCH v3 proxmox-backup 2/3] disks: also check for file systems with lsblk

[Fabian Ebner]

Reported-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v2:
    * rebase because of moved file

Ideally, we'd also show the type in the result, but the api macro
currently does not support enums with fields:
    FileSystem(String)
    Mounted(String) // showing the fs type here would also be nice
An alternative would be replacing the DiskUsageType with a String in
the DiskUsage struct, but that's not very nice...

 src/tools/disks/mod.rs | 34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/src/tools/disks/mod.rs b/src/tools/disks/mod.rs
index 6e68954c..af7f7fe1 100644
--- a/src/tools/disks/mod.rs
+++ b/src/tools/disks/mod.rs
@@ -54,6 +54,9 @@ pub struct LsblkInfo {
     /// Partition type GUID.
     #[serde(rename = "parttype")]
     partition_type: Option<String>,
+    /// File system label.
+    #[serde(rename = "fstype")]
+    file_system_type: Option<String>,
 }
 
 impl DiskManage {
@@ -565,11 +568,11 @@ pub struct BlockDevStat {
     pub io_ticks: u64, // milliseconds
 }
 
-/// Use lsblk to read partition type uuids.
+/// Use lsblk to read partition type uuids and file system types.
 pub fn get_lsblk_info() -> Result<Vec<LsblkInfo>, Error> {
 
     let mut command = std::process::Command::new("lsblk");
-    command.args(&["--json", "-o", "path,parttype"]);
+    command.args(&["--json", "-o", "path,parttype,fstype"]);
 
     let output = crate::tools::run_command(command, None)?;
 
@@ -578,6 +581,25 @@ pub fn get_lsblk_info() -> Result<Vec<LsblkInfo>, Error> {
     Ok(serde_json::from_value(output["blockdevices"].take())?)
 }
 
+/// Get set of devices with a file system label.
+///
+/// The set is indexed by using the unix raw device number (dev_t is u64)
+fn get_file_system_devices(
+    lsblk_info: &[LsblkInfo],
+) -> Result<HashSet<u64>, Error> {
+
+    let mut device_set: HashSet<u64> = HashSet::new();
+
+    for info in lsblk_info.iter() {
+        if info.file_system_type.is_some() {
+            let meta = std::fs::metadata(&info.path)?;
+            device_set.insert(meta.rdev());
+        }
+    }
+
+    Ok(device_set)
+}
+
 #[api()]
 #[derive(Debug, Serialize, Deserialize, PartialEq)]
 #[serde(rename_all="lowercase")]
@@ -594,6 +616,8 @@ pub enum DiskUsageType {
     DeviceMapper,
     /// Disk has partitions
     Partitions,
+    /// Disk contains a file system label
+    FileSystem,
 }
 
 #[api(
@@ -740,6 +764,8 @@ pub fn get_disks(
 
     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();
@@ -815,6 +841,10 @@ pub fn get_disks(
             };
         }
 
+        if usage == DiskUsageType::Unused && file_system_devices.contains(&devnum) {
+            usage = DiskUsageType::FileSystem;
+        }
+
         if usage == DiskUsageType::Unused && disk.has_holders()? {
             usage = DiskUsageType::DeviceMapper;
         }
-- 
2.30.2

[pbs-devel] [PATCH v3 proxmox-backup 3/3] api: disk list: sort by name

[Fabian Ebner]

So callers get more stable results. Most noticeable, the disk list in
the web UI doesn't jump around upon reloading, and while sorting could
be done directly there, like this other callers get the benefit too.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

New in v3.

Does not depend on the other patches.

 src/api2/node/disks/mod.rs | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/api2/node/disks/mod.rs b/src/api2/node/disks/mod.rs
index 44554157..e0c3d39c 100644
--- a/src/api2/node/disks/mod.rs
+++ b/src/api2/node/disks/mod.rs
@@ -66,6 +66,8 @@ pub fn list_disks(
         }
     }
 
+    list.sort_by(|a, b| a.name.cmp(&b.name));
+
     Ok(list)
 }
 
-- 
2.30.2

[pbs-devel] applied-series: [PATCH v3 proxmox-backup 1/3] disks: refactor partition type handling

[Thomas Lamprecht]

On 09.07.21 12:32, Fabian Ebner wrote:
> in preparation to also get the file system type from lsblk.
> 
> Co-developed-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> Changes from v2:
>     * rebase because of moved file
> 
>  src/tools/disks/lvm.rs | 18 ++++++++++--------
>  src/tools/disks/mod.rs | 37 ++++++++++++++++---------------------
>  src/tools/disks/zfs.rs | 17 ++++++++---------
>  3 files changed, 34 insertions(+), 38 deletions(-)
> 
>

applied all three patches, thanks!