[pbs-devel] [PATCH vma-to-pbs 0/9] Implement vma-to-pbs tool
Filip Schauer
f.schauer at proxmox.com
Tue Apr 9 14:17:36 CEST 2024
Patch v7 is available:
https://lists.proxmox.com/pipermail/pbs-devel/2024-April/008439.html
On 03/04/2024 16:51, Max Carrara wrote:
> On Wed Apr 3, 2024 at 11:49 AM CEST, 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 at host:port:datastore> \
>> --vmid 123 \
>> --password-file pbs_password
>>
>> Commit 2/9 requires
>> https://lists.proxmox.com/pipermail/pve-devel/2024-March/062182.html
>> to be applied first.
> You've made some excellent progress since the last version, very nicely
> done! There are a few more comments inline, but it's almost there.
>
> Some general notes:
>
> * The overall code is now much more readable ever since you broke up
> the few remaining pieces. (Especially that one fat closure I
> mentioned in v5). Great job!
>
> * If you introduce a change in one commit and then alter that change
> in a later commit in the same series, it's usually best to just
> introduce the final change all at once. This doesn't apply
> everywhere of course - I've left an example inline to show what I
> mean exactly.
>
> * In general we don't `use anyhow::Result;` and instead always opt to
> using `Result<T, Error>` with `use anyhow::Error;` - but no hard
> feelings on my side. Though, it would be great to follow this
> convention from the start.
>
> (Side note: We should probably write all of our conventions down
> somewhere, but that's not important now.)
>
> I was able to shove a few `.vma` files to my PBS instance just as you
> described above, so the converting-and-backing-up works as intended.
>
> The last couple things that IMO need changing are rather minor - so if
> you sort those out, this gets a definitive pass from me. Great to see
> how far this series has come along, you should be proud of yourself :)
>
>> Changes since v5:
>> * Switch argument parsing from clap to pico-args
>> * Change time printing format ISO 8601 timestamps
>> * use anyhow::bail, so it does not have to be written out everytime
>> * Force the usage of password files when passing the vma file to stdin
>> * Cut off a trailing new line when reading a password from a file
>> * Extract PBS error handling into a seperate function
>> * Reformat command line arguments to kebab-case
>> * Refactor VmaReader to be generic over Read trait
>> * Split up block device upload into smaller functions to improve readability
>>
>> 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
>>
>> 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 (9):
>> 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
>> makefile: remove reference to unused submodule
>> switch argument handling from clap to pico-args
>> reformat command line arguments to kebab-case
>>
>> Cargo.toml | 2 +-
>> Makefile | 2 +-
>> src/main.rs | 421 +++++++++++----------------------
>> src/vma.rs | 343 ++++++++++++---------------
>> src/vma2pbs.rs | 386 ++++++++++++++++++++++++++++++
>> submodules/proxmox-backup-qemu | 2 +-
>> 6 files changed, 686 insertions(+), 470 deletions(-)
>> create mode 100644 src/vma2pbs.rs
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
More information about the pbs-devel
mailing list