From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <m.carrara@proxmox.com> 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 0E619BB7AA for <pbs-devel@lists.proxmox.com>; Mon, 25 Mar 2024 13:38:44 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id E330F3FAD for <pbs-devel@lists.proxmox.com>; Mon, 25 Mar 2024 13:38:13 +0100 (CET) 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 <pbs-devel@lists.proxmox.com>; Mon, 25 Mar 2024 13:38:13 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 050C841FD3 for <pbs-devel@lists.proxmox.com>; Mon, 25 Mar 2024 13:38:13 +0100 (CET) Content-Type: text/plain; charset=UTF-8 Date: Mon, 25 Mar 2024 13:38:12 +0100 Message-Id: <D02U4YNJ8XHN.1EZHN46RYE93Y@proxmox.com> Cc: <w.bumiller@proxmox.com> From: "Max Carrara" <m.carrara@proxmox.com> To: "Proxmox Backup Server development discussion" <pbs-devel@lists.proxmox.com> Content-Transfer-Encoding: quoted-printable Mime-Version: 1.0 X-Mailer: aerc 0.17.0-72-g6a84f1331f1c References: <20240320141516.213930-1-f.schauer@proxmox.com> In-Reply-To: <20240320141516.213930-1-f.schauer@proxmox.com> X-TUID: iOP1ryO/WfnD X-SPAM-LEVEL: Spam detection results: 0 AWL 0.021 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 v5 vma-to-pbs 00/10] 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 <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> X-List-Received-Date: Mon, 25 Mar 2024 12:38:44 -0000 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 ~/p= bs-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 verificatio= n 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