From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 2F4E993A3A for ; Tue, 9 Apr 2024 14:17:38 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 194BE1D68F for ; Tue, 9 Apr 2024 14:17:38 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Tue, 9 Apr 2024 14:17:37 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 26BC241856 for ; Tue, 9 Apr 2024 14:17:37 +0200 (CEST) Message-ID: <59e4f542-1c2b-4922-a62b-4599e5909c6d@proxmox.com> Date: Tue, 9 Apr 2024 14:17:36 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: pbs-devel@lists.proxmox.com References: <20240403094913.107177-1-f.schauer@proxmox.com> From: Filip Schauer In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.077 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 SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pbs-devel] [PATCH vma-to-pbs 0/9] Implement vma-to-pbs tool X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Apr 2024 12:17:38 -0000 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 \ >> --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` 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@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel > >