Re: [pbs-devel] [PATCH proxmox-backup v12 13/26] datastore: handle deletion of removable datastore properly

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On September 4, 2024 4:11 pm, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  pbs-datastore/src/datastore.rs |  4 +++-
>  src/api2/config/datastore.rs   | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
> index 29f98b37..df038d62 100644
> --- a/pbs-datastore/src/datastore.rs
> +++ b/pbs-datastore/src/datastore.rs
> @@ -1510,7 +1510,9 @@ impl DataStore {
>                          // weird, but ok
>                      }
>                      Err(err) if err.is_errno(nix::errno::Errno::EBUSY) => {
> -                        warn!("Cannot delete datastore directory (is it a mount point?).")
> +                        if datastore_config.backing_device.is_none() {
> +                            warn!("Cannot delete datastore directory (is it a mount point?).")
> +                        }
>                      }
>                      Err(err) if err.is_errno(nix::errno::Errno::ENOTEMPTY) => {
>                          warn!("Datastore directory not empty, not deleting.")
> diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
> index c24b6e9d..c7d59c44 100644
> --- a/src/api2/config/datastore.rs
> +++ b/src/api2/config/datastore.rs
> @@ -29,9 +29,11 @@ use crate::api2::config::tape_backup_job::{delete_tape_backup_job, list_tape_bac
>  use crate::api2::config::verify::delete_verification_job;
>  use pbs_config::CachedUserInfo;
>  
> +use pbs_datastore::is_datastore_available;
>  use proxmox_rest_server::WorkerTask;
>  
>  use crate::server::jobstate;
> +use crate::tools::disks::unmount_by_mountpoint;
>  
>  #[api(
>      input: {
> @@ -523,6 +525,14 @@ pub async fn delete_datastore(
>          http_bail!(NOT_FOUND, "datastore '{}' does not exist.", name);
>      }
>  
> +    let store_config: DataStoreConfig = config.lookup("datastore", &name)?;
> +    if destroy_data && !is_datastore_available(&store_config) {
> +        http_bail!(
> +            BAD_REQUEST,
> +            "cannot destroy data on '{name}' unless the datastore is mounted"
> +        );
> +    }
> +
>      if !keep_job_configs {
>          for job in list_verification_jobs(Some(name.clone()), Value::Null, rpcenv)? {
>              delete_verification_job(job.config.id, None, rpcenv)?
> @@ -549,6 +559,22 @@ pub async fn delete_datastore(
>  
>      let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
>      let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
> +    let name_copy = name.clone();
> +    tokio::spawn(async move {
> +        if let Ok(proxy_pid) =
> +            proxmox_rest_server::read_pid(pbs_buildcfg::PROXMOX_BACKUP_PROXY_PID_FN)
> +        {
> +            let sock = proxmox_daemon::command_socket::path_from_pid(proxy_pid);
> +            let _ = proxmox_daemon::command_socket::send_raw(
> +                sock,
> +                &format!(
> +                    "{{\"command\":\"update-datastore-cache\",\"args\":\"{}\"}}\n",
> +                    name_copy
> +                ),
> +            )
> +            .await;
> +        }
> +    });

why is this tokio::spawned, but the one in unmount introduced in the
same series is not?

since we now have three call sites with basically the same code, should
we have a helper to do this?

>  
>      let upid = WorkerTask::new_thread(
>          "delete-datastore",
> @@ -561,6 +587,12 @@ pub async fn delete_datastore(
>              // ignore errors
>              let _ = jobstate::remove_state_file("prune", &name);
>              let _ = jobstate::remove_state_file("garbage_collection", &name);
> +            if destroy_data {
> +                if let Some(mount_point) = store_config.get_mount_point() {
> +                    let _ = unmount_by_mountpoint(&mount_point);
> +                    let _ = std::fs::remove_dir(&mount_point);
> +                }
> +            }
>  
>              if let Err(err) =
>                  proxmox_async::runtime::block_on(crate::server::notify_datastore_removed())
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
> 


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel