all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup 0/2] Fix DatastoreListSummary on limited permissions
@ 2023-12-07 14:35 Gabriel Goller
  2023-12-07 14:35 ` [pbs-devel] [PATCH proxmox-backup 1/2] ui: datastore summary handle non-existent values Gabriel Goller
  2023-12-07 14:35 ` [pbs-devel] [PATCH proxmox-backup 2/2] status: use Option on avail/used datastore attrs Gabriel Goller
  0 siblings, 2 replies; 7+ messages in thread
From: Gabriel Goller @ 2023-12-07 14:35 UTC (permalink / raw)
  To: pbs-devel

When the user has limited permissions on a datastore (e.g `NoAccess` on the whole 
datastore and `DatastoreBackup` on a namespace inside the datastore) the Datastore
List shows weird values. 

Instead of returning -1 we return an Option and handle missing values on the
frontend.



proxmox-backup:

Gabriel Goller (2):
  ui: datastore summary handle non-existent values
  status: use Option on avail/used datastore attrs

 pbs-api-types/src/datastore.rs        | 19 +++++++++++--------
 src/api2/status.rs                    |  6 +++---
 www/datastore/DataStoreListSummary.js | 20 ++++++++++++++------
 3 files changed, 28 insertions(+), 17 deletions(-)


Summary over all repositories:
  3 files changed, 28 insertions(+), 17 deletions(-)

-- 
murpp v0.4.0





^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 1/2] ui: datastore summary handle non-existent values
  2023-12-07 14:35 [pbs-devel] [PATCH proxmox-backup 0/2] Fix DatastoreListSummary on limited permissions Gabriel Goller
@ 2023-12-07 14:35 ` Gabriel Goller
  2023-12-07 14:35 ` [pbs-devel] [PATCH proxmox-backup 2/2] status: use Option on avail/used datastore attrs Gabriel Goller
  1 sibling, 0 replies; 7+ messages in thread
From: Gabriel Goller @ 2023-12-07 14:35 UTC (permalink / raw)
  To: pbs-devel

Correctly display missing 'avail' and 'used' attributes in the
datatstore summary. This simply sets it to 0, so that we don't get any
errors in the console.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 www/datastore/DataStoreListSummary.js | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/www/datastore/DataStoreListSummary.js b/www/datastore/DataStoreListSummary.js
index 968239b0..b908034d 100644
--- a/www/datastore/DataStoreListSummary.js
+++ b/www/datastore/DataStoreListSummary.js
@@ -52,12 +52,20 @@ Ext.define('PBS.datastore.DataStoreListSummary', {
 	    vm.set('maintenance', '');
 	}
 
-	let total = statusData.avail + statusData.used;
-	let usage = statusData.used / total;
-	let usagetext = Ext.String.format(gettext('{0} of {1}'),
-	    Proxmox.Utils.format_size(statusData.used, true),
-	    Proxmox.Utils.format_size(total, true),
-	);
+	let usagetext;
+	let usage;
+
+	if (Object.hasOwn(statusData, 'avail') && Object.hasOwn(statusData, 'used')) {
+	    let total = statusData.avail + statusData.used;
+	    usage = statusData.used / total;
+	    usagetext = Ext.String.format(gettext('{0} of {1}'),
+		Proxmox.Utils.format_size(statusData.used, true),
+		Proxmox.Utils.format_size(total, true),
+	    );
+	} else {
+	    usagetext = Ext.String.format(gettext('{0} of {1}'), 0, 0);
+	    usage = 0;
+	}
 
 	let usagePanel = me.lookup('usage');
 	usagePanel.updateValue(usage, usagetext);
-- 
2.39.2





^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 2/2] status: use Option on avail/used datastore attrs
  2023-12-07 14:35 [pbs-devel] [PATCH proxmox-backup 0/2] Fix DatastoreListSummary on limited permissions Gabriel Goller
  2023-12-07 14:35 ` [pbs-devel] [PATCH proxmox-backup 1/2] ui: datastore summary handle non-existent values Gabriel Goller
@ 2023-12-07 14:35 ` Gabriel Goller
  2023-12-07 17:07   ` Dietmar Maurer
  2023-12-07 17:33   ` Dietmar Maurer
  1 sibling, 2 replies; 7+ messages in thread
From: Gabriel Goller @ 2023-12-07 14:35 UTC (permalink / raw)
  To: pbs-devel

Instead of returning -1 if we can't get the attributes, we use an
Option which will not be serialized on `None`.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 pbs-api-types/src/datastore.rs | 19 +++++++++++--------
 src/api2/status.rs             |  6 +++---
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index d4ead1d1..c361dc30 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -1302,12 +1302,15 @@ pub struct DataStoreStatus {
 /// Status of a Datastore
 pub struct DataStoreStatusListItem {
     pub store: String,
-    /// The Size of the underlying storage in bytes. (-1 on error)
-    pub total: i64,
-    /// The used bytes of the underlying storage. (-1 on error)
-    pub used: i64,
+    /// The Size of the underlying storage in bytes.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub total: Option<i64>,
+    /// The used bytes of the underlying storage.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub used: Option<i64>,
     /// The available bytes of the underlying storage. (-1 on error)
-    pub avail: i64,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub avail: Option<i64>,
     /// A list of usages of the past (last Month).
     #[serde(skip_serializing_if = "Option::is_none")]
     pub history: Option<Vec<Option<f64>>>,
@@ -1335,9 +1338,9 @@ impl DataStoreStatusListItem {
     pub fn empty(store: &str, err: Option<String>) -> Self {
         DataStoreStatusListItem {
             store: store.to_owned(),
-            total: -1,
-            used: -1,
-            avail: -1,
+            total: None,
+            used: None,
+            avail: None,
             history: None,
             history_start: None,
             history_delta: None,
diff --git a/src/api2/status.rs b/src/api2/status.rs
index ff7d4cbd..6e758187 100644
--- a/src/api2/status.rs
+++ b/src/api2/status.rs
@@ -68,9 +68,9 @@ pub async fn datastore_status(
 
         let mut entry = DataStoreStatusListItem {
             store: store.clone(),
-            total: status.total as i64,
-            used: status.used as i64,
-            avail: status.available as i64,
+            total: Some(status.total as i64),
+            used: Some(status.used as i64),
+            avail: Some(status.available as i64),
             history: None,
             history_start: None,
             history_delta: None,
-- 
2.39.2





^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup 2/2] status: use Option on avail/used datastore attrs
  2023-12-07 14:35 ` [pbs-devel] [PATCH proxmox-backup 2/2] status: use Option on avail/used datastore attrs Gabriel Goller
@ 2023-12-07 17:07   ` Dietmar Maurer
  2023-12-07 17:10     ` Dietmar Maurer
  2023-12-07 17:33   ` Dietmar Maurer
  1 sibling, 1 reply; 7+ messages in thread
From: Dietmar Maurer @ 2023-12-07 17:07 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Gabriel Goller

But this is not backwards compatible. Deserialization fail now for -1 (when connecting to an old server which returns -1)?

> On 7.12.2023 15:35 CET Gabriel Goller <g.goller@proxmox.com> wrote:
> 
>  
> Instead of returning -1 if we can't get the attributes, we use an
> Option which will not be serialized on `None`.
> 
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>  pbs-api-types/src/datastore.rs | 19 +++++++++++--------
>  src/api2/status.rs             |  6 +++---
>  2 files changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
> index d4ead1d1..c361dc30 100644
> --- a/pbs-api-types/src/datastore.rs
> +++ b/pbs-api-types/src/datastore.rs
> @@ -1302,12 +1302,15 @@ pub struct DataStoreStatus {
>  /// Status of a Datastore
>  pub struct DataStoreStatusListItem {
>      pub store: String,
> -    /// The Size of the underlying storage in bytes. (-1 on error)
> -    pub total: i64,
> -    /// The used bytes of the underlying storage. (-1 on error)
> -    pub used: i64,
> +    /// The Size of the underlying storage in bytes.
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub total: Option<i64>,
> +    /// The used bytes of the underlying storage.
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub used: Option<i64>,
>      /// The available bytes of the underlying storage. (-1 on error)
> -    pub avail: i64,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub avail: Option<i64>,
>      /// A list of usages of the past (last Month).
>      #[serde(skip_serializing_if = "Option::is_none")]
>      pub history: Option<Vec<Option<f64>>>,
> @@ -1335,9 +1338,9 @@ impl DataStoreStatusListItem {
>      pub fn empty(store: &str, err: Option<String>) -> Self {
>          DataStoreStatusListItem {
>              store: store.to_owned(),
> -            total: -1,
> -            used: -1,
> -            avail: -1,
> +            total: None,
> +            used: None,
> +            avail: None,
>              history: None,
>              history_start: None,
>              history_delta: None,
> diff --git a/src/api2/status.rs b/src/api2/status.rs
> index ff7d4cbd..6e758187 100644
> --- a/src/api2/status.rs
> +++ b/src/api2/status.rs
> @@ -68,9 +68,9 @@ pub async fn datastore_status(
>  
>          let mut entry = DataStoreStatusListItem {
>              store: store.clone(),
> -            total: status.total as i64,
> -            used: status.used as i64,
> -            avail: status.available as i64,
> +            total: Some(status.total as i64),
> +            used: Some(status.used as i64),
> +            avail: Some(status.available as i64),
>              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




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup 2/2] status: use Option on avail/used datastore attrs
  2023-12-07 17:07   ` Dietmar Maurer
@ 2023-12-07 17:10     ` Dietmar Maurer
  0 siblings, 0 replies; 7+ messages in thread
From: Dietmar Maurer @ 2023-12-07 17:10 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Gabriel Goller

Oh, Ignore me - I forgot that we do not have any GUI that uses rust
for deserialization (it is only my prototype)...

> On 7.12.2023 18:07 CET Dietmar Maurer <dietmar@proxmox.com> wrote:
> 
>  
> But this is not backwards compatible. Deserialization fail now for -1 (when connecting to an old server which returns -1)?




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup 2/2] status: use Option on avail/used datastore attrs
  2023-12-07 14:35 ` [pbs-devel] [PATCH proxmox-backup 2/2] status: use Option on avail/used datastore attrs Gabriel Goller
  2023-12-07 17:07   ` Dietmar Maurer
@ 2023-12-07 17:33   ` Dietmar Maurer
  2023-12-11  8:46     ` Gabriel Goller
  1 sibling, 1 reply; 7+ messages in thread
From: Dietmar Maurer @ 2023-12-07 17:33 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Gabriel Goller

But why is it still i64? I would expect u64.


> On 7.12.2023 15:35 CET Gabriel Goller <g.goller@proxmox.com> wrote:
> 
>  
> Instead of returning -1 if we can't get the attributes, we use an
> Option which will not be serialized on `None`.
> 
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>  pbs-api-types/src/datastore.rs | 19 +++++++++++--------
>  src/api2/status.rs             |  6 +++---
>  2 files changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
> index d4ead1d1..c361dc30 100644
> --- a/pbs-api-types/src/datastore.rs
> +++ b/pbs-api-types/src/datastore.rs
> @@ -1302,12 +1302,15 @@ pub struct DataStoreStatus {
>  /// Status of a Datastore
>  pub struct DataStoreStatusListItem {
>      pub store: String,
> -    /// The Size of the underlying storage in bytes. (-1 on error)
> -    pub total: i64,
> -    /// The used bytes of the underlying storage. (-1 on error)
> -    pub used: i64,
> +    /// The Size of the underlying storage in bytes.
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub total: Option<i64>,
> +    /// The used bytes of the underlying storage.
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub used: Option<i64>,
>      /// The available bytes of the underlying storage. (-1 on error)
> -    pub avail: i64,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub avail: Option<i64>,
>      /// A list of usages of the past (last Month).
>      #[serde(skip_serializing_if = "Option::is_none")]
>      pub history: Option<Vec<Option<f64>>>,
> @@ -1335,9 +1338,9 @@ impl DataStoreStatusListItem {
>      pub fn empty(store: &str, err: Option<String>) -> Self {
>          DataStoreStatusListItem {
>              store: store.to_owned(),
> -            total: -1,
> -            used: -1,
> -            avail: -1,
> +            total: None,
> +            used: None,
> +            avail: None,
>              history: None,
>              history_start: None,
>              history_delta: None,
> diff --git a/src/api2/status.rs b/src/api2/status.rs
> index ff7d4cbd..6e758187 100644
> --- a/src/api2/status.rs
> +++ b/src/api2/status.rs
> @@ -68,9 +68,9 @@ pub async fn datastore_status(
>  
>          let mut entry = DataStoreStatusListItem {
>              store: store.clone(),
> -            total: status.total as i64,
> -            used: status.used as i64,
> -            avail: status.available as i64,
> +            total: Some(status.total as i64),
> +            used: Some(status.used as i64),
> +            avail: Some(status.available as i64),
>              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




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup 2/2] status: use Option on avail/used datastore attrs
  2023-12-07 17:33   ` Dietmar Maurer
@ 2023-12-11  8:46     ` Gabriel Goller
  0 siblings, 0 replies; 7+ messages in thread
From: Gabriel Goller @ 2023-12-11  8:46 UTC (permalink / raw)
  To: Dietmar Maurer, Proxmox Backup Server development discussion


On 12/7/23 18:33, Dietmar Maurer wrote:
> But why is it still i64? I would expect u64.
Yep, that makes more sense :)
I'll submit a v2.




^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-12-11  8:46 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-07 14:35 [pbs-devel] [PATCH proxmox-backup 0/2] Fix DatastoreListSummary on limited permissions Gabriel Goller
2023-12-07 14:35 ` [pbs-devel] [PATCH proxmox-backup 1/2] ui: datastore summary handle non-existent values Gabriel Goller
2023-12-07 14:35 ` [pbs-devel] [PATCH proxmox-backup 2/2] status: use Option on avail/used datastore attrs Gabriel Goller
2023-12-07 17:07   ` Dietmar Maurer
2023-12-07 17:10     ` Dietmar Maurer
2023-12-07 17:33   ` Dietmar Maurer
2023-12-11  8:46     ` Gabriel Goller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal