public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pdm-devel] [PATCH datacenter-manager v2 1/2] ui: improve foreach_drive_* functions
@ 2025-01-14 14:38 Dominik Csapak
  2025-01-14 14:38 ` [pdm-devel] [PATCH datacenter-manager v2 2/2] ui: remote migrate: correctly use node from selected endpoint Dominik Csapak
  2025-01-15  9:39 ` [pdm-devel] applied: [PATCH datacenter-manager v2 1/2] ui: improve foreach_drive_* functions Dietmar Maurer
  0 siblings, 2 replies; 3+ messages in thread
From: Dominik Csapak @ 2025-01-14 14:38 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>
---
no changes from v1
 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] 3+ messages in thread

* [pdm-devel] [PATCH datacenter-manager v2 2/2] ui: remote migrate: correctly use node from selected endpoint
  2025-01-14 14:38 [pdm-devel] [PATCH datacenter-manager v2 1/2] ui: improve foreach_drive_* functions Dominik Csapak
@ 2025-01-14 14:38 ` Dominik Csapak
  2025-01-15  9:39 ` [pdm-devel] applied: [PATCH datacenter-manager v2 1/2] ui: improve foreach_drive_* functions Dietmar Maurer
  1 sibling, 0 replies; 3+ messages in thread
From: Dominik Csapak @ 2025-01-14 14:38 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>
---
changes from v1:
* also give the node to the network selector in the detail mapping
* don't use 'localhost' as default value in the detail mapping directly,
  just pass through the option and let the network/storage selector
  handle that themselves.

 ui/src/widget/migrate_window.rs       |  4 +++-
 ui/src/widget/pve_migrate_mapping.rs  | 10 +++++++---
 ui/src/widget/pve_network_selector.rs |  6 +++---
 3 files changed, 13 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..79d15cf 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();
                 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();
@@ -383,6 +386,7 @@ fn columns(
                         .into(),
                     MapType::Network => PveNetworkSelector::new(remote.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] 3+ messages in thread

* [pdm-devel] applied: [PATCH datacenter-manager v2 1/2] ui: improve foreach_drive_* functions
  2025-01-14 14:38 [pdm-devel] [PATCH datacenter-manager v2 1/2] ui: improve foreach_drive_* functions Dominik Csapak
  2025-01-14 14:38 ` [pdm-devel] [PATCH datacenter-manager v2 2/2] ui: remote migrate: correctly use node from selected endpoint Dominik Csapak
@ 2025-01-15  9:39 ` Dietmar Maurer
  1 sibling, 0 replies; 3+ messages in thread
From: Dietmar Maurer @ 2025-01-15  9:39 UTC (permalink / raw)
  To: Proxmox Datacenter Manager development discussion, Dominik Csapak

applied both patches


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


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

end of thread, other threads:[~2025-01-15  9:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-01-14 14:38 [pdm-devel] [PATCH datacenter-manager v2 1/2] ui: improve foreach_drive_* functions Dominik Csapak
2025-01-14 14:38 ` [pdm-devel] [PATCH datacenter-manager v2 2/2] ui: remote migrate: correctly use node from selected endpoint Dominik Csapak
2025-01-15  9:39 ` [pdm-devel] applied: [PATCH datacenter-manager v2 1/2] ui: improve foreach_drive_* functions Dietmar Maurer

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