fix #3335: allow removing datastore contents on delete

Adds an optional 'destroy-data' parameter to the datastore
remove api call.

Based-on: https://lists.proxmox.com/pipermail/pbs-devel/2022-January/004574.html
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>

pbs-datastore/src/datastore.rs

@@ -11,6 +11,7 @@ use nix::unistd::{unlinkat, UnlinkatFlags};
 
 use proxmox_schema::ApiType;
 
+use proxmox_sys::error::SysError;
 use proxmox_sys::fs::{file_read_optional_string, replace_file, CreateOptions};
 use proxmox_sys::fs::{lock_dir_noblock, DirLockGuard};
 use proxmox_sys::process_locker::ProcessLockSharedGuard;
@@ -29,7 +30,7 @@ use crate::fixed_index::{FixedIndexReader, FixedIndexWriter};
 use crate::hierarchy::{ListGroups, ListGroupsType, ListNamespaces, ListNamespacesRecursive};
 use crate::index::IndexFile;
 use crate::manifest::{archive_type, ArchiveType};
-use crate::task_tracking::update_active_operations;
+use crate::task_tracking::{self, update_active_operations};
 use crate::DataBlob;
 
 lazy_static! {
@@ -124,6 +125,10 @@ impl DataStore {
         name: &str,
         operation: Option<Operation>,
     ) -> Result<Arc<DataStore>, Error> {
+        // Avoid TOCTOU between checking maintenance mode and updating active operation counter, as
+        // we use it to decide whether it is okay to delete the datastore.
+        let config_lock = pbs_config::datastore::lock_config()?;
+
         // we could use the ConfigVersionCache's generation for staleness detection, but  we load
         // the config anyway -> just use digest, additional benefit: manual changes get detected
         let (config, digest) = pbs_config::datastore::config()?;
@@ -139,6 +144,9 @@ impl DataStore {
             update_active_operations(name, operation, 1)?;
         }
 
+        // Our operation is registered, unlock the config.
+        drop(config_lock);
+
         let mut datastore_cache = DATASTORE_MAP.lock().unwrap();
         let entry = datastore_cache.get(name);
 
@@ -1325,4 +1333,110 @@ impl DataStore {
         }
         Ok(())
     }
+
+    /// Destroy a datastore. This requires that there are no active operations on the datastore.
+    ///
+    /// This is a synchronous operation and should be run in a worker-thread.
+    pub fn destroy(
+        name: &str,
+        destroy_data: bool,
+        worker: &dyn WorkerTaskContext,
+    ) -> Result<(), Error> {
+        let config_lock = pbs_config::datastore::lock_config()?;
+
+        let (mut config, _digest) = pbs_config::datastore::config()?;
+        let mut datastore_config: DataStoreConfig = config.lookup("datastore", name)?;
+
+        datastore_config.maintenance_mode = Some("type=delete".to_string());
+        config.set_data(name, "datastore", &datastore_config)?;
+        pbs_config::datastore::save_config(&config)?;
+        drop(config_lock);
+
+        let (operations, _lock) = task_tracking::get_active_operations_locked(name)?;
+
+        if operations.read != 0 || operations.write != 0 {
+            bail!("datastore is currently in use");
+        }
+
+        let base = PathBuf::from(&datastore_config.path);
+
+        let mut ok = true;
+        if destroy_data {
+            let remove = |subdir, ok: &mut bool| {
+                if let Err(err) = std::fs::remove_dir_all(base.join(subdir)) {
+                    if err.kind() != io::ErrorKind::NotFound {
+                        task_warn!(worker, "failed to remove {subdir:?} subdirectory: {err}");
+                        *ok = false;
+                    }
+                }
+            };
+
+            task_log!(worker, "Deleting datastore data...");
+            remove("ns", &mut ok); // ns first
+            remove("ct", &mut ok);
+            remove("vm", &mut ok);
+            remove("host", &mut ok);
+
+            if ok {
+                if let Err(err) = std::fs::remove_file(base.join(".gc-status")) {
+                    if err.kind() != io::ErrorKind::NotFound {
+                        task_warn!(worker, "failed to remove .gc-status file: {err}");
+                        ok = false;
+                    }
+                }
+            }
+
+            // chunks get removed last and only if the backups were successfully deleted
+            if ok {
+                remove(".chunks", &mut ok);
+            }
+        }
+
+        // now the config
+        if ok {
+            task_log!(worker, "Removing datastore from config...");
+            let _lock = pbs_config::datastore::lock_config()?;
+            let _ = config.sections.remove(name);
+            pbs_config::datastore::save_config(&config)?;
+        }
+
+        // finally the lock & toplevel directory
+        if destroy_data {
+            if ok {
+                if let Err(err) = std::fs::remove_file(base.join(".lock")) {
+                    if err.kind() != io::ErrorKind::NotFound {
+                        task_warn!(worker, "failed to remove .lock file: {err}");
+                        ok = false;
+                    }
+                }
+            }
+
+            if ok {
+                task_log!(worker, "Finished deleting data.");
+
+                match std::fs::remove_dir(base) {
+                    Ok(()) => task_log!(worker, "Removed empty datastore directory."),
+                    Err(err) if err.kind() == io::ErrorKind::NotFound => {
+                        // weird, but ok
+                    }
+                    Err(err) if err.is_errno(nix::errno::Errno::EBUSY) => {
+                        task_warn!(
+                            worker,
+                            "Cannot delete datastore directory (is it a mount point?)."
+                        )
+                    }
+                    Err(err) if err.is_errno(nix::errno::Errno::ENOTEMPTY) => {
+                        task_warn!(worker, "Datastore directory not empty, not deleting.")
+                    }
+                    Err(err) => {
+                        task_warn!(worker, "Failed to remove datastore directory: {err}");
+                    }
+                }
+            } else {
+                task_log!(worker, "There were errors deleting data.");
+            }
+        }
+
+        Ok(())
+    }
 }

pbs-datastore/src/task_tracking.rs

@@ -34,10 +34,37 @@ struct TaskOperations {
     active_operations: ActiveOperationStats,
 }
 
-pub fn get_active_operations(name: &str) -> Result<ActiveOperationStats, Error> {
-    let path = PathBuf::from(format!("{}/{}", crate::ACTIVE_OPERATIONS_DIR, name));
+fn open_lock_file(name: &str) -> Result<(std::fs::File, CreateOptions), Error> {
+    let user = pbs_config::backup_user()?;
 
-    Ok(match file_read_optional_string(&path)? {
+    let lock_path = PathBuf::from(format!("{}/{}.lock", crate::ACTIVE_OPERATIONS_DIR, name));
+
+    let options = CreateOptions::new()
+        .group(user.gid)
+        .owner(user.uid)
+        .perm(nix::sys::stat::Mode::from_bits_truncate(0o660));
+
+    let timeout = std::time::Duration::new(10, 0);
+
+    Ok((
+        open_file_locked(&lock_path, timeout, true, options.clone())?,
+        options,
+    ))
+}
+
+/// MUST return `Some(file)` when `lock` is `true`.
+fn get_active_operations_do(
+    name: &str,
+    lock: bool,
+) -> Result<(ActiveOperationStats, Option<std::fs::File>), Error> {
+    let path = PathBuf::from(format!("{}/{}", crate::ACTIVE_OPERATIONS_DIR, name));
+    let lock = if lock {
+        Some(open_lock_file(name)?.0)
+    } else {
+        None
+    };
+
+    let data = match file_read_optional_string(&path)? {
         Some(data) => serde_json::from_str::<Vec<TaskOperations>>(&data)?
             .iter()
             .filter_map(
@@ -48,21 +75,26 @@ pub fn get_active_operations(name: &str) -> Result<ActiveOperationStats, Error>
             )
             .sum(),
         None => ActiveOperationStats::default(),
-    })
+    };
+
+    Ok((data, lock))
+}
+
+pub fn get_active_operations(name: &str) -> Result<ActiveOperationStats, Error> {
+    Ok(get_active_operations_do(name, false)?.0)
+}
+
+pub fn get_active_operations_locked(
+    name: &str,
+) -> Result<(ActiveOperationStats, std::fs::File), Error> {
+    let (data, lock) = get_active_operations_do(name, true)?;
+    Ok((data, lock.unwrap()))
 }
 
 pub fn update_active_operations(name: &str, operation: Operation, count: i64) -> Result<(), Error> {
     let path = PathBuf::from(format!("{}/{}", crate::ACTIVE_OPERATIONS_DIR, name));
-    let lock_path = PathBuf::from(format!("{}/{}.lock", crate::ACTIVE_OPERATIONS_DIR, name));
 
-    let user = pbs_config::backup_user()?;
-    let options = CreateOptions::new()
-        .group(user.gid)
-        .owner(user.uid)
-        .perm(nix::sys::stat::Mode::from_bits_truncate(0o660));
-
-    let timeout = std::time::Duration::new(10, 0);
-    let _lock = open_file_locked(&lock_path, timeout, true, options.clone())?;
+    let (_lock, options) = open_lock_file(name)?;
 
     let pid = std::process::id();
     let starttime = procfs::PidStat::read_from_pid(Pid::from_raw(pid as pid_t))?.starttime;

src/api2/config/datastore.rs

@@ -8,12 +8,12 @@ use serde_json::Value;
 use proxmox_router::{http_bail, Permission, Router, RpcEnvironment, RpcEnvironmentType};
 use proxmox_schema::{api, param_bail, ApiType};
 use proxmox_section_config::SectionConfigData;
-use proxmox_sys::WorkerTaskContext;
+use proxmox_sys::{task_warn, WorkerTaskContext};
 
 use pbs_api_types::{
     Authid, DataStoreConfig, DataStoreConfigUpdater, DatastoreNotify, DatastoreTuning,
     DATASTORE_SCHEMA, PRIV_DATASTORE_ALLOCATE, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY,
-    PROXMOX_CONFIG_DIGEST_SCHEMA,
+    PROXMOX_CONFIG_DIGEST_SCHEMA, UPID_SCHEMA,
 };
 use pbs_config::BackupLockGuard;
 use pbs_datastore::chunk_store::ChunkStore;
@@ -386,6 +386,12 @@ pub fn update_datastore(
                 optional: true,
                 default: false,
             },
+            "destroy-data": {
+                description: "Delete the datastore's underlying contents",
+                optional: true,
+                type: bool,
+                default: false,
+            },
             digest: {
                 optional: true,
                 schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
@@ -395,28 +401,29 @@ pub fn update_datastore(
     access: {
         permission: &Permission::Privilege(&["datastore", "{name}"], PRIV_DATASTORE_ALLOCATE, false),
     },
+    returns: {
+        schema: UPID_SCHEMA,
+    },
 )]
-/// Remove a datastore configuration.
+/// Remove a datastore configuration and optionally delete all its contents.
 pub async fn delete_datastore(
     name: String,
     keep_job_configs: bool,
+    destroy_data: bool,
     digest: Option<String>,
     rpcenv: &mut dyn RpcEnvironment,
-) -> Result<(), Error> {
+) -> Result<String, Error> {
     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 => http_bail!(NOT_FOUND, "datastore '{}' does not exist.", name),
+    if !config.sections.contains_key(&name) {
+        http_bail!(NOT_FOUND, "datastore '{}' does not exist.", name);
     }
 
     if !keep_job_configs {
@@ -436,15 +443,32 @@ pub async fn delete_datastore(
         }
     }
 
-    pbs_config::datastore::save_config(&config)?;
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
 
-    // ignore errors
-    let _ = jobstate::remove_state_file("prune", &name);
-    let _ = jobstate::remove_state_file("garbage_collection", &name);
+    let upid = WorkerTask::new_thread(
+        "delete-datastore",
+        Some(name.clone()),
+        auth_id.to_string(),
+        to_stdout,
+        move |worker| {
+            pbs_datastore::DataStore::destroy(&name, destroy_data, &worker)?;
 
-    crate::server::notify_datastore_removed().await?;
+            // ignore errors
+            let _ = jobstate::remove_state_file("prune", &name);
+            let _ = jobstate::remove_state_file("garbage_collection", &name);
 
-    Ok(())
+            if let Err(err) =
+                proxmox_async::runtime::block_on(crate::server::notify_datastore_removed())
+            {
+                task_warn!(worker, "failed to notify after datastore removal: {err}");
+            }
+
+            Ok(())
+        },
+    )?;
+
+    Ok(upid)
 }
 
 const ITEM_ROUTER: Router = Router::new()

src/bin/proxmox_backup_manager/datastore.rs

@@ -4,7 +4,7 @@ use serde_json::Value;
 use proxmox_router::{cli::*, ApiHandler, RpcEnvironment};
 use proxmox_schema::api;
 
-use pbs_api_types::{DataStoreConfig, DATASTORE_SCHEMA};
+use pbs_api_types::{DataStoreConfig, DATASTORE_SCHEMA, PROXMOX_CONFIG_DIGEST_SCHEMA};
 use pbs_client::view_task_result;
 
 use proxmox_backup::api2;
@@ -99,6 +99,46 @@ 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,
+            },
+            "destroy-data": {
+                description: "Delete the datastore's underlying contents",
+                optional: true,
+                type: bool,
+                default: false,
+            },
+            digest: {
+                optional: true,
+                schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
+            },
+        },
+    },
+)]
+/// Remove a datastore configuration.
+async fn delete_datastore(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), 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(())
+}
+
 pub fn datastore_commands() -> CommandLineInterface {
     let cmd_def = CliCommandMap::new()
         .insert("list", CliCommand::new(&API_METHOD_LIST_DATASTORES))
@@ -128,7 +168,7 @@ pub fn datastore_commands() -> CommandLineInterface {
         )
         .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),
         );