Re: [pbs-devel] [RFC PATCH proxmox-backup 1/6] api2: Add `protected` parameter to `finish` endpoint

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

On January 18, 2023 11:48 am, Christoph Heiss wrote:
> Groundwork for fixing #4289. This way, a backup can be set as protected
> without a separate API call and thus avoid locking problems.
> 
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
>  pbs-client/src/backup_writer.rs |  4 ++--
>  src/api2/backup/environment.rs  | 11 +++++++++++
>  src/api2/backup/mod.rs          | 25 ++++++++++++++++++++-----
>  3 files changed, 33 insertions(+), 7 deletions(-)
> 
> diff --git a/pbs-client/src/backup_writer.rs b/pbs-client/src/backup_writer.rs
> index be6da2a6..3e4b4878 100644
> --- a/pbs-client/src/backup_writer.rs
> +++ b/pbs-client/src/backup_writer.rs
> @@ -165,10 +165,10 @@ impl BackupWriter {
>          self.h2.upload("PUT", path, param, content_type, data).await
>      }
> 
> -    pub async fn finish(self: Arc<Self>) -> Result<(), Error> {
> +    pub async fn finish(self: Arc<Self>, param: Option<Value>) -> Result<(), Error> {
>          let h2 = self.h2.clone();
> 
> -        h2.post("finish", None)
> +        h2.post("finish", param)
>              .map_ok(move |_| {
>                  self.abort.abort();
>              })

IMHO this hunk belongs in the next patch (it's not about the endpoint, but about
the client)

> diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs
> index 4f07f9b4..05ee12ea 100644
> --- a/src/api2/backup/environment.rs
> +++ b/src/api2/backup/environment.rs
> @@ -590,6 +590,17 @@ impl BackupEnvironment {
>          Ok(())
>      }
> 
> +    /// Sets the backup as protected.
> +    pub fn set_protected(&self) -> Result<(), Error> {
> +        let state = self.state.lock().unwrap();
> +        state.ensure_unfinished()?;
> +
> +        let protected_path = self.backup_dir.protected_file();
> +        std::fs::File::create(protected_path)
> +            .map(|_| ())
> +            .map_err(|err| format_err!("could not create protection file: {}", err))
> +    }
> +
>      /// Mark backup as finished
>      pub fn finish_backup(&self) -> Result<(), Error> {
>          let mut state = self.state.lock().unwrap();
> diff --git a/src/api2/backup/mod.rs b/src/api2/backup/mod.rs
> index 90e2ea7e..decad084 100644
> --- a/src/api2/backup/mod.rs
> +++ b/src/api2/backup/mod.rs
> @@ -344,10 +344,7 @@ const BACKUP_API_SUBDIRS: SubdirMap = &[
>      ),
>      (
>          "finish",
> -        &Router::new().post(&ApiMethod::new(
> -            &ApiHandler::Sync(&finish_backup),
> -            &ObjectSchema::new("Mark backup as finished.", &[]),
> -        )),
> +        &Router::new().post(&API_METHOD_FINISH_BACKUP),
>      ),
>      (
>          "fixed_chunk",
> @@ -771,12 +768,30 @@ fn close_fixed_index(
>      Ok(Value::Null)
>  }
> 
> +#[sortable]
> +const API_METHOD_FINISH_BACKUP: ApiMethod = ApiMethod::new(
> +    &ApiHandler::Sync(&finish_backup),
> +    &ObjectSchema::new(
> +        "Mark backup as finished.",
> +        &sorted!([
> +            ("protected", true, &BooleanSchema::new("Mark backup as protected").schema()),
> +        ]),
> +    ),
> +);
> +
>  fn finish_backup(
> -    _param: Value,
> +    param: Value,
>      _info: &ApiMethod,
>      rpcenv: &mut dyn RpcEnvironment,
>  ) -> Result<Value, Error> {
>      let env: &BackupEnvironment = rpcenv.as_ref();
> +    let protected = param["protected"].as_bool().unwrap_or(false);
> +
> +    if protected {
> +        if let Err(err) = env.set_protected() {
> +            env.log(format!("failed to set backup protected: {}", err));
> +        }
> +    }
> 
>      env.finish_backup()?;
>      env.log("successfully finished backup");
> --
> 2.34.1
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
>