From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id A523B1FF0B2 for ; Thu, 20 Aug 2026 13:07:29 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id BA9AF21573; Thu, 20 Aug 2026 13:07:28 +0200 (CEST) Content-Type: text/plain; charset=UTF-8 Date: Thu, 20 Aug 2026 13:07:20 +0200 Message-Id: Subject: Re: [PATCH datacenter-manager v2 1/9] lib: api types: add new ResourceView type and move accessors there From: "Lukas Wagner" To: "Dominik Csapak" , Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Mailer: aerc 0.21.0-0-g5549850facc2-dirty References: <20260818133000.3793412-1-d.csapak@proxmox.com> <20260818133000.3793412-2-d.csapak@proxmox.com> In-Reply-To: <20260818133000.3793412-2-d.csapak@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1787224015650 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.827 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: 2KA4VUJ7ECCAVHF5K67MIZKPHVQ4BCYL X-Message-ID-Hash: 2KA4VUJ7ECCAVHF5K67MIZKPHVQ4BCYL X-MailFrom: l.wagner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 > --- > 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/re= source.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/` > pub fn id(&self) -> String { > - match self { > - Resource::PveStorage(r) =3D> format!("storage/{}/{}", r.node= , r.storage), > - Resource::PveQemu(r) =3D> format!("qemu/{}", r.vmid), > - Resource::PveLxc(r) =3D> format!("lxc/{}", r.vmid), > - Resource::PveNode(r) =3D> format!("node/{}", r.node), > - Resource::PveNetwork(r) =3D> { > - if let PveNetworkResource::Zone(z) =3D r { > - if z.legacy { > - return format!("sdn/{}/{}", r.node(), r.name()); > - } > - } > - > - format!("network/{}/{}/{}", r.node(), r.network_type(), = r.name()) > - } > - Resource::PbsNode(r) =3D> format!("node/{}", r.name), > - Resource::PbsDatastore(r) =3D> r.name.clone(), > - } > + self.as_resource_view().id() > } > =20 > /// Returns the PDM global ID for the resource, e.g. > /// `remote//guest/` > pub fn global_id(&self) -> &str { > - match self { > - Resource::PveStorage(r) =3D> r.id.as_str(), > - Resource::PveQemu(r) =3D> r.id.as_str(), > - Resource::PveLxc(r) =3D> r.id.as_str(), > - Resource::PveNode(r) =3D> r.id.as_str(), > - Resource::PveNetwork(r) =3D> r.id(), > - Resource::PbsNode(r) =3D> r.id.as_str(), > - Resource::PbsDatastore(r) =3D> r.id.as_str(), > - } > + self.as_resource_view().global_id() > } > =20 > /// 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) =3D> r.storage.as_str(), > - Resource::PveQemu(r) =3D> r.name.as_str(), > - Resource::PveLxc(r) =3D> r.name.as_str(), > - Resource::PveNode(r) =3D> r.node.as_str(), > - Resource::PveNetwork(r) =3D> r.name(), > - Resource::PbsNode(r) =3D> r.name.as_str(), > - Resource::PbsDatastore(r) =3D> r.name.as_str(), > - } > + self.as_resource_view().name() > } > =20 While at it, you could add some doc strings here. > pub fn resource_type(&self) -> ResourceType { > - match self { > - Resource::PveStorage(_) =3D> ResourceType::PveStorage, > - Resource::PveQemu(_) =3D> ResourceType::PveQemu, > - Resource::PveLxc(_) =3D> ResourceType::PveLxc, > - Resource::PveNetwork(_) =3D> ResourceType::PveNetwork, > - Resource::PveNode(_) | Resource::PbsNode(_) =3D> ResourceTyp= e::Node, > - Resource::PbsDatastore(_) =3D> ResourceType::PbsDatastore, > - } > + self.as_resource_view().resource_type() > } > =20 While at it, you could add some doc strings here. > pub fn status(&self) -> &str { > - match self { > - Resource::PveStorage(r) =3D> r.status.as_str(), > - Resource::PveQemu(r) =3D> r.status.as_str(), > - Resource::PveLxc(r) =3D> r.status.as_str(), > - Resource::PveNode(r) =3D> r.status.as_str(), > - Resource::PveNetwork(r) =3D> r.status(), > - Resource::PbsNode(r) =3D> { > - if r.uptime > 0 { > - "online" > - } else { > - "offline" > - } > - } > - Resource::PbsDatastore(r) =3D> { > - if r.maintenance.is_none() { > - "online" > - } else { > - "under-maintenance" > - } > - } > - } > + self.as_resource_view().status() > } > =20 While at it, you could add some doc strings here. > pub fn properties(&self) -> String { > - let mut properties =3D Vec::new(); > - if let Resource::PbsDatastore(r) =3D self { > - if let Some(backend_type) =3D &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() > } > } > =20 > @@ -229,6 +158,184 @@ pub enum PveResource { > Network(PveNetworkResource), > } > =20 > +/// 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 re= gardless 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/` > + pub fn id(&self) -> String { > + match self { > + ResourceView::PveStorage(r) =3D> format!("storage/{}/{}", r.= node, r.storage), > + ResourceView::PveQemu(r) =3D> format!("qemu/{}", r.vmid), > + ResourceView::PveLxc(r) =3D> format!("lxc/{}", r.vmid), > + ResourceView::PveNode(r) =3D> format!("node/{}", r.node), > + ResourceView::PveNetwork(r) =3D> { > + if let PveNetworkResource::Zone(z) =3D r { > + if z.legacy { > + return format!("sdn/{}/{}", r.node(), r.name()); > + } > + } > + > + format!("network/{}/{}/{}", r.node(), r.network_type(), = r.name()) > + } > + ResourceView::PbsNode(r) =3D> format!("node/{}", r.name), > + ResourceView::PbsDatastore(r) =3D> r.name.clone(), > + } > + } > + > + /// Returns the PDM global ID for the resource, e.g. > + /// `remote//guest/` > + pub fn global_id(&self) -> &'a str { > + match self { > + ResourceView::PveStorage(r) =3D> r.id.as_str(), > + ResourceView::PveQemu(r) =3D> r.id.as_str(), > + ResourceView::PveLxc(r) =3D> r.id.as_str(), > + ResourceView::PveNode(r) =3D> r.id.as_str(), > + ResourceView::PveNetwork(r) =3D> r.id(), > + ResourceView::PbsNode(r) =3D> r.id.as_str(), > + ResourceView::PbsDatastore(r) =3D> 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) =3D> r.storage.as_str(), > + ResourceView::PveQemu(r) =3D> r.name.as_str(), > + ResourceView::PveLxc(r) =3D> r.name.as_str(), > + ResourceView::PveNode(r) =3D> r.node.as_str(), > + ResourceView::PveNetwork(r) =3D> r.name(), > + ResourceView::PbsNode(r) =3D> r.name.as_str(), > + ResourceView::PbsDatastore(r) =3D> r.name.as_str(), > + } > + } > + Missing doc strings here. > + pub fn resource_type(&self) -> ResourceType { > + match self { > + ResourceView::PveStorage(_) =3D> ResourceType::PveStorage, > + ResourceView::PveQemu(_) =3D> ResourceType::PveQemu, > + ResourceView::PveLxc(_) =3D> ResourceType::PveLxc, > + ResourceView::PveNetwork(_) =3D> ResourceType::PveNetwork, > + ResourceView::PveNode(_) | ResourceView::PbsNode(_) =3D> Res= ourceType::Node, > + ResourceView::PbsDatastore(_) =3D> ResourceType::PbsDatastor= e, > + } > + } > + here as well > + pub fn status(&self) -> &'a str { > + match self { > + ResourceView::PveStorage(r) =3D> r.status.as_str(), > + ResourceView::PveQemu(r) =3D> r.status.as_str(), > + ResourceView::PveLxc(r) =3D> r.status.as_str(), > + ResourceView::PveNode(r) =3D> r.status.as_str(), > + ResourceView::PveNetwork(r) =3D> r.status(), > + ResourceView::PbsNode(r) =3D> { > + if r.uptime > 0 { > + "online" > + } else { > + "offline" > + } > + } > + ResourceView::PbsDatastore(r) =3D> { > + if r.maintenance.is_none() { > + "online" > + } else { > + "under-maintenance" > + } > + } > + } > + } > + here as well > + pub fn properties(&self) -> String { > + let mut properties =3D Vec::new(); > + if let ResourceView::PbsDatastore(r) =3D self { > + if let Some(backend_type) =3D &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) =3D> ResourceView::PveStorage(r), > + Resource::PveQemu(r) =3D> ResourceView::PveQemu(r), > + Resource::PveLxc(r) =3D> ResourceView::PveLxc(r), > + Resource::PveNode(r) =3D> ResourceView::PveNode(r), > + Resource::PveNetwork(r) =3D> ResourceView::PveNetwork(r), > + Resource::PbsNode(r) =3D> ResourceView::PbsNode(r), > + Resource::PbsDatastore(r) =3D> ResourceView::PbsDatastore(r)= , > + } > + } > +} > + > +impl<'a> From<&'a PveResource> for ResourceView<'a> { > + fn from(value: &'a PveResource) -> Self { > + match value { > + PveResource::Storage(r) =3D> ResourceView::PveStorage(r), > + PveResource::Qemu(r) =3D> ResourceView::PveQemu(r), > + PveResource::Lxc(r) =3D> ResourceView::PveLxc(r), > + PveResource::Node(r) =3D> ResourceView::PveNode(r), > + PveResource::Network(r) =3D> 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: {