From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 6345D6FDFE for ; Wed, 1 Sep 2021 16:49:31 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 59E839DD3 for ; Wed, 1 Sep 2021 16:49:01 +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 id 3AE1E9D5D for ; Wed, 1 Sep 2021 16:48:57 +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 09693444AB; Wed, 1 Sep 2021 16:48:57 +0200 (CEST) Message-ID: <2ae56265-0733-1ae7-91bd-584a3b3648cd@proxmox.com> Date: Wed, 1 Sep 2021 16:48:55 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:92.0) Gecko/20100101 Thunderbird/92.0 Content-Language: en-US To: Proxmox Backup Server development discussion , Hannes Laimer References: <20210830111505.38694-1-h.laimer@proxmox.com> <20210830111505.38694-8-h.laimer@proxmox.com> From: Dominik Csapak In-Reply-To: <20210830111505.38694-8-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.917 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.932 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [datastore.rs] Subject: Re: [pbs-devel] [PATCH v2 proxmox-backup 07/15] api2: add (un)mount endpoint for removable ds's X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 01 Sep 2021 14:49:31 -0000 Comments inline On 8/30/21 13:14, Hannes Laimer wrote: > Add endpoints for mounting and unmounting removable datastores by their > name. > --- > src/api2/admin/datastore.rs | 144 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 144 insertions(+) > > diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs > index b236dfab..f6adc663 100644 > --- a/src/api2/admin/datastore.rs > +++ b/src/api2/admin/datastore.rs > @@ -39,6 +39,7 @@ use crate::config::datastore; > use crate::config::cached_user_info::CachedUserInfo; > > use crate::server::{jobstate::Job, WorkerTask}; > +use crate::tools::disks::{mount_by_uuid, unmount_by_mount_point}; > > use crate::config::acl::{ > PRIV_DATASTORE_AUDIT, > @@ -1859,6 +1860,139 @@ pub fn set_backup_owner( > Ok(()) > } > > +pub fn do_mount(store: String, auth_id: &Authid) -> Result { > + let (config, _digest) = datastore::config()?; > + let store_config = config.lookup("datastore", &store)?; > + > + let upid_str = WorkerTask::new_thread( > + "mount", > + Some(store.clone()), > + auth_id.clone(), > + false, > + move |_worker| { > + if check_if_available(&store_config).is_ok() { > + bail!( > + "Datastore '{}' can't be mounted because it is already available.", > + &store_config.name > + ); > + }; > + if let (Some(uuid), Some(mount_point)) = ( > + store_config.backing_device, > + store_config.backing_device_mount_point, > + ) { > + let mount_point_path = std::path::Path::new(&mount_point); > + mount_by_uuid(&uuid, &mount_point_path) > + } else { > + bail!( > + "Datastore '{}' can't be mounted because it is not removable.", > + &store_config.name > + ); > + } > + }, > + )?; > + > + Ok(upid_str) > +} > + > +#[api( > + protected: true, > + input: { > + properties: { > + store: { > + schema: DATASTORE_SCHEMA, > + }, > + } > + }, > + returns: { > + schema: UPID_SCHEMA, > + }, > + access: { > + permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, false), that permission looks wrong, maybe _MODIFY instead ? > + }, > +)] > +/// Mount removable datastore. > +pub fn mount(store: String, mut rpcenv: &mut dyn RpcEnvironment) -> Result { > + let (_config, digest) = datastore::config()?; > + > + rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into(); > + > + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; > + > + do_mount(store, &auth_id).map(|upid_str| json!(upid_str)) > +} > + > +#[api( > + protected: true, > + input: { > + properties: { > + store: { > + schema: DATASTORE_SCHEMA, > + }, > + }, > + }, > + returns: { > + schema: UPID_SCHEMA, > + }, > + access: { > + permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, false), same here > + }, > +)] > +/// Unmount removable datastore. > +pub fn unmount(store: String, mut rpcenv: &mut dyn RpcEnvironment) -> Result { > + let (config, digest) = datastore::config()?; > + > + let store_config = config.lookup("datastore", &store)?; > + rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into(); > + > + let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI; > + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; > + > + let upid_str = WorkerTask::new_thread( > + "unmount", > + Some(store.clone()), > + auth_id.clone(), > + to_stdout, > + move |_worker| { > + if !check_if_available(&store_config).is_ok() { > + bail!( > + "Datastore '{}' can't be unmounted because it is not available.", > + &store_config.name > + ); > + }; > + if let (Some(_uuid), Some(mount_point)) = ( > + store_config.backing_device, > + store_config.backing_device_mount_point, > + ) { > + let mount_point_path = std::path::Path::new(&mount_point); > + for job in crate::server::TaskListInfoIterator::new(true)? { > + let job = match job { > + Ok(job) => job, > + Err(_) => break, > + }; > + > + if !job.upid.worker_type.eq("unmount") > + && job.upid.worker_id.map_or(false, |id| id.contains(&store)) i know its what we do for the task filtering, but i don't think its the best approach for this if my datastore name is 'host', i'll not be able to unmount it during any completely unrelated backup of a 'host' sadly i dont have a good alternate solution at hand that can handle old daemons.... so we likely will have to go with this for now... but i would want it at least mark it with a FIXME or TODO so we see that it should be done right... but, since we unmount lazy anyway, we should wait here until its really unmounted, else we might give users a wrong feeling of safety to remove the drive, when in reality its still writing? if we do that, we can toss the job checking code completely, unmount it always lazy and wait until finished? > + { > + bail!( > + "Can't unmount '{}' because there is a '{}' job still running!", > + &store_config.name, > + job.upid.worker_type > + ); > + } > + } > + unmount_by_mount_point(&mount_point_path) > + } else { > + bail!( > + "Datastore '{}' can't be mounted because it is not removable.", > + &store_config.name > + ); > + } > + }, > + )?; > + > + Ok(json!(upid_str)) > +} > + > #[sortable] > const DATASTORE_INFO_SUBDIRS: SubdirMap = &[ > ( > @@ -1904,6 +2038,11 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[ > .get(&API_METHOD_LIST_GROUPS) > .delete(&API_METHOD_DELETE_GROUP) > ), > + ( > + "mount", > + &Router::new() > + .post(&API_METHOD_MOUNT) > + ), > ( > "notes", > &Router::new() > @@ -1941,6 +2080,11 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[ > &Router::new() > .get(&API_METHOD_STATUS) > ), > + ( > + "unmount", > + &Router::new() > + .post(&API_METHOD_UNMOUNT) > + ), > ( > "upload-backup-log", > &Router::new() >