[pve-devel] [PATCH installer 1/3] common: allow lowercase and uppercase zfs raid levels

* [pve-devel] [PATCH installer 1/3] common: allow lowercase and uppercase zfs raid levels
@ 2024-11-20 18:24 Daniel Kral
  2024-11-20 18:24 ` [pve-devel] [PATCH installer 2/3] common: allow lowercase and uppercase btrfs " Daniel Kral
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Daniel Kral @ 2024-11-20 18:24 UTC (permalink / raw)
  To: pve-devel

Allows the ZFS RAID levels to be either lowercase or uppercase when
deserializing them from string values, i.e. currently only the config
value of `zfs.raid` in auto-installer answer files.

This partly fixes a regression, where deserializing the `zfs.raid`
property in answer files were only possible with uppercase values for
the ZFS RAID Z-levels, opposed to only lowercase as in previous
versions. This breaks the user API, as users cannot use the same answer
files as before for ZFS RAID Z-levels (the prepare-iso command fails).

Fixes: 510b0c008fb1 ("common: simplifying filesystem type serializing & Display trait impl")
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
As suggested by Thomas off-list, we could improve this in the future to
use a case-insensitive deserializer, but this should work as a rather
quick fix to not break user API in the answer file.

Serde does not have a native case-insensitive deserializer and this was
the minimal option without having to implement a case-insensitive
deserializer on my own (or pulling in `serde_aux` as a dependency for
this), allowing the `serde_plain::derive_display_from_serialize!`
afterwards and showing the lowercase variants as the correct values on
error (e.g. using `zfs.raid = "Raidz-3"`, which is incorrect):

```
$ proxmox-auto-install-assistant validate-answer answer.toml
Error: Error parsing answer file: TOML parse error at line 23, column 12
   |
23 | zfs.raid = "Raidz-3"
   |            ^^^^^^^^^
unknown variant `Raidz-3`, expected one of `raid0`, `raid1`, `raid10`,
    `raidz-1`, `raidz-2`, `raidz-3`
```

I checked the correctness by using either lowercase or uppercase in a
answer file and checking the expanded version of this with
`cargo expand` in the proxmox-installer-common directory. These parts
show the following for deserialization (only the string variant next to
the bytes variant)...

```
fn visit_str<__E>(
    self,
    __value: &str,
) -> _serde::__private::Result<Self::Value, __E>
where
    __E: _serde::de::Error,
{
    match __value {
	"RAID0" | "raid0" => _serde::__private::Ok(__Field::__field0),
	"RAID1" | "raid1" => _serde::__private::Ok(__Field::__field1),
	"RAID10" | "raid10" => {
	    _serde::__private::Ok(__Field::__field2)
	}
	"RAIDZ-1" | "raidz-1" => {
	    _serde::__private::Ok(__Field::__field3)
	}
	"RAIDZ-2" | "raidz-2" => {
	    _serde::__private::Ok(__Field::__field4)
	}
	"RAIDZ-3" | "raidz-3" => {
	    _serde::__private::Ok(__Field::__field5)
	}
	_ => {
	    _serde::__private::Err(
		_serde::de::Error::unknown_variant(__value, VARIANTS),
	    )
	}
    }
}
```

...and the following serialization:

```
impl _serde::Serialize for ZfsRaidLevel {
fn serialize<__S>(
    &self,
    __serializer: __S,
) -> _serde::__private::Result<__S::Ok, __S::Error>
where
    __S: _serde::Serializer,
{
    match *self {
	ZfsRaidLevel::Raid0 => {
	    _serde::Serializer::serialize_unit_variant(
		__serializer,
		"ZfsRaidLevel",
		0u32,
		"RAID0",
	    )
	}
	ZfsRaidLevel::Raid1 => {
	    _serde::Serializer::serialize_unit_variant(
		__serializer,
		"ZfsRaidLevel",
		1u32,
		"RAID1",
	    )
	}
	ZfsRaidLevel::Raid10 => {
	    _serde::Serializer::serialize_unit_variant(
		__serializer,
		"ZfsRaidLevel",
		2u32,
		"RAID10",
	    )
	}
	ZfsRaidLevel::RaidZ => {
	    _serde::Serializer::serialize_unit_variant(
		__serializer,
		"ZfsRaidLevel",
		3u32,
		"RAIDZ-1",
	    )
	}
	ZfsRaidLevel::RaidZ2 => {
	    _serde::Serializer::serialize_unit_variant(
		__serializer,
		"ZfsRaidLevel",
		4u32,
		"RAIDZ-2",
	    )
	}
	ZfsRaidLevel::RaidZ3 => {
	    _serde::Serializer::serialize_unit_variant(
		__serializer,
		"ZfsRaidLevel",
		5u32,
		"RAIDZ-3",
	    )
	}
    }
}
}
```

 proxmox-installer-common/src/options.rs | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index 8b6c281..434ce26 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -24,14 +24,17 @@ serde_plain::derive_display_from_serialize!(BtrfsRaidLevel);
 #[derive(Copy, Clone, Debug, Deserialize, Serialize, Eq, PartialEq)]
 #[serde(rename_all(deserialize = "lowercase", serialize = "UPPERCASE"))]
 pub enum ZfsRaidLevel {
+    #[serde(alias = "RAID0")]
     Raid0,
+    #[serde(alias = "RAID1")]
     Raid1,
+    #[serde(alias = "RAID10")]
     Raid10,
-    #[serde(rename = "RAIDZ-1")]
+    #[serde(alias = "RAIDZ-1", rename(deserialize = "raidz-1", serialize = "RAIDZ-1"))]
     RaidZ,
-    #[serde(rename = "RAIDZ-2")]
+    #[serde(alias = "RAIDZ-2", rename(deserialize = "raidz-2", serialize = "RAIDZ-2"))]
     RaidZ2,
-    #[serde(rename = "RAIDZ-3")]
+    #[serde(alias = "RAIDZ-3", rename(deserialize = "raidz-3", serialize = "RAIDZ-3"))]
     RaidZ3,
 }
 
-- 
2.39.5



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


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