all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Christian Ebner <c.ebner@proxmox.com>, pbs-devel@lists.proxmox.com
Subject: Re: [PATCH proxmox-backup] fix #7305: client: restore: filter out last snapshot if not finished
Date: Thu, 12 Feb 2026 15:54:40 +0100	[thread overview]
Message-ID: <1770907549.yrzyvu2z27.astroid@yuna.none> (raw)
In-Reply-To: <20260211152452.894157-1-c.ebner@proxmox.com>

On February 11, 2026 4:24 pm, Christian Ebner wrote:
> When invoking the client CLI for restore via:
> ```
> proxmox-backup-client restore <snapshot> <archive> <target>
> ```
> the <archive> provide a backup group only, in which case the client
> falls back to restore the last snapshot of that group as convenience.
> 
> The snapshot listing used to identify the last snapshot returns
> however all snapshots, including unfinished ones, a restore on an
> unfinished one will therefore fail.
> 
> Fix this by filtering out snapshots which do not contain a manifest
> file yet, and therefore are not considered as finished. Also adapt
> the helper function name to reflect this change. Ideally this would
> happen on the server side, guarded by some flag, but that will not
> work with older server versions.
> 
> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=7305
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> Note:
> The behaviour can be easily tested by simply temporarily renaming the
> index.json.blob in the last snapshot of a backup group to simulate an
> ongoing backup for that snapshot.
> 
>  proxmox-backup-client/src/main.rs | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
> index 999e50205..7e20b9cc8 100644
> --- a/proxmox-backup-client/src/main.rs
> +++ b/proxmox-backup-client/src/main.rs
> @@ -160,7 +160,7 @@ async fn api_datastore_list_snapshots(
>      Ok(result["data"].take())
>  }
>  
> -pub async fn api_datastore_latest_snapshot(
> +pub async fn api_datastore_latest_finished_snapshot(
>      client: &HttpClient,
>      store: &str,
>      ns: &BackupNamespace,
> @@ -175,6 +175,21 @@ pub async fn api_datastore_latest_snapshot(
>  
>      list.sort_unstable_by(|a, b| b.backup.time.cmp(&a.backup.time));
>  
> +    // only once a snapshots contains the manifest it is considered finished
> +    let last_is_finished = list[0]
> +        .files
> +        .iter()
> +        .any(|content| content.filename == MANIFEST_BLOB_NAME.as_ref());
> +
> +    if !last_is_finished {
> +        if list.len() > 1 {
> +            log::info!("last snapshot of group {group} not finished, use previous instead");
> +            return Ok((group, list[1].backup.time).into());
> +        } else {
> +            bail!("backup group {group} does not contain finished snapshots.");
> +        }
> +    }

couldn't this be a combinator instead?

i.e., something like

    let last_finished_time = list
        .iter()
        .find_map(|snap| {
            snap.files.iter().find_map(|content| {
                if content.filename == MANIFEST_BLOB_NAME.as_ref() {
                    Some(snap.backup.time)
                } else {
                    None
                }
            })
        })
        .ok_or_else(|| {
            format_err!("backup group {group} does not contain any finished snapshots.")
        })?;

to account for multiple snapshots with missing manifests? those should
not occur "naturally", but might exist cause of accidents/..

> +
>      Ok((group, list[0].backup.time).into())
>  }
>  
> @@ -187,7 +202,7 @@ pub async fn dir_or_last_from_group(
>      match path.parse::<BackupPart>()? {
>          BackupPart::Dir(dir) => Ok(dir),
>          BackupPart::Group(group) => {
> -            api_datastore_latest_snapshot(client, repo.store(), ns, group).await
> +            api_datastore_latest_finished_snapshot(client, repo.store(), ns, group).await
>          }
>      }
>  }
> -- 
> 2.47.3
> 
> 
> 
> 
> 
> 




  reply	other threads:[~2026-02-12 14:54 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-11 15:24 Christian Ebner
2026-02-12 14:54 ` Fabian Grünbichler [this message]
2026-02-13  8:54   ` Christian Ebner
2026-02-13 10:44 ` superseded: " Christian Ebner
2026-02-13 10:43 Christian Ebner
2026-02-13 10:45 ` Christian Ebner
2026-02-13 10:51 ` Fabian Grünbichler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1770907549.yrzyvu2z27.astroid@yuna.none \
    --to=f.gruenbichler@proxmox.com \
    --cc=c.ebner@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal