From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <c.ebner@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id 782D396640
 for <pbs-devel@lists.proxmox.com>; Tue, 16 Apr 2024 09:50:23 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 57F7315669
 for <pbs-devel@lists.proxmox.com>; Tue, 16 Apr 2024 09:50:23 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [94.136.29.106])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS
 for <pbs-devel@lists.proxmox.com>; Tue, 16 Apr 2024 09:50:22 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 1296444DE1
 for <pbs-devel@lists.proxmox.com>; Tue, 16 Apr 2024 09:50:22 +0200 (CEST)
Message-ID: <d62f6fcf-746b-443d-a20c-5c6fc3d95a33@proxmox.com>
Date: Tue, 16 Apr 2024 09:50:20 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: Proxmox Backup Server development discussion
 <pbs-devel@lists.proxmox.com>, Hannes Laimer <h.laimer@proxmox.com>
References: <20240409110012.166472-1-h.laimer@proxmox.com>
 <20240409110012.166472-12-h.laimer@proxmox.com>
Content-Language: en-US, de-DE
From: Christian Ebner <c.ebner@proxmox.com>
In-Reply-To: <20240409110012.166472-12-h.laimer@proxmox.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.032 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: Re: [pbs-devel] [PATCH proxmox-backup v3 11/24] pb-manager: add
 (un)mount command
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Tue, 16 Apr 2024 07:50:23 -0000

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)