[pve-devel] [PATCH installer 04/14] 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>
---
 proxmox-installer-common/Cargo.toml     |   1 +
 proxmox-installer-common/src/options.rs | 105 +++++++++++++-----------
 proxmox-installer-common/src/setup.rs   |  59 +------------
 3 files changed, 61 insertions(+), 104 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 e77914b..9f6131b 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -1,4 +1,4 @@
-use serde::Deserialize;
+use serde::{Deserialize, Serialize};
 use std::net::{IpAddr, Ipv4Addr};
 use std::{cmp, fmt};
 
@@ -7,55 +7,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,
@@ -81,6 +59,49 @@ impl fmt::Display for FsType {
     }
 }
 
+impl<'de> Deserialize<'de> for FsType {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        let fs: String = Deserialize::deserialize(deserializer)?;
+
+        match fs.as_str() {
+            "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(serde::de::Error::custom("could not find file system: {fs}")),
+        }
+    }
+}
+
+impl Serialize for FsType {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        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)
+    }
+}
+
 #[derive(Clone, Debug)]
 pub struct LvmBootdiskOptions {
     pub total_size: f64,
@@ -119,8 +140,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 +153,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 +166,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 +175,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 9aa4063..c581100 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,
@@ -449,10 +444,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")]
@@ -511,51 +502,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.1



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