Re: [pbs-devel] [PATCH proxmox-backup v12 10/26] pbs-api-types: add removable/is-available flag to DataStoreListItem

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On September 4, 2024 4:11 pm, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  pbs-api-types/src/datastore.rs |  9 ++++++++-
>  src/api2/admin/datastore.rs    | 17 +++++++++--------
>  src/api2/status.rs             | 18 +++++++++++++++---
>  3 files changed, 32 insertions(+), 12 deletions(-)
> 
> diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
> index d6e2f97e..6a999372 100644
> --- a/pbs-api-types/src/datastore.rs
> +++ b/pbs-api-types/src/datastore.rs
> @@ -454,6 +454,10 @@ impl DataStoreConfig {
>  pub struct DataStoreListItem {
>      pub store: String,
>      pub comment: Option<String>,
> +    /// Datastore is removable
> +    pub removable: bool,
> +    /// Datastore is available
> +    pub available: bool,
>      /// If the datastore is in maintenance mode, information about it
>      #[serde(skip_serializing_if = "Option::is_none")]
>      pub maintenance: Option<String>,
> @@ -1453,6 +1457,8 @@ pub struct DataStoreStatusListItem {
>      /// The available bytes of the underlying storage. (-1 on error)
>      #[serde(skip_serializing_if = "Option::is_none")]
>      pub avail: Option<u64>,
> +    /// The datastore is available, relevant if removable
> +    pub is_available: bool,

but it doesn't explain what available is.. why is it only relevant if
removable? and wouldn't it then make more sense to make removable an
enum that encodes whether it's available or not?

>      /// A list of usages of the past (last Month).
>      #[serde(skip_serializing_if = "Option::is_none")]
>      pub history: Option<Vec<Option<f64>>>,
> @@ -1477,12 +1483,13 @@ pub struct DataStoreStatusListItem {
>  }
>  
>  impl DataStoreStatusListItem {
> -    pub fn empty(store: &str, err: Option<String>) -> Self {
> +    pub fn empty(store: &str, err: Option<String>, is_available: bool) -> Self {
>          DataStoreStatusListItem {
>              store: store.to_owned(),
>              total: None,
>              used: None,
>              avail: None,
> +            is_available,
>              history: None,
>              history_start: None,
>              history_delta: None,
> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> index 3c95888d..0b5dede7 100644
> --- a/src/api2/admin/datastore.rs
> +++ b/src/api2/admin/datastore.rs
> @@ -1310,8 +1310,8 @@ pub fn get_datastore_list(
>  
>      let mut list = Vec::new();
>  
> -    for (store, (_, data)) in &config.sections {
> -        let acl_path = &["datastore", store];
> +    for (store, (_, data)) in config.sections {
> +        let acl_path = &["datastore", &store];
>          let user_privs = user_info.lookup_privs(&auth_id, acl_path);
>          let allowed = (user_privs & (PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_BACKUP)) != 0;
>  
> @@ -1322,15 +1322,16 @@ pub fn get_datastore_list(
>              }
>          }
>  
> +        let store_config: DataStoreConfig = serde_json::from_value(data)?;
> +        let is_available = pbs_datastore::is_datastore_available(&store_config);
> +
>          if allowed || allow_id {
>              list.push(DataStoreListItem {
>                  store: store.clone(),
> -                comment: if !allowed {
> -                    None
> -                } else {
> -                    data["comment"].as_str().map(String::from)
> -                },
> -                maintenance: data["maintenance-mode"].as_str().map(String::from),
> +                comment: store_config.comment.filter(|_| allowed),
> +                removable: store_config.backing_device.is_some(),
> +                available: is_available,
> +                maintenance: store_config.maintenance_mode,
>              });
>          }
>      }
> diff --git a/src/api2/status.rs b/src/api2/status.rs
> index f1ae0ef5..a0dd46b6 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::{is_datastore_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 !is_datastore_available(&store_config) {
> +            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,
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
> 


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