[PATCH 1/2] refactor Rust logger to be diomatic

[PATCH 1/2] refactor Rust logger to be diomatic

[TruongSinh Tran-Nguyen]

This is also to allow more levels of logging (error, debug and trace)
---
 rust-src/Cargo.toml             |  2 +-
 rust-src/bch_bindgen/src/lib.rs |  1 -
 rust-src/bch_bindgen/src/log.rs | 57 ---------------------------------
 rust-src/src/cmd_list.rs        |  3 +-
 rust-src/src/cmd_mount.rs       | 19 +++++++++--
 rust-src/src/key.rs             |  3 +-
 rust-src/src/lib.rs             |  1 +
 rust-src/src/logger.rs          | 28 ++++++++++++++++
 8 files changed, 48 insertions(+), 66 deletions(-)
 delete mode 100644 rust-src/bch_bindgen/src/log.rs
 create mode 100644 rust-src/src/logger.rs

diff --git a/rust-src/Cargo.toml b/rust-src/Cargo.toml
index 64b195c..92a3853 100644
--- a/rust-src/Cargo.toml
+++ b/rust-src/Cargo.toml
@@ -9,7 +9,7 @@ crate-type = ["staticlib"]
 
 [dependencies]
 atty = "0.2.14"
-log = "0.4"
+log = { version = "0.4", features = ["std"] }
 chrono = "0.4"
 colored = "2"
 clap = { version = "4.0.32", features = ["derive", "wrap_help"] }
diff --git a/rust-src/bch_bindgen/src/lib.rs b/rust-src/bch_bindgen/src/lib.rs
index 86592c6..73aeef6 100644
--- a/rust-src/bch_bindgen/src/lib.rs
+++ b/rust-src/bch_bindgen/src/lib.rs
@@ -3,7 +3,6 @@ pub mod btree;
 pub mod bkey;
 pub mod errcode;
 pub mod keyutils;
-pub mod log;
 pub mod rs;
 pub mod fs;
 pub mod opts;
diff --git a/rust-src/bch_bindgen/src/log.rs b/rust-src/bch_bindgen/src/log.rs
deleted file mode 100644
index 32927f1..0000000
--- a/rust-src/bch_bindgen/src/log.rs
+++ /dev/null
@@ -1,57 +0,0 @@
-use std::sync::atomic::{AtomicU8, Ordering};
-
-pub const MUTE: u8 = 0;
-pub const ERROR: u8 = 1;
-pub const INFO: u8 = 2;
-pub const DEBUG: u8 = 3;
-
-// error level by default
-pub static VERBOSE: AtomicU8 = AtomicU8::new(ERROR);
-
-#[inline]
-pub fn set_verbose_level(level: u8) {
-    VERBOSE.store(level, Ordering::SeqCst);
-}
-
-pub fn max_level() -> u8 {
-    VERBOSE.load(Ordering::SeqCst)
-}
-
-#[macro_export]
-macro_rules! info {
-    ($($arg:tt)*) => {
-        if 2 <= $crate::log::max_level() {
-            println!("{} {} {}",
-                " INFO".green(),
-                format!("{}:", module_path!()).bright_black(),
-                format_args!($($arg)*)
-            );
-        }
-    }
-}
-
-#[macro_export]
-macro_rules! debug {
-    ($($arg:tt)*) => {
-        if 3 <= $crate::log::max_level() {
-            println!("{} {} {}",
-                "DEBUG".bright_blue(),
-                format!("{}:", module_path!()).bright_black(),
-                format_args!($($arg)*)
-            );
-        }
-    }
-}
-
-#[macro_export]
-macro_rules! error {
-    ($($arg:tt)*) => {
-        if 1 <= $crate::log::max_level() {
-            println!("{} {} {}",
-                "ERROR".bright_red(),
-                format!("{}:", module_path!()).bright_black(),
-                format_args!($($arg)*)
-            );
-        }
-    }
-}
diff --git a/rust-src/src/cmd_list.rs b/rust-src/src/cmd_list.rs
index f04efa2..2dc8d71 100644
--- a/rust-src/src/cmd_list.rs
+++ b/rust-src/src/cmd_list.rs
@@ -1,5 +1,5 @@
 use atty::Stream;
-use bch_bindgen::error;
+use log::{error};
 use bch_bindgen::bcachefs;
 use bch_bindgen::opt_set;
 use bch_bindgen::fs::Fs;
@@ -9,7 +9,6 @@ use bch_bindgen::btree::BtreeIter;
 use bch_bindgen::btree::BtreeNodeIter;
 use bch_bindgen::btree::BtreeIterFlags;
 use clap::Parser;
-use colored::Colorize;
 use std::ffi::{CStr, OsStr, c_int, c_char};
 use std::os::unix::ffi::OsStrExt;
 
diff --git a/rust-src/src/cmd_mount.rs b/rust-src/src/cmd_mount.rs
index 1251d0d..af370ef 100644
--- a/rust-src/src/cmd_mount.rs
+++ b/rust-src/src/cmd_mount.rs
@@ -1,11 +1,13 @@
 use atty::Stream;
-use bch_bindgen::{bcachefs, bcachefs::bch_sb_handle, debug, error, info};
+use bch_bindgen::{bcachefs, bcachefs::bch_sb_handle};
+use log::{info, warn, debug, error, trace, LevelFilter};
 use clap::Parser;
-use colored::Colorize;
 use uuid::Uuid;
+use std::convert::TryInto;
 use std::path::PathBuf;
 use crate::key;
 use crate::key::KeyLoc;
+use crate::logger::SimpleLogger;
 use std::ffi::{CStr, CString, OsStr, c_int, c_char, c_void};
 use std::os::unix::ffi::OsStrExt;
 
@@ -202,9 +204,20 @@ pub extern "C" fn cmd_mount(argc: c_int, argv: *const *const c_char) {
         .collect();
 
     let opt = Cli::parse_from(argv);
-    bch_bindgen::log::set_verbose_level(opt.verbose + bch_bindgen::log::ERROR);
+    
+    log::set_boxed_logger(Box::new(SimpleLogger)).unwrap();
+
+    // @TODO : more granular log levels via mount option
+    log::set_max_level(match opt.verbose {
+        0 => LevelFilter::Warn,
+        1 => LevelFilter::Trace,
+        2_u8..=u8::MAX => todo!(),
+    });
+
     colored::control::set_override(opt.colorize);
     if let Err(e) = cmd_mount_inner(opt) {
         error!("Fatal error: {}", e);
+    } else {
+        info!("Successfully mounted");
     }
 }
diff --git a/rust-src/src/key.rs b/rust-src/src/key.rs
index abea584..2af34b1 100644
--- a/rust-src/src/key.rs
+++ b/rust-src/src/key.rs
@@ -1,6 +1,5 @@
-use bch_bindgen::info;
+use log::{info};
 use bch_bindgen::bcachefs::bch_sb_handle;
-use colored::Colorize;
 use crate::c_str;
 use anyhow::anyhow;
 
diff --git a/rust-src/src/lib.rs b/rust-src/src/lib.rs
index a33e391..159d049 100644
--- a/rust-src/src/lib.rs
+++ b/rust-src/src/lib.rs
@@ -1,4 +1,5 @@
 pub mod key;
+pub mod logger;
 pub mod cmd_mount;
 pub mod cmd_list;
 
diff --git a/rust-src/src/logger.rs b/rust-src/src/logger.rs
new file mode 100644
index 0000000..2cd7b36
--- /dev/null
+++ b/rust-src/src/logger.rs
@@ -0,0 +1,28 @@
+use colored::Colorize;
+use log::{Level, Metadata, Record};
+
+pub struct SimpleLogger;
+
+impl log::Log for SimpleLogger {
+    fn enabled(&self, _: &Metadata) -> bool {
+        true
+    }
+
+    fn log(&self, record: &Record) {
+        let debug_prefix = match record.level() {
+            Level::Error => "ERROR".bright_red(),
+            Level::Warn => "WARN".bright_yellow(),
+            Level::Info => "INFO".green(),
+            Level::Debug => "DEBUG".bright_blue(),
+            Level::Trace => "TRACE".into(),
+        };
+        println!(
+            "{} - {}: {}",
+            debug_prefix,
+            record.module_path().unwrap_or_default().bright_black(),
+            record.args()
+        );
+    }
+
+    fn flush(&self) {}
+}
-- 
2.33.1

[PATCH 2/2] feature: allow cmd mount to accept only 1 block device, then mount by UUID

[TruongSinh Tran-Nguyen]

This allows bcachefs to be mounted by /etc/fstab
---
 rust-src/src/cmd_mount.rs | 45 ++++++++++++++++++++++++++++-----------
 1 file changed, 33 insertions(+), 12 deletions(-)

diff --git a/rust-src/src/cmd_mount.rs b/rust-src/src/cmd_mount.rs
index af370ef..da95f80 100644
--- a/rust-src/src/cmd_mount.rs
+++ b/rust-src/src/cmd_mount.rs
@@ -161,26 +161,47 @@ struct Cli {
     verbose:        u8,
 }
 
+fn get_devices_and_sbs_by_uuid(uuid: Uuid) -> anyhow::Result<(String, Vec<bch_sb_handle>)> {
+
+    let devs_sbs = get_devices_by_uuid(uuid)?;
+
+    let devs_strs: Vec<_> = devs_sbs.iter().map(|(dev, _)| dev.clone().into_os_string().into_string().unwrap()).collect();
+    let devs_str = devs_strs.join(":");
+    let sbs: Vec<bch_sb_handle> = devs_sbs.iter().map(|(_, sb)| *sb).collect();
+
+    Ok((devs_str, sbs))
+}
+
 fn cmd_mount_inner(opt: Cli) -> anyhow::Result<()> {
     let (devs, sbs) = if opt.dev.starts_with("UUID=") {
         let uuid = opt.dev.replacen("UUID=", "", 1);
         let uuid = Uuid::parse_str(&uuid)?;
-        let devs_sbs = get_devices_by_uuid(uuid)?;
+        get_devices_and_sbs_by_uuid(uuid)?
+    } else {
+        let sbs: Vec<bch_sb_handle> = opt.dev.split(':').map(|dev| {
+            let dev = PathBuf::from(dev);
+            bch_bindgen::rs::read_super(&dev)
+        }).collect::<Result<Vec<_>, _>>()?;
 
-        let devs_strs: Vec<_> = devs_sbs.iter().map(|(dev, _)| dev.clone().into_os_string().into_string().unwrap()).collect();
-        let devs_str = devs_strs.join(":");
-        let sbs = devs_sbs.iter().map(|(_, sb)| *sb).collect();
+        let uuid = sbs[0].sb().uuid();
+        let sbs_len: u8 = sbs.len().try_into().unwrap();
+        let nr_devices = sbs[0].sb().nr_devices;
 
-        (devs_str, sbs)
-    } else {
-        let mut sbs = Vec::new();
+        info!("{} is the external UUID", uuid);
 
-        for dev in opt.dev.split(':') {
-            let dev = PathBuf::from(dev);
-            sbs.push(bch_bindgen::rs::read_super(&dev)?);
+        if sbs_len == nr_devices {
+            info!("{} device(s) specified from mount command, matching required as specified from first device headers", sbs_len);
+            // @TODO looping through and check all devices actually have the same external UUID
+            (opt.dev, sbs)
+        } else {
+            warn!("{} device(s) specified from mount command", sbs_len);
+            warn!("{} device(s) required as specified from first device headers", nr_devices);
+            warn!("trying to scan all devices with external UUID instead");
+            let (devs_str, sbs) = get_devices_and_sbs_by_uuid(uuid)?;
+            info!("{} device(s) found from scanning all devices matching UUID", sbs.len());
+            // @TODO what if get_devices_and_sbs_by_uuid still cannot get all required?
+            (devs_str, sbs)
         }
-
-        (opt.dev, sbs)
     };
 
     if unsafe { bcachefs::bch2_sb_is_encrypted(sbs[0].sb) } {
-- 
2.33.1