More rust improvements

- passing of arguments from c -> rust code now works correctly
 - 'bcachefs mount' now handles being passed a device or devices

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>

bcachefs.c

@@ -177,20 +177,7 @@ int main(int argc, char *argv[])
 
 	setvbuf(stdout, NULL, _IOLBF, 0);
 
-	if (argc < 2) {
+	char *cmd = pop_cmd(&argc, argv);
-		puts("missing command\n");
-		goto usage;
-	}
-
-	/* Rust commands first - rust can't handle us mutating argv */
-	char *cmd = argv[1];
-
-	if (!strcmp(cmd, "mount")) {
-		cmd_mount();
-		return 0;
-	}
-
-	cmd = pop_cmd(&argc, argv);
 	if (!cmd) {
 		puts("missing command\n");
 		goto usage;
@@ -257,6 +244,11 @@ int main(int argc, char *argv[])
 	if (!strcmp(cmd, "setattr"))
 		return cmd_setattr(argc, argv);
 
+	if (!strcmp(cmd, "mount")) {
+		cmd_mount(argc, argv);
+		return 0;
+	}
+
 #ifdef BCACHEFS_FUSE
 	if (!strcmp(cmd, "fusemount"))
 		return cmd_fusemount(argc, argv);

cmds.h

@@ -61,6 +61,6 @@ int cmd_subvolume_delete(int argc, char *argv[]);
 int cmd_subvolume_snapshot(int argc, char *argv[]);
 
 int cmd_fusemount(int argc, char *argv[]);
-void cmd_mount(void);
+void cmd_mount(int agc, char *argv[]);
 
 #endif /* _CMDS_H */

rust-src/src/cmd_mount.rs

@@ -1,18 +1,128 @@
-use bch_bindgen::{error, info};
+use atty::Stream;
+use bch_bindgen::{bcachefs, bcachefs::bch_sb_handle, debug, error, info};
 use clap::Parser;
 use colored::Colorize;
-use atty::Stream;
 use uuid::Uuid;
-use crate::filesystem;
+use std::path::PathBuf;
 use crate::key;
 use crate::key::KeyLoc;
+use std::ffi::{CStr, CString, OsStr, c_int, c_char, c_void};
+use std::os::unix::ffi::OsStrExt;
 
-fn parse_fstab_uuid(uuid_raw: &str) -> Result<Uuid, uuid::Error> {
+fn mount_inner(
-    let mut uuid = String::from(uuid_raw);
+    src: String,
-    if uuid.starts_with("UUID=") {
+    target: impl AsRef<std::path::Path>,
-        uuid = uuid.replacen("UUID=", "", 1);
+    fstype: &str,
+    mountflags: u64,
+    data: Option<String>,
+) -> anyhow::Result<()> {
+
+    // bind the CStrings to keep them alive
+    let src = CString::new(src)?;
+    let target = CString::new(target.as_ref().as_os_str().as_bytes())?;
+    let data = data.map(CString::new).transpose()?;
+    let fstype = CString::new(fstype)?;
+
+    // convert to pointers for ffi
+    let src = src.as_c_str().to_bytes_with_nul().as_ptr() as *const c_char;
+    let target = target.as_c_str().to_bytes_with_nul().as_ptr() as *const c_char;
+    let data = data.as_ref().map_or(std::ptr::null(), |data| {
+        data.as_c_str().to_bytes_with_nul().as_ptr() as *const c_void
+    });
+    let fstype = fstype.as_c_str().to_bytes_with_nul().as_ptr() as *const c_char;
+
+    let ret = {
+        info!("mounting filesystem");
+        // REQUIRES: CAP_SYS_ADMIN
+        unsafe { libc::mount(src, target, fstype, mountflags, data) }
+    };
+    match ret {
+        0 => Ok(()),
+        _ => Err(crate::ErrnoError(errno::errno()).into()),
     }
-    return Uuid::parse_str(&uuid);
+}
+
+/// Parse a comma-separated mount options and split out mountflags and filesystem
+/// specific options.
+fn parse_mount_options(options: impl AsRef<str>) -> (Option<String>, u64) {
+    use either::Either::*;
+    debug!("parsing mount options: {}", options.as_ref());
+    let (opts, flags) = options
+        .as_ref()
+        .split(",")
+        .map(|o| match o {
+            "dirsync"       => Left(libc::MS_DIRSYNC),
+            "lazytime"      => Left(1 << 25), // MS_LAZYTIME
+            "mand"          => Left(libc::MS_MANDLOCK),
+            "noatime"       => Left(libc::MS_NOATIME),
+            "nodev"         => Left(libc::MS_NODEV),
+            "nodiratime"    => Left(libc::MS_NODIRATIME),
+            "noexec"        => Left(libc::MS_NOEXEC),
+            "nosuid"        => Left(libc::MS_NOSUID),
+            "relatime"      => Left(libc::MS_RELATIME),
+            "remount"       => Left(libc::MS_REMOUNT),
+            "ro"            => Left(libc::MS_RDONLY),
+            "rw"            => Left(0),
+            "strictatime"   => Left(libc::MS_STRICTATIME),
+            "sync"          => Left(libc::MS_SYNCHRONOUS),
+            ""              => Left(0),
+            o @ _           => Right(o),
+        })
+        .fold((Vec::new(), 0), |(mut opts, flags), next| match next {
+            Left(f) => (opts, flags | f),
+            Right(o) => {
+                opts.push(o);
+                (opts, flags)
+            }
+        });
+
+    use itertools::Itertools;
+    (
+        if opts.len() == 0 {
+            None
+        } else {
+            Some(opts.iter().join(","))
+        },
+        flags,
+    )
+}
+
+fn mount(
+    device: String,
+    target: impl AsRef<std::path::Path>,
+    options: impl AsRef<str>,
+) -> anyhow::Result<()> {
+    let (data, mountflags) = parse_mount_options(options);
+
+    info!(
+        "mounting bcachefs filesystem, {}",
+        target.as_ref().display()
+    );
+    mount_inner(device, target, "bcachefs", mountflags, data)
+}
+
+fn read_super_silent(path: &std::path::PathBuf) -> std::io::Result<bch_sb_handle> {
+    // Stop libbcachefs from spamming the output
+    let _gag = gag::BufferRedirect::stdout().unwrap();
+
+    bch_bindgen::rs::read_super(&path)?
+}
+
+fn get_devices_by_uuid(uuid: Uuid) -> anyhow::Result<Vec<(PathBuf, bch_sb_handle)>> {
+    debug!("enumerating udev devices");
+    let mut udev = udev::Enumerator::new()?;
+
+    udev.match_subsystem("block")?;
+
+    let devs = udev
+        .scan_devices()?
+        .into_iter()
+        .filter_map(|dev| dev.devnode().map(ToOwned::to_owned))
+        .map(|dev| (dev.clone(), read_super_silent(&dev)))
+        .filter_map(|(dev, sb)| sb.ok().map(|sb| (dev, sb)))
+        .filter(|(_, sb)| sb.sb().uuid() == uuid)
+        .collect();
+    Ok(devs)
 }
 
 fn stdout_isatty() -> &'static str {
@@ -26,7 +136,7 @@ fn stdout_isatty() -> &'static str {
 /// Mount a bcachefs filesystem by its UUID.
 #[derive(Parser, Debug)]
 #[command(author, version, about, long_about = None)]
-pub struct Cli {
+struct Cli {
     /// Where the password would be loaded from.
     ///
     /// Possible values are:
@@ -34,63 +144,71 @@ pub struct Cli {
     /// "wait" - wait for password to become available before mounting;
     /// "ask" -  prompt the user for password;
     #[arg(short, long, default_value = "", verbatim_doc_comment)]
-    pub key_location: KeyLoc,
+    key_location:   KeyLoc,
 
-    /// External UUID of the bcachefs filesystem
+    /// Device, or UUID=<UUID>
-    ///
+    dev:            String,
-    /// Accepts the UUID as is or as fstab style UUID=<UUID>
-    #[arg(value_parser = parse_fstab_uuid)]
-    pub uuid: uuid::Uuid,
 
     /// Where the filesystem should be mounted. If not set, then the filesystem
     /// won't actually be mounted. But all steps preceeding mounting the
     /// filesystem (e.g. asking for passphrase) will still be performed.
-    pub mountpoint: Option<std::path::PathBuf>,
+    mountpoint:     std::path::PathBuf,
 
     /// Mount options
     #[arg(short, default_value = "")]
-    pub options: String,
+    options:        String,
 
     /// Force color on/off. Default: autodetect tty
     #[arg(short, long, action = clap::ArgAction::Set, default_value=stdout_isatty())]
-    pub colorize: bool,
+    colorize:       bool,
 
     #[arg(short = 'v', long, action = clap::ArgAction::Count)]
-    pub verbose: u8,
+    verbose:        u8,
 }
 
-pub fn cmd_mount_inner(opt: Cli) -> anyhow::Result<()> {
+fn cmd_mount_inner(opt: Cli) -> anyhow::Result<()> {
-    unsafe {
+    let (devs, sbs) = if opt.dev.starts_with("UUID=") {
-        libc::setvbuf(filesystem::stdout, std::ptr::null_mut(), libc::_IONBF, 0);
+        let uuid = opt.dev.replacen("UUID=", "", 1);
-    }
+        let uuid = Uuid::parse_str(&uuid)?;
+        let devs_sbs = get_devices_by_uuid(uuid)?;
 
-    let fss = filesystem::probe_filesystems()?;
+        let devs_strs: Vec<_> = devs_sbs.iter().map(|(dev, _)| dev.clone().into_os_string().into_string().unwrap()).collect();
-    let fs = fss
+        let devs_str = devs_strs.join(":");
-        .get(&opt.uuid)
+        let sbs = devs_sbs.iter().map(|(_, sb)| *sb).collect();
-        .ok_or_else(|| anyhow::anyhow!("filesystem was not found"))?;
 
-    info!("found filesystem {}", fs);
+        (devs_str, sbs)
-    if fs.encrypted() {
+    } else {
+        let mut sbs = Vec::new();
+
+        for dev in opt.dev.split(':') {
+            let dev = PathBuf::from(dev);
+            sbs.push(bch_bindgen::rs::read_super(&dev)??);
+        }
+
+        (opt.dev, sbs)
+    };
+
+    if unsafe { bcachefs::bch2_sb_is_encrypted(sbs[0].sb) } {
         let key = opt
             .key_location
             .0
             .ok_or_else(|| anyhow::anyhow!("no keyoption specified for locked filesystem"))?;
 
-        key::prepare_key(&fs, key)?;
+        key::prepare_key(&sbs[0], key)?;
     }
 
-    let mountpoint = opt
+    mount(devs, &opt.mountpoint, &opt.options)?;
-        .mountpoint
-        .ok_or_else(|| anyhow::anyhow!("mountpoint option was not specified"))?;
-
-    fs.mount(&mountpoint, &opt.options)?;
-
     Ok(())
 }
 
 #[no_mangle]
-pub extern "C" fn cmd_mount() {
+pub extern "C" fn cmd_mount(argc: c_int, argv: *const *const c_char) {
-    let opt = Cli::parse();
+    let argv: Vec<_> = (0..argc)
+        .map(|i| unsafe { CStr::from_ptr(*argv.add(i as usize)) })
+        .map(|i| OsStr::from_bytes(i.to_bytes()))
+        .collect();
+
+    let opt = Cli::parse_from(argv);
     bch_bindgen::log::set_verbose_level(opt.verbose + bch_bindgen::log::ERROR);
     colored::control::set_override(opt.colorize);
     if let Err(e) = cmd_mount_inner(opt) {

rust-src/src/filesystem.rs

@@ -5,213 +5,5 @@ use bch_bindgen::{debug, info};
 use colored::Colorize;
 use getset::{CopyGetters, Getters};
 use std::path::PathBuf;
-#[derive(Getters, CopyGetters)]
+use bcachefs::bch_sb_handle;
-pub struct FileSystem {
-    /// External UUID of the bcachefs
-    #[getset(get = "pub")]
-    uuid: uuid::Uuid,
-    /// Whether filesystem is encrypted
-    #[getset(get_copy = "pub")]
-    encrypted: bool,
-    /// Super block
-    #[getset(get = "pub")]
-    sb: bcachefs::bch_sb_handle,
-    /// Member devices for this filesystem
-    #[getset(get = "pub")]
-    devices: Vec<PathBuf>,
-}
-impl std::fmt::Debug for FileSystem {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.debug_struct("FileSystem")
-            .field("uuid", &self.uuid)
-            .field("encrypted", &self.encrypted)
-            .field("devices", &self.device_string())
-            .finish()
-    }
-}
-use std::fmt;
-impl std::fmt::Display for FileSystem {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        let devs = self.device_string();
-        write!(
-            f,
-            "{:?}: locked?={lock} ({}) ",
-            self.uuid,
-            devs,
-            lock = self.encrypted
-        )
-    }
-}
 
-impl FileSystem {
-    pub(crate) fn new(sb: bcachefs::bch_sb_handle) -> Self {
-        Self {
-            uuid: sb.sb().uuid(),
-            encrypted: sb.sb().crypt().is_some(),
-            sb: sb,
-            devices: Vec::new(),
-        }
-    }
-
-    pub fn device_string(&self) -> String {
-        use itertools::Itertools;
-        self.devices.iter().map(|d| d.display()).join(":")
-    }
-
-    pub fn mount(
-        &self,
-        target: impl AsRef<std::path::Path>,
-        options: impl AsRef<str>,
-    ) -> anyhow::Result<()> {
-        let src = self.device_string();
-        let (data, mountflags) = parse_mount_options(options);
-
-        info!(
-            "mounting bcachefs filesystem, {}",
-            target.as_ref().display()
-        );
-        mount_inner(src, target, "bcachefs", mountflags, data)
-    }
-}
-
-fn mount_inner(
-    src: String,
-    target: impl AsRef<std::path::Path>,
-    fstype: &str,
-    mountflags: u64,
-    data: Option<String>,
-) -> anyhow::Result<()> {
-    use std::{
-        ffi::{c_void, CString},
-        os::{raw::c_char, unix::ffi::OsStrExt},
-    };
-
-    // bind the CStrings to keep them alive
-    let src = CString::new(src)?;
-    let target = CString::new(target.as_ref().as_os_str().as_bytes())?;
-    let data = data.map(CString::new).transpose()?;
-    let fstype = CString::new(fstype)?;
-
-    // convert to pointers for ffi
-    let src = src.as_c_str().to_bytes_with_nul().as_ptr() as *const c_char;
-    let target = target.as_c_str().to_bytes_with_nul().as_ptr() as *const c_char;
-    let data = data.as_ref().map_or(std::ptr::null(), |data| {
-        data.as_c_str().to_bytes_with_nul().as_ptr() as *const c_void
-    });
-    let fstype = fstype.as_c_str().to_bytes_with_nul().as_ptr() as *const c_char;
-
-    let ret = {
-        info!("mounting filesystem");
-        // REQUIRES: CAP_SYS_ADMIN
-        unsafe { libc::mount(src, target, fstype, mountflags, data) }
-    };
-    match ret {
-        0 => Ok(()),
-        _ => Err(crate::ErrnoError(errno::errno()).into()),
-    }
-}
-
-/// Parse a comma-separated mount options and split out mountflags and filesystem
-/// specific options.
-fn parse_mount_options(options: impl AsRef<str>) -> (Option<String>, u64) {
-    use either::Either::*;
-    debug!("parsing mount options: {}", options.as_ref());
-    let (opts, flags) = options
-        .as_ref()
-        .split(",")
-        .map(|o| match o {
-            "dirsync" => Left(libc::MS_DIRSYNC),
-            "lazytime" => Left(1 << 25), // MS_LAZYTIME
-            "mand" => Left(libc::MS_MANDLOCK),
-            "noatime" => Left(libc::MS_NOATIME),
-            "nodev" => Left(libc::MS_NODEV),
-            "nodiratime" => Left(libc::MS_NODIRATIME),
-            "noexec" => Left(libc::MS_NOEXEC),
-            "nosuid" => Left(libc::MS_NOSUID),
-            "relatime" => Left(libc::MS_RELATIME),
-            "remount" => Left(libc::MS_REMOUNT),
-            "ro" => Left(libc::MS_RDONLY),
-            "rw" => Left(0),
-            "strictatime" => Left(libc::MS_STRICTATIME),
-            "sync" => Left(libc::MS_SYNCHRONOUS),
-            "" => Left(0),
-            o @ _ => Right(o),
-        })
-        .fold((Vec::new(), 0), |(mut opts, flags), next| match next {
-            Left(f) => (opts, flags | f),
-            Right(o) => {
-                opts.push(o);
-                (opts, flags)
-            }
-        });
-
-    use itertools::Itertools;
-    (
-        if opts.len() == 0 {
-            None
-        } else {
-            Some(opts.iter().join(","))
-        },
-        flags,
-    )
-}
-
-use bch_bindgen::bcachefs;
-use std::collections::HashMap;
-use uuid::Uuid;
-
-pub fn probe_filesystems() -> anyhow::Result<HashMap<Uuid, FileSystem>> {
-    debug!("enumerating udev devices");
-    let mut udev = udev::Enumerator::new()?;
-
-    udev.match_subsystem("block")?; // find kernel block devices
-
-    let mut fs_map = HashMap::new();
-    let devresults = udev
-        .scan_devices()?
-        .into_iter()
-        .filter_map(|dev| dev.devnode().map(ToOwned::to_owned));
-
-    for pathbuf in devresults {
-        match get_super_block_uuid(&pathbuf)? {
-            Ok((uuid_key, superblock)) => {
-                let fs = fs_map.entry(uuid_key).or_insert_with(|| {
-                    info!("found bcachefs pool: {}", uuid_key);
-                    FileSystem::new(superblock)
-                });
-
-                fs.devices.push(pathbuf);
-            }
-
-            Err(e) => {
-                debug!("{}", e);
-            }
-        }
-    }
-
-    info!("found {} filesystems", fs_map.len());
-    Ok(fs_map)
-}
-
-// #[tracing_attributes::instrument(skip(dev, fs_map))]
-fn get_super_block_uuid(
-    path: &std::path::Path,
-) -> std::io::Result<std::io::Result<(Uuid, bcachefs::bch_sb_handle)>> {
-    use gag::BufferRedirect;
-    // Stop libbcachefs from spamming the output
-    let gag = BufferRedirect::stdout().unwrap();
-
-    let sb = bch_bindgen::rs::read_super(&path)?;
-    let super_block = match sb {
-        Err(e) => {
-            return Ok(Err(e));
-        }
-        Ok(sb) => sb,
-    };
-    drop(gag);
-
-    let uuid = (&super_block).sb().uuid();
-    debug!("bcachefs superblock path={} uuid={}", path.display(), uuid);
-
-    Ok(Ok((uuid, super_block)))
-}

rust-src/src/key.rs

@@ -1,4 +1,5 @@
 use bch_bindgen::info;
+use bch_bindgen::bcachefs::bch_sb_handle;
 use colored::Colorize;
 use crate::c_str;
 use anyhow::anyhow;
@@ -23,11 +24,11 @@ impl std::str::FromStr for KeyLoc {
     type Err = anyhow::Error;
     fn from_str(s: &str) -> anyhow::Result<Self> {
         match s {
-            "" => Ok(KeyLoc(None)),
+            ""      => Ok(KeyLoc(None)),
-            "fail" => Ok(KeyLoc(Some(KeyLocation::Fail))),
+            "fail"  => Ok(KeyLoc(Some(KeyLocation::Fail))),
-            "wait" => Ok(KeyLoc(Some(KeyLocation::Wait))),
+            "wait"  => Ok(KeyLoc(Some(KeyLocation::Wait))),
-            "ask" => Ok(KeyLoc(Some(KeyLocation::Ask))),
+            "ask"   => Ok(KeyLoc(Some(KeyLocation::Ask))),
-            _ => Err(anyhow!("invalid password option")),
+            _       => Err(anyhow!("invalid password option")),
         }
     }
 }
@@ -60,19 +61,18 @@ fn wait_for_key(uuid: &uuid::Uuid) -> anyhow::Result<()> {
 }
 
 const BCH_KEY_MAGIC: &str = "bch**key";
-use crate::filesystem::FileSystem;
+fn ask_for_key(sb: &bch_sb_handle) -> anyhow::Result<()> {
-fn ask_for_key(fs: &FileSystem) -> anyhow::Result<()> {
     use bch_bindgen::bcachefs::{self, bch2_chacha_encrypt_key, bch_encrypted_key, bch_key};
     use byteorder::{LittleEndian, ReadBytesExt};
     use std::os::raw::c_char;
 
-    let key_name = std::ffi::CString::new(format!("bcachefs:{}", fs.uuid())).unwrap();
+    let key_name = std::ffi::CString::new(format!("bcachefs:{}", sb.sb().uuid())).unwrap();
     if check_for_key(&key_name)? {
         return Ok(());
     }
 
     let bch_key_magic = BCH_KEY_MAGIC.as_bytes().read_u64::<LittleEndian>().unwrap();
-    let crypt = fs.sb().sb().crypt().unwrap();
+    let crypt = sb.sb().crypt().unwrap();
     let pass = rpassword::read_password_from_tty(Some("Enter passphrase: "))?;
     let pass = std::ffi::CString::new(pass.trim_end())?; // bind to keep the CString alive
     let mut output: bch_key = unsafe {
@@ -86,7 +86,7 @@ fn ask_for_key(fs: &FileSystem) -> anyhow::Result<()> {
     let ret = unsafe {
         bch2_chacha_encrypt_key(
             &mut output as *mut _,
-            fs.sb().sb().nonce(),
+            sb.sb().nonce(),
             &mut key as *mut _ as *mut _,
             std::mem::size_of::<bch_encrypted_key>() as usize,
         )
@@ -114,11 +114,11 @@ fn ask_for_key(fs: &FileSystem) -> anyhow::Result<()> {
     }
 }
 
-pub fn prepare_key(fs: &FileSystem, password: KeyLocation) -> anyhow::Result<()> {
+pub fn prepare_key(sb: &bch_sb_handle, password: KeyLocation) -> anyhow::Result<()> {
-    info!("checking if key exists for filesystem {}", fs.uuid());
+    info!("checking if key exists for filesystem {}", sb.sb().uuid());
     match password {
         KeyLocation::Fail => Err(anyhow!("no key available")),
-        KeyLocation::Wait => Ok(wait_for_key(fs.uuid())?),
+        KeyLocation::Wait => Ok(wait_for_key(&sb.sb().uuid())?),
-        KeyLocation::Ask => ask_for_key(fs),
+        KeyLocation::Ask => ask_for_key(sb),
     }
 }

rust-src/src/lib.rs

@@ -1,4 +1,3 @@
-pub mod filesystem;
 pub mod key;
 pub mod cmd_mount;