Re: [pbs-devel] [PATCH v5 vma-to-pbs 00/10] Implement vma-to-pbs tool

["Max Carrara" <m.carrara@proxmox.com>]

On Wed Mar 20, 2024 at 3:15 PM CET, Filip Schauer wrote:
> Implement a tool to import VMA files into a Proxmox Backup Server
>
> Example usage:
>
> zstd -d --stdout vzdump.vma.zst | vma-to-pbs \
>     --repository <auth_id@host:port:datastore> \
>     --vmid 123 \
>     --password_file pbs_password
>
> Commit 07/10 requires
> https://lists.proxmox.com/pipermail/pve-devel/2024-March/062182.html
> to be applied first.
>
> Changes since v4:
> * Bump proxmox-backup-qemu
> * Remove unnecessary "extern crate" declarations
> * Refactor error handling with anyhow
> * vma.rs: Improve code readability by adding constants and using more
>   descriptive variable/type names.
> * vma.rs: Move duplicate code into read_string function
> * Print elapsed time in minutes, seconds and ms
> * Refactor block device id and size retrieval logic
> * vma: Document break statement when reaching end of file
> * Use selected imports instead of glob imports
> * Split up vma2pbs logic into seperate functions
> * Makefile: remove reference to unused submodule

To preface all of my following comments: You really did incorporate a
lot of my feedback, hats off to you! That sure must've been a lot of
work and it certainly does not go unnoticed nor unappreciated.

I do have some more comments, but the vast majority of what I had
mentioned in v4 has been resolved. See more below and in my replies to
individual patches.

First and foremost, the patches 1 - 3 can just be dropped - as they're
already applied, there's no need to keep including them in this series.
(Nevermind, ignore this - you just mentioned it yourself as I was
writing this.)

Furthermore, there are still a handful of things that could use some
untangling and some minor changes otherwise, but it's nowhere near as
much as I had mentioned last time. For more details see my comments
inline on individual patches.

That being said, I was able to successfully build the whole thing and
tried to test it against some locally stored backups - unfortunately
without success. Here's the (adapted) output:


zstd -d --stdout vzdump-qemu-300-2024_03_25-10_21_10.vma.zst | vma-to-pbs --repository root@pam@my.pbs.tld:8007:default --vmid 300 --password_file ~/pbs-pw.txt
PBS repository: root@pam@my.pbs.tld:8007:default
PBS fingerprint: [...]
compress: false
encrypt: false
backup time: 1711360054
Error: proxmox_backup_connect failed: "command error: parameter verification failed - \'password\': value does not match the regex pattern"

(Fingerprint was set via env var.)

I'm 100% certain that my password is correct - it seems to be related to
proxmox-schema. I haven't really looked into that otherwise yet; maybe
@Wolfgang could weigh in here?

>
> Changes since v3:
> * Add the ability to provide credentials via files
> * Add support for streaming the VMA file via stdin
> * Add a fallback for the --fingerprint argument
>
> Changes since v2:
> * Use the deb packages from the proxmox-io and proxmox-sys dependencies
>   instead of the proxmox submodule
> * Remove the proxmox submodule
> * Update the proxmox-backup-qemu submodule to make it buildable with
>   the newest librust dependencies
>
> Changes since v1:
> * Remove unused crates and uses
> * Format the code
> * Use anyhow for error handling
> * Use clap for parsing arguments instead of getopts
> * Fix blocks being reindexed on every read
> * Make sure ProxmoxBackupHandle is dropped properly on error
> * Move image_chunk_buffer from stack to heap
> * Move the block_index in VmaReader to the heap completely
> * Initialize vectors with `Vec::with_capacity` and `resize` instead of
>   the `vec!` macro, to potentially improve performance on debug builds.
> * Add comments to code filling the MD5 sum field with zeros
> * Change device_id arguments to usize
> * Handle devices that have a size that is not aligned to 4096 properly
>   in read_device_contents, when the caller provides a buffer that would
>   exceed the device size.
> * Avoid unnecessary loop iterations in read_device_contents when the
>   buffer size is not aligned to 65536
>
> Filip Schauer (8):
>   Initial commit
>   Add the ability to provide credentials via files
>   bump proxmox-backup-qemu
>   remove unnecessary "extern crate" declarations
>   add support for streaming the VMA file via stdin
>   add a fallback for the --fingerprint argument
>   refactor error handling with anyhow
>   Makefile: remove reference to unused submodule
>
> Wolfgang Bumiller (2):
>   cargo fmt
>   bump proxmox-backup-qemu