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

[pbs-devel] [PATCH proxmox-backup v2 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 calculation for "unpriv_total" is as follows:
= (total blocks - (free blocks in fs - avail for unpriv user)) * bsize

Note: Statistics for 'unpriv_total' were not tracked in the rrdb
before. Existing datastores will therefore show a somewhat incorrect
value for 'estimated_full', at least until it is populated with enough
new values.

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
Changes from v1:
* 1/3 Should now use the correct values for the calculation of the
  estimated time full
* 1/3 Started tracking the unprivileged total in the backend disk
  status types and in the rrdb
* 2/3 Revised the usage of "total" in the GUI to display the total
  left for unprivileged users
* 3/3 Fixed displaying "Never" when a datastore was full. Now displays
  "Full" instead

 pbs-api-types/src/lib.rs        | 2 ++
 src/api2/status.rs              | 3 ++-
 src/bin/proxmox-backup-proxy.rs | 2 ++
 src/tools/disks/mod.rs          | 1 +
 4 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/pbs-api-types/src/lib.rs b/pbs-api-types/src/lib.rs
index d9c8cee1..629a6812 100644
--- a/pbs-api-types/src/lib.rs
+++ b/pbs-api-types/src/lib.rs
@@ -345,6 +345,8 @@ pub struct StorageStatus {
     pub used: u64,
     /// Available space (bytes).
     pub avail: u64,
+    /// Total space for the unpriviliged user (bytes)
+    pub unpriv_total: u64,
 }
 
 pub const PASSWORD_HINT_SCHEMA: Schema = StringSchema::new("Password hint.")
diff --git a/src/api2/status.rs b/src/api2/status.rs
index 55c811a5..3575932b 100644
--- a/src/api2/status.rs
+++ b/src/api2/status.rs
@@ -84,7 +84,8 @@ pub 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 aadd7c93..d25aa08f 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -1129,6 +1129,8 @@ fn gather_disk_stats(disk_manager: Arc<DiskManage>, path: &Path, rrd_prefix: &st
             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.unpriv_total as f64);
         }
         Err(err) => {
             eprintln!("read disk_usage on {:?} failed - {}", path, err);
diff --git a/src/tools/disks/mod.rs b/src/tools/disks/mod.rs
index 568dccbf..08f516df 100644
--- a/src/tools/disks/mod.rs
+++ b/src/tools/disks/mod.rs
@@ -543,6 +543,7 @@ pub fn disk_usage(path: &std::path::Path) -> Result<StorageStatus, Error> {
         total: stat.f_blocks * bsize,
         used: (stat.f_blocks - stat.f_bfree) * bsize,
         avail: stat.f_bavail * bsize,
+        unpriv_total: (stat.f_blocks - (stat.f_bfree - stat.f_bavail)) * bsize,
     })
 }
 
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 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
blocks on the filesystem. On ext4 filesystems this number will not
match the observed behaviour though when some amount of blocks are
reserved, as the proxmox-backup-proxy uses the unpriviliged 'backup'
user. Therefore backuping 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, both the 'unpriv_total' and 'available' fields will
increase by this amount, keeping the usage at 100%.

Note: While the value for total is no longer used in the GUI, I did
not overwrite it in the backend as it could break existing code which
might depend on the API returning the total byte size for the
underlying disk.

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
 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 bb20e149..fcf21ff1 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -1250,6 +1250,8 @@ impl Default for GarbageCollectionStatus {
 pub struct DataStoreStatus {
     /// Total space (bytes).
     pub total: u64,
+    /// Total unreserved space (bytes)
+    pub unpriv_total: u64,
     /// Used space (bytes).
     pub used: u64,
     /// Available space (bytes).
@@ -1284,6 +1286,8 @@ pub struct DataStoreStatusListItem {
     pub store: String,
     /// The Size of the underlying storage in bytes. (-1 on error)
     pub total: i64,
+    /// The Size of the total blocks available to the unpriviliged user (-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)
@@ -1316,6 +1320,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 44208a4c..52488732 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -678,6 +678,7 @@ pub fn status(
         let storage = crate::tools::disks::disk_usage(&datastore.base_path())?;
         DataStoreStatus {
             total: storage.total,
+            unpriv_total: storage.unpriv_total,
             used: storage.used,
             avail: storage.avail,
             gc_status,
@@ -686,6 +687,7 @@ pub fn status(
     } else {
         DataStoreStatus {
             total: 0,
+            unpriv_total: 0,
             used: 0,
             avail: 0,
             gc_status,
@@ -1768,6 +1770,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 3575932b..d9ccc97a 100644
--- a/src/api2/status.rs
+++ b/src/api2/status.rs
@@ -69,6 +69,7 @@ pub fn datastore_status(
         let mut entry = DataStoreStatusListItem {
             store: store.clone(),
             total: status.total as i64,
+            unpriv_total: status.unpriv_total as i64,
             used: status.used as i64,
             avail: status.avail 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 v2 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>
---
 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