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.he...@proxmox.com> --- Changes v1 -> v2: * implement FromStr instead of Deserialize and use serde_plain::derive_deserialize_from_fromstr!() Signed-off-by: Christoph Heiss <c.he...@proxmox.com> --- 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 e77914b..32c62a7 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 804da1a..e0ac8d6 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