From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pbs-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
by lore.proxmox.com (Postfix) with ESMTPS id DE9B21FF15E
for <inbox@lore.proxmox.com>; Tue, 28 Jan 2025 17:39:26 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 20D0A123C3;
Tue, 28 Jan 2025 17:39:25 +0100 (CET)
Message-ID: <9ffbed92-a6d4-46ac-acf2-0462fe08ca94@proxmox.com>
Date: Tue, 28 Jan 2025 17:38:50 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Content-Language: en-US
To: Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>, Christian Ebner <c.ebner@proxmox.com>
References: <20250128150907.437556-1-c.ebner@proxmox.com>
From: Markus Frank <m.frank@proxmox.com>
In-Reply-To: <20250128150907.437556-1-c.ebner@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.017 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
T_SCC_BODY_TEXT_LINE -0.01 -
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [lib.rs, restore.rs]
Subject: Re: [pbs-devel] [PATCH proxmox-backup-qemu 1/2] fix: never append
archive name extensions twice, use type instead
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
<pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com>
Thanks for the fix.
I have run a few qmrestore tasks with this patch applied.
pbs-restore now works as expected (with both .img.fidx and .img as filename extensions).
Tested-by: Markus Frank <m.frank@proxmox.com>
On 2025-01-28 16:09, Christian Ebner wrote:
> Commit 16298c5f ("update to current PBS master") introduced a helper
> method to generate the backup archive names from a device name.
>
> However, the helper was also used incorrectly to generate archive
> names from strings already having the `.img.fidx` filename extension.
> As the extension is appended unconditionally, this leads to an
> archive name which will parse successfully, but not be the expected
> archive name to be found in the backup manifest.
>
> Fix this by dropping the incorrect helper call-sites and adapt the
> function parameters to take a `BackupArchiveName` directly, reducing
> potential for future mishandling.
>
> Fixes: Commit 16298c5f ("update to current PBS master")
> Reported-by: Markus Frank <m.frank@proxmox.com>
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> src/lib.rs | 14 +++++++++++---
> src/restore.rs | 10 ++++------
> 2 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/src/lib.rs b/src/lib.rs
> index 0a1239b..ed94c9c 100644
> --- a/src/lib.rs
> +++ b/src/lib.rs
> @@ -8,7 +8,7 @@ use std::sync::{Arc, Condvar, Mutex};
>
> use proxmox_lang::try_block;
>
> -use pbs_api_types::{Authid, BackupDir, BackupNamespace, BackupType, CryptMode};
> +use pbs_api_types::{Authid, BackupArchiveName, BackupDir, BackupNamespace, BackupType, CryptMode};
> use pbs_client::BackupRepository;
>
> pub mod capi_types;
> @@ -979,6 +979,7 @@ pub extern "C" fn proxmox_restore_image(
> let result: Result<_, Error> = try_block!({
> let archive_name = tools::utf8_c_string(archive_name)?
> .ok_or_else(|| format_err!("archive_name must not be NULL"))?;
> + let archive_name: BackupArchiveName = archive_name.parse()?;
>
> let write_data_callback = move |offset: u64, data: &[u8]| {
> callback(callback_data, offset, data.as_ptr(), data.len() as u64)
> @@ -988,7 +989,7 @@ pub extern "C" fn proxmox_restore_image(
> move |offset: u64, len: u64| callback(callback_data, offset, std::ptr::null(), len);
>
> proxmox_async::runtime::block_on(restore_task.restore_image(
> - archive_name,
> + &archive_name,
> write_data_callback,
> write_zero_callback,
> verbose,
> @@ -1051,9 +1052,16 @@ pub extern "C" fn proxmox_restore_open_image_async(
>
> param_not_null!(archive_name, callback_info);
> let archive_name = unsafe { tools::utf8_c_string_lossy_non_null(archive_name) };
> + let archive_name = match archive_name.parse() {
> + Ok(archive_name) => archive_name,
> + Err(err) => {
> + callback_info.send_result(Err(err));
> + return;
> + }
> + };
>
> restore_task.runtime().spawn(async move {
> - let result = match restore_task.open_image(archive_name).await {
> + let result = match restore_task.open_image(&archive_name).await {
> Ok(res) => Ok(res as i32),
> Err(err) => Err(err),
> };
> diff --git a/src/restore.rs b/src/restore.rs
> index d3818e9..a879b99 100644
> --- a/src/restore.rs
> +++ b/src/restore.rs
> @@ -3,6 +3,7 @@ use std::sync::{Arc, Mutex};
>
> use anyhow::{bail, format_err, Error};
> use once_cell::sync::OnceCell;
> +use pbs_api_types::BackupArchiveName;
> use tokio::runtime::Runtime;
>
> use proxmox_async::runtime::get_runtime_with_builder;
> @@ -19,7 +20,6 @@ use pbs_tools::crypt_config::CryptConfig;
>
> use super::BackupSetup;
> use crate::capi_types::DataPointer;
> -use crate::commands::archive_name;
> use crate::registry::Registry;
> use crate::shared_cache::get_shared_chunk_cache;
>
> @@ -119,12 +119,11 @@ impl RestoreTask {
>
> pub async fn restore_image(
> &self,
> - archive_str: String,
> + archive_name: &BackupArchiveName,
> write_data_callback: impl Fn(u64, &[u8]) -> i32,
> write_zero_callback: impl Fn(u64, u64) -> i32,
> verbose: bool,
> ) -> Result<(), Error> {
> - let archive_name = archive_name(&archive_str)?;
> if verbose {
> eprintln!("download and verify backup index");
> }
> @@ -218,8 +217,7 @@ impl RestoreTask {
> Ok(info.archive_size)
> }
>
> - pub async fn open_image(&self, archive_str: String) -> Result<u8, Error> {
> - let archive_name = archive_name(&archive_str)?;
> + pub async fn open_image(&self, archive_name: &BackupArchiveName) -> Result<u8, Error> {
> let client = match self.client.get() {
> Some(reader) => Arc::clone(reader),
> None => bail!("not connected"),
> @@ -254,7 +252,7 @@ impl RestoreTask {
>
> let info = ImageAccessInfo {
> archive_size,
> - _archive_name: archive_str,
> + _archive_name: archive_name.to_string(),
> // useful to debug
> reader,
> };
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel