Re: [PATCH datacenter-manager v2 1/9] lib: api types: add new ResourceView type and move accessors there

["Lukas Wagner" <l.wagner@proxmox.com>]

Hi Dominik,

looking good - some of the code is missing doc-strings that could be
added, but that's just a minor nit-pick.

On Tue Aug 18, 2026 at 3:25 PM CEST, Dominik Csapak wrote:
> This is intended as a view type for Resource and PveResource, which will
> always share some enum type. This way, we can reuse all accessors
> cheaply for PveResource values without cloning or consuming.
>
> This makes it easier to share code for both of them, for instance in the
> UI where we want to render properties consistently for resources.
>
> Introduces also a AsResourceView trait to make converting more
> ergonomic.
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  lib/pdm-api-types/src/resource.rs | 261 +++++++++++++++++++++---------
>  1 file changed, 184 insertions(+), 77 deletions(-)
>
> diff --git a/lib/pdm-api-types/src/resource.rs b/lib/pdm-api-types/src/resource.rs
> index 303ac3cb..4447aedb 100644
> --- a/lib/pdm-api-types/src/resource.rs
> +++ b/lib/pdm-api-types/src/resource.rs
> @@ -45,102 +45,31 @@ impl Resource {
>      /// Returns the local ID, not a globally unique one, e.g.
>      /// `qemu/<vmid>`
>      pub fn id(&self) -> String {
> -        match self {
> -            Resource::PveStorage(r) => format!("storage/{}/{}", r.node, r.storage),
> -            Resource::PveQemu(r) => format!("qemu/{}", r.vmid),
> -            Resource::PveLxc(r) => format!("lxc/{}", r.vmid),
> -            Resource::PveNode(r) => format!("node/{}", r.node),
> -            Resource::PveNetwork(r) => {
> -                if let PveNetworkResource::Zone(z) = r {
> -                    if z.legacy {
> -                        return format!("sdn/{}/{}", r.node(), r.name());
> -                    }
> -                }
> -
> -                format!("network/{}/{}/{}", r.node(), r.network_type(), r.name())
> -            }
> -            Resource::PbsNode(r) => format!("node/{}", r.name),
> -            Resource::PbsDatastore(r) => r.name.clone(),
> -        }
> +        self.as_resource_view().id()
>      }
>  
>      /// Returns the PDM global ID for the resource, e.g.
>      /// `remote/<remote-id>/guest/<vmid>`
>      pub fn global_id(&self) -> &str {
> -        match self {
> -            Resource::PveStorage(r) => r.id.as_str(),
> -            Resource::PveQemu(r) => r.id.as_str(),
> -            Resource::PveLxc(r) => r.id.as_str(),
> -            Resource::PveNode(r) => r.id.as_str(),
> -            Resource::PveNetwork(r) => r.id(),
> -            Resource::PbsNode(r) => r.id.as_str(),
> -            Resource::PbsDatastore(r) => r.id.as_str(),
> -        }
> +        self.as_resource_view().global_id()
>      }
>  
>      /// Returns the "name" of the resource, e.g. the guest name for VMs/Containers or
>      /// the hostname for nodes
>      pub fn name(&self) -> &str {
> -        match self {
> -            Resource::PveStorage(r) => r.storage.as_str(),
> -            Resource::PveQemu(r) => r.name.as_str(),
> -            Resource::PveLxc(r) => r.name.as_str(),
> -            Resource::PveNode(r) => r.node.as_str(),
> -            Resource::PveNetwork(r) => r.name(),
> -            Resource::PbsNode(r) => r.name.as_str(),
> -            Resource::PbsDatastore(r) => r.name.as_str(),
> -        }
> +        self.as_resource_view().name()
>      }
>  

While at it, you could add some doc strings here.

>      pub fn resource_type(&self) -> ResourceType {
> -        match self {
> -            Resource::PveStorage(_) => ResourceType::PveStorage,
> -            Resource::PveQemu(_) => ResourceType::PveQemu,
> -            Resource::PveLxc(_) => ResourceType::PveLxc,
> -            Resource::PveNetwork(_) => ResourceType::PveNetwork,
> -            Resource::PveNode(_) | Resource::PbsNode(_) => ResourceType::Node,
> -            Resource::PbsDatastore(_) => ResourceType::PbsDatastore,
> -        }
> +        self.as_resource_view().resource_type()
>      }
>  
While at it, you could add some doc strings here.

>      pub fn status(&self) -> &str {
> -        match self {
> -            Resource::PveStorage(r) => r.status.as_str(),
> -            Resource::PveQemu(r) => r.status.as_str(),
> -            Resource::PveLxc(r) => r.status.as_str(),
> -            Resource::PveNode(r) => r.status.as_str(),
> -            Resource::PveNetwork(r) => r.status(),
> -            Resource::PbsNode(r) => {
> -                if r.uptime > 0 {
> -                    "online"
> -                } else {
> -                    "offline"
> -                }
> -            }
> -            Resource::PbsDatastore(r) => {
> -                if r.maintenance.is_none() {
> -                    "online"
> -                } else {
> -                    "under-maintenance"
> -                }
> -            }
> -        }
> +        self.as_resource_view().status()
>      }
>  
While at it, you could add some doc strings here.

>      pub fn properties(&self) -> String {
> -        let mut properties = Vec::new();
> -        if let Resource::PbsDatastore(r) = self {
> -            if let Some(backend_type) = &r.backend_type {
> -                properties.push(backend_type.to_string());
> -            }
> -            if r.backing_device.is_some() {
> -                properties.push("removable".to_string());
> -            }
> -            if r.usage > PBS_DATASTORE_HIGH_USAGE_THRESHOLD {
> -                properties.push("high-usage".to_string());
> -            }
> -        }
> -        properties.join(",")
> +        self.as_resource_view().properties()
>      }
>  }
>  
> @@ -229,6 +158,184 @@ pub enum PveResource {
>      Network(PveNetworkResource),
>  }
>  
> +/// A borrowed view of a resource that can come from a [Resource] or a [PveResource]
> +/// This is useful for having a single implementation of field access regardless which
> +/// of the types is used.
> +#[derive(Clone, Debug, PartialEq)]
> +pub enum ResourceView<'a> {
> +    /// Storage resource in PVE.
> +    PveStorage(&'a PveStorageResource),
> +    /// QEMU guest resource in PVE.
> +    PveQemu(&'a PveQemuResource),
> +    /// LXC guest resource in PVE.
> +    PveLxc(&'a PveLxcResource),
> +    /// A PVE node.
> +    PveNode(&'a PveNodeResource),
> +    /// Network in PVE.
> +    PveNetwork(&'a PveNetworkResource),
> +    /// A PBS node.
> +    PbsNode(&'a PbsNodeResource),
> +    /// Datastore on a PBS node.
> +    PbsDatastore(&'a PbsDatastoreResource),
> +}
> +
> +impl<'a> ResourceView<'a> {
> +    /// Returns the local ID, not a globally unique one, e.g.
> +    /// `qemu/<vmid>`
> +    pub fn id(&self) -> String {
> +        match self {
> +            ResourceView::PveStorage(r) => format!("storage/{}/{}", r.node, r.storage),
> +            ResourceView::PveQemu(r) => format!("qemu/{}", r.vmid),
> +            ResourceView::PveLxc(r) => format!("lxc/{}", r.vmid),
> +            ResourceView::PveNode(r) => format!("node/{}", r.node),
> +            ResourceView::PveNetwork(r) => {
> +                if let PveNetworkResource::Zone(z) = r {
> +                    if z.legacy {
> +                        return format!("sdn/{}/{}", r.node(), r.name());
> +                    }
> +                }
> +
> +                format!("network/{}/{}/{}", r.node(), r.network_type(), r.name())
> +            }
> +            ResourceView::PbsNode(r) => format!("node/{}", r.name),
> +            ResourceView::PbsDatastore(r) => r.name.clone(),
> +        }
> +    }
> +
> +    /// Returns the PDM global ID for the resource, e.g.
> +    /// `remote/<remote-id>/guest/<vmid>`
> +    pub fn global_id(&self) -> &'a str {
> +        match self {
> +            ResourceView::PveStorage(r) => r.id.as_str(),
> +            ResourceView::PveQemu(r) => r.id.as_str(),
> +            ResourceView::PveLxc(r) => r.id.as_str(),
> +            ResourceView::PveNode(r) => r.id.as_str(),
> +            ResourceView::PveNetwork(r) => r.id(),
> +            ResourceView::PbsNode(r) => r.id.as_str(),
> +            ResourceView::PbsDatastore(r) => r.id.as_str(),
> +        }
> +    }
> +
> +    /// Returns the "name" of the resource, e.g. the guest name for VMs/Containers or
> +    /// the hostname for nodes
> +    pub fn name(&self) -> &'a str {
> +        match self {
> +            ResourceView::PveStorage(r) => r.storage.as_str(),
> +            ResourceView::PveQemu(r) => r.name.as_str(),
> +            ResourceView::PveLxc(r) => r.name.as_str(),
> +            ResourceView::PveNode(r) => r.node.as_str(),
> +            ResourceView::PveNetwork(r) => r.name(),
> +            ResourceView::PbsNode(r) => r.name.as_str(),
> +            ResourceView::PbsDatastore(r) => r.name.as_str(),
> +        }
> +    }
> +

Missing doc strings here.
> +    pub fn resource_type(&self) -> ResourceType {
> +        match self {
> +            ResourceView::PveStorage(_) => ResourceType::PveStorage,
> +            ResourceView::PveQemu(_) => ResourceType::PveQemu,
> +            ResourceView::PveLxc(_) => ResourceType::PveLxc,
> +            ResourceView::PveNetwork(_) => ResourceType::PveNetwork,
> +            ResourceView::PveNode(_) | ResourceView::PbsNode(_) => ResourceType::Node,
> +            ResourceView::PbsDatastore(_) => ResourceType::PbsDatastore,
> +        }
> +    }
> +

here as well
> +    pub fn status(&self) -> &'a str {
> +        match self {
> +            ResourceView::PveStorage(r) => r.status.as_str(),
> +            ResourceView::PveQemu(r) => r.status.as_str(),
> +            ResourceView::PveLxc(r) => r.status.as_str(),
> +            ResourceView::PveNode(r) => r.status.as_str(),
> +            ResourceView::PveNetwork(r) => r.status(),
> +            ResourceView::PbsNode(r) => {
> +                if r.uptime > 0 {
> +                    "online"
> +                } else {
> +                    "offline"
> +                }
> +            }
> +            ResourceView::PbsDatastore(r) => {
> +                if r.maintenance.is_none() {
> +                    "online"
> +                } else {
> +                    "under-maintenance"
> +                }
> +            }
> +        }
> +    }
> +

here as well
> +    pub fn properties(&self) -> String {
> +        let mut properties = Vec::new();
> +        if let ResourceView::PbsDatastore(r) = self {
> +            if let Some(backend_type) = &r.backend_type {
> +                properties.push(backend_type.to_string());
> +            }
> +            if r.backing_device.is_some() {
> +                properties.push("removable".to_string());
> +            }
> +            if r.usage > PBS_DATASTORE_HIGH_USAGE_THRESHOLD {
> +                properties.push("high-usage".to_string());
> +            }
> +        }
> +        properties.join(",")
> +    }
> +}
> +
> +impl<'a> From<&'a Resource> for ResourceView<'a> {
> +    fn from(value: &'a Resource) -> Self {
> +        match value {
> +            Resource::PveStorage(r) => ResourceView::PveStorage(r),
> +            Resource::PveQemu(r) => ResourceView::PveQemu(r),
> +            Resource::PveLxc(r) => ResourceView::PveLxc(r),
> +            Resource::PveNode(r) => ResourceView::PveNode(r),
> +            Resource::PveNetwork(r) => ResourceView::PveNetwork(r),
> +            Resource::PbsNode(r) => ResourceView::PbsNode(r),
> +            Resource::PbsDatastore(r) => ResourceView::PbsDatastore(r),
> +        }
> +    }
> +}
> +
> +impl<'a> From<&'a PveResource> for ResourceView<'a> {
> +    fn from(value: &'a PveResource) -> Self {
> +        match value {
> +            PveResource::Storage(r) => ResourceView::PveStorage(r),
> +            PveResource::Qemu(r) => ResourceView::PveQemu(r),
> +            PveResource::Lxc(r) => ResourceView::PveLxc(r),
> +            PveResource::Node(r) => ResourceView::PveNode(r),
> +            PveResource::Network(r) => ResourceView::PveNetwork(r),
> +        }
> +    }
> +}
> +

missing doc string here

> +pub trait AsResourceView {
> +    fn as_resource_view(&self) -> ResourceView<'_>;
> +}
> +
> +impl AsResourceView for Resource {
> +    fn as_resource_view<'a>(&'a self) -> ResourceView<'a> {
> +        ResourceView::from(self)
> +    }
> +}
> +
> +impl AsResourceView for &Resource {
> +    fn as_resource_view<'a>(&'a self) -> ResourceView<'a> {
> +        ResourceView::from(*self)
> +    }
> +}
> +
> +impl AsResourceView for PveResource {
> +    fn as_resource_view<'a>(&'a self) -> ResourceView<'a> {
> +        ResourceView::from(self)
> +    }
> +}
> +
> +impl AsResourceView for &PveResource {
> +    fn as_resource_view<'a>(&'a self) -> ResourceView<'a> {
> +        ResourceView::from(*self)
> +    }
> +}
> +
>  #[api(
>      properties: {
>          tags: {