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 3ED8E78711 for ; Fri, 30 Apr 2021 11:55:36 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 342422692E for ; Fri, 30 Apr 2021 11:55:36 +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 id 4C08226923 for ; Fri, 30 Apr 2021 11:55:35 +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 16D58429B0 for ; Fri, 30 Apr 2021 11:55:35 +0200 (CEST) Date: Fri, 30 Apr 2021 11:55:33 +0200 From: Wolfgang Bumiller To: Hannes Laimer Cc: pbs-devel@lists.proxmox.com Message-ID: <20210430095533.qvvu5z62bu2cqa22@wobu-vie.proxmox.com> References: <20210429110016.1467670-1-h.laimer@proxmox.com> <20210429110016.1467670-5-h.laimer@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210429110016.1467670-5-h.laimer@proxmox.com> User-Agent: NeoMutt/20180716 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.021 Adjusted score from AWL reputation of From: address 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [mod.rs, proxmox-backup-debug.rs, recover.rs] Subject: Re: [pbs-devel] [PATCH v5 proxmox-backup 4/4] add index recovery to pb-debug 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: Fri, 30 Apr 2021 09:55:36 -0000 On Thu, Apr 29, 2021 at 01:00:16PM +0200, Hannes Laimer wrote: > Adds possibility to recover data from an index file. Options: > - chunks: path to the directory where the chunks are saved > - file: the index file that should be recovered(must be either .fidx or > didx) > - [opt] keyfile: path to a keyfile, if the data was encrypted, a keyfile is > needed > - [opt] skip-crc: boolean, if true, read chunks wont be verified with their > crc-sum, increases the restore speed by a lot > > Signed-off-by: Hannes Laimer > --- > v5: > - try to load decryption key at the beginning > - simplified usage of the key > - allocate 4 MiB for chunks instead of 1 MiB > > v4: > - there might be a better way to do the match magic block > > src/bin/proxmox-backup-debug.rs | 6 +- > src/bin/proxmox_backup_debug/mod.rs | 2 + > src/bin/proxmox_backup_debug/recover.rs | 115 ++++++++++++++++++++++++ > 3 files changed, 121 insertions(+), 2 deletions(-) > create mode 100644 src/bin/proxmox_backup_debug/recover.rs > > diff --git a/src/bin/proxmox-backup-debug.rs b/src/bin/proxmox-backup-debug.rs > index fb548a31..83328701 100644 > --- a/src/bin/proxmox-backup-debug.rs > +++ b/src/bin/proxmox-backup-debug.rs > @@ -2,7 +2,7 @@ use proxmox::api::cli::*; > use proxmox::api::schema::{Schema, StringSchema}; > > mod proxmox_backup_debug; > -use proxmox_backup_debug::inspect_commands; > +use proxmox_backup_debug::{inspect_commands, recover_commands}; > > pub mod proxmox_client_tools; > use proxmox_client_tools::key_source::{get_encryption_key_password, KEYFILE_SCHEMA}; > @@ -12,7 +12,9 @@ pub const PATH_SCHEMA: Schema = StringSchema::new("Path to a file or a directory > fn main() { > proxmox_backup::tools::setup_safe_path_env(); > > - let cmd_def = CliCommandMap::new().insert("inspect", inspect_commands()); > + let cmd_def = CliCommandMap::new() > + .insert("inspect", inspect_commands()) > + .insert("recover", recover_commands()); > > let rpcenv = CliEnvironment::new(); > run_cli_command( > diff --git a/src/bin/proxmox_backup_debug/mod.rs b/src/bin/proxmox_backup_debug/mod.rs > index 644583db..62df7754 100644 > --- a/src/bin/proxmox_backup_debug/mod.rs > +++ b/src/bin/proxmox_backup_debug/mod.rs > @@ -1,2 +1,4 @@ > mod inspect; > pub use inspect::*; > +mod recover; > +pub use recover::*; > diff --git a/src/bin/proxmox_backup_debug/recover.rs b/src/bin/proxmox_backup_debug/recover.rs > new file mode 100644 > index 00000000..2355b50f > --- /dev/null > +++ b/src/bin/proxmox_backup_debug/recover.rs > @@ -0,0 +1,115 @@ > +use std::fs::File; > +use std::io::{Read, Seek, SeekFrom, Write}; > +use std::path::Path; > + > +use anyhow::{format_err, Error}; > + > +use proxmox::api::api; > +use proxmox::api::cli::{CliCommand, CliCommandMap, CommandLineInterface}; > +use proxmox_backup::backup::{DYNAMIC_SIZED_CHUNK_INDEX_1_0, FIXED_SIZED_CHUNK_INDEX_1_0}; > +use serde_json::Value; > + > +use proxmox_backup::backup::{ > + load_and_decrypt_key, CryptConfig, DataBlob, DynamicIndexReader, FixedIndexReader, IndexFile, > +}; > + > +use crate::{get_encryption_key_password, KEYFILE_SCHEMA, PATH_SCHEMA}; > + > +use proxmox::tools::digest_to_hex; > + > +#[api( > + input: { > + properties: { > + file: { > + schema: PATH_SCHEMA, Actually, since this schema has no restrictions, all this does is give the same description to all parameters using it. So please, replace `schema: PATH_SCHEMA` with a `description: "Something more useful"` ;-) Here, below, and the other commands as well. It's particularly weird to see *this* command's `--help` output since it has 2 of them: $ proxmox-backup-debug recover index --help Usage: proxmox-backup-debug recover index --chunks --file [OPTIONS] --chunks Path to a file or a directory --file Path to a file or a directory > + }, > + chunks: { > + schema: PATH_SCHEMA, > + }, > + "keyfile": { > + schema: KEYFILE_SCHEMA, > + optional: true, > + }, > + "skip-crc": { > + type: Boolean, > + optional: true, > + default: false, > + description: "Skip the crc verification, increases the restore speed immensely", > + } > + } > + } > +)] > +/// Restore the data from an index file, given the directory of where chunks > +/// are saved, the index file and a keyfile, if needed for decryption. > +fn recover_index( > + file: String, > + chunks: String, > + keyfile: Option, > + skip_crc: bool, > + _param: Value, > +) -> Result<(), Error> { > + let file_path = Path::new(&file); > + let chunks_path = Path::new(&chunks); > + > + let key_file_path = keyfile.as_ref().map(Path::new); > + > + let mut file = File::open(Path::new(&file))?; > + let mut magic = [0; 8]; > + file.read_exact(&mut magic)?; > + file.seek(SeekFrom::Start(0))?; > + let index: Box = match magic { > + FIXED_SIZED_CHUNK_INDEX_1_0 => { > + Ok(Box::new(FixedIndexReader::new(file)?) as Box) Please drop the `Ok()` wrapping here > + } > + DYNAMIC_SIZED_CHUNK_INDEX_1_0 => { > + Ok(Box::new(DynamicIndexReader::new(file)?) as Box) and here > + } > + _ => Err(format_err!( and use bail!() here instead of `Err(format_err!(...))` > + "index file must either be a .fidx or a .didx file" > + )), > + }?; and drop the `?` here. > + > + let mut crypt_conf_opt = None; > + > + if key_file_path.is_some() { Also please combine the above 2 lines to: let crypt_conf_opt = if key_file_path.is_some() { ... } else { None }; Btw. another variant with short optional things would be: let crypt_conf_opt = key_file_path .map(|key_file_path| { let (key, _created, _fingerprint) = load_and_decrypt_key(key_file_path, &get_encryption_key_password)?; CryptConfig::new(key) }) .transpose()?; ^ The `transpose` helps bubbling up the error. > + let (key, _created, _fingerprint) = > + load_and_decrypt_key(&key_file_path.unwrap(), &get_encryption_key_password)?; > + crypt_conf_opt = Some(CryptConfig::new(key)?); > + } > + > + let output_filename = file_path.file_stem().unwrap().to_str().unwrap(); > + let output_path = Path::new(output_filename); > + let mut output_file = File::create(output_path) > + .map_err(|e| format_err!("could not create output file - {}", e))?; > + > + let mut data = Vec::with_capacity(4 * 1024 * 1024); > + for pos in 0..index.index_count() { > + let chunk_digest = index.index_digest(pos).unwrap(); > + let digest_str = digest_to_hex(chunk_digest); > + let digest_prefix = &digest_str[0..4]; > + let chunk_path = chunks_path.join(digest_prefix).join(digest_str); > + let mut chunk_file = std::fs::File::open(&chunk_path) > + .map_err(|e| format_err!("could not open chunk file - {}", e))?; > + > + data.clear(); > + chunk_file.read_to_end(&mut data)?; > + let chunk_blob = DataBlob::from_raw(data.clone())?; > + > + if !skip_crc { > + chunk_blob.verify_crc()?; > + } > + > + output_file.write_all( > + chunk_blob > + .decode(crypt_conf_opt.as_ref(), Some(chunk_digest))? > + .as_slice(), > + )?; > + } > + > + Ok(()) > +} > + > +pub fn recover_commands() -> CommandLineInterface { > + let cmd_def = CliCommandMap::new().insert("index", CliCommand::new(&API_METHOD_RECOVER_INDEX)); > + cmd_def.into() > +} > -- > 2.20.1