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

[Dominik Csapak <d.csapak@proxmox.com>]

On 8/20/26 1:06 PM, Lukas Wagner wrote:
> Hi Dominik,
> 
> looking good - some of the code is missing doc-strings that could be
> added, but that's just a minor nit-pick.

yes of course, i'll send a v3

> 
> 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: {
>