[pdm-devel] [PATCH datacenter-manager 1/2] ui: improve foreach_drive_* functions

* [pdm-devel] [PATCH datacenter-manager 1/2] ui: improve foreach_drive_* functions
@ 2025-01-14 14:20 Dominik Csapak
  2025-01-14 14:20 ` [pdm-devel] [PATCH datacenter-manager 2/2] ui: remote migrate: correctly use node from selected endpoint Dominik Csapak
  0 siblings, 1 reply; 2+ messages in thread
From: Dominik Csapak @ 2025-01-14 14:20 UTC (permalink / raw)
  To: pdm-devel

by

* using the new ArrayMap feature of pve-api-types

  This new way of de-serialization of the pve array types such as `scsiX`
  makes it possible to use them directly as an iterator instead of
  serializing into a serde value again, so make use of that.

* improve the error handling of that function

  Instead of aborting on the first error, pass the error through to the
  given function parameter and let the caller decide what to do with
  errors. With this, we can now remove the return value of the
  foreach_drive_* functions completely.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 ui/src/pve/utils.rs                  | 114 ++++++++++++---------------
 ui/src/widget/pve_migrate_mapping.rs |  22 +++++-
 2 files changed, 70 insertions(+), 66 deletions(-)

diff --git a/ui/src/pve/utils.rs b/ui/src/pve/utils.rs
index d00a6b5..3cb45a9 100644
--- a/ui/src/pve/utils.rs
+++ b/ui/src/pve/utils.rs
@@ -6,7 +6,7 @@ use pdm_client::types::{
     LxcConfig, LxcConfigMp, LxcConfigRootfs, LxcConfigUnused, PveQmIde, QemuConfig, QemuConfigSata,
     QemuConfigScsi, QemuConfigUnused, QemuConfigVirtio,
 };
-use proxmox_schema::ApiType;
+use proxmox_schema::property_string::PropertyString;
 use proxmox_yew_comp::{GuestState, NodeState, StorageState};
 use pwt::{
     css::Opacity,
@@ -148,56 +148,49 @@ impl PveDriveQemu {
 // note: uses to_value so we can iterate over the keys
 // unwrap is ok here, since we know that those are structs and strings
 /// Iterates over every drive from the config
-pub fn foreach_drive_qemu<F>(config: &QemuConfig, mut f: F) -> Result<(), Error>
+pub fn foreach_drive_qemu<F>(config: &QemuConfig, mut f: F)
 where
-    F: FnMut(&str, PveDriveQemu),
+    F: FnMut(&str, Result<PveDriveQemu, Error>),
 {
-    let ide = serde_json::to_value(&config.ide)?;
-    for (key, value) in ide.as_object().unwrap() {
-        let value = serde_json::from_value(
-            PveQmIde::API_SCHEMA.parse_property_string(value.as_str().unwrap())?,
-        )?;
-
-        f(key, PveDriveQemu::Ide(value))
+    for (idx, value) in (&*config.ide).into_iter() {
+        let key = format!("ide{idx}");
+        let res = value
+            .parse::<PropertyString<PveQmIde>>()
+            .map(|value| PveDriveQemu::Ide(value.into_inner()));
+        f(&key, res.map_err(Error::from));
     }
 
-    let sata = serde_json::to_value(&config.sata)?;
-    for (key, value) in sata.as_object().unwrap() {
-        let value = serde_json::from_value(
-            QemuConfigSata::API_SCHEMA.parse_property_string(value.as_str().unwrap())?,
-        )?;
-
-        f(key, PveDriveQemu::Sata(value))
+    for (idx, value) in (&*config.sata).into_iter() {
+        let key = format!("sata{idx}");
+        let res = value
+            .parse::<PropertyString<QemuConfigSata>>()
+            .map(|value| PveDriveQemu::Sata(value.into_inner()));
+        f(&key, res.map_err(Error::from));
     }
 
-    let scsi = serde_json::to_value(&config.scsi)?;
-    for (key, value) in scsi.as_object().unwrap() {
-        let value = serde_json::from_value(
-            QemuConfigScsi::API_SCHEMA.parse_property_string(value.as_str().unwrap())?,
-        )?;
-
-        f(key, PveDriveQemu::Scsi(value))
+    for (idx, value) in (&*config.scsi).into_iter() {
+        let key = format!("scsi{idx}");
+        let res = value
+            .parse::<PropertyString<QemuConfigScsi>>()
+            .map(|value| PveDriveQemu::Scsi(value.into_inner()));
+        f(&key, res.map_err(Error::from));
     }
 
-    let virtio = serde_json::to_value(&config.virtio)?;
-    for (key, value) in virtio.as_object().unwrap() {
-        let value = serde_json::from_value(
-            QemuConfigVirtio::API_SCHEMA.parse_property_string(value.as_str().unwrap())?,
-        )?;
-
-        f(key, PveDriveQemu::Virtio(value))
+    for (idx, value) in (&*config.virtio).into_iter() {
+        let key = format!("virtio{idx}");
+        let res = value
+            .parse::<PropertyString<QemuConfigVirtio>>()
+            .map(|value| PveDriveQemu::Virtio(value.into_inner()));
+        f(&key, res.map_err(Error::from));
     }
 
-    let unused = serde_json::to_value(&config.unused)?;
-    for (key, value) in unused.as_object().unwrap() {
-        let value = serde_json::from_value(
-            QemuConfigUnused::API_SCHEMA.parse_property_string(value.as_str().unwrap())?,
-        )?;
-
-        f(key, PveDriveQemu::Unused(value))
+    for (idx, value) in (&*config.unused).into_iter() {
+        let key = format!("unused{idx}");
+        let res = value
+            .parse::<PropertyString<QemuConfigUnused>>()
+            .map(|value| PveDriveQemu::Unused(value.into_inner()));
+        f(&key, res.map_err(Error::from));
     }
-
-    Ok(())
 }
 
 /// Represents a drive from an LXC config
@@ -219,34 +212,31 @@ impl PveDriveLxc {
 }
 
 /// Iterates over every drive from the config
-pub fn foreach_drive_lxc<F>(config: &LxcConfig, mut f: F) -> Result<(), Error>
+pub fn foreach_drive_lxc<F>(config: &LxcConfig, mut f: F)
 where
-    F: FnMut(&str, PveDriveLxc),
+    F: FnMut(&str, Result<PveDriveLxc, Error>),
 {
     if let Some(rootfs) = &config.rootfs {
-        let value =
-            serde_json::from_value(LxcConfigRootfs::API_SCHEMA.parse_property_string(&rootfs)?)?;
-
-        f("rootfs", PveDriveLxc::RootFs(value))
+        let key = "rootfs";
+        let res = rootfs
+            .parse::<PropertyString<LxcConfigRootfs>>()
+            .map(|value| PveDriveLxc::RootFs(value.into_inner()));
+        f(key, res.map_err(Error::from));
     }
 
-    let mp = serde_json::to_value(&config.mp)?;
-    for (key, value) in mp.as_object().unwrap() {
-        let value = serde_json::from_value(
-            LxcConfigMp::API_SCHEMA.parse_property_string(value.as_str().unwrap())?,
-        )?;
-
-        f(key, PveDriveLxc::Mp(value))
+    for (idx, value) in (&*config.mp).into_iter() {
+        let key = format!("mp{idx}");
+        let res = value
+            .parse::<PropertyString<LxcConfigMp>>()
+            .map(|value| PveDriveLxc::Mp(value.into_inner()));
+        f(&key, res.map_err(Error::from));
     }
 
-    let unused = serde_json::to_value(&config.unused)?;
-    for (key, value) in unused.as_object().unwrap() {
-        let value = serde_json::from_value(
-            LxcConfigUnused::API_SCHEMA.parse_property_string(value.as_str().unwrap())?,
-        )?;
-
-        f(key, PveDriveLxc::Unused(value))
+    for (idx, value) in (&*config.unused).into_iter() {
+        let key = format!("unused{idx}");
+        let res = value
+            .parse::<PropertyString<LxcConfigUnused>>()
+            .map(|value| PveDriveLxc::Unused(value.into_inner()));
+        f(&key, res.map_err(Error::from));
     }
-
-    Ok(())
 }
diff --git a/ui/src/widget/pve_migrate_mapping.rs b/ui/src/widget/pve_migrate_mapping.rs
index cce4453..b8ab3f9 100644
--- a/ui/src/widget/pve_migrate_mapping.rs
+++ b/ui/src/widget/pve_migrate_mapping.rs
@@ -139,7 +139,14 @@ impl PveMigrateMapComp {
 
                 let mut storages = HashSet::new();
 
-                foreach_drive_qemu(&config, |key, value| {
+                foreach_drive_qemu(&config, |key, res| {
+                    let value = match res {
+                        Ok(value) => value,
+                        Err(err) => {
+                            log::error!("could not parse {key}: {err}");
+                            return;
+                        }
+                    };
                     let file = value.get_file();
                     if let Some(captures) = pdm_client::types::VOLUME_ID.captures(file) {
                         let storage = captures.get(1).unwrap();
@@ -147,7 +154,7 @@ impl PveMigrateMapComp {
                     } else {
                         log::error!("could not parse 'file' property of '{key}");
                     }
-                })?;
+                });
 
                 let mut networks = HashSet::new();
 
@@ -178,7 +185,14 @@ impl PveMigrateMapComp {
 
                 let mut storages = HashSet::new();
 
-                foreach_drive_lxc(&config, |key, value| {
+                foreach_drive_lxc(&config, |key, res| {
+                    let value = match res {
+                        Ok(value) => value,
+                        Err(err) => {
+                            log::error!("could not parse {key}: {err}");
+                            return;
+                        }
+                    };
                     let volume = value.get_volume();
                     if let Some(captures) = pdm_client::types::VOLUME_ID.captures(volume) {
                         let storage = captures.get(1).unwrap();
@@ -186,7 +200,7 @@ impl PveMigrateMapComp {
                     } else {
                         log::error!("could not parse 'file' property of '{key}");
                     }
-                })?;
+                });
 
                 let mut networks = HashSet::new();
 
-- 
2.39.5



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel


^ permalink raw reply	[flat|nested] 2+ messages in thread