[pdm-devel] [RFC proxmox{, -yew-comp, -datacenter-manager}/yew-mobile-gui 0/6] Add fallback variant to enum properties in Proxmox VE Rust API types

[pdm-devel] [RFC proxmox{, -yew-comp, -datacenter-manager}/yew-mobile-gui 0/6] Add fallback variant to enum properties in Proxmox VE Rust API types

[Stefan Hanreich]

## Introduction

This patch series adds a new fallback variant to all enums that are
auto-generated in pve-api-types. This ensures that the addition of new enum
variants in the Proxmox VE API does not break the deserialization of Proxmox VE
API types in PDM. Rather, the unknown enum variant is now populated with the
parsed enum variant name. For that matter this patch series introduces a new
string type, FixedString. It can store up to 23 characters and is Copy, which is
the main reason for introducing this custom string type. This lets us keep Copy
in our generated enums.


## Open Questions

### Resubmitting unknown variants

I had some off list discussions with Dominik and Dietmar regarding how consumers
of the API (mainly the UI) could handle encountering unknown values when there
is the possibility of re-submission of this data. Two main options emerged:

* Resubmitting unknown values as-is, without preventing the user from doing so

This has the upside of not preventing the user from submitting perfectly fine
values if they haven't been implemented in PDM. An argument in favor would be
that the PVE API should act as the last safeguard against invalid objects in the
configuration anyway, so we could rely on PVE validation only in the case of
unknown enums.
The downside is that we need a round-trip to the Proxmox VE API and can only
show the resulting error in the UI, without any validation. It also does not
take care of situations where setting a certain, new, enum potentially requires
special behavior when submitting values.

* Showing the value, but preventing the user from re-submitting the value

This is the safest option, since it prevents any, potentially destructive,
action from the user due to the consumer not being aware of special . It also
gives the user immediate feedback in the UI, without requiring a round-trip to
the Proxmox VE instance, but of course users lose the ability to submit
perfectly valid configurations (without upgrading).


The answer is, of course, it depends on the context - but in the general case it
seems preferable to me to show the value to the user, but prevent the user from
re-submitting the value, since the UI cannot know if there is some special
handling required in case the new enum variant is used in the UI. If there is a
good argument for letting the user re-submit certain fields, then that seems
like a fine option too, but imo *iff* there's good reasoning attached to it.

What do you think? Any options that were missed?


### Storing unknown Variants

When only reading and displaying / storing data, then including unknown values
is desired behavior imo (again, in the general case) - since it allows storing
data (e.g. metrics) that isn't yet understood but could potentially be
understood perfectly fine after an upgrade of PDM.


### Unsafe FixedString

I implemented as_bytes and as_str in FixedString via the unsafe variants of the
methods and included tests that check for correct behavior of those methods.
Using the safe variants and panicking here should be fine too imo if we don't
want to go down that road. FixedString should only get used in rare, exceptional
cases anyway - and even then not in a hot path. The adaption should be trivial.


### Truncating

Unknown enum variants longer than 23 characters can still lead to the same
issues as before. I shortly thought about truncating all longer variants instead
of throwing a hard error, but imo that's a bad idea because:

* deserialize followed by a serialize is no longer an idempotent operation
* accidentally resubmitting truncated values would be *really* bad, whereas
  resubmitting exact values is way less likely to cause trouble.
* we control the length of future enum variants and can take this limit into
  consideration

Nevertheless, I left the implementation for truncating in the impl, since it was
complete and in case it is potentially needed in the future. We could also just
hide it from the public API or remove it altogether, since it is currently
unused and potentially dangerous due to the reasons listed above.


## Adapting the existing code

One difficulty when adapting the UI was identifying places in which a struct
gets returned directly from the API, deserialized (but the value isn't used
anywhere in the frontend), then serialized again and sent back to the backend.
Since the FixedString serializes indistinguishable from a String, this could
introduce situations where we submit unkown enum values when instead an error
should be shown and the user prevented from submitting a form.

I tried to look at each component in the backend / frontend and check if it uses
and potentially re-submits am enum without checking its variants for unknown
values. Since we model enums with checkboxes in the UI and have fixed values
there, this should usually work and prevent users from submitting values that
are unknown to the UI. Nevertheless, it's very much possible I missed some
places, so here some group effort in trying to identify potentially problematic
spots would be nice.


## Maintainers Notes

pve-api-types breaks building:
* proxmox-datacenter-manager
* proxmox-yew-comp
* pve-yew-mobile-gui

proxmox-datacenter-manager and pve-yew-mobile-gui additionally need the newer
version of proxmox-yew-comp to build.

proxmox:

Stefan Hanreich (3):
  pve-api-types: add FixedString type
  pve-api-types: generate fallback variant for enums
  pve-api-types: regenerate

 pve-api-types/generator-lib/Schema2Rust.pm |   3 +
 pve-api-types/src/generated/types.rs       | 207 +++++++++++++
 pve-api-types/src/types/fixed_string.rs    | 331 +++++++++++++++++++++
 pve-api-types/src/types/mod.rs             |   3 +
 4 files changed, 544 insertions(+)
 create mode 100644 pve-api-types/src/types/fixed_string.rs


proxmox-yew-comp:

Stefan Hanreich (1):
  pve: qemu: handle fallback enum variants

 .../qemu_property/qemu_amd_sev_property.rs    |  1 +
 .../pve/qemu_property/qemu_bios_property.rs   |  1 +
 .../qemu_property/qemu_machine_property.rs    | 20 +++++++++++--------
 3 files changed, 14 insertions(+), 8 deletions(-)


proxmox-datacenter-manager:

Stefan Hanreich (1):
  tree-wide: handle new unknown enum variants

 server/src/metric_collection/rrd_task.rs | 4 ++++
 ui/src/pve/utils.rs                      | 1 +
 ui/src/widget/migrate_window.rs          | 6 ++++++
 3 files changed, 11 insertions(+)


pve-yew-mobile-gui:

Stefan Hanreich (1):
  tree-wide: handle fallback enum values

 src/pages/page_resources.rs   | 4 +++-
 src/pages/page_task_status.rs | 7 +++++++
 2 files changed, 10 insertions(+), 1 deletion(-)


Summary over all repositories:
  12 files changed, 579 insertions(+), 9 deletions(-)

-- 
Generated by git-murpp 0.8.0

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

[pdm-devel] [PATCH proxmox 1/3] pve-api-types: add FixedString type

[Stefan Hanreich]

A custom, immutable, string type, that is copiable, which can hold up
to 23 characters. It will be used later for storing the name of
unknown enum variants when parsing the return value of enum
properties. The main reason for introducing this type is that its copy
as opposed to String, which lets us keep the Copy implementation for
the existing enums.

Main reason for choosing 23 characters is the fact that String is 24
bytes large as well (ptr, len, capacity).

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 pve-api-types/src/types/fixed_string.rs | 331 ++++++++++++++++++++++++
 pve-api-types/src/types/mod.rs          |   3 +
 2 files changed, 334 insertions(+)
 create mode 100644 pve-api-types/src/types/fixed_string.rs

diff --git a/pve-api-types/src/types/fixed_string.rs b/pve-api-types/src/types/fixed_string.rs
new file mode 100644
index 00000000..f1692227
--- /dev/null
+++ b/pve-api-types/src/types/fixed_string.rs
@@ -0,0 +1,331 @@
+use std::cmp::Ordering;
+use std::error::Error;
+use std::fmt::{Debug, Display, Formatter};
+use std::ops::Deref;
+use std::str::FromStr;
+
+use serde::{Deserialize, Serialize};
+
+/// Error type used by constructors of [`FixedString`]
+#[derive(Clone, Copy, Debug)]
+pub enum FixedStringError {
+    TooLong,
+}
+
+impl Error for FixedStringError {}
+
+impl Display for FixedStringError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
+        f.write_str(match self {
+            Self::TooLong => "string is longer than 23 characters",
+        })
+    }
+}
+
+/// An immutable string type with a maximum size of 23 bytes.
+///
+/// After construction it is guaranteed that its contents are:
+/// * valid utf-8
+/// * not longer than 23 characters
+///
+/// FixedString is immutable, therefore it is sufficient to validate the invariants only at
+/// construction time to guarantee that they will always hold during the lifecycle of the
+/// struct.
+#[derive(Clone, Copy)]
+pub struct FixedString {
+    buf: [u8; 23],
+    len: u8,
+}
+
+impl FixedString {
+    /// Creates a new FixedString instance from a str reference.
+    ///
+    /// # Errors
+    /// This function will return an error if:
+    /// * The passed string is longer than 23 bytes
+    pub fn new(value: &str) -> Result<Self, FixedStringError> {
+        if value.len() > 23 {
+            return Err(FixedStringError::TooLong);
+        }
+
+        let mut buf = [0; 23];
+        buf[..value.len()].copy_from_slice(value.as_bytes());
+
+        Ok(Self {
+            buf,
+            // SAFETY: self.len is at least 0 and at most 23, which fits into u8
+            len: value.len() as u8,
+        })
+    }
+
+    /// Creates a new FixedString instance from a str reference, truncating if required
+    ///
+    /// It works by copying the bytes from the supplied value into its own local buffer. If the
+    /// string is longer than 23 bytes, then this methods will truncate the string at the nearest
+    /// utf-8 boundary, so even if a string longer than 23 characters is passed in, it is possible
+    /// that it gets truncated to a length less than 23 if the utf-8 boundary doesn't perfectly
+    /// align with the 23 bytes limit.
+    pub fn new_truncated(value: &str) -> Self {
+        // TODO: replace, once [`str::floor_char_boundary`] has been stabilized
+        let len = if value.len() > 23 {
+            let mut index = 23;
+
+            while index > 0 && !value.is_char_boundary(index) {
+                index = index - 1;
+            }
+
+            index
+        } else {
+            value.len()
+        };
+
+        let mut buf = [0; 23];
+        buf[..len].copy_from_slice(&value.as_bytes()[..len]);
+
+        Self {
+            buf,
+            // SAFETY: self.len is at least 0 and at most 23, which fits into u8
+            len: len as u8,
+        }
+    }
+
+    /// Returns a str reference to the stored data
+    #[inline]
+    pub fn as_str(&self) -> &str {
+        // SAFETY: self.buf must be a valid utf-8 string by construction
+        unsafe { str::from_utf8_unchecked(self.as_bytes()) }
+    }
+
+    /// Returns a reference to the set bytes in the stored buffer
+    #[inline]
+    pub fn as_bytes(&self) -> &[u8] {
+        // SAFETY: self.len >= 0 and self.len <= 23 by construction
+        unsafe { self.buf.get_unchecked(..self.len as usize) }
+    }
+}
+
+impl PartialEq for FixedString {
+    #[inline]
+    fn eq(&self, other: &FixedString) -> bool {
+        self.as_bytes() == other.as_bytes()
+    }
+}
+
+impl Eq for FixedString {}
+
+impl PartialOrd for FixedString {
+    fn partial_cmp(&self, other: &FixedString) -> Option<Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl Ord for FixedString {
+    #[inline]
+    fn cmp(&self, other: &FixedString) -> Ordering {
+        self.as_bytes().cmp(other.as_bytes())
+    }
+}
+
+impl Display for FixedString {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
+        f.write_str(self.as_str())
+    }
+}
+
+impl Debug for FixedString {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
+        f.write_str(self.as_str())
+    }
+}
+
+impl Deref for FixedString {
+    type Target = str;
+
+    fn deref(&self) -> &str {
+        self.as_str()
+    }
+}
+
+impl AsRef<str> for FixedString {
+    fn as_ref(&self) -> &str {
+        self.as_str()
+    }
+}
+
+impl TryFrom<String> for FixedString {
+    type Error = FixedStringError;
+
+    fn try_from(value: String) -> Result<Self, Self::Error> {
+        FixedString::new(value.as_str())
+    }
+}
+
+impl FromStr for FixedString {
+    type Err = FixedStringError;
+
+    fn from_str(value: &str) -> Result<Self, Self::Err> {
+        FixedString::new(value)
+    }
+}
+
+impl TryFrom<&str> for FixedString {
+    type Error = FixedStringError;
+
+    fn try_from(value: &str) -> Result<Self, Self::Error> {
+        FixedString::new(value)
+    }
+}
+
+impl PartialEq<str> for FixedString {
+    fn eq(&self, other: &str) -> bool {
+        self.as_str().eq(other)
+    }
+}
+
+impl PartialEq<FixedString> for &str {
+    fn eq(&self, other: &FixedString) -> bool {
+        self.eq(&other.as_str())
+    }
+}
+
+impl PartialOrd<str> for FixedString {
+    fn partial_cmp(&self, other: &str) -> Option<Ordering> {
+        Some(self.as_str().cmp(&other))
+    }
+}
+
+impl PartialOrd<FixedString> for &str {
+    fn partial_cmp(&self, other: &FixedString) -> Option<Ordering> {
+        Some(self.cmp(&other.as_str()))
+    }
+}
+
+impl Serialize for FixedString {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        serializer.serialize_str(self.as_str())
+    }
+}
+
+impl<'de> Deserialize<'de> for FixedString {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        deserializer.deserialize_str(FixedStringVisitor)
+    }
+}
+
+struct FixedStringVisitor;
+
+impl<'de> serde::de::Visitor<'de> for FixedStringVisitor {
+    type Value = FixedString;
+
+    fn expecting(&self, f: &mut Formatter) -> std::fmt::Result {
+        f.write_str("a string that is at most 23 bytes long")
+    }
+
+    fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
+    where
+        E: serde::de::Error,
+    {
+        v.try_into().map_err(E::custom)
+    }
+
+    fn visit_string<E>(self, v: String) -> Result<Self::Value, E>
+    where
+        E: serde::de::Error,
+    {
+        v.try_into().map_err(E::custom)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    use serde_plain;
+
+    #[test]
+    fn test_construct() {
+        let fixed_string = FixedString::new("").expect("empty string is valid");
+        assert_eq!("", fixed_string);
+
+        let fixed_string = FixedString::new("a").expect("valid string");
+        assert_eq!("a", fixed_string);
+
+        let fixed_string = FixedString::new("🌏🌏🌏🌏🌏").expect("valid string");
+        assert_eq!("🌏🌏🌏🌏🌏", fixed_string);
+
+        let fixed_string =
+            FixedString::new("aaaaaaaaaaaaaaaaaaaaaaa").expect("23 characters are allowed");
+        assert_eq!("aaaaaaaaaaaaaaaaaaaaaaa", fixed_string);
+
+        FixedString::new("🌏🌏🌏🌏🌏🌏").expect_err("string too long");
+        FixedString::new("aaaaaaaaaaaaaaaaaaaaaaaa").expect_err("string too long");
+    }
+
+    #[test]
+    fn test_serialize_deserialize() {
+        let valid_string = "aaaaaaaaaaaaaaaaaaaaaaa";
+
+        let fixed_string: FixedString =
+            serde_plain::from_str("aaaaaaaaaaaaaaaaaaaaaaa").expect("deserialization works");
+        assert_eq!(valid_string, fixed_string);
+
+        let serialized_string =
+            serde_plain::to_string(&fixed_string).expect("can be serialized into a string");
+        assert_eq!(valid_string, serialized_string);
+
+        serde_plain::from_str::<FixedString>("aaaaaaaaaaaaaaaaaaaaaaaa")
+            .expect_err("cannot deserialize string that is too long");
+    }
+
+    #[test]
+    fn test_ord() {
+        let fixed_string = FixedString::new("abc").expect("valid string");
+
+        assert!(fixed_string == fixed_string);
+        assert!(fixed_string >= fixed_string);
+        assert!(fixed_string <= fixed_string);
+
+        assert!("ab" < fixed_string);
+        assert!("abc" == fixed_string);
+        assert!("abcd" > fixed_string);
+
+        let larger_fixed_string = FixedString::new("abcde").expect("valid string");
+
+        assert!(larger_fixed_string > fixed_string);
+        assert!(fixed_string < larger_fixed_string);
+    }
+
+    #[test]
+    fn test_truncate() {
+        let fixed_string = FixedString::new_truncated("aaaaaaaaaaaaaaaaaaaaaaaaaaaa");
+        assert_eq!("aaaaaaaaaaaaaaaaaaaaaaa", fixed_string);
+
+        let fixed_string = FixedString::new_truncated("aaaaaaaaaaaaaaaaaa💯");
+        assert_eq!("aaaaaaaaaaaaaaaaaa💯", fixed_string);
+
+        // '💯' is 4 bytes long
+        // first byte of '💯' is inside the 23 byte limit, but it's not a valid utf-8 boundary so
+        // it should get truncated
+        let fixed_string = FixedString::new_truncated("aaaaaaaaaaaaaaaaaaaaa💯");
+        assert_eq!("aaaaaaaaaaaaaaaaaaaaa", fixed_string);
+
+        let fixed_string = FixedString::new_truncated("aaaaaaaaaaaaaaaaaaaaaa💯");
+        assert_eq!("aaaaaaaaaaaaaaaaaaaaaa", fixed_string);
+
+        let fixed_string = FixedString::new_truncated("aaaaaaaaaaaaaaaaaaaaaaa💯");
+        assert_eq!("aaaaaaaaaaaaaaaaaaaaaaa", fixed_string);
+
+        // '🌏' is 4 bytes long
+        let fixed_string = FixedString::new_truncated("🌏🌏🌏🌏🌏");
+        assert_eq!("🌏🌏🌏🌏🌏", fixed_string);
+
+        let fixed_string = FixedString::new_truncated("🌏🌏🌏🌏🌏🌏");
+        assert_eq!("🌏🌏🌏🌏🌏", fixed_string);
+    }
+}
diff --git a/pve-api-types/src/types/mod.rs b/pve-api-types/src/types/mod.rs
index fe52a169..9653dd46 100644
--- a/pve-api-types/src/types/mod.rs
+++ b/pve-api-types/src/types/mod.rs
@@ -17,6 +17,9 @@ pub mod array;
 pub mod stringlist;
 pub mod verifiers;
 
+mod fixed_string;
+pub use fixed_string::{FixedString, FixedStringError};
+
 include!("../generated/types.rs");
 
 /// A PVE Upid, contrary to a PBS Upid, contains no 'task-id' number.
-- 
2.47.3


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

Re: [pdm-devel] [PATCH proxmox 1/3] pve-api-types: add FixedString type

[Wolfgang Bumiller]

On Wed, Nov 12, 2025 at 10:22:05AM +0100, Stefan Hanreich wrote:
> A custom, immutable, string type, that is copiable, which can hold up
> to 23 characters. It will be used later for storing the name of
> unknown enum variants when parsing the return value of enum
> properties. The main reason for introducing this type is that its copy
> as opposed to String, which lets us keep the Copy implementation for
> the existing enums.
> 
> Main reason for choosing 23 characters is the fact that String is 24
> bytes large as well (ptr, len, capacity).
> 
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>  pve-api-types/src/types/fixed_string.rs | 331 ++++++++++++++++++++++++
>  pve-api-types/src/types/mod.rs          |   3 +
>  2 files changed, 334 insertions(+)
>  create mode 100644 pve-api-types/src/types/fixed_string.rs
> 
> diff --git a/pve-api-types/src/types/fixed_string.rs b/pve-api-types/src/types/fixed_string.rs
> new file mode 100644
> index 00000000..f1692227
> --- /dev/null
> +++ b/pve-api-types/src/types/fixed_string.rs
> @@ -0,0 +1,331 @@
> +use std::cmp::Ordering;
> +use std::error::Error;
> +use std::fmt::{Debug, Display, Formatter};
> +use std::ops::Deref;
> +use std::str::FromStr;
> +
> +use serde::{Deserialize, Serialize};
> +
> +/// Error type used by constructors of [`FixedString`]
> +#[derive(Clone, Copy, Debug)]
> +pub enum FixedStringError {
> +    TooLong,
> +}

IMO this does not need to be an enum. `pub struct TooLongError` should
be enough.

> +
> +impl Error for FixedStringError {}
> +
> +impl Display for FixedStringError {
> +    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {

^ you're importing `Formatter`.
And on that note: personally I prefer to import `std::fmt` and prefix
its items with `fmt::*`.

> +        f.write_str(match self {
> +            Self::TooLong => "string is longer than 23 characters",
> +        })
> +    }
> +}
> +
> +/// An immutable string type with a maximum size of 23 bytes.
> +///
> +/// After construction it is guaranteed that its contents are:
> +/// * valid utf-8
> +/// * not longer than 23 characters
> +///
> +/// FixedString is immutable, therefore it is sufficient to validate the invariants only at
> +/// construction time to guarantee that they will always hold during the lifecycle of the
> +/// struct.
> +#[derive(Clone, Copy)]
> +pub struct FixedString {
> +    buf: [u8; 23],
> +    len: u8,
> +}
> +
> +impl FixedString {
> +    /// Creates a new FixedString instance from a str reference.
> +    ///
> +    /// # Errors
> +    /// This function will return an error if:
> +    /// * The passed string is longer than 23 bytes
> +    pub fn new(value: &str) -> Result<Self, FixedStringError> {
> +        if value.len() > 23 {
> +            return Err(FixedStringError::TooLong);
> +        }
> +
> +        let mut buf = [0; 23];
> +        buf[..value.len()].copy_from_slice(value.as_bytes());
> +
> +        Ok(Self {
> +            buf,
> +            // SAFETY: self.len is at least 0 and at most 23, which fits into u8
> +            len: value.len() as u8,
> +        })
> +    }
> +
> +    /// Creates a new FixedString instance from a str reference, truncating if required
> +    ///
> +    /// It works by copying the bytes from the supplied value into its own local buffer. If the
> +    /// string is longer than 23 bytes, then this methods will truncate the string at the nearest
> +    /// utf-8 boundary, so even if a string longer than 23 characters is passed in, it is possible
> +    /// that it gets truncated to a length less than 23 if the utf-8 boundary doesn't perfectly
> +    /// align with the 23 bytes limit.
> +    pub fn new_truncated(value: &str) -> Self {

I don't think we should keep this unless we also have a way of checking
for this.
In case we do want to use this: since values up to 31 (a potential
still-cheap-enough extension to this `FixedString`'s length (since it
would then just be 4 pointers in size)) only require 5 bits, we have 3
bits remaining in the length byte which could be used to signal that
this was a truncated string, which could be returned via a
`.is_truncated()` method, and we could refuse to serialize such a string
without calling some extra method first (and include some marker in the
Debug impl)?

But IMO: for now, we just don't need it.

> +        // TODO: replace, once [`str::floor_char_boundary`] has been stabilized
> +        let len = if value.len() > 23 {
> +            let mut index = 23;
> +
> +            while index > 0 && !value.is_char_boundary(index) {
> +                index = index - 1;

index -= 1; ;-)

or:
    panic!("Who let Dominik add emojis to enums?")

> +            }
> +
> +            index
> +        } else {
> +            value.len()
> +        };

^ Actually the above could just be shortened to

let mut len = value.len();
if len > 23 {
    len = 23;
    while ... { len -= 1 }
}

> +
> +        let mut buf = [0; 23];
> +        buf[..len].copy_from_slice(&value.as_bytes()[..len]);
> +
> +        Self {
> +            buf,
> +            // SAFETY: self.len is at least 0 and at most 23, which fits into u8
> +            len: len as u8,
> +        }
> +    }
> +
> +    /// Returns a str reference to the stored data
> +    #[inline]
> +    pub fn as_str(&self) -> &str {
> +        // SAFETY: self.buf must be a valid utf-8 string by construction
> +        unsafe { str::from_utf8_unchecked(self.as_bytes()) }
> +    }
> +
> +    /// Returns a reference to the set bytes in the stored buffer
> +    #[inline]
> +    pub fn as_bytes(&self) -> &[u8] {
> +        // SAFETY: self.len >= 0 and self.len <= 23 by construction
> +        unsafe { self.buf.get_unchecked(..self.len as usize) }
> +    }
> +}
> +
> +impl PartialEq for FixedString {

IMO - regarding ordering where it matters - it would be easier to read
such impls when they're purely forwarded, eg.:

    PartialEq::eq(self.as_str(), other)

since then all implementations look the same (no need for `Some()`
wrapping in the `PartialOrd` impl, for instance.

Also, it could be generated with a simple forwarding macro which would
IMO also make review easier than having to go through each of them
individually:

    macro_rules! forward_impl_to_bytes {
        ($($trait:ident {$fn:ident -> $out:ty })+) => {
            $(
                impl $trait for FixedString {
                    fn $fn(&self, other: &Self) -> $out {
                        <[u8] as $trait>::$fn(self.as_bytes(), other.as_bytes())
                    }
                }
            )+
        };
    }

    forward_impl_to_bytes! {
        PartialEq { eq -> bool }
        PartialOrd { partial_cmp -> Option<Ordering> }
        Ord { cmp -> Ordering }
    }


And for the ones below:

    macro_rules! forward_impl_to_str_bidir {
        ($($trait:ident {$fn:ident -> $out:ty })+) => {
            $(
                impl $trait<str> for FixedString {
                    fn $fn(&self, other: &str) -> $out {
                        <str as $trait>::$fn(self.as_str(), other)
                    }
                }

                impl $trait<FixedString> for &str {
                    fn $fn(&self, other: &FixedString) -> $out {
                        <str as $trait>::$fn(self, other.as_str())
                    }
                }
            )+
        };
    }

    forward_impl_to_str_bidir! {
        PartialEq { eq -> bool }
        PartialOrd { partial_cmp -> Option<Ordering> }
    }

Yes I realize that for 2 traits this doesn't save much, but it would
make the intention clearer and IMO is still easier to review.

> +    #[inline]
> +    fn eq(&self, other: &FixedString) -> bool {
> +        self.as_bytes() == other.as_bytes()
> +    }
> +}
> +
> +impl Eq for FixedString {}
> +
> +impl PartialOrd for FixedString {
> +    fn partial_cmp(&self, other: &FixedString) -> Option<Ordering> {
> +        Some(self.cmp(other))
> +    }
> +}
> +
> +impl Ord for FixedString {
> +    #[inline]
> +    fn cmp(&self, other: &FixedString) -> Ordering {
> +        self.as_bytes().cmp(other.as_bytes())
> +    }
> +}
> +
> +impl Display for FixedString {
> +    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
> +        f.write_str(self.as_str())

Consider forwarding to &str's fmt instead so that things such as
alignment etc. work:

    Display::fmt(self.as_str(), f)

so that `println!("{the_enum:>8}")` will print "     foo" instead of
just "foo", and `{x:0^8}` produces "00foo000" ;-)

> +    }
> +}
> +
> +impl Debug for FixedString {
> +    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
> +        f.write_str(self.as_str())

Same here with `Debug::fmt(self.as_str(), f)`.

> +    }
> +}
> +
> +impl Deref for FixedString {
> +    type Target = str;
> +
> +    fn deref(&self) -> &str {
> +        self.as_str()
> +    }
> +}
> +
> +impl AsRef<str> for FixedString {
> +    fn as_ref(&self) -> &str {
> +        self.as_str()
> +    }
> +}

I'd also add `AsRef<[u8]>`, and `Borrow<str>`. (The latter would be
required to allow a `HashMap<FixedString, T>` to use `.get("key")` with
`&str` for a key - not that that's something we'd expect to see...

> +
> +impl TryFrom<String> for FixedString {
> +    type Error = FixedStringError;
> +
> +    fn try_from(value: String) -> Result<Self, Self::Error> {
> +        FixedString::new(value.as_str())
> +    }
> +}
> +
> +impl FromStr for FixedString {
> +    type Err = FixedStringError;
> +
> +    fn from_str(value: &str) -> Result<Self, Self::Err> {
> +        FixedString::new(value)
> +    }
> +}
> +
> +impl TryFrom<&str> for FixedString {
> +    type Error = FixedStringError;
> +
> +    fn try_from(value: &str) -> Result<Self, Self::Error> {
> +        FixedString::new(value)
> +    }
> +}
> +
> +impl PartialEq<str> for FixedString {
> +    fn eq(&self, other: &str) -> bool {
> +        self.as_str().eq(other)
> +    }
> +}
> +
> +impl PartialEq<FixedString> for &str {
> +    fn eq(&self, other: &FixedString) -> bool {
> +        self.eq(&other.as_str())
> +    }
> +}
> +
> +impl PartialOrd<str> for FixedString {
> +    fn partial_cmp(&self, other: &str) -> Option<Ordering> {
> +        Some(self.as_str().cmp(&other))
> +    }
> +}
> +
> +impl PartialOrd<FixedString> for &str {
> +    fn partial_cmp(&self, other: &FixedString) -> Option<Ordering> {
> +        Some(self.cmp(&other.as_str()))
> +    }
> +}
> +
> +impl Serialize for FixedString {
> +    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
> +    where
> +        S: serde::Serializer,
> +    {
> +        serializer.serialize_str(self.as_str())
> +    }
> +}
> +
> +impl<'de> Deserialize<'de> for FixedString {
> +    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
> +    where
> +        D: serde::Deserializer<'de>,
> +    {
> +        deserializer.deserialize_str(FixedStringVisitor)
> +    }
> +}
> +
> +struct FixedStringVisitor;

^ I'd prefer to move the type and Visitor impl into the `fn deserialize`.

> +
> +impl<'de> serde::de::Visitor<'de> for FixedStringVisitor {
> +    type Value = FixedString;
> +
> +    fn expecting(&self, f: &mut Formatter) -> std::fmt::Result {
> +        f.write_str("a string that is at most 23 bytes long")
> +    }
> +
> +    fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
> +    where
> +        E: serde::de::Error,
> +    {
> +        v.try_into().map_err(E::custom)
> +    }
> +
> +    fn visit_string<E>(self, v: String) -> Result<Self::Value, E>
> +    where
> +        E: serde::de::Error,
> +    {

^ You can drop this. `visit_string` and `visit_borrowed_str` by default
forward to `visit_str`.

> +        v.try_into().map_err(E::custom)
> +    }
> +}
> +
> +#[cfg(test)]
> +mod tests {
> +    use super::*;
> +
> +    use serde_plain;
> +
> +    #[test]
> +    fn test_construct() {
> +        let fixed_string = FixedString::new("").expect("empty string is valid");
> +        assert_eq!("", fixed_string);
> +
> +        let fixed_string = FixedString::new("a").expect("valid string");
> +        assert_eq!("a", fixed_string);
> +
> +        let fixed_string = FixedString::new("🌏🌏🌏🌏🌏").expect("valid string");
> +        assert_eq!("🌏🌏🌏🌏🌏", fixed_string);
> +
> +        let fixed_string =
> +            FixedString::new("aaaaaaaaaaaaaaaaaaaaaaa").expect("23 characters are allowed");
> +        assert_eq!("aaaaaaaaaaaaaaaaaaaaaaa", fixed_string);
> +
> +        FixedString::new("🌏🌏🌏🌏🌏🌏").expect_err("string too long");
> +        FixedString::new("aaaaaaaaaaaaaaaaaaaaaaaa").expect_err("string too long");
> +    }
> +
> +    #[test]
> +    fn test_serialize_deserialize() {
> +        let valid_string = "aaaaaaaaaaaaaaaaaaaaaaa";
> +
> +        let fixed_string: FixedString =
> +            serde_plain::from_str("aaaaaaaaaaaaaaaaaaaaaaa").expect("deserialization works");
> +        assert_eq!(valid_string, fixed_string);
> +
> +        let serialized_string =
> +            serde_plain::to_string(&fixed_string).expect("can be serialized into a string");
> +        assert_eq!(valid_string, serialized_string);
> +
> +        serde_plain::from_str::<FixedString>("aaaaaaaaaaaaaaaaaaaaaaaa")
> +            .expect_err("cannot deserialize string that is too long");
> +    }
> +
> +    #[test]
> +    fn test_ord() {
> +        let fixed_string = FixedString::new("abc").expect("valid string");
> +
> +        assert!(fixed_string == fixed_string);
> +        assert!(fixed_string >= fixed_string);
> +        assert!(fixed_string <= fixed_string);
> +
> +        assert!("ab" < fixed_string);
> +        assert!("abc" == fixed_string);
> +        assert!("abcd" > fixed_string);
> +
> +        let larger_fixed_string = FixedString::new("abcde").expect("valid string");
> +
> +        assert!(larger_fixed_string > fixed_string);
> +        assert!(fixed_string < larger_fixed_string);
> +    }
> +
> +    #[test]
> +    fn test_truncate() {
> +        let fixed_string = FixedString::new_truncated("aaaaaaaaaaaaaaaaaaaaaaaaaaaa");
> +        assert_eq!("aaaaaaaaaaaaaaaaaaaaaaa", fixed_string);
> +
> +        let fixed_string = FixedString::new_truncated("aaaaaaaaaaaaaaaaaa💯");
> +        assert_eq!("aaaaaaaaaaaaaaaaaa💯", fixed_string);
> +
> +        // '💯' is 4 bytes long
> +        // first byte of '💯' is inside the 23 byte limit, but it's not a valid utf-8 boundary so
> +        // it should get truncated
> +        let fixed_string = FixedString::new_truncated("aaaaaaaaaaaaaaaaaaaaa💯");
> +        assert_eq!("aaaaaaaaaaaaaaaaaaaaa", fixed_string);
> +
> +        let fixed_string = FixedString::new_truncated("aaaaaaaaaaaaaaaaaaaaaa💯");
> +        assert_eq!("aaaaaaaaaaaaaaaaaaaaaa", fixed_string);
> +
> +        let fixed_string = FixedString::new_truncated("aaaaaaaaaaaaaaaaaaaaaaa💯");
> +        assert_eq!("aaaaaaaaaaaaaaaaaaaaaaa", fixed_string);
> +
> +        // '🌏' is 4 bytes long
> +        let fixed_string = FixedString::new_truncated("🌏🌏🌏🌏🌏");
> +        assert_eq!("🌏🌏🌏🌏🌏", fixed_string);
> +
> +        let fixed_string = FixedString::new_truncated("🌏🌏🌏🌏🌏🌏");
> +        assert_eq!("🌏🌏🌏🌏🌏", fixed_string);
> +    }
> +}
> diff --git a/pve-api-types/src/types/mod.rs b/pve-api-types/src/types/mod.rs
> index fe52a169..9653dd46 100644
> --- a/pve-api-types/src/types/mod.rs
> +++ b/pve-api-types/src/types/mod.rs
> @@ -17,6 +17,9 @@ pub mod array;
>  pub mod stringlist;
>  pub mod verifiers;
>  
> +mod fixed_string;
> +pub use fixed_string::{FixedString, FixedStringError};
> +
>  include!("../generated/types.rs");
>  
>  /// A PVE Upid, contrary to a PBS Upid, contains no 'task-id' number.
> -- 
> 2.47.3


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

Re: [pdm-devel] [PATCH proxmox 1/3] pve-api-types: add FixedString type

[Stefan Hanreich]

thanks for the review! Incorporated all your suggestions, waiting with
the v2 til tomorrow, in case something else comes up.

On 11/12/25 11:49 AM, Wolfgang Bumiller wrote:
> On Wed, Nov 12, 2025 at 10:22:05AM +0100, Stefan Hanreich wrote:
>> A custom, immutable, string type, that is copiable, which can hold up
>> to 23 characters. It will be used later for storing the name of
>> unknown enum variants when parsing the return value of enum
>> properties. The main reason for introducing this type is that its copy
>> as opposed to String, which lets us keep the Copy implementation for
>> the existing enums.
>>
>> Main reason for choosing 23 characters is the fact that String is 24
>> bytes large as well (ptr, len, capacity).
>>
>> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
>> ---
>>  pve-api-types/src/types/fixed_string.rs | 331 ++++++++++++++++++++++++
>>  pve-api-types/src/types/mod.rs          |   3 +
>>  2 files changed, 334 insertions(+)
>>  create mode 100644 pve-api-types/src/types/fixed_string.rs
>>
>> diff --git a/pve-api-types/src/types/fixed_string.rs b/pve-api-types/src/types/fixed_string.rs
>> new file mode 100644
>> index 00000000..f1692227
>> --- /dev/null
>> +++ b/pve-api-types/src/types/fixed_string.rs
>> @@ -0,0 +1,331 @@
>> +use std::cmp::Ordering;
>> +use std::error::Error;
>> +use std::fmt::{Debug, Display, Formatter};
>> +use std::ops::Deref;
>> +use std::str::FromStr;
>> +
>> +use serde::{Deserialize, Serialize};
>> +
>> +/// Error type used by constructors of [`FixedString`]
>> +#[derive(Clone, Copy, Debug)]
>> +pub enum FixedStringError {
>> +    TooLong,
>> +}
> 
> IMO this does not need to be an enum. `pub struct TooLongError` should
> be enough.

also thought about it, wanted it to be potentially extendable without
too much hassle - but works for me as well. Additional error variants
are unlikely anyway..

>> +
>> +impl Error for FixedStringError {}
>> +
>> +impl Display for FixedStringError {
>> +    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
> 
> ^ you're importing `Formatter`.
> And on that note: personally I prefer to import `std::fmt` and prefix
> its items with `fmt::*`.
> 
>> +        f.write_str(match self {
>> +            Self::TooLong => "string is longer than 23 characters",
>> +        })
>> +    }
>> +}
>> +
>> +/// An immutable string type with a maximum size of 23 bytes.
>> +///
>> +/// After construction it is guaranteed that its contents are:
>> +/// * valid utf-8
>> +/// * not longer than 23 characters
>> +///
>> +/// FixedString is immutable, therefore it is sufficient to validate the invariants only at
>> +/// construction time to guarantee that they will always hold during the lifecycle of the
>> +/// struct.
>> +#[derive(Clone, Copy)]
>> +pub struct FixedString {
>> +    buf: [u8; 23],
>> +    len: u8,
>> +}
>> +
>> +impl FixedString {
>> +    /// Creates a new FixedString instance from a str reference.
>> +    ///
>> +    /// # Errors
>> +    /// This function will return an error if:
>> +    /// * The passed string is longer than 23 bytes
>> +    pub fn new(value: &str) -> Result<Self, FixedStringError> {
>> +        if value.len() > 23 {
>> +            return Err(FixedStringError::TooLong);
>> +        }
>> +
>> +        let mut buf = [0; 23];
>> +        buf[..value.len()].copy_from_slice(value.as_bytes());
>> +
>> +        Ok(Self {
>> +            buf,
>> +            // SAFETY: self.len is at least 0 and at most 23, which fits into u8
>> +            len: value.len() as u8,
>> +        })
>> +    }
>> +
>> +    /// Creates a new FixedString instance from a str reference, truncating if required
>> +    ///
>> +    /// It works by copying the bytes from the supplied value into its own local buffer. If the
>> +    /// string is longer than 23 bytes, then this methods will truncate the string at the nearest
>> +    /// utf-8 boundary, so even if a string longer than 23 characters is passed in, it is possible
>> +    /// that it gets truncated to a length less than 23 if the utf-8 boundary doesn't perfectly
>> +    /// align with the 23 bytes limit.
>> +    pub fn new_truncated(value: &str) -> Self {
> 
> I don't think we should keep this unless we also have a way of checking
> for this.
> In case we do want to use this: since values up to 31 (a potential
> still-cheap-enough extension to this `FixedString`'s length (since it
> would then just be 4 pointers in size)) only require 5 bits, we have 3
> bits remaining in the length byte which could be used to signal that
> this was a truncated string, which could be returned via a
> `.is_truncated()` method, and we could refuse to serialize such a string
> without calling some extra method first (and include some marker in the
> Debug impl)?
> 
> But IMO: for now, we just don't need it.
> 

dropped it now then

>> +        // TODO: replace, once [`str::floor_char_boundary`] has been stabilized
>> +        let len = if value.len() > 23 {
>> +            let mut index = 23;
>> +
>> +            while index > 0 && !value.is_char_boundary(index) {
>> +                index = index - 1;
> 
> index -= 1; ;-)
> 
> or:
>     panic!("Who let Dominik add emojis to enums?")
> 
>> +            }
>> +
>> +            index
>> +        } else {
>> +            value.len()
>> +        };
> 
> ^ Actually the above could just be shortened to
> 
> let mut len = value.len();
> if len > 23 {
>     len = 23;
>     while ... { len -= 1 }
> }
> 
>> +
>> +        let mut buf = [0; 23];
>> +        buf[..len].copy_from_slice(&value.as_bytes()[..len]);
>> +
>> +        Self {
>> +            buf,
>> +            // SAFETY: self.len is at least 0 and at most 23, which fits into u8
>> +            len: len as u8,
>> +        }
>> +    }
>> +
>> +    /// Returns a str reference to the stored data
>> +    #[inline]
>> +    pub fn as_str(&self) -> &str {
>> +        // SAFETY: self.buf must be a valid utf-8 string by construction
>> +        unsafe { str::from_utf8_unchecked(self.as_bytes()) }
>> +    }
>> +
>> +    /// Returns a reference to the set bytes in the stored buffer
>> +    #[inline]
>> +    pub fn as_bytes(&self) -> &[u8] {
>> +        // SAFETY: self.len >= 0 and self.len <= 23 by construction
>> +        unsafe { self.buf.get_unchecked(..self.len as usize) }
>> +    }
>> +}
>> +
>> +impl PartialEq for FixedString {
> 
> IMO - regarding ordering where it matters - it would be easier to read
> such impls when they're purely forwarded, eg.:
> 
>     PartialEq::eq(self.as_str(), other)
> 
> since then all implementations look the same (no need for `Some()`
> wrapping in the `PartialOrd` impl, for instance.

yeah, that's fair - idk why I didn't do it in the first place...

> Also, it could be generated with a simple forwarding macro which would
> IMO also make review easier than having to go through each of them
> individually:
> 
>     macro_rules! forward_impl_to_bytes {
>         ($($trait:ident {$fn:ident -> $out:ty })+) => {
>             $(
>                 impl $trait for FixedString {
>                     fn $fn(&self, other: &Self) -> $out {
>                         <[u8] as $trait>::$fn(self.as_bytes(), other.as_bytes())
>                     }
>                 }
>             )+
>         };
>     }
> 
>     forward_impl_to_bytes! {
>         PartialEq { eq -> bool }
>         PartialOrd { partial_cmp -> Option<Ordering> }
>         Ord { cmp -> Ordering }
>     }
> 
> 
> And for the ones below:
> 
>     macro_rules! forward_impl_to_str_bidir {
>         ($($trait:ident {$fn:ident -> $out:ty })+) => {
>             $(
>                 impl $trait<str> for FixedString {
>                     fn $fn(&self, other: &str) -> $out {
>                         <str as $trait>::$fn(self.as_str(), other)
>                     }
>                 }
> 
>                 impl $trait<FixedString> for &str {
>                     fn $fn(&self, other: &FixedString) -> $out {
>                         <str as $trait>::$fn(self, other.as_str())
>                     }
>                 }
>             )+
>         };
>     }
> 
>     forward_impl_to_str_bidir! {
>         PartialEq { eq -> bool }
>         PartialOrd { partial_cmp -> Option<Ordering> }
>     }
> 
> Yes I realize that for 2 traits this doesn't save much, but it would
> make the intention clearer and IMO is still easier to review.

still, this makes adding additional traits a breeze and I'm sure we
might need to do that at some point. I kept the inline for both cases
though - what do you think? makes sense imo.

>> +    #[inline]
>> +    fn eq(&self, other: &FixedString) -> bool {
>> +        self.as_bytes() == other.as_bytes()
>> +    }
>> +}
>> +
>> +impl Eq for FixedString {}
>> +
>> +impl PartialOrd for FixedString {
>> +    fn partial_cmp(&self, other: &FixedString) -> Option<Ordering> {
>> +        Some(self.cmp(other))
>> +    }
>> +}
>> +
>> +impl Ord for FixedString {
>> +    #[inline]
>> +    fn cmp(&self, other: &FixedString) -> Ordering {
>> +        self.as_bytes().cmp(other.as_bytes())
>> +    }
>> +}
>> +
>> +impl Display for FixedString {
>> +    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
>> +        f.write_str(self.as_str())
> 
> Consider forwarding to &str's fmt instead so that things such as
> alignment etc. work:
> 
>     Display::fmt(self.as_str(), f)
> 
> so that `println!("{the_enum:>8}")` will print "     foo" instead of
> just "foo", and `{x:0^8}` produces "00foo000" ;-)
> 

done

>> +    }
>> +}
>> +
>> +impl Debug for FixedString {
>> +    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
>> +        f.write_str(self.as_str())
> 
> Same here with `Debug::fmt(self.as_str(), f)`.
> 

done

>> +    }
>> +}
>> +
>> +impl Deref for FixedString {
>> +    type Target = str;
>> +
>> +    fn deref(&self) -> &str {
>> +        self.as_str()
>> +    }
>> +}
>> +
>> +impl AsRef<str> for FixedString {
>> +    fn as_ref(&self) -> &str {
>> +        self.as_str()
>> +    }
>> +}
> 
> I'd also add `AsRef<[u8]>`, and `Borrow<str>`. (The latter would be
> required to allow a `HashMap<FixedString, T>` to use `.get("key")` with
> `&str` for a key - not that that's something we'd expect to see...
> 

done

>> +
>> +impl TryFrom<String> for FixedString {
>> +    type Error = FixedStringError;
>> +
>> +    fn try_from(value: String) -> Result<Self, Self::Error> {
>> +        FixedString::new(value.as_str())
>> +    }
>> +}
>> +
>> +impl FromStr for FixedString {
>> +    type Err = FixedStringError;
>> +
>> +    fn from_str(value: &str) -> Result<Self, Self::Err> {
>> +        FixedString::new(value)
>> +    }
>> +}
>> +
>> +impl TryFrom<&str> for FixedString {
>> +    type Error = FixedStringError;
>> +
>> +    fn try_from(value: &str) -> Result<Self, Self::Error> {
>> +        FixedString::new(value)
>> +    }
>> +}
>> +
>> +impl PartialEq<str> for FixedString {
>> +    fn eq(&self, other: &str) -> bool {
>> +        self.as_str().eq(other)
>> +    }
>> +}
>> +
>> +impl PartialEq<FixedString> for &str {
>> +    fn eq(&self, other: &FixedString) -> bool {
>> +        self.eq(&other.as_str())
>> +    }
>> +}
>> +
>> +impl PartialOrd<str> for FixedString {
>> +    fn partial_cmp(&self, other: &str) -> Option<Ordering> {
>> +        Some(self.as_str().cmp(&other))
>> +    }
>> +}
>> +
>> +impl PartialOrd<FixedString> for &str {
>> +    fn partial_cmp(&self, other: &FixedString) -> Option<Ordering> {
>> +        Some(self.cmp(&other.as_str()))
>> +    }
>> +}
>> +
>> +impl Serialize for FixedString {
>> +    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
>> +    where
>> +        S: serde::Serializer,
>> +    {
>> +        serializer.serialize_str(self.as_str())
>> +    }
>> +}
>> +
>> +impl<'de> Deserialize<'de> for FixedString {
>> +    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
>> +    where
>> +        D: serde::Deserializer<'de>,
>> +    {
>> +        deserializer.deserialize_str(FixedStringVisitor)
>> +    }
>> +}
>> +
>> +struct FixedStringVisitor;
> 
> ^ I'd prefer to move the type and Visitor impl into the `fn deserialize`.
> 

done

>> +
>> +impl<'de> serde::de::Visitor<'de> for FixedStringVisitor {
>> +    type Value = FixedString;
>> +
>> +    fn expecting(&self, f: &mut Formatter) -> std::fmt::Result {
>> +        f.write_str("a string that is at most 23 bytes long")
>> +    }
>> +
>> +    fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
>> +    where
>> +        E: serde::de::Error,
>> +    {
>> +        v.try_into().map_err(E::custom)
>> +    }
>> +
>> +    fn visit_string<E>(self, v: String) -> Result<Self::Value, E>
>> +    where
>> +        E: serde::de::Error,
>> +    {
> 
> ^ You can drop this. `visit_string` and `visit_borrowed_str` by default
> forward to `visit_str`.
> 

done

>> +        v.try_into().map_err(E::custom)
>> +    }
>> +}
>> +
>> +#[cfg(test)]
>> +mod tests {
>> +    use super::*;
>> +
>> +    use serde_plain;
>> +
>> +    #[test]
>> +    fn test_construct() {
>> +        let fixed_string = FixedString::new("").expect("empty string is valid");
>> +        assert_eq!("", fixed_string);
>> +
>> +        let fixed_string = FixedString::new("a").expect("valid string");
>> +        assert_eq!("a", fixed_string);
>> +
>> +        let fixed_string = FixedString::new("🌏🌏🌏🌏🌏").expect("valid string");
>> +        assert_eq!("🌏🌏🌏🌏🌏", fixed_string);
>> +
>> +        let fixed_string =
>> +            FixedString::new("aaaaaaaaaaaaaaaaaaaaaaa").expect("23 characters are allowed");
>> +        assert_eq!("aaaaaaaaaaaaaaaaaaaaaaa", fixed_string);
>> +
>> +        FixedString::new("🌏🌏🌏🌏🌏🌏").expect_err("string too long");
>> +        FixedString::new("aaaaaaaaaaaaaaaaaaaaaaaa").expect_err("string too long");
>> +    }
>> +
>> +    #[test]
>> +    fn test_serialize_deserialize() {
>> +        let valid_string = "aaaaaaaaaaaaaaaaaaaaaaa";
>> +
>> +        let fixed_string: FixedString =
>> +            serde_plain::from_str("aaaaaaaaaaaaaaaaaaaaaaa").expect("deserialization works");
>> +        assert_eq!(valid_string, fixed_string);
>> +
>> +        let serialized_string =
>> +            serde_plain::to_string(&fixed_string).expect("can be serialized into a string");
>> +        assert_eq!(valid_string, serialized_string);
>> +
>> +        serde_plain::from_str::<FixedString>("aaaaaaaaaaaaaaaaaaaaaaaa")
>> +            .expect_err("cannot deserialize string that is too long");
>> +    }
>> +
>> +    #[test]
>> +    fn test_ord() {
>> +        let fixed_string = FixedString::new("abc").expect("valid string");
>> +
>> +        assert!(fixed_string == fixed_string);
>> +        assert!(fixed_string >= fixed_string);
>> +        assert!(fixed_string <= fixed_string);
>> +
>> +        assert!("ab" < fixed_string);
>> +        assert!("abc" == fixed_string);
>> +        assert!("abcd" > fixed_string);
>> +
>> +        let larger_fixed_string = FixedString::new("abcde").expect("valid string");
>> +
>> +        assert!(larger_fixed_string > fixed_string);
>> +        assert!(fixed_string < larger_fixed_string);
>> +    }
>> +
>> +    #[test]
>> +    fn test_truncate() {
>> +        let fixed_string = FixedString::new_truncated("aaaaaaaaaaaaaaaaaaaaaaaaaaaa");
>> +        assert_eq!("aaaaaaaaaaaaaaaaaaaaaaa", fixed_string);
>> +
>> +        let fixed_string = FixedString::new_truncated("aaaaaaaaaaaaaaaaaa💯");
>> +        assert_eq!("aaaaaaaaaaaaaaaaaa💯", fixed_string);
>> +
>> +        // '💯' is 4 bytes long
>> +        // first byte of '💯' is inside the 23 byte limit, but it's not a valid utf-8 boundary so
>> +        // it should get truncated
>> +        let fixed_string = FixedString::new_truncated("aaaaaaaaaaaaaaaaaaaaa💯");
>> +        assert_eq!("aaaaaaaaaaaaaaaaaaaaa", fixed_string);
>> +
>> +        let fixed_string = FixedString::new_truncated("aaaaaaaaaaaaaaaaaaaaaa💯");
>> +        assert_eq!("aaaaaaaaaaaaaaaaaaaaaa", fixed_string);
>> +
>> +        let fixed_string = FixedString::new_truncated("aaaaaaaaaaaaaaaaaaaaaaa💯");
>> +        assert_eq!("aaaaaaaaaaaaaaaaaaaaaaa", fixed_string);
>> +
>> +        // '🌏' is 4 bytes long
>> +        let fixed_string = FixedString::new_truncated("🌏🌏🌏🌏🌏");
>> +        assert_eq!("🌏🌏🌏🌏🌏", fixed_string);
>> +
>> +        let fixed_string = FixedString::new_truncated("🌏🌏🌏🌏🌏🌏");
>> +        assert_eq!("🌏🌏🌏🌏🌏", fixed_string);
>> +    }
>> +}
>> diff --git a/pve-api-types/src/types/mod.rs b/pve-api-types/src/types/mod.rs
>> index fe52a169..9653dd46 100644
>> --- a/pve-api-types/src/types/mod.rs
>> +++ b/pve-api-types/src/types/mod.rs
>> @@ -17,6 +17,9 @@ pub mod array;
>>  pub mod stringlist;
>>  pub mod verifiers;
>>  
>> +mod fixed_string;
>> +pub use fixed_string::{FixedString, FixedStringError};
>> +
>>  include!("../generated/types.rs");
>>  
>>  /// A PVE Upid, contrary to a PBS Upid, contains no 'task-id' number.
>> -- 
>> 2.47.3



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

[pdm-devel] [PATCH proxmox 2/3] pve-api-types: generate fallback variant for enums

[Stefan Hanreich]

Generate a new variant, UnknownEnumValue, which is used as a fallback
in case the enum variant is unknown in PDM. This ensures
forwards-compatibility with newer Proxmox VE versions, in case they
introduce a new variant to an enum string type. Otherwise,
deserializing return values from the Proxmox VE API fails and can
cause severe issues in the PDM backend / frontend.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 pve-api-types/generator-lib/Schema2Rust.pm | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/pve-api-types/generator-lib/Schema2Rust.pm b/pve-api-types/generator-lib/Schema2Rust.pm
index f54b3683..ab1c1700 100644
--- a/pve-api-types/generator-lib/Schema2Rust.pm
+++ b/pve-api-types/generator-lib/Schema2Rust.pm
@@ -295,6 +295,9 @@ sub print_types : prototype($) {
                 print {$out} "    /// $orig.\n";
                 print {$out} "    $named,\n";
             };
+            print {$out} "    /// Unknown variants for forward compatibility.\n";
+            print {$out} "    #[serde(untagged)]\n";
+            print {$out} "    UnknownEnumValue(FixedString)\n";
             print {$out} "}\n";
             print {$out} "serde_plain::derive_display_from_serialize!($def->{name});\n";
             print {$out} "serde_plain::derive_fromstr_from_deserialize!($def->{name});\n";
-- 
2.47.3


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

[pdm-devel] [PATCH proxmox 3/3] pve-api-types: regenerate

[Stefan Hanreich]

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 pve-api-types/src/generated/types.rs | 207 +++++++++++++++++++++++++++
 1 file changed, 207 insertions(+)

diff --git a/pve-api-types/src/generated/types.rs b/pve-api-types/src/generated/types.rs
index 6c42b620..a0480a2b 100644
--- a/pve-api-types/src/generated/types.rs
+++ b/pve-api-types/src/generated/types.rs
@@ -101,6 +101,9 @@ pub enum AptUpdateInfoArch {
     #[serde(rename = "s390x")]
     /// s390x.
     S390x,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(AptUpdateInfoArch);
 serde_plain::derive_fromstr_from_deserialize!(AptUpdateInfoArch);
@@ -444,6 +447,9 @@ pub enum ClusterMetricsDataType {
     #[serde(rename = "derive")]
     /// derive.
     Derive,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(ClusterMetricsDataType);
 serde_plain::derive_fromstr_from_deserialize!(ClusterMetricsDataType);
@@ -549,6 +555,9 @@ pub enum ClusterNodeIndexResponseStatus {
     #[serde(rename = "offline")]
     /// offline.
     Offline,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(ClusterNodeIndexResponseStatus);
 serde_plain::derive_fromstr_from_deserialize!(ClusterNodeIndexResponseStatus);
@@ -663,6 +672,9 @@ pub enum ClusterNodeStatusType {
     #[serde(rename = "node")]
     /// node.
     Node,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(ClusterNodeStatusType);
 serde_plain::derive_fromstr_from_deserialize!(ClusterNodeStatusType);
@@ -970,6 +982,9 @@ pub enum ClusterResourceKind {
     #[serde(rename = "sdn")]
     /// sdn.
     Sdn,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(ClusterResourceKind);
 serde_plain::derive_fromstr_from_deserialize!(ClusterResourceKind);
@@ -999,6 +1014,9 @@ pub enum ClusterResourceType {
     #[serde(rename = "sdn")]
     /// sdn.
     Sdn,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(ClusterResourceType);
 serde_plain::derive_fromstr_from_deserialize!(ClusterResourceType);
@@ -1655,6 +1673,9 @@ pub enum IsRunning {
     #[serde(rename = "stopped")]
     /// stopped.
     Stopped,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(IsRunning);
 serde_plain::derive_fromstr_from_deserialize!(IsRunning);
@@ -1675,6 +1696,9 @@ pub enum ListControllersType {
     #[serde(rename = "isis")]
     /// isis.
     Isis,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(ListControllersType);
 serde_plain::derive_fromstr_from_deserialize!(ListControllersType);
@@ -1725,6 +1749,9 @@ pub enum ListNetworksType {
     #[serde(rename = "include_sdn")]
     /// include_sdn.
     IncludeSdn,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(ListNetworksType);
 serde_plain::derive_fromstr_from_deserialize!(ListNetworksType);
@@ -1776,6 +1803,9 @@ pub enum ListRealmTfa {
     #[serde(rename = "oath")]
     /// oath.
     Oath,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(ListRealmTfa);
 serde_plain::derive_fromstr_from_deserialize!(ListRealmTfa);
@@ -1986,6 +2016,9 @@ pub enum ListTasksSource {
     #[serde(rename = "all")]
     /// all.
     All,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(ListTasksSource);
 serde_plain::derive_fromstr_from_deserialize!(ListTasksSource);
@@ -2012,6 +2045,9 @@ pub enum ListZonesType {
     #[serde(rename = "vxlan")]
     /// vxlan.
     Vxlan,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(ListZonesType);
 serde_plain::derive_fromstr_from_deserialize!(ListZonesType);
@@ -2414,6 +2450,9 @@ pub enum LxcConfigArch {
     #[serde(rename = "riscv64")]
     /// riscv64.
     Riscv64,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(LxcConfigArch);
 serde_plain::derive_fromstr_from_deserialize!(LxcConfigArch);
@@ -2435,6 +2474,9 @@ pub enum LxcConfigCmode {
     #[default]
     /// tty.
     Tty,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(LxcConfigCmode);
 serde_plain::derive_fromstr_from_deserialize!(LxcConfigCmode);
@@ -2607,6 +2649,9 @@ pub enum LxcConfigLock {
     #[serde(rename = "snapshot-delete")]
     /// snapshot-delete.
     SnapshotDelete,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(LxcConfigLock);
 serde_plain::derive_fromstr_from_deserialize!(LxcConfigLock);
@@ -2868,6 +2913,9 @@ pub enum LxcConfigNetType {
     #[serde(rename = "veth")]
     /// veth.
     Veth,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(LxcConfigNetType);
 serde_plain::derive_fromstr_from_deserialize!(LxcConfigNetType);
@@ -2912,6 +2960,9 @@ pub enum LxcConfigOstype {
     #[serde(rename = "unmanaged")]
     /// unmanaged.
     Unmanaged,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(LxcConfigOstype);
 serde_plain::derive_fromstr_from_deserialize!(LxcConfigOstype);
@@ -4106,6 +4157,9 @@ pub enum NetworkInterfaceBondMode {
     #[serde(rename = "lacp-balance-tcp")]
     /// lacp-balance-tcp.
     LacpBalanceTcp,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(NetworkInterfaceBondMode);
 serde_plain::derive_fromstr_from_deserialize!(NetworkInterfaceBondMode);
@@ -4124,6 +4178,9 @@ pub enum NetworkInterfaceBondXmitHashPolicy {
     #[serde(rename = "layer3+4")]
     /// layer3+4.
     Layer3_4,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(NetworkInterfaceBondXmitHashPolicy);
 serde_plain::derive_fromstr_from_deserialize!(NetworkInterfaceBondXmitHashPolicy);
@@ -4138,6 +4195,9 @@ pub enum NetworkInterfaceFamilies {
     #[serde(rename = "inet6")]
     /// inet6.
     Inet6,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(NetworkInterfaceFamilies);
 serde_plain::derive_fromstr_from_deserialize!(NetworkInterfaceFamilies);
@@ -4161,6 +4221,9 @@ pub enum NetworkInterfaceMethod {
     #[serde(rename = "auto")]
     /// auto.
     Auto,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(NetworkInterfaceMethod);
 serde_plain::derive_fromstr_from_deserialize!(NetworkInterfaceMethod);
@@ -4205,6 +4268,9 @@ pub enum NetworkInterfaceType {
     #[serde(rename = "unknown")]
     /// unknown.
     Unknown,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(NetworkInterfaceType);
 serde_plain::derive_fromstr_from_deserialize!(NetworkInterfaceType);
@@ -4219,6 +4285,9 @@ pub enum NetworkInterfaceVlanProtocol {
     #[serde(rename = "802.1q")]
     /// 802.1q.
     Ieee802_1q,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(NetworkInterfaceVlanProtocol);
 serde_plain::derive_fromstr_from_deserialize!(NetworkInterfaceVlanProtocol);
@@ -4262,6 +4331,9 @@ pub enum NodeShellTermproxyCmd {
     #[serde(rename = "upgrade")]
     /// upgrade.
     Upgrade,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(NodeShellTermproxyCmd);
 serde_plain::derive_fromstr_from_deserialize!(NodeShellTermproxyCmd);
@@ -4392,6 +4464,9 @@ pub enum NodeStatusBootInfoMode {
     #[serde(rename = "legacy-bios")]
     /// legacy-bios.
     LegacyBios,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(NodeStatusBootInfoMode);
 serde_plain::derive_fromstr_from_deserialize!(NodeStatusBootInfoMode);
@@ -4662,6 +4737,9 @@ pub enum NodeSubscriptionInfoStatus {
     #[serde(rename = "suspended")]
     /// suspended.
     Suspended,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(NodeSubscriptionInfoStatus);
 serde_plain::derive_fromstr_from_deserialize!(NodeSubscriptionInfoStatus);
@@ -4774,6 +4852,9 @@ pub enum PveQemuSevFmtType {
     #[serde(rename = "snp")]
     /// snp.
     Snp,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(PveQemuSevFmtType);
 serde_plain::derive_fromstr_from_deserialize!(PveQemuSevFmtType);
@@ -5364,6 +5445,9 @@ pub enum PveQmIdeAio {
     #[serde(rename = "io_uring")]
     /// io_uring.
     IoUring,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(PveQmIdeAio);
 serde_plain::derive_fromstr_from_deserialize!(PveQmIdeAio);
@@ -5387,6 +5471,9 @@ pub enum PveQmIdeCache {
     #[serde(rename = "directsync")]
     /// directsync.
     Directsync,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(PveQmIdeCache);
 serde_plain::derive_fromstr_from_deserialize!(PveQmIdeCache);
@@ -5401,6 +5488,9 @@ pub enum PveQmIdeDiscard {
     #[serde(rename = "on")]
     /// on.
     On,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(PveQmIdeDiscard);
 serde_plain::derive_fromstr_from_deserialize!(PveQmIdeDiscard);
@@ -5427,6 +5517,9 @@ pub enum PveQmIdeFormat {
     #[serde(rename = "cloop")]
     /// cloop.
     Cloop,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(PveQmIdeFormat);
 serde_plain::derive_fromstr_from_deserialize!(PveQmIdeFormat);
@@ -5442,6 +5535,9 @@ pub enum PveQmIdeMedia {
     #[default]
     /// disk.
     Disk,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(PveQmIdeMedia);
 serde_plain::derive_fromstr_from_deserialize!(PveQmIdeMedia);
@@ -5459,6 +5555,9 @@ pub enum PveQmIdeRerror {
     #[serde(rename = "stop")]
     /// stop.
     Stop,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(PveQmIdeRerror);
 serde_plain::derive_fromstr_from_deserialize!(PveQmIdeRerror);
@@ -5479,6 +5578,9 @@ pub enum PveQmIdeWerror {
     #[serde(rename = "stop")]
     /// stop.
     Stop,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(PveQmIdeWerror);
 serde_plain::derive_fromstr_from_deserialize!(PveQmIdeWerror);
@@ -5583,6 +5685,9 @@ pub enum PveQmRngSource {
     #[serde(rename = "/dev/hwrng")]
     /// /dev/hwrng.
     DevHwrng,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(PveQmRngSource);
 serde_plain::derive_fromstr_from_deserialize!(PveQmRngSource);
@@ -5706,6 +5811,9 @@ pub enum PveQmWatchdogAction {
     #[serde(rename = "none")]
     /// none.
     None,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(PveQmWatchdogAction);
 serde_plain::derive_fromstr_from_deserialize!(PveQmWatchdogAction);
@@ -5721,6 +5829,9 @@ pub enum PveQmWatchdogModel {
     #[serde(rename = "ib700")]
     /// ib700.
     Ib700,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(PveQmWatchdogModel);
 serde_plain::derive_fromstr_from_deserialize!(PveQmWatchdogModel);
@@ -6031,6 +6142,9 @@ pub enum PveVmCpuConfReportedModel {
     #[serde(rename = "Westmere-IBRS")]
     /// Westmere-IBRS.
     WestmereIbrs,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(PveVmCpuConfReportedModel);
 serde_plain::derive_fromstr_from_deserialize!(PveVmCpuConfReportedModel);
@@ -7083,6 +7197,9 @@ pub enum QemuConfigAgentType {
     #[serde(rename = "isa")]
     /// isa.
     Isa,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuConfigAgentType);
 serde_plain::derive_fromstr_from_deserialize!(QemuConfigAgentType);
@@ -7097,6 +7214,9 @@ pub enum QemuConfigArch {
     #[serde(rename = "aarch64")]
     /// aarch64.
     Aarch64,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuConfigArch);
 serde_plain::derive_fromstr_from_deserialize!(QemuConfigArch);
@@ -7134,6 +7254,9 @@ pub enum QemuConfigAudio0Device {
     #[serde(rename = "AC97")]
     /// AC97.
     Ac97,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuConfigAudio0Device);
 serde_plain::derive_fromstr_from_deserialize!(QemuConfigAudio0Device);
@@ -7149,6 +7272,9 @@ pub enum QemuConfigAudio0Driver {
     #[serde(rename = "none")]
     /// none.
     None,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuConfigAudio0Driver);
 serde_plain::derive_fromstr_from_deserialize!(QemuConfigAudio0Driver);
@@ -7164,6 +7290,9 @@ pub enum QemuConfigBios {
     #[serde(rename = "ovmf")]
     /// ovmf.
     Ovmf,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuConfigBios);
 serde_plain::derive_fromstr_from_deserialize!(QemuConfigBios);
@@ -7183,6 +7312,9 @@ pub enum QemuConfigCitype {
     #[serde(rename = "opennebula")]
     /// opennebula.
     Opennebula,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuConfigCitype);
 serde_plain::derive_fromstr_from_deserialize!(QemuConfigCitype);
@@ -7263,6 +7395,9 @@ pub enum QemuConfigEfidisk0Efitype {
     #[serde(rename = "4m")]
     /// 4m.
     Mb4,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuConfigEfidisk0Efitype);
 serde_plain::derive_fromstr_from_deserialize!(QemuConfigEfidisk0Efitype);
@@ -7280,6 +7415,9 @@ pub enum QemuConfigHugepages {
     #[serde(rename = "1024")]
     /// 1024.
     Mb1024,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuConfigHugepages);
 serde_plain::derive_fromstr_from_deserialize!(QemuConfigHugepages);
@@ -7389,6 +7527,9 @@ pub enum QemuConfigKeyboard {
     #[serde(rename = "tr")]
     /// tr.
     Tr,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuConfigKeyboard);
 serde_plain::derive_fromstr_from_deserialize!(QemuConfigKeyboard);
@@ -7424,6 +7565,9 @@ pub enum QemuConfigLock {
     #[serde(rename = "suspended")]
     /// suspended.
     Suspended,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuConfigLock);
 serde_plain::derive_fromstr_from_deserialize!(QemuConfigLock);
@@ -7498,6 +7642,9 @@ pub enum QemuConfigMachineViommu {
     #[serde(rename = "virtio")]
     /// virtio.
     Virtio,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuConfigMachineViommu);
 serde_plain::derive_fromstr_from_deserialize!(QemuConfigMachineViommu);
@@ -7742,6 +7889,9 @@ pub enum QemuConfigNetModel {
     #[serde(rename = "vmxnet3")]
     /// vmxnet3.
     Vmxnet3,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuConfigNetModel);
 serde_plain::derive_fromstr_from_deserialize!(QemuConfigNetModel);
@@ -7793,6 +7943,9 @@ pub enum QemuConfigNumaPolicy {
     #[serde(rename = "interleave")]
     /// interleave.
     Interleave,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuConfigNumaPolicy);
 serde_plain::derive_fromstr_from_deserialize!(QemuConfigNumaPolicy);
@@ -7841,6 +7994,9 @@ pub enum QemuConfigOstype {
     #[serde(rename = "solaris")]
     /// solaris.
     Solaris,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuConfigOstype);
 serde_plain::derive_fromstr_from_deserialize!(QemuConfigOstype);
@@ -8590,6 +8746,9 @@ pub enum QemuConfigScsihw {
     #[serde(rename = "pvscsi")]
     /// pvscsi.
     Pvscsi,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuConfigScsihw);
 serde_plain::derive_fromstr_from_deserialize!(QemuConfigScsihw);
@@ -8633,6 +8792,9 @@ pub enum QemuConfigSpiceEnhancementsVideostreaming {
     #[serde(rename = "filter")]
     /// filter.
     Filter,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuConfigSpiceEnhancementsVideostreaming);
 serde_plain::derive_fromstr_from_deserialize!(QemuConfigSpiceEnhancementsVideostreaming);
@@ -8692,6 +8854,9 @@ pub enum QemuConfigTpmstate0Version {
     #[serde(rename = "v2.0")]
     /// v2.0.
     V2_0,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuConfigTpmstate0Version);
 serde_plain::derive_fromstr_from_deserialize!(QemuConfigTpmstate0Version);
@@ -8820,6 +8985,9 @@ pub enum QemuConfigVgaClipboard {
     #[serde(rename = "vnc")]
     /// vnc.
     Vnc,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuConfigVgaClipboard);
 serde_plain::derive_fromstr_from_deserialize!(QemuConfigVgaClipboard);
@@ -8871,6 +9039,9 @@ pub enum QemuConfigVgaType {
     #[serde(rename = "vmware")]
     /// vmware.
     Vmware,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuConfigVgaType);
 serde_plain::derive_fromstr_from_deserialize!(QemuConfigVgaType);
@@ -9291,6 +9462,9 @@ pub enum QemuConfigVirtiofsCache {
     #[serde(rename = "never")]
     /// never.
     Never,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuConfigVirtiofsCache);
 serde_plain::derive_fromstr_from_deserialize!(QemuConfigVirtiofsCache);
@@ -9488,6 +9662,9 @@ pub enum QemuMigratePreconditionsNotAllowedNodesBlockingHaResourcesCause {
     #[serde(rename = "resource-affinity")]
     /// resource-affinity.
     ResourceAffinity,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(
     QemuMigratePreconditionsNotAllowedNodesBlockingHaResourcesCause
@@ -10547,6 +10724,9 @@ pub enum QemuMoveDiskDisk {
     #[serde(rename = "unused255")]
     /// unused255.
     Unused255,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuMoveDiskDisk);
 serde_plain::derive_fromstr_from_deserialize!(QemuMoveDiskDisk);
@@ -10564,6 +10744,9 @@ pub enum QemuMoveDiskFormat {
     #[serde(rename = "vmdk")]
     /// vmdk.
     Vmdk,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuMoveDiskFormat);
 serde_plain::derive_fromstr_from_deserialize!(QemuMoveDiskFormat);
@@ -10789,6 +10972,9 @@ pub enum QemuResizeDisk {
     #[serde(rename = "tpmstate0")]
     /// tpmstate0.
     Tpmstate0,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(QemuResizeDisk);
 serde_plain::derive_fromstr_from_deserialize!(QemuResizeDisk);
@@ -11824,6 +12010,9 @@ pub enum SdnObjectState {
     #[serde(rename = "changed")]
     /// changed.
     Changed,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(SdnObjectState);
 serde_plain::derive_fromstr_from_deserialize!(SdnObjectState);
@@ -11984,6 +12173,9 @@ pub enum SdnVnetType {
     #[serde(rename = "vnet")]
     /// vnet.
     Vnet,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(SdnVnetType);
 serde_plain::derive_fromstr_from_deserialize!(SdnVnetType);
@@ -12258,6 +12450,9 @@ pub enum SdnZoneDhcp {
     #[serde(rename = "dnsmasq")]
     /// dnsmasq.
     Dnsmasq,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(SdnZoneDhcp);
 serde_plain::derive_fromstr_from_deserialize!(SdnZoneDhcp);
@@ -12733,6 +12928,9 @@ pub enum StartQemuMigrationType {
     #[serde(rename = "insecure")]
     /// insecure.
     Insecure,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(StartQemuMigrationType);
 serde_plain::derive_fromstr_from_deserialize!(StartQemuMigrationType);
@@ -12859,6 +13057,9 @@ pub enum StorageContent {
     #[serde(rename = "vztmpl")]
     /// vztmpl.
     Vztmpl,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(StorageContent);
 serde_plain::derive_fromstr_from_deserialize!(StorageContent);
@@ -13021,6 +13222,9 @@ pub enum StorageInfoFormatsDefault {
     #[serde(rename = "vmdk")]
     /// vmdk.
     Vmdk,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(StorageInfoFormatsDefault);
 serde_plain::derive_fromstr_from_deserialize!(StorageInfoFormatsDefault);
@@ -16588,6 +16792,9 @@ pub enum VersionResponseConsole {
     #[serde(rename = "xtermjs")]
     /// xtermjs.
     Xtermjs,
+    /// Unknown variants for forward compatibility.
+    #[serde(untagged)]
+    UnknownEnumValue(FixedString),
 }
 serde_plain::derive_display_from_serialize!(VersionResponseConsole);
 serde_plain::derive_fromstr_from_deserialize!(VersionResponseConsole);
-- 
2.47.3


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

[pdm-devel] [PATCH proxmox-yew-comp 1/1] pve: qemu: handle fallback enum variants

[Stefan Hanreich]

pve-api-types introduced a fallback variant for all enums returned by
the Proxmox VE API. Add code for handling those new variants to the
use sites of those enums.

For AMD SEV and BIOS it is sufficient to print the unknown values,
since the Comboboxes prevent the re-submission of unknown values
anyway, due to the force_selection property being set to true.

For the OS type, we cannot uniquely identify if an OS is Windows or
not, since it is possible for an unknown value to be either Windows or
something else. For that case, the respective input field (machine
type), that needs this information to render content, is simply
disabled.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 .../qemu_property/qemu_amd_sev_property.rs    |  1 +
 .../pve/qemu_property/qemu_bios_property.rs   |  1 +
 .../qemu_property/qemu_machine_property.rs    | 20 +++++++++++--------
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/src/form/pve/qemu_property/qemu_amd_sev_property.rs b/src/form/pve/qemu_property/qemu_amd_sev_property.rs
index 72d2dfd..30be125 100644
--- a/src/form/pve/qemu_property/qemu_amd_sev_property.rs
+++ b/src/form/pve/qemu_property/qemu_amd_sev_property.rs
@@ -118,6 +118,7 @@ pub fn qemu_amd_sev_property(mobile: bool) -> EditableProperty {
                         PveQemuSevFmtType::Std => "AMD SEV",
                         PveQemuSevFmtType::Es => "AMD SEV-ES",
                         PveQemuSevFmtType::Snp => "AMD SEV-SNP",
+                        PveQemuSevFmtType::UnknownEnumValue(value) => &format!("unknown '{value}'"),
                     };
                     format!("{text} ({v})").into()
                 }
diff --git a/src/form/pve/qemu_property/qemu_bios_property.rs b/src/form/pve/qemu_property/qemu_bios_property.rs
index 1ae78b3..4acfda4 100644
--- a/src/form/pve/qemu_property/qemu_bios_property.rs
+++ b/src/form/pve/qemu_property/qemu_bios_property.rs
@@ -59,6 +59,7 @@ pub fn qemu_bios_property(mobile: bool) -> EditableProperty {
                 Ok(bios) => match bios {
                     QemuConfigBios::Seabios => "SeaBIOS".into(),
                     QemuConfigBios::Ovmf => "OVMF (UEFI)".into(),
+                    QemuConfigBios::UnknownEnumValue(value) => format!("unknown '{value}'").into(),
                 },
                 Err(_) => v.into(),
             },
diff --git a/src/form/pve/qemu_property/qemu_machine_property.rs b/src/form/pve/qemu_property/qemu_machine_property.rs
index 31ace00..02c1bdb 100644
--- a/src/form/pve/qemu_property/qemu_machine_property.rs
+++ b/src/form/pve/qemu_property/qemu_machine_property.rs
@@ -15,7 +15,7 @@ use crate::form::pve::QemuMachineVersionSelector;
 use crate::pve_api_types::QemuMachineType;
 use crate::{EditableProperty, PropertyEditorState, RenderPropertyInputPanelFn};
 
-fn ostype_is_windows(ostype: &QemuConfigOstype) -> bool {
+fn ostype_is_windows(ostype: &QemuConfigOstype) -> Option<bool> {
     match ostype {
         QemuConfigOstype::Wxp
         | QemuConfigOstype::W2k
@@ -25,11 +25,12 @@ fn ostype_is_windows(ostype: &QemuConfigOstype) -> bool {
         | QemuConfigOstype::Win7
         | QemuConfigOstype::Win8
         | QemuConfigOstype::Win10
-        | QemuConfigOstype::Win11 => true,
+        | QemuConfigOstype::Win11 => Some(true),
         QemuConfigOstype::L24
         | QemuConfigOstype::L26
         | QemuConfigOstype::Solaris
-        | QemuConfigOstype::Other => false,
+        | QemuConfigOstype::Other => Some(false),
+        QemuConfigOstype::UnknownEnumValue(_) => None,
     }
 }
 
@@ -91,12 +92,14 @@ fn input_panel(mobile: bool) -> RenderPropertyInputPanelFn {
         };
 
         let add_version_selector = |panel: &mut InputPanel, ty| {
-            let disabled = machine_type != ty;
+            let is_windows = ostype_is_windows(&ostype);
+            let disabled = machine_type != ty || is_windows.is_none();
             let name = get_version_prop_name(ty.to_string());
+
             let field = QemuMachineVersionSelector::new(ty)
                 .name(name)
-                .disabled(machine_type != ty)
-                .required(ostype_is_windows(&ostype))
+                .disabled(disabled)
+                .required(is_windows.unwrap_or_default())
                 .submit(false);
             panel.add_field_with_options(
                 FieldPosition::Left,
@@ -181,8 +184,9 @@ pub fn qemu_machine_property(mobile: bool) -> EditableProperty {
                 serde_json::from_value(record["ostype"].clone()).ok();
             let ostype = ostype.unwrap_or(QemuConfigOstype::Other);
             match (v.as_str(), ostype_is_windows(&ostype)) {
-                (None | Some("pc"), true) => "pc-i440fx-5.1".into(),
-                (Some("q35"), true) => "pc-q35-5.1".into(),
+                (_, None) => format!("unknown ostype: {v}").into(),
+                (None | Some("pc"), Some(true)) => "pc-i440fx-5.1".into(),
+                (Some("q35"), Some(true)) => "pc-q35-5.1".into(),
                 (Some(machine), _) => machine.into(),
                 (None, _) => placeholder().into(),
             }
-- 
2.47.3


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

[pdm-devel] [PATCH proxmox-datacenter-manager 1/1] tree-wide: handle new unknown enum variants

[Stefan Hanreich]

pve-api-types now generates a fallback enum variant in order to handle
encountering unknown values for enums more gracefully. Fix all
occurrences of enums that have changed that lead to compiler errors.

In the case of migrations an error is thrown, since it is impossible
to tell if it is okay to proceed migrating a guest with an unknown
state. In other cases simply log / print the unknown value.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 server/src/metric_collection/rrd_task.rs | 4 ++++
 ui/src/pve/utils.rs                      | 1 +
 ui/src/widget/migrate_window.rs          | 6 ++++++
 3 files changed, 11 insertions(+)

diff --git a/server/src/metric_collection/rrd_task.rs b/server/src/metric_collection/rrd_task.rs
index 507d6b2..f48968b 100644
--- a/server/src/metric_collection/rrd_task.rs
+++ b/server/src/metric_collection/rrd_task.rs
@@ -140,6 +140,10 @@ fn store_metric_pve(cache: &RrdCache, remote_name: &str, data_point: &ClusterMet
         ClusterMetricsDataType::Gauge => DataSourceType::Gauge,
         ClusterMetricsDataType::Counter => DataSourceType::Counter,
         ClusterMetricsDataType::Derive => DataSourceType::Derive,
+        ClusterMetricsDataType::UnknownEnumValue(value) => {
+            log::warn!("encountered unknown metric type: {value}");
+            return;
+        }
     };
 
     cache.update_value(
diff --git a/ui/src/pve/utils.rs b/ui/src/pve/utils.rs
index 5923855..ef536da 100644
--- a/ui/src/pve/utils.rs
+++ b/ui/src/pve/utils.rs
@@ -289,5 +289,6 @@ pub(crate) fn render_content_type(ty: &StorageContent) -> String {
         StorageContent::Snippets => tr!("Snippets"),
         StorageContent::Vztmpl => tr!("Container template"),
         StorageContent::None => tr!("None"),
+        StorageContent::UnknownEnumValue(value) => tr!("unknown content type ({0})", value),
     }
 }
diff --git a/ui/src/widget/migrate_window.rs b/ui/src/widget/migrate_window.rs
index cb12a15..5fe7278 100644
--- a/ui/src/widget/migrate_window.rs
+++ b/ui/src/widget/migrate_window.rs
@@ -122,6 +122,9 @@ impl PdmMigrateWindow {
                 match status.status {
                     pdm_client::types::IsRunning::Running => tr!("Online"),
                     pdm_client::types::IsRunning::Stopped => tr!("Offline"),
+                    pdm_client::types::IsRunning::UnknownEnumValue(value) => {
+                        bail!("encountered unknown guest status: {value}")
+                    }
                 }
             }
             crate::pve::GuestType::Lxc => {
@@ -131,6 +134,9 @@ impl PdmMigrateWindow {
                 match status.status {
                     pdm_client::types::IsRunning::Running => tr!("Restart"),
                     pdm_client::types::IsRunning::Stopped => tr!("Offline"),
+                    pdm_client::types::IsRunning::UnknownEnumValue(value) => {
+                        bail!("encountered unknown guest status: {value}")
+                    }
                 }
             }
         };
-- 
2.47.3


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

Re: [pdm-devel] [PATCH proxmox-datacenter-manager 1/1] tree-wide: handle new unknown enum variants

[Lukas Wagner]

Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>

One comment inline.

On Wed Nov 12, 2025 at 10:22 AM CET, Stefan Hanreich wrote:
> pve-api-types now generates a fallback enum variant in order to handle
> encountering unknown values for enums more gracefully. Fix all
> occurrences of enums that have changed that lead to compiler errors.
>
> In the case of migrations an error is thrown, since it is impossible
> to tell if it is okay to proceed migrating a guest with an unknown
> state. In other cases simply log / print the unknown value.
>
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>  server/src/metric_collection/rrd_task.rs | 4 ++++
>  ui/src/pve/utils.rs                      | 1 +
>  ui/src/widget/migrate_window.rs          | 6 ++++++
>  3 files changed, 11 insertions(+)
>
> diff --git a/server/src/metric_collection/rrd_task.rs b/server/src/metric_collection/rrd_task.rs
> index 507d6b2..f48968b 100644
> --- a/server/src/metric_collection/rrd_task.rs
> +++ b/server/src/metric_collection/rrd_task.rs
> @@ -140,6 +140,10 @@ fn store_metric_pve(cache: &RrdCache, remote_name: &str, data_point: &ClusterMet
>          ClusterMetricsDataType::Gauge => DataSourceType::Gauge,
>          ClusterMetricsDataType::Counter => DataSourceType::Counter,
>          ClusterMetricsDataType::Derive => DataSourceType::Derive,
> +        ClusterMetricsDataType::UnknownEnumValue(value) => {
> +            log::warn!("encountered unknown metric type: {value}");
> +            return;
> +        }
>      };
>  

At lunch, we briefly talked about whether it would be possible to store
datapoints with an unknown data type enum value in some form until PDM
is updated and is able to understand the new variant.
If we wanted to do this, we would need to store the raw datapoints,
since we cannot just dump data with an unknown type into proxmox_rrd.

I think just bailing out and logging is fine, since
  - it's pretty unlikely that there will be a new enum variant here
  - keeping the raw datapoints around could mean that we have to store
    large amounts of data for an unknown time period

>      cache.update_value(
> diff --git a/ui/src/pve/utils.rs b/ui/src/pve/utils.rs
> index 5923855..ef536da 100644
> --- a/ui/src/pve/utils.rs
> +++ b/ui/src/pve/utils.rs
> @@ -289,5 +289,6 @@ pub(crate) fn render_content_type(ty: &StorageContent) -> String {
>          StorageContent::Snippets => tr!("Snippets"),
>          StorageContent::Vztmpl => tr!("Container template"),
>          StorageContent::None => tr!("None"),
> +        StorageContent::UnknownEnumValue(value) => tr!("unknown content type ({0})", value),
>      }
>  }



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

[pdm-devel] [PATCH pve-yew-mobile-gui 1/1] tree-wide: handle fallback enum values

[Stefan Hanreich]

pve-api-types introduced a new fallback variant when encountering
unknown enum variants in the Proxmox VE API response.

For the resources overview, simply ignore any unknown resource type
and do nothing, as is alreadu the case with known resource types that
are not yet implemented.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/pages/page_resources.rs   | 4 +++-
 src/pages/page_task_status.rs | 7 +++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/pages/page_resources.rs b/src/pages/page_resources.rs
index f0c1e2d..50b2545 100644
--- a/src/pages/page_resources.rs
+++ b/src/pages/page_resources.rs
@@ -231,7 +231,8 @@ impl PvePageResources {
                     ClusterResourceType::Node => Some(self.create_node_list_item(ctx, item)),
                     ClusterResourceType::Pool
                     | ClusterResourceType::Openvz
-                    | ClusterResourceType::Sdn => {
+                    | ClusterResourceType::Sdn
+                    | ClusterResourceType::UnknownEnumValue(_) => {
                         /* ignore for now  */
                         None
                     }
@@ -343,6 +344,7 @@ fn type_ordering(ty: ClusterResourceType) -> usize {
         ClusterResourceType::Node => 4,
         ClusterResourceType::Pool => 5,
         ClusterResourceType::Sdn => 6,
+        ClusterResourceType::UnknownEnumValue(_) => 1000,
     }
 }
 
diff --git a/src/pages/page_task_status.rs b/src/pages/page_task_status.rs
index 4424eed..dda34a5 100644
--- a/src/pages/page_task_status.rs
+++ b/src/pages/page_task_status.rs
@@ -125,6 +125,13 @@ impl PvePageTaskStatus {
                         (IsRunning::Stopped, None) => {
                             format!("{} ({})", tr!("stopped"), tr!("unknown"))
                         }
+                        (IsRunning::UnknownEnumValue(value), msg) => {
+                            format!(
+                                "{} ({})",
+                                tr!("unknown status '{0}'", value),
+                                msg.as_ref().unwrap_or(&tr!("unknown"))
+                            )
+                        }
                     },
                 ));
 
-- 
2.47.3


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