[pbs-devel] [PATCH proxmox-backup 1/1] ext4 filesystems no longer reserve blocks when created as a datastore

[pbs-devel] [PATCH proxmox-backup 1/1] ext4 filesystems no longer reserve blocks when created as a datastore

[Daniel Tschlatscher]

Ext4 Filesystems reserve about 5% of all available blocks for usage by
the root user. While this can be helpful for root partitions and mount
points, it is significantly less so for datastores. Therefore, ext4
filesystems are created with no reserved blocks, increasing the
available storage for unpriviliged backup users.
Some more rationale can be found in #4077:
https://bugzilla.proxmox.com/show_bug.cgi?id=4077

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
 src/tools/disks/mod.rs | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/tools/disks/mod.rs b/src/tools/disks/mod.rs
index 568dccbf..9a19f0c1 100644
--- a/src/tools/disks/mod.rs
+++ b/src/tools/disks/mod.rs
@@ -979,10 +979,14 @@ pub fn create_file_system(disk: &Disk, fs_type: FileSystemType) -> Result<(), Er
         None => bail!("disk {:?} has no node in /dev", disk.syspath()),
     };
 
-    let fs_type = fs_type.to_string();
-
     let mut command = std::process::Command::new("mkfs");
-    command.args(&["-t", &fs_type]);
+
+    command.args(&["-t", &fs_type.to_string()]);
+
+    if matches!(fs_type, FileSystemType::Ext4) {
+        command.args(&["-m", "0"]);
+    }
+
     command.arg(disk_path);
 
     proxmox_sys::command::run_command(command, None)?;
-- 
2.30.2

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

[Daniel Tschlatscher]

The estimated full statistics metric no longer uses the total amount
of space on a disk, but rather the still available. Also refactored
some variable names for the 'estimated_full' calculation to make it
clearer what changed/is used now.

Note: Statistics for 'available' 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>
---
 src/api2/status.rs              | 14 +++++++-------
 src/bin/proxmox-backup-proxy.rs |  2 ++
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/api2/status.rs b/src/api2/status.rs
index 45aa7fd7..f7d96c49 100644
--- a/src/api2/status.rs
+++ b/src/api2/status.rs
@@ -88,25 +88,25 @@ 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")?;
+        let avail_res = get_rrd("available")?;
         let used_res = get_rrd("used")?;
 
-        if let (Some((start, reso, total_list)), Some((_, _, used_list))) = (total_res, used_res) {
+        if let (Some((start, reso, avail_list)), Some((_, _, used_list))) = (avail_res, used_res) {
             let mut usage_list: Vec<f64> = Vec::new();
             let mut time_list: Vec<u64> = Vec::new();
             let mut history = Vec::new();
 
             for (idx, used) in used_list.iter().enumerate() {
-                let total = if idx < total_list.len() {
-                    total_list[idx]
+                let available = if idx < avail_list.len() {
+		    avail_list[idx]
                 } else {
                     None
                 };
 
-                match (total, used) {
-                    (Some(total), Some(used)) if total != 0.0 => {
+                match (available, used) {
+                    (Some(available), Some(used)) if available != 0.0 => {
                         time_list.push(start + (idx as u64) * reso);
-                        let usage = used / total;
+                        let usage = used / available;
                         usage_list.push(usage);
                         history.push(Some(usage));
                     }
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index 659f7b4a..82f40a54 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -1140,6 +1140,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!("{}/available", rrd_prefix);
+            rrd_update_gauge(&rrd_key, status.avail as f64);
         }
         Err(err) => {
             eprintln!("read disk_usage on {:?} failed - {}", path, err);
-- 
2.30.2

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

[Daniel Tschlatscher]

Talked with Dominic off-list:

I had a bit of an error in reasoning with the usage of 'available' here 
which means
it does not work quite as I expected here.

Therefore, please ignore this patch. I will try to re(think) and rewrite 
this patch in v2.

The first patch should still be applicable though, as it does not impact 
any of this.

On 5/27/22 12:57, Daniel Tschlatscher wrote:
> The estimated full statistics metric no longer uses the total amount
> of space on a disk, but rather the still available. Also refactored
> some variable names for the 'estimated_full' calculation to make it
> clearer what changed/is used now.
>
> Note: Statistics for 'available' 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>
> ---
>   src/api2/status.rs              | 14 +++++++-------
>   src/bin/proxmox-backup-proxy.rs |  2 ++
>   2 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/src/api2/status.rs b/src/api2/status.rs
> index 45aa7fd7..f7d96c49 100644
> --- a/src/api2/status.rs
> +++ b/src/api2/status.rs
> @@ -88,25 +88,25 @@ 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")?;
> +        let avail_res = get_rrd("available")?;
>           let used_res = get_rrd("used")?;
>   
> -        if let (Some((start, reso, total_list)), Some((_, _, used_list))) = (total_res, used_res) {
> +        if let (Some((start, reso, avail_list)), Some((_, _, used_list))) = (avail_res, used_res) {
>               let mut usage_list: Vec<f64> = Vec::new();
>               let mut time_list: Vec<u64> = Vec::new();
>               let mut history = Vec::new();
>   
>               for (idx, used) in used_list.iter().enumerate() {
> -                let total = if idx < total_list.len() {
> -                    total_list[idx]
> +                let available = if idx < avail_list.len() {
> +		    avail_list[idx]
>                   } else {
>                       None
>                   };
>   
> -                match (total, used) {
> -                    (Some(total), Some(used)) if total != 0.0 => {
> +                match (available, used) {
> +                    (Some(available), Some(used)) if available != 0.0 => {
>                           time_list.push(start + (idx as u64) * reso);
> -                        let usage = used / total;
> +                        let usage = used / available;
>                           usage_list.push(usage);
>                           history.push(Some(usage));
>                       }
> diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
> index 659f7b4a..82f40a54 100644
> --- a/src/bin/proxmox-backup-proxy.rs
> +++ b/src/bin/proxmox-backup-proxy.rs
> @@ -1140,6 +1140,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!("{}/available", rrd_prefix);
> +            rrd_update_gauge(&rrd_key, status.avail as f64);
>           }
>           Err(err) => {
>               eprintln!("read disk_usage on {:?} failed - {}", path, err);