[pbs-devel] [PATCH v4 proxmox-backup 3/3] add index recovery to pb-debug

[Dominik Csapak]

small comments inline, rest looks ok

On 2/19/21 13:09, 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 <h.laimer at proxmox.com>
> ---
> v4:
>   - there might be a better way to do the match magic block
> 
>   src/bin/proxmox-backup-debug.rs         |   8 +-
>   src/bin/proxmox_backup_debug/mod.rs     |   2 +
>   src/bin/proxmox_backup_debug/recover.rs | 117 ++++++++++++++++++++++++
>   3 files changed, 124 insertions(+), 3 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 ae747c5d..00f06870 100644
> --- a/src/bin/proxmox-backup-debug.rs
> +++ b/src/bin/proxmox-backup-debug.rs
> @@ -5,7 +5,7 @@ use proxmox::api::schema::{Schema, StringSchema};
>   use proxmox::sys::linux::tty;
>   
>   mod proxmox_backup_debug;
> -use proxmox_backup_debug::inspect_commands;
> +use proxmox_backup_debug::{inspect_commands, recover_commands};
>   
>   pub fn get_encryption_key_password() -> Result<Vec<u8>, Error> {
>       tty::read_password("Encryption Key Password: ")
> @@ -21,7 +21,9 @@ pub const KEYFILE_SCHEMA: Schema = StringSchema::new(
>   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(
> @@ -29,4 +31,4 @@ fn main() {
>           rpcenv,
>           Some(|future| proxmox_backup::tools::runtime::main(future)),
>       );
> -}
> +}
> \ No newline at end of file
> 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..451686bc
> --- /dev/null
> +++ b/src/bin/proxmox_backup_debug/recover.rs
> @@ -0,0 +1,117 @@
> +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,
> +            },
> +            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<String>,
> +    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<dyn IndexFile> = match magic {
> +        FIXED_SIZED_CHUNK_INDEX_1_0 => {
> +            Ok(Box::new(FixedIndexReader::new(file)?) as Box<dyn IndexFile>)
> +        }
> +        DYNAMIC_SIZED_CHUNK_INDEX_1_0 => {
> +            Ok(Box::new(DynamicIndexReader::new(file)?) as Box<dyn IndexFile>)
> +        }
> +        _ => Err(format_err!(
> +            "index file must either be a .fidx or a .didx file"
> +        )),
> +    }?;
> +
> +    let mut crypt_conf_opt = None;
> +    let mut crypt_conf;

i do not get this, why not simply

crypt_conf_opt = Some(CryptConfig::new(key)?);

below ?

> +
> +    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(1024 * 1024);

nit: we generally target 4 MiB chunks, so it would make sense
to try to allocate as much here

> +    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()?;
> +        }
> +
> +        if key_file_path.is_some() && chunk_blob.is_encrypted() && crypt_conf_opt.is_none() {
> +            let (key, _created, _fingerprint) =
> +                load_and_decrypt_key(&key_file_path.unwrap(), &get_encryption_key_password)?;
> +            crypt_conf = CryptConfig::new(key)?;
> +            crypt_conf_opt = Some(&crypt_conf);
> +        }

we probably want to load the key once at the start of the function and 
always give it to decode? this only tries to decrypt if the blob
has the ENCRYPTED magic header anyway

> +
> +        output_file.write_all(
> +            chunk_blob
> +                .decode(crypt_conf_opt, 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()
> +}
>