Re: [pbs-devel] [PATCH proxmox-backup v3 11/24] pb-manager: add (un)mount command

[Christian Ebner <c.ebner@proxmox.com>]

one question inline

On 4/9/24 12:59, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>   pbs-config/src/datastore.rs                 | 14 ++++
>   src/bin/proxmox_backup_manager/datastore.rs | 76 ++++++++++++++++++++-
>   2 files changed, 89 insertions(+), 1 deletion(-)
> 
> diff --git a/pbs-config/src/datastore.rs b/pbs-config/src/datastore.rs
> index 5844a174..a540788f 100644
> --- a/pbs-config/src/datastore.rs
> +++ b/pbs-config/src/datastore.rs
> @@ -63,6 +63,20 @@ pub fn complete_datastore_name(_arg: &str, _param: &HashMap<String, String>) ->
>       }
>   }
>   
> +pub fn complete_removable_datastore_name(
> +    _arg: &str,
> +    _param: &HashMap<String, String>,
> +) -> Vec<String> {
> +    match config() {
> +        Ok((data, _digest)) => data
> +            .sections
> +            .into_iter()
> +            .filter_map(|(name, (_, c))| c.get("backing-device").map(|_| name))
> +            .collect(),
> +        Err(_) => Vec::new(),
> +    }
> +}
> +
>   pub fn complete_acl_path(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
>       let mut list = vec![
>           String::from("/"),
> diff --git a/src/bin/proxmox_backup_manager/datastore.rs b/src/bin/proxmox_backup_manager/datastore.rs
> index 383bcd24..f8462459 100644
> --- a/src/bin/proxmox_backup_manager/datastore.rs
> +++ b/src/bin/proxmox_backup_manager/datastore.rs
> @@ -1,4 +1,4 @@
> -use anyhow::Error;
> +use anyhow::{format_err, Error};
>   use serde_json::Value;
>   
>   use proxmox_router::{cli::*, ApiHandler, RpcEnvironment};
> @@ -40,6 +40,34 @@ fn list_datastores(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Valu
>       Ok(Value::Null)
>   }
>   
> +#[api(
> +    protected: true,
> +    input: {
> +        properties: {
> +            store: {
> +                schema: DATASTORE_SCHEMA,
> +            },
> +            digest: {
> +                optional: true,
> +                schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
> +            },
> +        },
> +    },
> +)]
> +/// Mount a removable a datastore configuration.
> +async fn mount_datastore(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
> +    param["node"] = "localhost".into();
> +
> +    let info = &api2::admin::datastore::API_METHOD_MOUNT;
> +    let result = match info.handler {
> +        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
> +        _ => unreachable!(),
> +    };
> +
> +    crate::wait_for_local_worker(result.as_str().unwrap()).await?;

as already mentioned in the patch introducing the mount api method, is 
the dedicated worker task needed for the mount? Is this expected to take 
some prolonged time in some cases?

> +    Ok(())
> +}
> +
>   #[api(
>       input: {
>           properties: {
> @@ -99,6 +127,34 @@ async fn create_datastore(mut param: Value) -> Result<Value, Error> {
>       Ok(Value::Null)
>   }
>   
> +#[api(
> +    protected: true,
> +    input: {
> +        properties: {
> +            store: {
> +                schema: DATASTORE_SCHEMA,
> +            },
> +            digest: {
> +                optional: true,
> +                schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
> +            },
> +        },
> +    },
> +)]
> +/// Unmunt a removable a datastore configuration.
> +async fn unmount_datastore(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
> +    param["node"] = "localhost".into();
> +
> +    let info = &api2::admin::datastore::API_METHOD_UNMOUNT;
> +    let result = match info.handler {
> +        ApiHandler::Async(handler) => (handler)(param, info, rpcenv).await?,
> +        _ => unreachable!(),
> +    };
> +
> +    crate::wait_for_local_worker(result.as_str().unwrap()).await?;
> +    Ok(())
> +}
> +
>   #[api(
>       protected: true,
>       input: {
> @@ -142,6 +198,15 @@ async fn delete_datastore(mut param: Value, rpcenv: &mut dyn RpcEnvironment) ->
>   pub fn datastore_commands() -> CommandLineInterface {
>       let cmd_def = CliCommandMap::new()
>           .insert("list", CliCommand::new(&API_METHOD_LIST_DATASTORES))
> +        .insert(
> +            "mount",
> +            CliCommand::new(&API_METHOD_MOUNT_DATASTORE)
> +                .arg_param(&["store"])
> +                .completion_cb(
> +                    "store",
> +                    pbs_config::datastore::complete_removable_datastore_name,
> +                ),
> +        )
>           .insert(
>               "show",
>               CliCommand::new(&API_METHOD_SHOW_DATASTORE)
> @@ -152,6 +217,15 @@ pub fn datastore_commands() -> CommandLineInterface {
>               "create",
>               CliCommand::new(&API_METHOD_CREATE_DATASTORE).arg_param(&["name", "path"]),
>           )
> +        .insert(
> +            "unmount",
> +            CliCommand::new(&API_METHOD_UNMOUNT_DATASTORE)
> +                .arg_param(&["store"])
> +                .completion_cb(
> +                    "store",
> +                    pbs_config::datastore::complete_removable_datastore_name,
> +                ),
> +        )
>           .insert(
>               "update",
>               CliCommand::new(&api2::config::datastore::API_METHOD_UPDATE_DATASTORE)