[pbs-devel] [RFC proxmox-backup v3 0/2] more flexible HumanByte type

[pbs-devel] [RFC proxmox-backup v3 0/2] more flexible HumanByte type

[Dietmar Maurer]

Changes since v2:

 - reverted changes from v2

 - restrict number of possibleunits (always store KB instead of K)
 
 - store size without scaling to unit (avoid rounding errors in
   deserialze/serialze)

 - allow to specify precision in Display

 - new with_unit() constructor

 - improved dev docs

Changes since v1:

- do not expose auto_unit functions
- store size as u64 (suggested by thomas)
- do not store unit (suggested by thomas)


*** BLURB HERE ***

Dietmar Maurer (2):
  pbs-api-types: more flexible HumanByte type
  use HumanByte for traffic-control config

 pbs-api-types/src/human_byte.rs      | 353 +++++++++++++++++++++++++++
 pbs-api-types/src/lib.rs             |   3 +
 pbs-api-types/src/traffic_control.rs |  18 +-
 pbs-client/src/backup_writer.rs      |  19 +-
 pbs-datastore/src/datastore.rs       |  13 +-
 pbs-tools/Cargo.toml                 |   1 +
 pbs-tools/src/format.rs              |  55 +----
 src/bin/proxmox-tape.rs              |   7 +-
 src/cached_traffic_control.rs        |  18 +-
 src/server/email_notifications.rs    |   5 +-
 10 files changed, 403 insertions(+), 89 deletions(-)
 create mode 100644 pbs-api-types/src/human_byte.rs

-- 
2.30.2

[pbs-devel] [RFC proxmox-backup v3 1/2] pbs-api-types: more flexible HumanByte type

[Dietmar Maurer]

This implements a parser for human readabkle byte sizes.

Examples: "1KB", "1.5 Mib", "3T"

Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
---
 pbs-api-types/src/human_byte.rs   | 353 ++++++++++++++++++++++++++++++
 pbs-api-types/src/lib.rs          |   3 +
 pbs-client/src/backup_writer.rs   |  19 +-
 pbs-datastore/src/datastore.rs    |  13 +-
 pbs-tools/Cargo.toml              |   1 +
 pbs-tools/src/format.rs           |  55 +----
 src/bin/proxmox-tape.rs           |   7 +-
 src/server/email_notifications.rs |   5 +-
 8 files changed, 382 insertions(+), 74 deletions(-)
 create mode 100644 pbs-api-types/src/human_byte.rs

diff --git a/pbs-api-types/src/human_byte.rs b/pbs-api-types/src/human_byte.rs
new file mode 100644
index 00000000..95435961
--- /dev/null
+++ b/pbs-api-types/src/human_byte.rs
@@ -0,0 +1,353 @@
+use std::str::FromStr;
+
+use anyhow::{bail, Error};
+
+use proxmox_schema::{ApiStringFormat, ApiType, StringSchema, Schema, UpdaterType};
+
+/// Size units for byte sizes
+#[derive(Debug,Copy,Clone, PartialEq)]
+pub enum SizeUnit {
+    None,
+    KByte,
+    MByte,
+    GByte,
+    TByte,
+    PByte,
+    Kibi,
+    Mebi,
+    Gibi,
+    Tebi,
+    Pebi,
+}
+
+impl SizeUnit {
+    /// Resturns the scaling factor
+    pub fn factor(&self) -> f64 {
+        match self {
+            SizeUnit::None => 1.0,
+
+            SizeUnit::KByte  => 1_000.0,
+            SizeUnit::MByte => 1_000_000.0,
+            SizeUnit::GByte => 1_000_000_000.0,
+            SizeUnit::TByte => 1_000_000_000_000.0,
+            SizeUnit::PByte => 1_000_000_000_000_000.0,
+
+            SizeUnit::Kibi => 1024.0,
+            SizeUnit::Mebi => 1024.0*1024.0,
+            SizeUnit::Gibi => 1024.0*1024.0*1024.0,
+            SizeUnit::Tebi => 1024.0*1024.0*1024.0*1024.0,
+            SizeUnit::Pebi => 1024.0*1024.0*1024.0*1024.0*1024.0,
+        }
+    }
+
+    /// Resturns the string repesentation
+    pub fn unit_str(&self) -> &'static str {
+        match self {
+            SizeUnit::None => "",
+
+            SizeUnit::KByte => "KB",
+            SizeUnit::MByte => "MB",
+            SizeUnit::GByte => "GB",
+            SizeUnit::TByte => "TB",
+            SizeUnit::PByte => "PB",
+
+            SizeUnit::Kibi => "KiB",
+            SizeUnit::Mebi => "MiB",
+            SizeUnit::Gibi => "GiB",
+            SizeUnit::Tebi => "TiB",
+            SizeUnit::Pebi => "PiB",
+       }
+    }
+}
+
+fn strip_unit(v: &str) -> (&str, SizeUnit) {
+
+    if let Some(v) = v.strip_suffix(&['k', 'K'][..]) {
+        return (v, SizeUnit::KByte);
+    }
+    if let Some(v) = v.strip_suffix(&['m', 'M'][..]) {
+        return (v, SizeUnit::MByte);
+    }
+    if let Some(v) = v.strip_suffix(&['g', 'G'][..]) {
+        return (v, SizeUnit::GByte);
+    }
+    if let Some(v) = v.strip_suffix(&['t', 'T'][..]) {
+        return (v, SizeUnit::TByte);
+    }
+    if let Some(v) = v.strip_suffix(&['p', 'P'][..]) {
+        return (v, SizeUnit::PByte);
+    }
+
+    if let Some(mut v) = v.strip_suffix(&['b', 'B'][..]) {
+
+        let binary = if let Some(n) = v.strip_suffix('i') {
+            v = n;
+            true
+        } else {
+            false
+        };
+
+        if let Some(v) = v.strip_suffix(&['k', 'K'][..]) {
+            return (v, if binary { SizeUnit::Kibi } else { SizeUnit::KByte });
+        }
+        if let Some(v) = v.strip_suffix(&['m', 'M'][..]) {
+            return (v, if binary { SizeUnit::Mebi } else { SizeUnit::MByte });
+        }
+        if let Some(v) = v.strip_suffix(&['g', 'G'][..]) {
+            return (v, if binary { SizeUnit::Gibi } else { SizeUnit::GByte });
+        }
+        if let Some(v) = v.strip_suffix(&['t', 'T'][..]) {
+            return (v, if binary { SizeUnit::Tebi } else { SizeUnit::TByte });
+        }
+        if let Some(v) = v.strip_suffix(&['p', 'P'][..]) {
+            return (v, if binary { SizeUnit::Pebi } else { SizeUnit::PByte });
+        }
+        return (v, SizeUnit::None);
+    }
+
+    return (v, SizeUnit::None);
+}
+
+#[derive(Debug, Copy, Clone, UpdaterType)]
+/// Byte size with unit
+pub struct HumanByte {
+    size: f64, // factor not applied
+    unit: SizeUnit,
+}
+
+fn verify_human_byte(s: &str) -> Result<(), Error> {
+    match s.parse::<HumanByte>() {
+        Ok(_) => Ok(()),
+        Err(err) => bail!("byte size parse error: {}", err),
+    }
+}
+
+impl ApiType for HumanByte {
+    const API_SCHEMA: Schema = StringSchema::new("Byte size with optional unit (KB, MB, GB, TB, PB, KiB, MiB, Gib, ...).")
+        .format(&ApiStringFormat::VerifyFn(verify_human_byte))
+        .min_length(1)
+        .max_length(64)
+        .schema();
+}
+
+impl HumanByte {
+
+    /// Create instance with size and unit (size must be positive)
+    pub fn with_unit(size: f64, unit: SizeUnit) -> Result<Self, Error> {
+        if size < 0.0 {
+            bail!("byte size may not be negative");
+        }
+        Ok(HumanByte { size, unit })
+    }
+
+    /// Create a new instance with optimal decimal unit computed
+    pub fn new_decimal(size: u64) -> Self {
+        let this = HumanByte { size: size as f64, unit: SizeUnit::None };
+        this.auto_unit_decimal()
+    }
+
+    /// Create a new instance with optimal binary unit computed
+    pub fn new_binary(size: u64) -> Self {
+        let this = HumanByte { size: size as f64, unit: SizeUnit::None };
+        this.auto_unit_binary()
+    }
+
+    /// Returns the scaled size as u64
+    pub fn as_u64(&self) -> u64 {
+        self.as_f64() as u64
+    }
+
+    /// Returns the scaled size as u64
+    pub fn as_f64(&self) -> f64 {
+        self.size * self.unit.factor()
+    }
+
+    /// Returns a copy with optimal binary unit computed
+    pub fn auto_unit_binary(mut self) -> Self {
+        let size = self.as_f64();
+        let zeroes =  (size as u64).leading_zeros();
+        let bits = 63 - zeroes;
+        self.unit = if bits >= 50 {
+            SizeUnit::Pebi
+        } else if bits >= 40 {
+            SizeUnit::Tebi
+        } else if bits >= 30 {
+            SizeUnit::Gibi
+        } else if bits >= 20 {
+            SizeUnit::Mebi
+        } else if bits >= 10 {
+            SizeUnit::Kibi
+        } else {
+            SizeUnit::None
+        };
+        self.size = size / self.unit.factor();
+        self
+    }
+
+    /// Returns a copy with optimal decimal unit computed
+    pub fn auto_unit_decimal(mut self) -> Self {
+        let size = self.as_f64();
+        self.unit = if size >= 1_000_000_000_000_000.0 {
+            SizeUnit::PByte
+        } else if size >= 1_000_000_000_000.0 {
+            SizeUnit::TByte
+        } else if size >= 1_000_000_000.0 {
+            SizeUnit::GByte
+        } else if size >= 1_000_000.0 {
+            SizeUnit::MByte
+        } else if size >= 1_000.0 {
+            SizeUnit::KByte
+        } else {
+            SizeUnit::None
+        };
+        self.size = size / self.unit.factor();
+        self
+    }
+}
+
+impl std::fmt::Display for HumanByte {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let unit = self.unit;
+
+        let unit_str = unit.unit_str();
+
+        let precision = f.precision().unwrap_or(3);
+        let mut pf = 1.0; for _ in 0..precision { pf = pf * 10.0; }
+
+        let size = self.size;
+        let size = ((size*pf).floor())/pf;
+        if unit_str.is_empty() {
+            write!(f, "{}", size)
+        } else {
+            write!(f, "{} {}", size, unit.unit_str())
+        }
+    }
+}
+
+impl FromStr for HumanByte {
+    type Err = Error;
+
+    fn from_str(v: &str) -> Result<Self, Error> {
+        let (mut v, unit) = strip_unit(v);
+
+        if unit != SizeUnit::None {
+            v = v.trim_end();
+        }
+        let size: f64 = v.parse()?;
+
+        let h = HumanByte::with_unit(size, unit)?;
+        Ok(h)
+    }
+}
+
+proxmox::forward_deserialize_to_from_str!(HumanByte);
+proxmox::forward_serialize_to_display!(HumanByte);
+
+#[test]
+fn test_human_byte_parser() -> Result<(), Error> {
+
+    assert!("-10".parse::<HumanByte>().is_err()); // negative size
+
+    fn test(v: &str, size: f64, unit: SizeUnit, as_str: &str) -> Result<(), Error> {
+        let h: HumanByte = v.parse()?;
+
+        if h.size != size {
+            bail!("got unexpected size for '{}' ({} != {})", v, h.size, size);
+        }
+        if h.unit != unit {
+            bail!("got unexpected unit for '{}' ({:?} != {:?})", v, h.unit, unit);
+        }
+
+        let new = h.to_string();
+        if &new != as_str {
+            bail!("to_string failed for '{}' ({:?} != {:?})", v, new, as_str);
+
+        }
+
+        Ok(())
+    }
+
+    test("14", 14.0, SizeUnit::None, "14")?;
+    test("14.4", 14.4, SizeUnit::None, "14.4")?;
+    test("14.45", 14.45, SizeUnit::None, "14.45")?;
+    test("14.456", 14.456, SizeUnit::None, "14.456")?;
+    test("14.4567", 14.4567, SizeUnit::None, "14.456")?;
+
+    let h: HumanByte = "1.2345678".parse()?;
+    assert_eq!(&format!("{:.0}", h), "1");
+    assert_eq!(&format!("{:.1}", h), "1.2");
+    assert_eq!(&format!("{:.2}", h), "1.23");
+    assert_eq!(&format!("{:.3}", h), "1.234");
+    assert_eq!(&format!("{:.4}", h), "1.2345");
+    assert_eq!(&format!("{:.5}", h), "1.23456");
+    assert_eq!(&format!("{:.6}", h), "1.234567");
+    assert_eq!(&format!("{:.7}", h), "1.2345678");
+    assert_eq!(&format!("{:.8}", h), "1.2345678");
+
+    test("987654321", 987654321.0, SizeUnit::None, "987654321")?;
+
+    test("1300b", 1300.0, SizeUnit::None, "1300")?;
+    test("1300B", 1300.0, SizeUnit::None, "1300")?;
+
+    test("1.5KB", 1.5, SizeUnit::KByte, "1.5 KB")?;
+    test("1.5kb", 1.5, SizeUnit::KByte, "1.5 KB")?;
+    test("1.654321MB", 1.654_321, SizeUnit::MByte, "1.654 MB")?;
+
+    test("2.0GB", 2.0, SizeUnit::GByte, "2 GB")?;
+
+    test("1.4TB", 1.4, SizeUnit::TByte, "1.4 TB")?;
+    test("1.4tb", 1.4, SizeUnit::TByte, "1.4 TB")?;
+
+    test("2KiB", 2.0, SizeUnit::Kibi, "2 KiB")?;
+    test("2kib", 2.0, SizeUnit::Kibi, "2 KiB")?;
+
+    test("2.3456MiB", 2.3456, SizeUnit::Mebi, "2.345 MiB")?;
+
+    test("4gib", 4.0, SizeUnit::Gibi, "4 GiB")?;
+
+    Ok(())
+}
+
+#[test]
+fn test_human_byte_auto_unit_decimal() {
+    fn convert(b: u64) -> String {
+         HumanByte::new_decimal(b).to_string()
+    }
+    assert_eq!(convert(987), "987");
+    assert_eq!(convert(1022), "1.022 KB");
+    assert_eq!(convert(9_000), "9 KB");
+    assert_eq!(convert(1_000), "1 KB");
+    assert_eq!(convert(1_000_000), "1 MB");
+    assert_eq!(convert(1_000_000_000), "1 GB");
+    assert_eq!(convert(1_000_000_000_000), "1 TB");
+    assert_eq!(convert(1_000_000_000_000_000), "1 PB");
+
+    assert_eq!(convert((1<<30) + 103 * (1<<20)), "1.181 GB");
+    assert_eq!(convert((1<<30) + 128 * (1<<20)), "1.207 GB");
+    assert_eq!(convert((2<<50) + 500 * (1<<40)), "2.801 PB");
+}
+
+#[test]
+fn test_human_byte_auto_unit_binary() {
+    fn convert(b: u64) -> String {
+         HumanByte::new_binary(b).to_string()
+    }
+    assert_eq!(convert(987), "987");
+    assert_eq!(convert(1022), "1022");
+    assert_eq!(convert(9_000), "8.789 KiB");
+    assert_eq!(convert(10_000_000), "9.536 MiB");
+    assert_eq!(convert(10_000_000_000), "9.313 GiB");
+    assert_eq!(convert(10_000_000_000_000), "9.094 TiB");
+
+    assert_eq!(convert(1<<10), "1 KiB");
+    assert_eq!(convert((1<<10)*10), "10 KiB");
+    assert_eq!(convert(1<<20), "1 MiB");
+    assert_eq!(convert(1<<30), "1 GiB");
+    assert_eq!(convert(1<<40), "1 TiB");
+    assert_eq!(convert(1<<50), "1 PiB");
+
+    assert_eq!(convert((1<<30) + 103 * (1<<20)), "1.1 GiB");
+    assert_eq!(convert((1<<30) + 128 * (1<<20)), "1.125 GiB");
+    assert_eq!(convert((1<<40) + 128 * (1<<30)), "1.125 TiB");
+    assert_eq!(convert((2<<50) + 512 * (1<<40)), "2.5 PiB");
+}
diff --git a/pbs-api-types/src/lib.rs b/pbs-api-types/src/lib.rs
index 01c14cc4..daf8fd54 100644
--- a/pbs-api-types/src/lib.rs
+++ b/pbs-api-types/src/lib.rs
@@ -39,6 +39,9 @@ pub use acl::*;
 mod datastore;
 pub use datastore::*;
 
+mod human_byte;
+pub use human_byte::HumanByte;
+
 mod jobs;
 pub use jobs::*;
 
diff --git a/pbs-client/src/backup_writer.rs b/pbs-client/src/backup_writer.rs
index 531de2e8..98f78652 100644
--- a/pbs-client/src/backup_writer.rs
+++ b/pbs-client/src/backup_writer.rs
@@ -3,6 +3,7 @@ use std::future::Future;
 use std::os::unix::fs::OpenOptionsExt;
 use std::sync::atomic::{AtomicU64, AtomicUsize, Ordering};
 use std::sync::{Arc, Mutex};
+use std::convert::TryInto;
 
 use anyhow::{bail, format_err, Error};
 use futures::future::{self, AbortHandle, Either, FutureExt, TryFutureExt};
@@ -14,8 +15,8 @@ use tokio_stream::wrappers::ReceiverStream;
 
 use proxmox::tools::digest_to_hex;
 
+use pbs_api_types::HumanByte;
 use pbs_tools::crypt_config::CryptConfig;
-use pbs_tools::format::HumanByte;
 use pbs_datastore::{CATALOG_NAME, PROXMOX_BACKUP_PROTOCOL_ID_V1};
 use pbs_datastore::data_blob::{ChunkInfo, DataBlob, DataChunkBuilder};
 use pbs_datastore::dynamic_index::DynamicIndexReader;
@@ -336,17 +337,19 @@ impl BackupWriter {
         .await?;
 
         let size_dirty = upload_stats.size - upload_stats.size_reused;
-        let size: HumanByte = upload_stats.size.into();
+        let size = HumanByte::new_binary(upload_stats.size as u64);
         let archive = if self.verbose {
             archive_name
         } else {
             pbs_tools::format::strip_server_file_extension(archive_name)
         };
         if archive_name != CATALOG_NAME {
-            let speed: HumanByte =
-                ((size_dirty * 1_000_000) / (upload_stats.duration.as_micros() as usize)).into();
-            let size_dirty: HumanByte = size_dirty.into();
-            let size_compressed: HumanByte = upload_stats.size_compressed.into();
+            let speed = HumanByte::new_binary(
+                ((size_dirty as u128 * 1_000_000) / (upload_stats.duration.as_micros()))
+                    .try_into()?
+            );
+            let size_dirty = HumanByte::new_binary(size_dirty as u64);
+            let size_compressed = HumanByte::new_binary(upload_stats.size_compressed as u64);
             println!(
                 "{}: had to backup {} of {} (compressed {}) in {:.2}s",
                 archive,
@@ -362,7 +365,7 @@ impl BackupWriter {
 
         if upload_stats.size_reused > 0 && upload_stats.size > 1024 * 1024 {
             let reused_percent = upload_stats.size_reused as f64 * 100. / upload_stats.size as f64;
-            let reused: HumanByte = upload_stats.size_reused.into();
+            let reused = HumanByte::new_binary(upload_stats.size_reused as u64);
             println!(
                 "{}: backup was done incrementally, reused {} ({:.1}%)",
                 archive, reused, reused_percent
@@ -376,7 +379,7 @@ impl BackupWriter {
             println!(
                 "{}: Average chunk size was {}.",
                 archive,
-                HumanByte::from(upload_stats.size / upload_stats.chunk_count)
+                HumanByte::new_binary((upload_stats.size / upload_stats.chunk_count) as u64),
             );
             println!(
                 "{}: Average time per request: {} microseconds.",
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 5049cb3d..f76554f2 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -11,8 +11,7 @@ use lazy_static::lazy_static;
 
 use proxmox::tools::fs::{replace_file, file_read_optional_string, CreateOptions};
 
-use pbs_api_types::{UPID, DataStoreConfig, Authid, GarbageCollectionStatus};
-use pbs_tools::format::HumanByte;
+use pbs_api_types::{UPID, DataStoreConfig, Authid, GarbageCollectionStatus, HumanByte};
 use pbs_tools::fs::{lock_dir_noblock, DirLockGuard};
 use pbs_tools::process_locker::ProcessLockSharedGuard;
 use pbs_tools::{task_log, task_warn, task::WorkerTaskContext};
@@ -654,14 +653,14 @@ impl DataStore {
             task_log!(
                 worker,
                 "Removed garbage: {}",
-                HumanByte::from(gc_status.removed_bytes),
+                HumanByte::new_binary(gc_status.removed_bytes),
             );
             task_log!(worker, "Removed chunks: {}", gc_status.removed_chunks);
             if gc_status.pending_bytes > 0 {
                 task_log!(
                     worker,
                     "Pending removals: {} (in {} chunks)",
-                    HumanByte::from(gc_status.pending_bytes),
+                    HumanByte::new_binary(gc_status.pending_bytes),
                     gc_status.pending_chunks,
                 );
             }
@@ -676,7 +675,7 @@ impl DataStore {
             task_log!(
                 worker,
                 "Original data usage: {}",
-                HumanByte::from(gc_status.index_data_bytes),
+                HumanByte::new_binary(gc_status.index_data_bytes),
             );
 
             if gc_status.index_data_bytes > 0 {
@@ -684,7 +683,7 @@ impl DataStore {
                 task_log!(
                     worker,
                     "On-Disk usage: {} ({:.2}%)",
-                    HumanByte::from(gc_status.disk_bytes),
+                    HumanByte::new_binary(gc_status.disk_bytes),
                     comp_per,
                 );
             }
@@ -701,7 +700,7 @@ impl DataStore {
 
             if gc_status.disk_chunks > 0 {
                 let avg_chunk = gc_status.disk_bytes/(gc_status.disk_chunks as u64);
-                task_log!(worker, "Average chunk size: {}", HumanByte::from(avg_chunk));
+                task_log!(worker, "Average chunk size: {}", HumanByte::new_binary(avg_chunk));
             }
 
             if let Ok(serialized) = serde_json::to_string(&gc_status) {
diff --git a/pbs-tools/Cargo.toml b/pbs-tools/Cargo.toml
index 635933e1..8c5f4671 100644
--- a/pbs-tools/Cargo.toml
+++ b/pbs-tools/Cargo.toml
@@ -40,6 +40,7 @@ proxmox-time = { version = "1" }
 
 pbs-buildcfg = { path = "../pbs-buildcfg" }
 pbs-runtime = { path = "../pbs-runtime" }
+pbs-api-types = { path = "../pbs-api-types" }
 
 [dev-dependencies]
 tokio = { version = "1.6", features = [ "macros" ] }
diff --git a/pbs-tools/src/format.rs b/pbs-tools/src/format.rs
index feabd8f1..048a588d 100644
--- a/pbs-tools/src/format.rs
+++ b/pbs-tools/src/format.rs
@@ -3,6 +3,8 @@ use std::borrow::Borrow;
 use anyhow::{Error};
 use serde_json::Value;
 
+use pbs_api_types::HumanByte;
+
 pub fn strip_server_file_extension(name: &str) -> &str {
     if name.ends_with(".didx") || name.ends_with(".fidx") || name.ends_with(".blob") {
         &name[..name.len()-5]
@@ -55,7 +57,7 @@ pub fn render_bytes_human_readable(value: &Value, _record: &Value) -> Result<Str
     if value.is_null() { return Ok(String::new()); }
     let text = match value.as_u64() {
         Some(bytes) => {
-            HumanByte::from(bytes).to_string()
+            HumanByte::new_binary(bytes).to_string()
         }
         None => {
             value.to_string()
@@ -63,54 +65,3 @@ pub fn render_bytes_human_readable(value: &Value, _record: &Value) -> Result<Str
     };
     Ok(text)
 }
-
-pub struct HumanByte {
-    b: usize,
-}
-impl std::fmt::Display for HumanByte {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        if self.b < 1024 {
-            return write!(f, "{} B", self.b);
-        }
-        let kb: f64 = self.b as f64 / 1024.0;
-        if kb < 1024.0 {
-            return write!(f, "{:.2} KiB", kb);
-        }
-        let mb: f64 = kb / 1024.0;
-        if mb < 1024.0 {
-            return write!(f, "{:.2} MiB", mb);
-        }
-        let gb: f64 = mb / 1024.0;
-        if gb < 1024.0 {
-            return write!(f, "{:.2} GiB", gb);
-        }
-        let tb: f64 = gb / 1024.0;
-        if tb < 1024.0 {
-            return write!(f, "{:.2} TiB", tb);
-        }
-        let pb: f64 = tb / 1024.0;
-        return write!(f, "{:.2} PiB", pb);
-    }
-}
-impl From<usize> for HumanByte {
-    fn from(v: usize) -> Self {
-        HumanByte { b: v }
-    }
-}
-impl From<u64> for HumanByte {
-    fn from(v: u64) -> Self {
-        HumanByte { b: v as usize }
-    }
-}
-
-#[test]
-fn correct_byte_convert() {
-    fn convert(b: usize) -> String {
-         HumanByte::from(b).to_string()
-    }
-    assert_eq!(convert(1023), "1023 B");
-    assert_eq!(convert(1<<10), "1.00 KiB");
-    assert_eq!(convert(1<<20), "1.00 MiB");
-    assert_eq!(convert((1<<30) + 103 * (1<<20)), "1.10 GiB");
-    assert_eq!(convert((2<<50) + 500 * (1<<40)), "2.49 PiB");
-}
diff --git a/src/bin/proxmox-tape.rs b/src/bin/proxmox-tape.rs
index 5de727c1..8f3a1248 100644
--- a/src/bin/proxmox-tape.rs
+++ b/src/bin/proxmox-tape.rs
@@ -10,7 +10,6 @@ use proxmox_time::strftime_local;
 
 use pbs_client::view_task_result;
 use pbs_tools::format::{
-    HumanByte,
     render_epoch,
     render_bytes_human_readable,
 };
@@ -21,7 +20,7 @@ use pbs_config::datastore::complete_datastore_name;
 
 use pbs_api_types::{
     Userid, Authid, DATASTORE_SCHEMA, DATASTORE_MAP_LIST_SCHEMA,
-    DRIVE_NAME_SCHEMA, MEDIA_LABEL_SCHEMA, MEDIA_POOL_NAME_SCHEMA,
+    DRIVE_NAME_SCHEMA, HumanByte, MEDIA_LABEL_SCHEMA, MEDIA_POOL_NAME_SCHEMA,
     TAPE_RESTORE_SNAPSHOT_SCHEMA,
 };
 use pbs_tape::{
@@ -599,7 +598,7 @@ fn debug_scan(mut param: Value) -> Result<(), Error> {
                             println!("got content header: {}", name);
                             println!("  uuid:  {}", header.content_uuid());
                             println!("  ctime: {}", strftime_local("%c", header.ctime)?);
-                            println!("  hsize: {}", HumanByte::from(header.size as usize));
+                            println!("  hsize: {}", HumanByte::new_binary(header.size as u64));
                             println!("  part:  {}", header.part_number);
                         } else {
                             println!("got unknown content header: {:?}", header.content_magic);
@@ -610,7 +609,7 @@ fn debug_scan(mut param: Value) -> Result<(), Error> {
                     }
                 }
                 let bytes = reader.skip_data()?;
-                println!("skipped {}", HumanByte::from(bytes));
+                println!("skipped {}", HumanByte::new_binary(bytes as u64));
                 if let Ok(true) = reader.has_end_marker() {
                     if reader.is_incomplete()? {
                         println!("WARNING: file is incomplete");
diff --git a/src/server/email_notifications.rs b/src/server/email_notifications.rs
index b462bd4c..8b57a67a 100644
--- a/src/server/email_notifications.rs
+++ b/src/server/email_notifications.rs
@@ -7,10 +7,9 @@ use proxmox::tools::email::sendmail;
 use proxmox_lang::try_block;
 use proxmox_schema::{parse_property_string, ApiType};
 
-use pbs_tools::format::HumanByte;
 use pbs_api_types::{
     User, TapeBackupJobSetup, SyncJobConfig, VerificationJobConfig,
-    APTUpdateInfo, GarbageCollectionStatus,
+    APTUpdateInfo, GarbageCollectionStatus, HumanByte,
     Userid, Notify, DatastoreNotify, DataStoreConfig,
 };
 
@@ -601,7 +600,7 @@ fn handlebars_humam_bytes_helper(
         .flatten()
         .ok_or_else(|| RenderError::new("human-bytes: param not found"))?;
 
-    out.write(&HumanByte::from(param).to_string())?;
+    out.write(&HumanByte::new_binary(param).to_string())?;
 
     Ok(())
 }
-- 
2.30.2

[pbs-devel] [RFC proxmox-backup v3 2/2] use HumanByte for traffic-control config

[Dietmar Maurer]

---
 pbs-api-types/src/traffic_control.rs | 18 +++++++++---------
 src/cached_traffic_control.rs        | 18 ++++++++++++------
 2 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/pbs-api-types/src/traffic_control.rs b/pbs-api-types/src/traffic_control.rs
index 0dd7ed58..210f53ac 100644
--- a/pbs-api-types/src/traffic_control.rs
+++ b/pbs-api-types/src/traffic_control.rs
@@ -3,7 +3,7 @@ use serde::{Deserialize, Serialize};
 use proxmox_schema::{api, Schema, IntegerSchema, StringSchema, Updater};
 
 use crate::{
-    CIDR_SCHEMA, DAILY_DURATION_FORMAT,
+    HumanByte, CIDR_SCHEMA, DAILY_DURATION_FORMAT,
     PROXMOX_SAFE_ID_FORMAT, SINGLE_LINE_COMMENT_SCHEMA,
 };
 
@@ -38,19 +38,19 @@ pub const TRAFFIC_CONTROL_BURST_SCHEMA: Schema = IntegerSchema::new(
             schema: SINGLE_LINE_COMMENT_SCHEMA,
         },
         "rate-in": {
-            schema: TRAFFIC_CONTROL_RATE_SCHEMA,
+            type: HumanByte,
             optional: true,
         },
         "burst-in": {
-            schema: TRAFFIC_CONTROL_BURST_SCHEMA,
+            type: HumanByte,
             optional: true,
         },
         "rate-out": {
-            schema: TRAFFIC_CONTROL_RATE_SCHEMA,
+            type: HumanByte,
             optional: true,
         },
         "burst-out": {
-            schema: TRAFFIC_CONTROL_BURST_SCHEMA,
+            type: HumanByte,
             optional: true,
         },
         network: {
@@ -79,13 +79,13 @@ pub struct TrafficControlRule {
     /// Rule applies to Source IPs within this networks
     pub network: Vec<String>,
     #[serde(skip_serializing_if="Option::is_none")]
-    pub rate_in: Option<u64>,
+    pub rate_in: Option<HumanByte>,
     #[serde(skip_serializing_if="Option::is_none")]
-    pub burst_in: Option<u64>,
+    pub burst_in: Option<HumanByte>,
     #[serde(skip_serializing_if="Option::is_none")]
-    pub rate_out: Option<u64>,
+    pub rate_out: Option<HumanByte>,
     #[serde(skip_serializing_if="Option::is_none")]
-    pub burst_out: Option<u64>,
+    pub burst_out: Option<HumanByte>,
     // fixme: expose this?
     //    /// Bandwidth is shared accross all connections
     //    #[serde(skip_serializing_if="Option::is_none")]
diff --git a/src/cached_traffic_control.rs b/src/cached_traffic_control.rs
index be554f6c..b45e564c 100644
--- a/src/cached_traffic_control.rs
+++ b/src/cached_traffic_control.rs
@@ -224,7 +224,10 @@ impl TrafficControlCache {
                 Some(ref read_limiter) => {
                     match rule.rate_in {
                         Some(rate_in) => {
-                            read_limiter.update_rate(rate_in, rule.burst_in.unwrap_or(rate_in));
+                            read_limiter.update_rate(
+                                rate_in.as_u64(),
+                                rule.burst_in.unwrap_or(rate_in).as_u64(),
+                            );
                         }
                         None => entry.0 = None,
                     }
@@ -235,8 +238,8 @@ impl TrafficControlCache {
                         let limiter = create_limiter(
                             self.use_shared_memory,
                             &name,
-                            rate_in,
-                            rule.burst_in.unwrap_or(rate_in),
+                            rate_in.as_u64(),
+                            rule.burst_in.unwrap_or(rate_in).as_u64(),
                         )?;
                         entry.0 = Some(limiter);
                     }
@@ -247,7 +250,10 @@ impl TrafficControlCache {
                 Some(ref write_limiter) => {
                     match rule.rate_out {
                         Some(rate_out) => {
-                            write_limiter.update_rate(rate_out, rule.burst_out.unwrap_or(rate_out));
+                            write_limiter.update_rate(
+                                rate_out.as_u64(),
+                                rule.burst_out.unwrap_or(rate_out).as_u64(),
+                            );
                         }
                         None => entry.1 = None,
                     }
@@ -258,8 +264,8 @@ impl TrafficControlCache {
                         let limiter = create_limiter(
                             self.use_shared_memory,
                             &name,
-                            rate_out,
-                            rule.burst_out.unwrap_or(rate_out),
+                            rate_out.as_u64(),
+                            rule.burst_out.unwrap_or(rate_out).as_u64(),
                         )?;
                         entry.1 = Some(limiter);
                     }
-- 
2.30.2

Re: [pbs-devel] [RFC proxmox-backup v3 0/2] more flexible HumanByte type

[Fabian Ebner]

Am 18.11.21 um 08:29 schrieb Dietmar Maurer:
> Changes since v2:
> 
>   - reverted changes from v2

Should there be separate types for byte rates that are represented with 
float + unit, and for byte sizes that are represented with an integer 
internally? After all, 1.2MiB/s "exists", but 1.2MiB does not ;)

> 
>   - restrict number of possibleunits (always store KB instead of K)
>   
>   - store size without scaling to unit (avoid rounding errors in
>     deserialze/serialze)
> 
>   - allow to specify precision in Display
> 
>   - new with_unit() constructor
> 
>   - improved dev docs
> 
> Changes since v1:
> 
> - do not expose auto_unit functions
> - store size as u64 (suggested by thomas)
> - do not store unit (suggested by thomas)
> 
> 
> *** BLURB HERE ***
> 
> Dietmar Maurer (2):
>    pbs-api-types: more flexible HumanByte type
>    use HumanByte for traffic-control config
> 
>   pbs-api-types/src/human_byte.rs      | 353 +++++++++++++++++++++++++++
>   pbs-api-types/src/lib.rs             |   3 +
>   pbs-api-types/src/traffic_control.rs |  18 +-
>   pbs-client/src/backup_writer.rs      |  19 +-
>   pbs-datastore/src/datastore.rs       |  13 +-
>   pbs-tools/Cargo.toml                 |   1 +
>   pbs-tools/src/format.rs              |  55 +----
>   src/bin/proxmox-tape.rs              |   7 +-
>   src/cached_traffic_control.rs        |  18 +-
>   src/server/email_notifications.rs    |   5 +-
>   10 files changed, 403 insertions(+), 89 deletions(-)
>   create mode 100644 pbs-api-types/src/human_byte.rs
> 

Re: [pbs-devel] [RFC proxmox-backup v3 1/2] pbs-api-types: more flexible HumanByte type

[Thomas Lamprecht]

On 18.11.21 08:29, Dietmar Maurer wrote:
> This implements a parser for human readabkle byte sizes.
> 
> Examples: "1KB", "1.5 Mib", "3T"
> 
> Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
> ---
>  pbs-api-types/src/human_byte.rs   | 353 ++++++++++++++++++++++++++++++
>  pbs-api-types/src/lib.rs          |   3 +
>  pbs-client/src/backup_writer.rs   |  19 +-
>  pbs-datastore/src/datastore.rs    |  13 +-
>  pbs-tools/Cargo.toml              |   1 +
>  pbs-tools/src/format.rs           |  55 +----
>  src/bin/proxmox-tape.rs           |   7 +-
>  src/server/email_notifications.rs |   5 +-
>  8 files changed, 382 insertions(+), 74 deletions(-)
>  create mode 100644 pbs-api-types/src/human_byte.rs
> 
>

I replaced this commit by four separate ones:

42ba4cd3 human byte: make proper proxmox API type
ab1c07a6 human byte: add from string parser
930a7146 human byte: add proper unit type and support base-10
a58a5cf7 move HumanByte to pbs-abi-types crate

I based it quite a bit on the semantics of yours. But made a few changes, the
two commits in the middle contain a bit more info in the commit message.

Also incorporated a bit of your code 1:1 and all of your (adapted) tests, plus
added a few more.

IMO the result is OK'ish and definitively better than my lousy original one, if
you find issues or don't like some changes feel free to adapt, thanks!

[pbs-devel] applied: [RFC proxmox-backup v3 2/2] use HumanByte for traffic-control config

[Thomas Lamprecht]

On 18.11.21 08:29, Dietmar Maurer wrote:
> ---
>  pbs-api-types/src/traffic_control.rs | 18 +++++++++---------
>  src/cached_traffic_control.rs        | 18 ++++++++++++------
>  2 files changed, 21 insertions(+), 15 deletions(-)
> 
>

applied, thanks!

Re: [pbs-devel] [RFC proxmox-backup v3 0/2] more flexible HumanByte type

[Dietmar Maurer]

Another solution would be to simply use MiB/s as base unit in traffic-control.cfg, and store it as a simple float number.

This is less flexible, but much simpler to implement...

Any thought on that?