[pbs-devel] [PATCH proxmox-backup v3 1/3] fix #4077: Estimated Full metric on ext4 file systems

[pbs-devel] [PATCH proxmox-backup v3 1/3] fix #4077: Estimated Full metric on ext4 file systems

[Daniel Tschlatscher]

The rrd data now includes tracking the total disk usage for the unpri-
vileged backup user. The calculation for the estimated_time_full was
adapted to use the total for the unpriviliged user.

The unpriv_total is the sum of the used space in the file system, plus
the available space for the unpriviliged user.

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
Changes from v2:
* Rebased this patch for the current master

 src/api2/status.rs              | 3 ++-
 src/bin/proxmox-backup-proxy.rs | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/api2/status.rs b/src/api2/status.rs
index d6dec9d2..650dde98 100644
--- a/src/api2/status.rs
+++ b/src/api2/status.rs
@@ -84,7 +84,8 @@ pub async fn datastore_status(
         let get_rrd =
             |what: &str| extract_rrd_data(&rrd_dir, what, RRDTimeFrame::Month, RRDMode::Average);
 
-        let total_res = get_rrd("total")?;
+        // Use the space for the unpriviliged user, as e.g. ext4 reserves 5% of disks for root only
+        let total_res = get_rrd("unpriv_total")?;
         let used_res = get_rrd("used")?;
 
         if let (Some((start, reso, total_list)), Some((_, _, used_list))) = (total_res, used_res) {
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index 4c57b2dd..b91aa66d 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -1273,6 +1273,8 @@ fn rrd_update_disk_stat(disk: &DiskStat, rrd_prefix: &str) {
         rrd_update_gauge(&rrd_key, status.total as f64);
         let rrd_key = format!("{}/used", rrd_prefix);
         rrd_update_gauge(&rrd_key, status.used as f64);
+        let rrd_key = format!("{}/unpriv_total", rrd_prefix);
+        rrd_update_gauge(&rrd_key, (status.used + status.available) as f64);
     }
 
     if let Some(stat) = &disk.dev {
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v3 2/3] expose the unpriviliged total in the api and use it in the GUI

[Daniel Tschlatscher]

Until now, the total size for a datastore was reported as the total
space on the filesystem. On ext4 filesystems this number will not
match the observed behaviour when some amount of blocks are reserved
though, as the proxmox-backup-proxy uses the unpriviliged 'backup'
user. Therefore backup-ing new data would fail, even though the GUI
still displays that X% of the datastore is free.
I think using the unpriviliged total makes more sense as it
communicates to the user how much they can actually still store on the
datastore, rather than the full total of which some part might not be
usable.

This will also not lead to issues in the GUI when the reserved space
is written to, because the value reported by statfs will automatically
increase accordingly.
I.e. when the unprivliged space is full and a 500MB file is written by
the root user, the fields for "unpriv_total" and "used" will both
increase by this amount, simply keeping the usage at 100%.

Note: I've opted to create a new field in the API result for the
unpriviliged total of the datastore, as overwriting the existing total
might break compatibility for users who retrieve the actual datastore
disk size this way.

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
Changes from v2:
* Rebased this patch for the current master
* Additionally, I found that there is a much simpler way to calculate
  the total size available to unprivileged users. (used + available)
  This also means that it's no longer necessary to make the calculation
  where the statfs call happens, but rather I've chosen to keep it as a
  new API return value, "unpriv_total".
  However, one could also just calculate this in the frontend, while
  leaving this parameter out of the API completely, if this is a more
  desired state.

 pbs-api-types/src/datastore.rs        | 5 +++++
 src/api2/admin/datastore.rs           | 3 +++
 src/api2/status.rs                    | 1 +
 www/dashboard/DataStoreStatistics.js  | 6 +++---
 www/datastore/DataStoreListSummary.js | 4 ++--
 5 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index fbd01fc4..6283dd06 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -1235,6 +1235,8 @@ pub struct GarbageCollectionStatus {
 pub struct DataStoreStatus {
     /// Total space (bytes).
     pub total: u64,
+    /// Total space available to unpriviliged users (bytes)
+    pub unpriv_total: u64,
     /// Used space (bytes).
     pub used: u64,
     /// Available space (bytes).
@@ -1269,6 +1271,8 @@ pub struct DataStoreStatusListItem {
     pub store: String,
     /// The Size of the underlying storage in bytes. (-1 on error)
     pub total: i64,
+    /// The total space available to unpriviliged users (-1 on error)
+    pub unpriv_total: i64,
     /// The used bytes of the underlying storage. (-1 on error)
     pub used: i64,
     /// The available bytes of the underlying storage. (-1 on error)
@@ -1301,6 +1305,7 @@ impl DataStoreStatusListItem {
         DataStoreStatusListItem {
             store: store.to_owned(),
             total: -1,
+            unpriv_total: -1,
             used: -1,
             avail: -1,
             history: None,
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index ff2f1f87..1fdccc1d 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -715,12 +715,14 @@ pub async fn status(
             total: storage.total,
             used: storage.used,
             avail: storage.available,
+            unpriv_total: storage.used + storage.available,
             gc_status,
             counts,
         }
     } else {
         DataStoreStatus {
             total: 0,
+            unpriv_total: 0,
             used: 0,
             avail: 0,
             gc_status,
@@ -1805,6 +1807,7 @@ pub fn get_rrd_stats(
 
     let mut rrd_fields = vec![
         "total",
+        "unpriv_total",
         "used",
         "read_ios",
         "read_bytes",
diff --git a/src/api2/status.rs b/src/api2/status.rs
index 650dde98..3db4df2c 100644
--- a/src/api2/status.rs
+++ b/src/api2/status.rs
@@ -69,6 +69,7 @@ pub async fn datastore_status(
         let mut entry = DataStoreStatusListItem {
             store: store.clone(),
             total: status.total as i64,
+            unpriv_total: (status.used + status.available) as i64,
             used: status.used as i64,
             avail: status.available as i64,
             history: None,
diff --git a/www/dashboard/DataStoreStatistics.js b/www/dashboard/DataStoreStatistics.js
index 38f7a2fe..8dbd1caf 100644
--- a/www/dashboard/DataStoreStatistics.js
+++ b/www/dashboard/DataStoreStatistics.js
@@ -3,7 +3,7 @@ Ext.define('pbs-datastore-statistics', {
 
     fields: [
 	'store',
-	'total',
+	'unpriv_total',
 	'used',
 	'avail',
 	'estimated-full-date',
@@ -27,7 +27,7 @@ Ext.define('pbs-datastore-statistics', {
 	    name: 'usage',
 	    calculate: function(data) {
 		let used = data.used || 0;
-		let total = data.total || 0;
+		let total = data["unpriv-total"] || 0;
 		if (total > 0) {
 		    return used/total;
 		} else {
@@ -78,7 +78,7 @@ Ext.define('PBS.DatastoreStatistics', {
 	},
 	{
 	    text: gettext('Size'),
-	    dataIndex: 'total',
+	    dataIndex: 'unpriv-total',
 	    sortable: true,
 	    width: 90,
 	    renderer: v => v === undefined || v < 0 ? '-' : Proxmox.Utils.format_size(v, true),
diff --git a/www/datastore/DataStoreListSummary.js b/www/datastore/DataStoreListSummary.js
index c7b67d56..3714528e 100644
--- a/www/datastore/DataStoreListSummary.js
+++ b/www/datastore/DataStoreListSummary.js
@@ -52,10 +52,10 @@ Ext.define('PBS.datastore.DataStoreListSummary', {
 	    vm.set('maintenance', '');
 	}
 
-	let usage = statusData.used/statusData.total;
+	let usage = statusData.used/statusData["unpriv-total"];
 	let usagetext = Ext.String.format(gettext('{0} of {1}'),
 	    Proxmox.Utils.format_size(statusData.used, true),
-	    Proxmox.Utils.format_size(statusData.total, true),
+	    Proxmox.Utils.format_size(statusData["unpriv-total"], true),
 	);
 
 	let usagePanel = me.lookup('usage');
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v3 3/3] gui: change reporting of the estimated_time_full to "Full" if no space

[Daniel Tschlatscher]

is left in the datastore. Before, the GUI would report "Never" for the
estimated time full, because the value provided in the backend was in
the past. To get around this, the GUI now report "Full" if the value
for available reaches 0.

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
Changes from v2:
* Rebased this patch for the current master

 www/dashboard/DataStoreStatistics.js  | 8 +++++++-
 www/datastore/DataStoreListSummary.js | 5 ++++-
 www/datastore/Summary.js              | 6 +++---
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/www/dashboard/DataStoreStatistics.js b/www/dashboard/DataStoreStatistics.js
index 8dbd1caf..daac461d 100644
--- a/www/dashboard/DataStoreStatistics.js
+++ b/www/dashboard/DataStoreStatistics.js
@@ -118,7 +118,13 @@ Ext.define('PBS.DatastoreStatistics', {
 	    text: gettext('Estimated Full'),
 	    dataIndex: 'estimated-full-date',
 	    sortable: true,
-	    renderer: PBS.Utils.render_estimate,
+	    renderer: (value, metaData, record, rowIndex, colIndex, store) => {
+		if (record.get("avail") === 0) {
+		    return gettext("Full");
+		}
+
+		return PBS.Utils.render_estimate(value);
+	    },
 	    flex: 1,
 	    minWidth: 130,
 	    maxWidth: 200,
diff --git a/www/datastore/DataStoreListSummary.js b/www/datastore/DataStoreListSummary.js
index 3714528e..60ad0461 100644
--- a/www/datastore/DataStoreListSummary.js
+++ b/www/datastore/DataStoreListSummary.js
@@ -61,7 +61,10 @@ Ext.define('PBS.datastore.DataStoreListSummary', {
 	let usagePanel = me.lookup('usage');
 	usagePanel.updateValue(usage, usagetext);
 
-	let estimate = PBS.Utils.render_estimate(statusData['estimated-full-date']);
+	let estimate = statusData.avail > 0
+	    ? PBS.Utils.render_estimate(statusData['estimated-full-date'])
+	    : gettext("Full");
+
 	vm.set('full', estimate);
 	vm.set('deduplication', PBS.Utils.calculate_dedup_factor(statusData['gc-status']).toFixed(2));
 	vm.set('stillbad', statusData['gc-status']['still-bad']);
diff --git a/www/datastore/Summary.js b/www/datastore/Summary.js
index 94be9559..4025949c 100644
--- a/www/datastore/Summary.js
+++ b/www/datastore/Summary.js
@@ -2,7 +2,7 @@ Ext.define('pve-rrd-datastore', {
     extend: 'Ext.data.Model',
     fields: [
 	'used',
-	'total',
+	'unpriv_total',
 	'read_ios',
 	'read_bytes',
 	'write_ios',
@@ -66,7 +66,7 @@ Ext.define('PBS.DataStoreInfo', {
 	    let vm = me.getViewModel();
 
 	    let counts = store.getById('counts').data.value;
-	    let total = store.getById('total').data.value;
+	    let total = store.getById('unpriv-total').data.value;
 	    let used = store.getById('used').data.value;
 
 	    let usage = Proxmox.Utils.render_size_usage(used, total, true);
@@ -236,7 +236,7 @@ Ext.define('PBS.DataStoreSummary', {
 	{
 	    xtype: 'proxmoxRRDChart',
 	    title: gettext('Storage usage (bytes)'),
-	    fields: ['total', 'used'],
+	    fields: ['unpriv_total', 'used'],
 	    fieldTitles: [gettext('Total'), gettext('Storage usage')],
 	},
 	{
-- 
2.30.2

Re: [pbs-devel] [PATCH proxmox-backup v3 2/3] expose the unpriviliged total in the api and use it in the GUI

[Matthias Heiserer]

On 26.07.2022 15:44, Daniel Tschlatscher wrote:
> Until now, the total size for a datastore was reported as the total
> space on the filesystem. On ext4 filesystems this number will not
> match the observed behaviour when some amount of blocks are reserved
> though, as the proxmox-backup-proxy uses the unpriviliged 'backup'
> user. Therefore backup-ing new data would fail, even though the GUI
> still displays that X% of the datastore is free.
> I think using the unpriviliged total makes more sense as it
> communicates to the user how much they can actually still store on the
> datastore, rather than the full total of which some part might not be
> usable.
> 
> This will also not lead to issues in the GUI when the reserved space
> is written to, because the value reported by statfs will automatically
> increase accordingly.
> I.e. when the unprivliged space is full and a 500MB file is written by
> the root user, the fields for "unpriv_total" and "used" will both
> increase by this amount, simply keeping the usage at 100%.
> 
> Note: I've opted to create a new field in the API result for the
> unpriviliged total of the datastore, as overwriting the existing total
> might break compatibility for users who retrieve the actual datastore
> disk size this way.
It's "privileged" :)

Looks good to me. I filled up the datastore on a pbs, and it said "Full".

Tested-by: Matthias Heiserer <m.heiserer@proxmox.com>

Re: [pbs-devel] [PATCH proxmox-backup v3 3/3] gui: change reporting of the estimated_time_full to "Full" if no space

[Matthias Heiserer]

On 26.07.2022 15:44, Daniel Tschlatscher wrote:
> is left in the datastore. Before, the GUI would report "Never" for the
> estimated time full, because the value provided in the backend was in
> the past. To get around this, the GUI now report "Full" if the value
> for available reaches 0.
> 
> Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
> ---
> Changes from v2:
> * Rebased this patch for the current master
> 
>   www/dashboard/DataStoreStatistics.js  | 8 +++++++-
>   www/datastore/DataStoreListSummary.js | 5 ++++-
>   www/datastore/Summary.js              | 6 +++---
>   3 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/www/dashboard/DataStoreStatistics.js b/www/dashboard/DataStoreStatistics.js
> index 8dbd1caf..daac461d 100644
> --- a/www/dashboard/DataStoreStatistics.js
> +++ b/www/dashboard/DataStoreStatistics.js
> @@ -118,7 +118,13 @@ Ext.define('PBS.DatastoreStatistics', {
>   	    text: gettext('Estimated Full'),
>   	    dataIndex: 'estimated-full-date',
>   	    sortable: true,
> -	    renderer: PBS.Utils.render_estimate,
> +	    renderer: (value, metaData, record, rowIndex, colIndex, store) => {
> +		if (record.get("avail") === 0) {
> +		    return gettext("Full");
> +		}
> +
> +		return PBS.Utils.render_estimate(value);
> +	    },
Here...
>   	    flex: 1,
>   	    minWidth: 130,
>   	    maxWidth: 200,
> diff --git a/www/datastore/DataStoreListSummary.js b/www/datastore/DataStoreListSummary.js
> index 3714528e..60ad0461 100644
> --- a/www/datastore/DataStoreListSummary.js
> +++ b/www/datastore/DataStoreListSummary.js
> @@ -61,7 +61,10 @@ Ext.define('PBS.datastore.DataStoreListSummary', {
>   	let usagePanel = me.lookup('usage');
>   	usagePanel.updateValue(usage, usagetext);
>   
> -	let estimate = PBS.Utils.render_estimate(statusData['estimated-full-date']);
> +	let estimate = statusData.avail > 0
> +	    ? PBS.Utils.render_estimate(statusData['estimated-full-date'])
> +	    : gettext("Full");
> +
... and here, you do the same. I would move the check if the datastore 
is full into render_estimate and pass it value and available.
>   	vm.set('full', estimate);
>   	vm.set('deduplication', PBS.Utils.calculate_dedup_factor(statusData['gc-status']).toFixed(2));
>   	vm.set('stillbad', statusData['gc-status']['still-bad']);
> diff --git a/www/datastore/Summary.js b/www/datastore/Summary.js
> index 94be9559..4025949c 100644
> --- a/www/datastore/Summary.js
> +++ b/www/datastore/Summary.js
> @@ -2,7 +2,7 @@ Ext.define('pve-rrd-datastore', {
>       extend: 'Ext.data.Model',
>       fields: [
>   	'used',
> -	'total',
> +	'unpriv_total',
>   	'read_ios',
>   	'read_bytes',
>   	'write_ios',
> @@ -66,7 +66,7 @@ Ext.define('PBS.DataStoreInfo', {
>   	    let vm = me.getViewModel();
>   
>   	    let counts = store.getById('counts').data.value;
> -	    let total = store.getById('total').data.value;
> +	    let total = store.getById('unpriv-total').data.value;
>   	    let used = store.getById('used').data.value;
>   
>   	    let usage = Proxmox.Utils.render_size_usage(used, total, true);
> @@ -236,7 +236,7 @@ Ext.define('PBS.DataStoreSummary', {
>   	{
>   	    xtype: 'proxmoxRRDChart',
>   	    title: gettext('Storage usage (bytes)'),
> -	    fields: ['total', 'used'],
> +	    fields: ['unpriv_total', 'used'],
>   	    fieldTitles: [gettext('Total'), gettext('Storage usage')],
>   	},
>   	{