From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 3288F965EE for ; Tue, 16 Apr 2024 09:38:27 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 0D1E315382 for ; Tue, 16 Apr 2024 09:37:57 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Tue, 16 Apr 2024 09:37:56 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 4FD8244DDD for ; Tue, 16 Apr 2024 09:37:55 +0200 (CEST) Message-ID: Date: Tue, 16 Apr 2024 09:37:53 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Proxmox Backup Server development discussion , Hannes Laimer References: <20240409110012.166472-1-h.laimer@proxmox.com> <20240409110012.166472-11-h.laimer@proxmox.com> Content-Language: en-US, de-DE From: Christian Ebner In-Reply-To: <20240409110012.166472-11-h.laimer@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -1.270 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy ENA_SUBJ_ODD_CASE 2.6 Subject has odd case KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pbs-devel] [PATCH proxmox-backup v3 10/24] pbs-api-types: add removable/is-available flag to DataStoreListItem X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Apr 2024 07:38:27 -0000 there is a typo in commit title, s/scheme/schema also maybe you can explicitly include the usage of `SchemaDeserializer` by stating something like: `pbs-api-types: use SchemaDeserializer for maintenance mode` and a comment inline On 4/9/24 12:59, Hannes Laimer wrote: > Signed-off-by: Hannes Laimer > --- > pbs-api-types/src/datastore.rs | 7 ++++++- > src/api2/admin/datastore.rs | 1 + > src/api2/status.rs | 18 +++++++++++++++--- > 3 files changed, 22 insertions(+), 4 deletions(-) > > diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs > index 738ba96f..ce53c375 100644 > --- a/pbs-api-types/src/datastore.rs > +++ b/pbs-api-types/src/datastore.rs > @@ -390,6 +390,8 @@ impl DataStoreConfig { > pub struct DataStoreListItem { > pub store: String, > pub comment: Option, > + /// Datastore is removable > + pub removable: bool, > /// If the datastore is in maintenance mode, information about it > #[serde(skip_serializing_if = "Option::is_none")] > pub maintenance: Option, > @@ -1357,6 +1359,8 @@ pub struct DataStoreStatusListItem { > /// The available bytes of the underlying storage. (-1 on error) > #[serde(skip_serializing_if = "Option::is_none")] > pub avail: Option, > + /// The datastore is available, relevant if removable > + pub is_available: bool, > /// A list of usages of the past (last Month). > #[serde(skip_serializing_if = "Option::is_none")] > pub history: Option>>, > @@ -1381,12 +1385,13 @@ pub struct DataStoreStatusListItem { > } > > impl DataStoreStatusListItem { > - pub fn empty(store: &str, err: Option) -> Self { > + pub fn empty(store: &str, err: Option, is_available: bool) -> Self { > DataStoreStatusListItem { > store: store.to_owned(), > total: None, > used: None, > avail: None, > + is_available, this could default to true, and instead of passing the `is_available` flag as argument. That could be set by a dedicates `fn datastore_unavailable(self) -> Self` method for `DataStoreStatusListItem`, which takes the list item, sets the flag and returns it to be pushed to the list, so the opaque argument to `empty` is avoided. So you would get: `list.push(DataStoreStatusListItem::empty(store, None).datastore_unavailable());` But not sure about this... > history: None, > history_start: None, > history_delta: None, > diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs > index 92f6adc2..acf19fa2 100644 > --- a/src/api2/admin/datastore.rs > +++ b/src/api2/admin/datastore.rs > @@ -1245,6 +1245,7 @@ pub fn get_datastore_list( > } else { > data["comment"].as_str().map(String::from) > }, > + removable: data["backing-device"].as_str().is_some(), > maintenance: data["maintenance-mode"].as_str().map(String::from), > }); > } > diff --git a/src/api2/status.rs b/src/api2/status.rs > index 78bc06b5..43f0b55d 100644 > --- a/src/api2/status.rs > +++ b/src/api2/status.rs > @@ -13,7 +13,7 @@ use pbs_api_types::{ > }; > > use pbs_config::CachedUserInfo; > -use pbs_datastore::DataStore; > +use pbs_datastore::{check_if_available, DataStore}; > > use crate::rrd_cache::extract_rrd_data; > use crate::tools::statistics::linear_regression; > @@ -48,10 +48,17 @@ pub async fn datastore_status( > for (store, (_, _)) in &config.sections { > let user_privs = user_info.lookup_privs(&auth_id, &["datastore", store]); > let allowed = (user_privs & (PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_BACKUP)) != 0; > + > + let store_config = config.lookup("datastore", store)?; > + if check_if_available(&store_config).is_err() { > + list.push(DataStoreStatusListItem::empty(store, None, false)); > + continue; > + } > + > if !allowed { > if let Ok(datastore) = DataStore::lookup_datastore(store, Some(Operation::Lookup)) { > if can_access_any_namespace(datastore, &auth_id, &user_info) { > - list.push(DataStoreStatusListItem::empty(store, None)); > + list.push(DataStoreStatusListItem::empty(store, None, true)); > } > } > continue; > @@ -60,7 +67,11 @@ pub async fn datastore_status( > let datastore = match DataStore::lookup_datastore(store, Some(Operation::Read)) { > Ok(datastore) => datastore, > Err(err) => { > - list.push(DataStoreStatusListItem::empty(store, Some(err.to_string()))); > + list.push(DataStoreStatusListItem::empty( > + store, > + Some(err.to_string()), > + true, > + )); > continue; > } > }; > @@ -71,6 +82,7 @@ pub async fn datastore_status( > total: Some(status.total), > used: Some(status.used), > avail: Some(status.available), > + is_available: true, > history: None, > history_start: None, > history_delta: None,