[pbs-devel] [PATCH proxmox-backup] Fix 3335: Allow removing datastore contents on delete
Dylan Whyte
d.whyte at proxmox.com
Tue Jan 25 18:44:39 CET 2022
Please ignore. Immediately after sending, I realised a last minute change (which i thought i tested) broke the build. Sending a new version..
Dylan
> On 25.01.2022 18:33 Dylan Whyte <d.whyte at proxmox.com> wrote:
>
>
> This adds an option to 'datastore remove', to additionally remove the
> datatore's underlying contents.
>
> In addition, the task is now carried out in a worker, in order to
> prevent the GUI from blocking or timing out during a removal (GUI patch
> to follow).
>
> Signed-off-by: Dylan Whyte <d.whyte at proxmox.com>
> ---
> src/api2/config/datastore.rs | 85 ++++++++++++++++++---
> src/bin/proxmox_backup_manager/datastore.rs | 55 ++++++++++++-
> 2 files changed, 126 insertions(+), 14 deletions(-)
>
> diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
> index 60bc3c0e..2605cab1 100644
> --- a/src/api2/config/datastore.rs
> +++ b/src/api2/config/datastore.rs
> @@ -8,7 +8,7 @@ use hex::FromHex;
> use proxmox_router::{Router, RpcEnvironment, RpcEnvironmentType, Permission};
> use proxmox_schema::{api, ApiType};
> use proxmox_section_config::SectionConfigData;
> -use proxmox_sys::WorkerTaskContext;
> +use proxmox_sys::{WorkerTaskContext, task_log};
>
> use pbs_datastore::chunk_store::ChunkStore;
> use pbs_config::BackupLockGuard;
> @@ -16,7 +16,7 @@ use pbs_api_types::{
> Authid, DatastoreNotify,
> DATASTORE_SCHEMA, PROXMOX_CONFIG_DIGEST_SCHEMA,
> PRIV_DATASTORE_ALLOCATE, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY,
> - DataStoreConfig, DataStoreConfigUpdater,
> + PRIV_SYS_MODIFY, DataStoreConfig, DataStoreConfigUpdater,
> };
>
> use crate::api2::config::sync::delete_sync_job;
> @@ -329,6 +329,12 @@ pub fn update_datastore(
> optional: true,
> schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
> },
> + "destroy-data": {
> + description: "Delete the datastore's underlying contents",
> + optional: true,
> + type: bool,
> + default: false,
> + },
> },
> },
> access: {
> @@ -340,23 +346,19 @@ pub async fn delete_datastore(
> name: String,
> keep_job_configs: bool,
> digest: Option<String>,
> + destroy_data: Option<bool>,
> rpcenv: &mut dyn RpcEnvironment,
> -) -> Result<(), Error> {
> +) -> Result<String, Error> {
>
> - let _lock = pbs_config::datastore::lock_config()?;
> + let lock = pbs_config::datastore::lock_config()?;
>
> - let (mut config, expected_digest) = pbs_config::datastore::config()?;
> + let (config, expected_digest) = pbs_config::datastore::config()?;
>
> if let Some(ref digest) = digest {
> let digest = <[u8; 32]>::from_hex(digest)?;
> crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
> }
>
> - match config.sections.get(&name) {
> - Some(_) => { config.sections.remove(&name); },
> - None => bail!("datastore '{}' does not exist.", name),
> - }
> -
> if !keep_job_configs {
> for job in list_verification_jobs(Some(name.clone()), Value::Null, rpcenv)? {
> delete_verification_job(job.config.id, None, rpcenv)?
> @@ -371,7 +373,23 @@ pub async fn delete_datastore(
> }
> }
>
> - pbs_config::datastore::save_config(&config)?;
> + let datastore_config: DataStoreConfig = config.lookup("datastore", &name)?;
> +
> + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> + let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
> +
> + let upid = WorkerTask::new_thread(
> + "delete-datastore",
> + Some(datastore_config.name.to_string()),
> + auth_id.to_string(),
> + to_stdout,
> + move |worker| do_delete_datastore(
> + lock,
> + config,
> + datastore_config,
> + destroy_data,
> + &worker),
> + )?;
>
> // ignore errors
> let _ = jobstate::remove_state_file("prune", &name);
> @@ -379,6 +397,51 @@ pub async fn delete_datastore(
>
> crate::server::notify_datastore_removed().await?;
>
> + Ok(upid)
> +}
> +
> +fn do_delete_datastore(
> + _lock: BackupLockGuard,
> + mut config: SectionConfigData,
> + datastore: DataStoreConfig,
> + destroy_data: Option<bool>,
> + worker: &dyn WorkerTaskContext,
> +) -> Result<(), Error> {
> + task_log!(worker, "Removing datastore: {}...", datastore.name);
> + match config.sections.get(&datastore.name) {
> + Some(_) => {
> + if let Some(_destroy_data) = destroy_data {
> + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> + let user_info = CachedUserInfo::new()?;
> + user_info.check_privs(&auth_id, &["datastore", &datastore_config.name], PRIV_SYS_MODIFY, true)?;
> +
> + task_log!(worker, "Destroying datastore data...");
> + match std::fs::read_dir(&datastore.path) {
> + Ok(dir) => {
> + for entry in dir {
> + if let Ok(entry) = entry {
> + let path = entry.path();
> + task_log!(worker, "Removing {}...", path.to_str().unwrap());
> + if path.is_dir() {
> + std::fs::remove_dir_all(path)?;
> + } else {
> + std::fs::remove_file(path)?;
> + }
> + };
> + }
> + task_log!(worker, "Finished destroying data...");
> + },
> + Err(err) => bail!("Failed to read {}: {}", &datastore.path, err),
> + }
> + }
> + task_log!(worker, "Removing datastore from config...");
> + config.sections.remove(&datastore.name);
> + },
> + None => bail!("datastore '{}' does not exist.", datastore.name),
> + }
> +
> + pbs_config::datastore::save_config(&config)?;
> +
> Ok(())
> }
>
> diff --git a/src/bin/proxmox_backup_manager/datastore.rs b/src/bin/proxmox_backup_manager/datastore.rs
> index a35e7bf5..98a0b1b2 100644
> --- a/src/bin/proxmox_backup_manager/datastore.rs
> +++ b/src/bin/proxmox_backup_manager/datastore.rs
> @@ -1,11 +1,13 @@
> use anyhow::Error;
> use serde_json::Value;
>
> -use proxmox_router::{cli::*, ApiHandler, RpcEnvironment};
> +use proxmox_router::{cli::*, ApiHandler, RpcEnvironment, Permission};
> use proxmox_schema::api;
>
> use pbs_client::view_task_result;
> -use pbs_api_types::{DataStoreConfig, DATASTORE_SCHEMA};
> +use pbs_api_types::{DataStoreConfig, DATASTORE_SCHEMA, PROXMOX_CONFIG_DIGEST_SCHEMA,
> + PRIV_DATASTORE_ALLOCATE
> +};
>
> use proxmox_backup::api2;
> use proxmox_backup::client_helpers::connect_to_localhost;
> @@ -100,6 +102,53 @@ async fn create_datastore(mut param: Value) -> Result<Value, Error> {
> Ok(Value::Null)
> }
>
> +#[api(
> + protected: true,
> + input: {
> + properties: {
> + name: {
> + schema: DATASTORE_SCHEMA,
> + },
> + "keep-job-configs": {
> + description: "If enabled, the job configurations related to this datastore will be kept.",
> + type: bool,
> + optional: true,
> + default: false,
> + },
> + digest: {
> + optional: true,
> + schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
> + },
> + "destroy-data": {
> + description: "Delete the datastore's underlying contents",
> + optional: true,
> + type: bool,
> + default: false,
> + }
> + },
> + },
> + access: {
> + permission: &Permission::Privilege(&["datastore", "{name}"], PRIV_DATASTORE_ALLOCATE, false),
> + },
> +)]
> +/// Remove a datastore configuration.
> +async fn delete_datastore(
> + mut param: Value,
> + rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<Value, Error> {
> + param["node"] = "localhost".into();
> +
> + let info = &api2::config::datastore::API_METHOD_DELETE_DATASTORE;
> + 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(Value::Null)
> +
> +}
> +
> pub fn datastore_commands() -> CommandLineInterface {
>
> let cmd_def = CliCommandMap::new()
> @@ -121,7 +170,7 @@ pub fn datastore_commands() -> CommandLineInterface {
> .completion_cb("prune-schedule", pbs_config::datastore::complete_calendar_event)
> )
> .insert("remove",
> - CliCommand::new(&api2::config::datastore::API_METHOD_DELETE_DATASTORE)
> + CliCommand::new(&API_METHOD_DELETE_DATASTORE)
> .arg_param(&["name"])
> .completion_cb("name", pbs_config::datastore::complete_datastore_name)
> );
> --
> 2.30.2
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
More information about the pbs-devel
mailing list