public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [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

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

for network selector and detailed mapping too, not only for the target
storage selector.

Change the 'node' property to an option, so it's consistent across the
components.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 ui/src/widget/migrate_window.rs       | 4 +++-
 ui/src/widget/pve_migrate_mapping.rs  | 9 ++++++---
 ui/src/widget/pve_network_selector.rs | 6 +++---
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/ui/src/widget/migrate_window.rs b/ui/src/widget/migrate_window.rs
index 7214ff4..559d04c 100644
--- a/ui/src/widget/migrate_window.rs
+++ b/ui/src/widget/migrate_window.rs
@@ -457,7 +457,7 @@ impl PdmMigrateWindow {
             PveStorageSelector::new(target_remote.clone())
                 .key(format!("storage-{target_remote}"))
                 .name("target_storage")
-                .node(target_node)
+                .node(target_node.clone())
                 .disabled(!show_target_storage)
                 .autoselect(!same_remote)
                 .content_types(content_types.clone())
@@ -471,6 +471,7 @@ impl PdmMigrateWindow {
             PveNetworkSelector::new(target_remote.clone())
                 .key(format!("network-{target_remote}"))
                 .name("target_network")
+                .node(target_node.clone())
                 .disabled(detail_mode)
                 .required(!detail_mode),
         );
@@ -481,6 +482,7 @@ impl PdmMigrateWindow {
             PveMigrateMap::new(target_remote, guest_info)
                 .content_types(content_types)
                 .name("detail-map")
+                .node(target_node)
                 .submit(detail_mode)
                 .required(detail_mode),
         );
diff --git a/ui/src/widget/pve_migrate_mapping.rs b/ui/src/widget/pve_migrate_mapping.rs
index b8ab3f9..3318eb3 100644
--- a/ui/src/widget/pve_migrate_mapping.rs
+++ b/ui/src/widget/pve_migrate_mapping.rs
@@ -81,8 +81,8 @@ pub struct PveMigrateMap {
 
     /// The node to query
     #[builder(IntoPropValue, into_prop_value)]
-    #[prop_or(AttrValue::from("localhost"))]
-    pub node: AttrValue,
+    #[prop_or_default]
+    pub node: Option<AttrValue>,
 
     /// The target node for the storage
     #[builder(IntoPropValue, into_prop_value)]
@@ -350,7 +350,8 @@ fn columns(
     ctx: &ManagedFieldContext<'_, PveMigrateMapComp>,
     remote: AttrValue,
 ) -> Rc<Vec<DataTableHeader<MapEntry>>> {
-    let content_types = ctx.props().content_types.clone();
+    let props = ctx.props();
+    let content_types = props.content_types.clone();
     Rc::new(vec![
         DataTableColumn::new(tr!("Type"))
             .get_property(|entry: &MapEntry| &entry.map_type)
@@ -366,10 +367,12 @@ fn columns(
             .flex(2)
             .render({
                 let link = ctx.link();
+                let node = props.node.clone().unwrap_or(AttrValue::from("localhost"));
                 move |entry: &MapEntry| match entry.map_type {
                     MapType::Storage => PveStorageSelector::new(remote.clone())
                         .content_types(content_types.clone())
                         .default(entry.target.clone())
+                        .node(node.clone())
                         .on_change({
                             let link = link.clone();
                             let entry = entry.clone();
diff --git a/ui/src/widget/pve_network_selector.rs b/ui/src/widget/pve_network_selector.rs
index 1fcaed7..fddbf9c 100644
--- a/ui/src/widget/pve_network_selector.rs
+++ b/ui/src/widget/pve_network_selector.rs
@@ -44,8 +44,8 @@ pub struct PveNetworkSelector {
 
     /// The node to select the network from
     #[builder(IntoPropValue, into_prop_value)]
-    #[prop_or(AttrValue::from("localhost"))]
-    pub node: AttrValue,
+    #[prop_or_default]
+    pub node: Option<AttrValue>,
 
     /// The interface types to list
     #[builder(IntoPropValue, into_prop_value)]
@@ -82,7 +82,7 @@ impl PveNetworkSelectorComp {
     fn create_load_callback(ctx: &yew::Context<Self>) -> LoadCallback<Vec<NetworkInterface>> {
         let props = ctx.props();
         let remote = props.remote.clone();
-        let node = props.node.clone();
+        let node = props.node.clone().unwrap_or(AttrValue::from("localhost"));
         let ty = props.interface_type;
 
         (move || Self::get_network_list(remote.clone(), node.clone(), ty)).into()
-- 
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

end of thread, other threads:[~2025-01-14 14:20 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-01-14 14:20 [pdm-devel] [PATCH datacenter-manager 1/2] ui: improve foreach_drive_* functions 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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal