From 1bdb914f167d18f5dbc04f2209e653f418293685 Mon Sep 17 00:00:00 2001 From: simonsan <14062932+simonsan@users.noreply.github.com> Date: Mon, 25 Nov 2024 15:21:01 +0100 Subject: [PATCH] refactor(mount): rename fields for clarity, add user options for mount (#1353) Addresses review feedback from #973 --------- Signed-off-by: simonsan <14062932+simonsan@users.noreply.github.com> --- Cargo.lock | 4 +- Cargo.toml | 2 +- config/full.toml | 7 +- src/commands/mount.rs | 131 +++++++++++++++++++++-------------- src/commands/mount/fusefs.rs | 1 - 5 files changed, 88 insertions(+), 57 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4040e2fb3..e92216882 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -882,9 +882,9 @@ dependencies = [ [[package]] name = "conflate" -version = "0.3.1" +version = "0.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ce1803bd64597191f7f4afa2dd0437f76b7b3c065d0e4566380012406c28eaac" +checksum = "0911e70a8cf23eb9cab4282eb9cf050421ee4d8b4464a366b2eb5e8f41c6ddec" dependencies = [ "conflate_derive", "num-traits", diff --git a/Cargo.toml b/Cargo.toml index 3ed7556fd..21e5e5982 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -98,7 +98,7 @@ bytesize = "1" cached = "0.54.0" clap = { version = "4", features = ["derive", "env", "wrap_help"] } clap_complete = "4" -conflate = "0.3.1" +conflate = "0.3.3" convert_case = "0.6.0" dateparser = "0.2.1" derive_more = { version = "1", features = ["debug"] } diff --git a/config/full.toml b/config/full.toml index 903748373..20b43a1a8 100644 --- a/config/full.toml +++ b/config/full.toml @@ -209,7 +209,10 @@ snapshot-path = "latest:/dir" # Default: not set - if not set, generate a virtua [mount] path-template = "[{hostname}]/[{label}]/{time}" # The path template to use for snapshots. {id}, {id_long}, {time}, {username}, {hostname}, {label}, {tags}, {backup_start}, {backup_end} are replaced. [default: "[{hostname}]/[{label}]/{time}"]. Only relevant if no snapshot-path is given. time-template = "%Y-%m-%d_%H-%M-%S" # only relevant if no snapshot-path is given -no-allow-other = true +exclusive = true # don't allow other users to access the mount point file-access = "read" # Default: "forbidden" for hot/cold repos, else "read" -mountpoint = "~/mnt" +mount-point = "~/mnt" snapshot-path = "latest:/dir" # Default: not set - if not set, generate a virtual tree with all snapshots using path-template +options = [ + "kernel_cache", # Default: ["kernel_cache] +] diff --git a/src/commands/mount.rs b/src/commands/mount.rs index 8f459198a..bfed98501 100644 --- a/src/commands/mount.rs +++ b/src/commands/mount.rs @@ -6,18 +6,19 @@ mod fusefs; use fusefs::FuseFS; +use abscissa_core::{ + config::Override, Command, FrameworkError, FrameworkErrorKind::ParseError, Runnable, Shutdown, +}; +use anyhow::{bail, Result}; +use clap::Parser; +use conflate::{Merge, MergeFrom}; +use fuse_mt::{mount, FuseMT}; +use rustic_core::vfs::{FilePolicy, IdenticalSnapshot, Latest, Vfs}; use std::{ffi::OsStr, path::PathBuf}; use crate::{repository::CliIndexedRepo, status_err, Application, RusticConfig, RUSTIC_APP}; -use abscissa_core::{config::Override, Command, FrameworkError, Runnable, Shutdown}; -use anyhow::{anyhow, Result}; -use conflate::Merge; -use fuse_mt::{mount, FuseMT}; -use rustic_core::vfs::{FilePolicy, IdenticalSnapshot, Latest, Vfs}; -use serde::{Deserialize, Serialize}; - -#[derive(Clone, Command, Default, Debug, clap::Parser, Serialize, Deserialize, Merge)] +#[derive(Clone, Debug, Default, Command, Parser, Merge, serde::Serialize, serde::Deserialize)] #[serde(default, rename_all = "kebab-case", deny_unknown_fields)] pub struct MountCmd { /// The path template to use for snapshots. {id}, {id_long}, {time}, {username}, {hostname}, {label}, {tags}, {backup_start}, {backup_end} are replaced. [default: "[{hostname}]/[{label}]/{time}"] @@ -31,24 +32,29 @@ pub struct MountCmd { time_template: Option, /// Don't allow other users to access the mount point - #[clap(long)] + #[clap(short, long)] #[merge(strategy=conflate::bool::overwrite_false)] - no_allow_other: bool, + exclusive: bool, /// How to handle access to files. [default: "forbidden" for hot/cold repositories, else "read"] #[clap(long)] #[merge(strategy=conflate::option::overwrite_none)] - file_access: Option, + file_access: Option, /// The mount point to use #[clap(value_name = "PATH")] #[merge(strategy=conflate::option::overwrite_none)] - mountpoint: Option, + mount_point: Option, /// Specify directly which snapshot/path to mount #[clap(value_name = "SNAPSHOT[:PATH]")] #[merge(strategy=conflate::option::overwrite_none)] snapshot_path: Option, + + /// Other options to use for mount + #[clap(short, long = "option", value_name = "OPTION")] + #[merge(strategy = conflate::vec::overwrite_empty)] + options: Vec, } impl Override for MountCmd { @@ -56,10 +62,22 @@ impl Override for MountCmd { // a configuration file using explicit flags taken from command-line // arguments. fn override_config(&self, mut config: RusticConfig) -> Result { - let mut self_config = self.clone(); - // merge "mount" section from config file, if given - self_config.merge(config.mount); + // Merge by precedence, cli <- config <- default + let self_config = self + .clone() + .merge_from(config.mount) + .merge_from(Self::with_default_config()); + + // Other values + if self_config.mount_point.is_none() { + return Err(ParseError + .context("Please specify a valid mount point!") + .into()); + } + + // rewrite the "mount" section in the config file config.mount = self_config; + Ok(config) } } @@ -78,24 +96,32 @@ impl Runnable for MountCmd { } impl MountCmd { + fn with_default_config() -> Self { + Self { + path_template: Some(String::from("[{hostname}]/[{label}]/{time}")), + time_template: Some(String::from("%Y-%m-%d_%H-%M-%S")), + options: vec![String::from("kernel_cache")], + ..Default::default() + } + } + fn inner_run(&self, repo: CliIndexedRepo) -> Result<()> { let config = RUSTIC_APP.config(); - let mountpoint = config - .mount - .mountpoint - .as_ref() - .ok_or_else(|| anyhow!("please specify a mountpoint"))?; - - let path_template = config - .mount - .path_template - .clone() - .unwrap_or_else(|| "[{hostname}]/[{label}]/{time}".to_string()); - let time_template = config - .mount - .time_template - .clone() - .unwrap_or_else(|| "%Y-%m-%d_%H-%M-%S".to_string()); + + // We have merged the config file, the command line options, and the + // default values into a single struct. Now we can use the values. + // If a value is missing, we can return an error. + let Some(path_template) = config.mount.path_template.clone() else { + bail!("Please specify a path template!"); + }; + + let Some(time_template) = config.mount.time_template.clone() else { + bail!("Please specify a time template!"); + }; + + let Some(mount_point) = config.mount.mount_point.clone() else { + bail!("Please specify a mount point!"); + }; let sn_filter = |sn: &_| config.snapshot_filter.matches(sn); let vfs = if let Some(snap_path) = &config.mount.snapshot_path { @@ -112,36 +138,39 @@ impl MountCmd { )? }; - let name_opt = format!("fsname=rusticfs:{}", repo.config().id); - let mut options = vec![ - OsStr::new("-o"), - OsStr::new(&name_opt), - OsStr::new("-o"), - OsStr::new("kernel_cache"), - ]; - - if !config.mount.no_allow_other { - options.extend_from_slice(&[ - OsStr::new("-o"), - OsStr::new("allow_other"), - OsStr::new("-o"), - OsStr::new("default_permissions"), - ]); + // Prepare the mount options + let mut mount_options = config.mount.options.clone(); + + mount_options.push(format!("fsname=rusticfs:{}", repo.config().id)); + + if !config.mount.exclusive { + mount_options + .extend_from_slice(&["allow_other".to_string(), "default_permissions".to_string()]); } let file_access = config.mount.file_access.as_ref().map_or_else( || { if repo.config().is_hot == Some(true) { - Ok(FilePolicy::Forbidden) + FilePolicy::Forbidden } else { - Ok(FilePolicy::Read) + FilePolicy::Read } }, - |s| s.parse(), - )?; + |s| *s, + ); let fs = FuseMT::new(FuseFS::new(repo, vfs, file_access), 1); - mount(fs, mountpoint, &options)?; + + // Sort and deduplicate options + mount_options.sort_unstable(); + mount_options.dedup(); + + // join options into a single comma-delimited string and prepent "-o " + // this should be parsed just fine by fuser, here + // https://github.com/cberner/fuser/blob/9f6ced73a36f1d99846e28be9c5e4903939ee9d5/src/mnt/mount_options.rs#L157 + let opt_string = format!("-o {}", mount_options.join(",")); + + mount(fs, mount_point, &[OsStr::new(&opt_string)])?; Ok(()) } diff --git a/src/commands/mount/fusefs.rs b/src/commands/mount/fusefs.rs index 3fc143dd3..eb2f4d185 100644 --- a/src/commands/mount/fusefs.rs +++ b/src/commands/mount/fusefs.rs @@ -164,7 +164,6 @@ impl FilesystemMT for FuseFS { fh: u64, offset: u64, size: u32, - callback: impl FnOnce(ResultSlice<'_>) -> CallbackResult, ) -> CallbackResult { if let Some(open_file) = self.open_files.read().unwrap().get(&fh) {