public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup] close #5571: client: fix regression for `map` command
@ 2024-07-02  7:24 Christian Ebner
  2024-07-02  7:50 ` Gabriel Goller
  2024-07-02  8:44 ` [pbs-devel] applied: " Fabian Grünbichler
  0 siblings, 2 replies; 4+ messages in thread
From: Christian Ebner @ 2024-07-02  7:24 UTC (permalink / raw)
  To: pbs-devel

Fixes a regression introduced by commit 08fe5052:
"client: mount: make split pxar archives mountable"

This commit introduced functionality to mount split pxar archives
(sharing code with the map command), moving the manifest lookup
exclusive to fixed index archives.
However, the lookup now uses the incorrect archive name, not
containing the `.fidx` extension, which is however required for the
lookup in the manifest.

Fix the issue by calling the method with the correct server archive
name including the required extension.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 proxmox-backup-client/src/mount.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/proxmox-backup-client/src/mount.rs b/proxmox-backup-client/src/mount.rs
index d96690839..538a40e68 100644
--- a/proxmox-backup-client/src/mount.rs
+++ b/proxmox-backup-client/src/mount.rs
@@ -302,7 +302,7 @@ async fn mount_do(param: Value, pipe: Option<OwnedFd>) -> Result<Value, Error> {
             }
         }
     } else if server_archive_name.ends_with(".fidx") {
-        let file_info = manifest.lookup_file_info(archive_name)?;
+        let file_info = manifest.lookup_file_info(&server_archive_name)?;
         let index = client
             .download_fixed_index(&manifest, &server_archive_name)
             .await?;
-- 
2.39.2



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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup] close #5571: client: fix regression for `map` command
  2024-07-02  7:24 [pbs-devel] [PATCH proxmox-backup] close #5571: client: fix regression for `map` command Christian Ebner
@ 2024-07-02  7:50 ` Gabriel Goller
  2024-07-02  8:13   ` Christian Ebner
  2024-07-02  8:44 ` [pbs-devel] applied: " Fabian Grünbichler
  1 sibling, 1 reply; 4+ messages in thread
From: Gabriel Goller @ 2024-07-02  7:50 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion

On 02.07.2024 09:24, Christian Ebner wrote:
>Fixes a regression introduced by commit 08fe5052:
>"client: mount: make split pxar archives mountable"
>
>This commit introduced functionality to mount split pxar archives
>(sharing code with the map command), moving the manifest lookup
>exclusive to fixed index archives.
>However, the lookup now uses the incorrect archive name, not
>containing the `.fidx` extension, which is however required for the
>lookup in the manifest.
>
>Fix the issue by calling the method with the correct server archive
>name including the required extension.
>
>Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
>---
> proxmox-backup-client/src/mount.rs | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/proxmox-backup-client/src/mount.rs b/proxmox-backup-client/src/mount.rs
>index d96690839..538a40e68 100644
>--- a/proxmox-backup-client/src/mount.rs
>+++ b/proxmox-backup-client/src/mount.rs
>@@ -302,7 +302,7 @@ async fn mount_do(param: Value, pipe: Option<OwnedFd>) -> Result<Value, Error> {
>             }
>         }
>     } else if server_archive_name.ends_with(".fidx") {
>-        let file_info = manifest.lookup_file_info(archive_name)?;
>+        let file_info = manifest.lookup_file_info(&server_archive_name)?;
>         let index = client
>             .download_fixed_index(&manifest, &server_archive_name)
>             .await?;

Works great!

Consider:
Tested-by: Gabriel Goller <g.goller@proxmox.com>
Reviewed-by: Gabriel Goller <g.goller@proxmox.com>


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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup] close #5571: client: fix regression for `map` command
  2024-07-02  7:50 ` Gabriel Goller
@ 2024-07-02  8:13   ` Christian Ebner
  0 siblings, 0 replies; 4+ messages in thread
From: Christian Ebner @ 2024-07-02  8:13 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Gabriel Goller

> On 02.07.2024 09:50 CEST Gabriel Goller <g.goller@proxmox.com> wrote:
> 
> Works great!
> 
> Consider:
> Tested-by: Gabriel Goller <g.goller@proxmox.com>
> Reviewed-by: Gabriel Goller <g.goller@proxmox.com>

Thanks for double checking Gabriel!

To add, I think it would make sense to create a dedicated wrapper type for archive names, so that such issues are catched by the Rust type system at compile time. Will have a closer look at that.


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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [pbs-devel] applied: [PATCH proxmox-backup] close #5571: client: fix regression for `map` command
  2024-07-02  7:24 [pbs-devel] [PATCH proxmox-backup] close #5571: client: fix regression for `map` command Christian Ebner
  2024-07-02  7:50 ` Gabriel Goller
@ 2024-07-02  8:44 ` Fabian Grünbichler
  1 sibling, 0 replies; 4+ messages in thread
From: Fabian Grünbichler @ 2024-07-02  8:44 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion

with Gabriel's T-b/R-b, and the commit reference reformatted into a
proper "Fixes:" trailer.

On July 2, 2024 9:24 am, Christian Ebner wrote:
> Fixes a regression introduced by commit 08fe5052:
> "client: mount: make split pxar archives mountable"
> 
> This commit introduced functionality to mount split pxar archives
> (sharing code with the map command), moving the manifest lookup
> exclusive to fixed index archives.
> However, the lookup now uses the incorrect archive name, not
> containing the `.fidx` extension, which is however required for the
> lookup in the manifest.
> 
> Fix the issue by calling the method with the correct server archive
> name including the required extension.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
>  proxmox-backup-client/src/mount.rs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/proxmox-backup-client/src/mount.rs b/proxmox-backup-client/src/mount.rs
> index d96690839..538a40e68 100644
> --- a/proxmox-backup-client/src/mount.rs
> +++ b/proxmox-backup-client/src/mount.rs
> @@ -302,7 +302,7 @@ async fn mount_do(param: Value, pipe: Option<OwnedFd>) -> Result<Value, Error> {
>              }
>          }
>      } else if server_archive_name.ends_with(".fidx") {
> -        let file_info = manifest.lookup_file_info(archive_name)?;
> +        let file_info = manifest.lookup_file_info(&server_archive_name)?;
>          let index = client
>              .download_fixed_index(&manifest, &server_archive_name)
>              .await?;
> -- 
> 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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-07-02  8:44 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-02  7:24 [pbs-devel] [PATCH proxmox-backup] close #5571: client: fix regression for `map` command Christian Ebner
2024-07-02  7:50 ` Gabriel Goller
2024-07-02  8:13   ` Christian Ebner
2024-07-02  8:44 ` [pbs-devel] applied: " Fabian Grünbichler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal