[pve-devel] [PATCH installer v3 04/20] common: simplify filesystem type serializing & Display trait impl

[Christoph Heiss <c.heiss@proxmox.com>]

Implements the proper de-/serializer directly on the type and then
use serde_plain::derive_display_from_serialize where applicable, instead
of separate serializer functions somewhere else.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
  * no changes

Changes v1 -> v2:
  * implement FromStr instead of Deserialize and use
    serde_plain::derive_deserialize_from_fromstr!()

 proxmox-installer-common/Cargo.toml     |   1 +
 proxmox-installer-common/src/options.rs | 116 +++++++++++++-----------
 proxmox-installer-common/src/setup.rs   |  59 +-----------
 3 files changed, 67 insertions(+), 109 deletions(-)

diff --git a/proxmox-installer-common/Cargo.toml b/proxmox-installer-common/Cargo.toml
index 70f828a..4b72041 100644
--- a/proxmox-installer-common/Cargo.toml
+++ b/proxmox-installer-common/Cargo.toml
@@ -11,3 +11,4 @@ homepage = "https://www.proxmox.com"
 regex = "1.7"
 serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0"
+serde_plain = "1.0"
diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index 9375ded..7a733e5 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -1,5 +1,6 @@
-use serde::Deserialize;
+use serde::{Deserialize, Serialize};
 use std::net::{IpAddr, Ipv4Addr};
+use std::str::FromStr;
 use std::{cmp, fmt};
 
 use crate::setup::{
@@ -7,55 +8,33 @@ use crate::setup::{
 };
 use crate::utils::{CidrAddress, Fqdn};
 
-#[derive(Copy, Clone, Debug, Deserialize, Eq, PartialEq)]
-#[serde(rename_all = "lowercase")]
+#[derive(Copy, Clone, Debug, Deserialize, Serialize, Eq, PartialEq)]
+#[serde(rename_all(deserialize = "lowercase", serialize = "UPPERCASE"))]
 pub enum BtrfsRaidLevel {
     Raid0,
     Raid1,
     Raid10,
 }
 
-impl fmt::Display for BtrfsRaidLevel {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        use BtrfsRaidLevel::*;
-        match self {
-            Raid0 => write!(f, "RAID0"),
-            Raid1 => write!(f, "RAID1"),
-            Raid10 => write!(f, "RAID10"),
-        }
-    }
-}
+serde_plain::derive_display_from_serialize!(BtrfsRaidLevel);
 
-#[derive(Copy, Clone, Debug, Deserialize, Eq, PartialEq)]
-#[serde(rename_all = "lowercase")]
+#[derive(Copy, Clone, Debug, Deserialize, Serialize, Eq, PartialEq)]
+#[serde(rename_all(deserialize = "lowercase", serialize = "UPPERCASE"))]
 pub enum ZfsRaidLevel {
     Raid0,
     Raid1,
     Raid10,
-    #[serde(rename = "raidz-1")]
+    #[serde(rename = "RAIDZ-1")]
     RaidZ,
-    #[serde(rename = "raidz-2")]
+    #[serde(rename = "RAIDZ-2")]
     RaidZ2,
-    #[serde(rename = "raidz-3")]
+    #[serde(rename = "RAIDZ-3")]
     RaidZ3,
 }
 
-impl fmt::Display for ZfsRaidLevel {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        use ZfsRaidLevel::*;
-        match self {
-            Raid0 => write!(f, "RAID0"),
-            Raid1 => write!(f, "RAID1"),
-            Raid10 => write!(f, "RAID10"),
-            RaidZ => write!(f, "RAIDZ-1"),
-            RaidZ2 => write!(f, "RAIDZ-2"),
-            RaidZ3 => write!(f, "RAIDZ-3"),
-        }
-    }
-}
+serde_plain::derive_display_from_serialize!(ZfsRaidLevel);
 
-#[derive(Copy, Clone, Debug, Deserialize, Eq, PartialEq)]
-#[serde(rename_all = "lowercase")]
+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
 pub enum FsType {
     Ext4,
     Xfs,
@@ -71,16 +50,59 @@ impl FsType {
 
 impl fmt::Display for FsType {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        use FsType::*;
+        // Values displayed to the user in the installer UI
         match self {
-            Ext4 => write!(f, "ext4"),
-            Xfs => write!(f, "XFS"),
-            Zfs(level) => write!(f, "ZFS ({level})"),
-            Btrfs(level) => write!(f, "Btrfs ({level})"),
+            FsType::Ext4 => write!(f, "ext4"),
+            FsType::Xfs => write!(f, "XFS"),
+            FsType::Zfs(level) => write!(f, "ZFS ({level})"),
+            FsType::Btrfs(level) => write!(f, "Btrfs ({level})"),
+        }
+    }
+}
+
+impl Serialize for FsType {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        // These values must match exactly what the low-level installer expects
+        let value = match self {
+            // proxinstall::$fssetup
+            FsType::Ext4 => "ext4",
+            FsType::Xfs => "xfs",
+            // proxinstall::get_zfs_raid_setup()
+            FsType::Zfs(level) => &format!("zfs ({level})"),
+            // proxinstall::get_btrfs_raid_setup()
+            FsType::Btrfs(level) => &format!("btrfs ({level})"),
+        };
+
+        serializer.collect_str(value)
+    }
+}
+
+impl FromStr for FsType {
+    type Err = String;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match s {
+            "ext4" => Ok(FsType::Ext4),
+            "xfs" => Ok(FsType::Xfs),
+            "zfs (RAID0)" => Ok(FsType::Zfs(ZfsRaidLevel::Raid0)),
+            "zfs (RAID1)" => Ok(FsType::Zfs(ZfsRaidLevel::Raid1)),
+            "zfs (RAID10)" => Ok(FsType::Zfs(ZfsRaidLevel::Raid10)),
+            "zfs (RAIDZ-1)" => Ok(FsType::Zfs(ZfsRaidLevel::RaidZ)),
+            "zfs (RAIDZ-2)" => Ok(FsType::Zfs(ZfsRaidLevel::RaidZ2)),
+            "zfs (RAIDZ-3)" => Ok(FsType::Zfs(ZfsRaidLevel::RaidZ3)),
+            "btrfs (RAID0)" => Ok(FsType::Btrfs(BtrfsRaidLevel::Raid0)),
+            "btrfs (RAID1)" => Ok(FsType::Btrfs(BtrfsRaidLevel::Raid1)),
+            "btrfs (RAID10)" => Ok(FsType::Btrfs(BtrfsRaidLevel::Raid10)),
+            _ => Err(format!("Could not find file system: {s}")),
         }
     }
 }
 
+serde_plain::derive_deserialize_from_fromstr!(FsType, "valid filesystem");
+
 #[derive(Clone, Debug)]
 pub struct LvmBootdiskOptions {
     pub total_size: f64,
@@ -119,8 +141,8 @@ impl BtrfsBootdiskOptions {
     }
 }
 
-#[derive(Copy, Clone, Debug, Default, Deserialize, Eq, PartialEq)]
-#[serde(rename_all(deserialize = "lowercase"))]
+#[derive(Copy, Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)]
+#[serde(rename_all = "lowercase")]
 pub enum ZfsCompressOption {
     #[default]
     On,
@@ -132,11 +154,7 @@ pub enum ZfsCompressOption {
     Zstd,
 }
 
-impl fmt::Display for ZfsCompressOption {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "{}", format!("{self:?}").to_lowercase())
-    }
-}
+serde_plain::derive_display_from_serialize!(ZfsCompressOption);
 
 impl From<&ZfsCompressOption> for String {
     fn from(value: &ZfsCompressOption) -> Self {
@@ -149,7 +167,7 @@ pub const ZFS_COMPRESS_OPTIONS: &[ZfsCompressOption] = {
     &[On, Off, Lzjb, Lz4, Zle, Gzip, Zstd]
 };
 
-#[derive(Copy, Clone, Debug, Default, Deserialize, Eq, PartialEq)]
+#[derive(Copy, Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)]
 #[serde(rename_all = "kebab-case")]
 pub enum ZfsChecksumOption {
     #[default]
@@ -158,11 +176,7 @@ pub enum ZfsChecksumOption {
     Sha256,
 }
 
-impl fmt::Display for ZfsChecksumOption {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "{}", format!("{self:?}").to_lowercase())
-    }
-}
+serde_plain::derive_display_from_serialize!(ZfsChecksumOption);
 
 impl From<&ZfsChecksumOption> for String {
     fn from(value: &ZfsChecksumOption) -> Self {
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index e4e609b..ad77308 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -12,10 +12,7 @@ use std::{
 use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
 
 use crate::{
-    options::{
-        BtrfsRaidLevel, Disk, FsType, ZfsBootdiskOptions, ZfsChecksumOption, ZfsCompressOption,
-        ZfsRaidLevel,
-    },
+    options::{Disk, FsType, ZfsBootdiskOptions, ZfsChecksumOption, ZfsCompressOption},
     utils::CidrAddress,
 };
 
@@ -201,9 +198,7 @@ pub fn installer_setup(in_test_mode: bool) -> Result<(SetupInfo, LocaleInfo, Run
 #[derive(Debug, Deserialize, Serialize)]
 pub struct InstallZfsOption {
     pub ashift: usize,
-    #[serde(serialize_with = "serialize_as_display")]
     pub compress: ZfsCompressOption,
-    #[serde(serialize_with = "serialize_as_display")]
     pub checksum: ZfsChecksumOption,
     pub copies: usize,
     pub arc_max: usize,
@@ -457,10 +452,6 @@ pub fn spawn_low_level_installer(test_mode: bool) -> io::Result<process::Child>
 pub struct InstallConfig {
     pub autoreboot: usize,
 
-    #[serde(
-        serialize_with = "serialize_fstype",
-        deserialize_with = "deserialize_fs_type"
-    )]
     pub filesys: FsType,
     pub hdsize: f64,
     #[serde(skip_serializing_if = "Option::is_none")]
@@ -519,51 +510,3 @@ where
         serializer.serialize_none()
     }
 }
-
-fn serialize_fstype<S>(value: &FsType, serializer: S) -> Result<S::Ok, S::Error>
-where
-    S: Serializer,
-{
-    use FsType::*;
-    let value = match value {
-        // proxinstall::$fssetup
-        Ext4 => "ext4",
-        Xfs => "xfs",
-        // proxinstall::get_zfs_raid_setup()
-        Zfs(ZfsRaidLevel::Raid0) => "zfs (RAID0)",
-        Zfs(ZfsRaidLevel::Raid1) => "zfs (RAID1)",
-        Zfs(ZfsRaidLevel::Raid10) => "zfs (RAID10)",
-        Zfs(ZfsRaidLevel::RaidZ) => "zfs (RAIDZ-1)",
-        Zfs(ZfsRaidLevel::RaidZ2) => "zfs (RAIDZ-2)",
-        Zfs(ZfsRaidLevel::RaidZ3) => "zfs (RAIDZ-3)",
-        // proxinstall::get_btrfs_raid_setup()
-        Btrfs(BtrfsRaidLevel::Raid0) => "btrfs (RAID0)",
-        Btrfs(BtrfsRaidLevel::Raid1) => "btrfs (RAID1)",
-        Btrfs(BtrfsRaidLevel::Raid10) => "btrfs (RAID10)",
-    };
-
-    serializer.collect_str(value)
-}
-
-pub fn deserialize_fs_type<'de, D>(deserializer: D) -> Result<FsType, D::Error>
-where
-    D: Deserializer<'de>,
-{
-    use FsType::*;
-    let de_fs: String = Deserialize::deserialize(deserializer)?;
-
-    match de_fs.as_str() {
-        "ext4" => Ok(Ext4),
-        "xfs" => Ok(Xfs),
-        "zfs (RAID0)" => Ok(Zfs(ZfsRaidLevel::Raid0)),
-        "zfs (RAID1)" => Ok(Zfs(ZfsRaidLevel::Raid1)),
-        "zfs (RAID10)" => Ok(Zfs(ZfsRaidLevel::Raid10)),
-        "zfs (RAIDZ-1)" => Ok(Zfs(ZfsRaidLevel::RaidZ)),
-        "zfs (RAIDZ-2)" => Ok(Zfs(ZfsRaidLevel::RaidZ2)),
-        "zfs (RAIDZ-3)" => Ok(Zfs(ZfsRaidLevel::RaidZ3)),
-        "btrfs (RAID0)" => Ok(Btrfs(BtrfsRaidLevel::Raid0)),
-        "btrfs (RAID1)" => Ok(Btrfs(BtrfsRaidLevel::Raid1)),
-        "btrfs (RAID10)" => Ok(Btrfs(BtrfsRaidLevel::Raid10)),
-        _ => Err(de::Error::custom("could not find file system: {de_fs}")),
-    }
-}
-- 
2.45.2



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