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

["Hannes Laimer" <h.laimer@proxmox.com>]

On Mon Oct 14, 2024 at 3:42 PM CEST, Fabian Grünbichler wrote:
> 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?
>

good point, that actually would make sense 

> >      /// 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



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