Re: [pbs-devel] [PATCH v4 proxmox-backup 2/3] add file inspection to pb-debug

[Dominik Csapak <d.csapak@proxmox.com>]

small comment inline, rest looks ok

On 2/19/21 13:09, Hannes Laimer wrote:
> Adds possibility to inspect .blob, .fidx and .didx files. For index
> files a list of the chunks referenced will be printed in addition to
> some other inforation. .blob files can be decoded into file or directly
> into stdout. Options:
>   - file: path to the file
>   - [opt] decode: path to a file or stdout(-), if specidied, the file will be
>     decoded into the specified location [only for blob files, no effect
>     with index files]
>   - [opt] keyfile: path to a keyfile, needed if decode is specified and the
>     data was encrypted
> 
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> v4:
>   - only the types of file that are passed by the user are check with the
>     magic number, when looking for index files just the filename ending
>     is checked -> don't have to open the file for that
>   - not sure if a function for the magic nr reading, seek reset makes
>     sense(?), it's just two lines
> 
>   src/bin/proxmox_backup_debug/inspect.rs | 132 ++++++++++++++++++++++--
>   1 file changed, 125 insertions(+), 7 deletions(-)
> 
> diff --git a/src/bin/proxmox_backup_debug/inspect.rs b/src/bin/proxmox_backup_debug/inspect.rs
> index b211857e..f4f7f53e 100644
> --- a/src/bin/proxmox_backup_debug/inspect.rs
> +++ b/src/bin/proxmox_backup_debug/inspect.rs
> @@ -1,18 +1,25 @@
> +use std::collections::HashSet;
> +use std::fs::File;
> +use std::io::{Read, Seek, SeekFrom};
>   use std::path::Path;
>   
>   use anyhow::{format_err, Error};
> -use proxmox::api::cli::{
> -    format_and_print_result, get_output_format, CliCommand, CliCommandMap, CommandLineInterface,
> -};
> -use proxmox::api::{api, cli::*};
>   use serde_json::{json, Value};
>   use walkdir::WalkDir;
>   
> +use crate::{get_encryption_key_password, KEYFILE_SCHEMA, PATH_SCHEMA};
> +use proxmox::api::{
> +    api,
> +    cli::{
> +        format_and_print_result, get_output_format, CliCommand, CliCommandMap,
> +        CommandLineInterface, OUTPUT_FORMAT,
> +    },
> +};
>   use proxmox_backup::backup::{
>       load_and_decrypt_key, CryptConfig, DataBlob, DynamicIndexReader, FixedIndexReader, IndexFile,
> +    COMPRESSED_BLOB_MAGIC_1_0, DYNAMIC_SIZED_CHUNK_INDEX_1_0, ENCRYPTED_BLOB_MAGIC_1_0,
> +    ENCR_COMPR_BLOB_MAGIC_1_0, FIXED_SIZED_CHUNK_INDEX_1_0, UNCOMPRESSED_BLOB_MAGIC_1_0,
>   };
> -
> -use crate::{get_encryption_key_password, KEYFILE_SCHEMA, PATH_SCHEMA};
>   use proxmox_backup::tools::output_or_stdout;
>   
>   /// Decodes a blob and writes its content either to stdout or into a file
> @@ -35,6 +42,115 @@ fn decode_blob(
>       output_or_stdout(output_path, blob.decode(crypt_conf_opt, digest)?.as_slice())
>   }
>   
> +#[api(
> +    input: {
> +        properties: {
> +            file: {
> +                schema: PATH_SCHEMA,
> +            },
> +            "decode": {
> +                schema: PATH_SCHEMA,
> +                optional: true,
> +            },
> +            "keyfile": {
> +                schema: KEYFILE_SCHEMA,
> +                optional: true,
> +            },
> +            "output-format": {
> +                schema: OUTPUT_FORMAT,
> +                optional: true,
> +            },
> +        }
> +    }
> +)]
> +/// Inspect a file
> +fn inspect_file(
> +    file: String,
> +    decode: Option<String>,
> +    keyfile: Option<String>,
> +    param: Value,
> +) -> Result<(), Error> {
> +    let output_format = get_output_format(&param);
> +
> +    let mut file = File::open(Path::new(&file))?;
> +    let mut magic = [0; 8];
> +    file.read_exact(&mut magic)?;
> +    file.seek(SeekFrom::Start(0))?;
> +    let val = match magic {
> +        UNCOMPRESSED_BLOB_MAGIC_1_0
> +        | COMPRESSED_BLOB_MAGIC_1_0
> +        | ENCRYPTED_BLOB_MAGIC_1_0
> +        | ENCR_COMPR_BLOB_MAGIC_1_0 => {
> +            let data_blob = DataBlob::load_from_reader(&mut file)?;
> +            let key_file_path = keyfile.as_ref().map(Path::new);
> +
> +            let (decode_output_path, to_stdout) = (
> +                decode.as_ref().map(Path::new),
> +                decode.clone().map_or(false, |p| p.eq("-")),
> +            );
> +
> +            if decode_output_path.is_some() || to_stdout {
> +                decode_blob(decode_output_path, key_file_path, None, &data_blob)?;
> +            }
> +
> +            let crypt_mode = data_blob.crypt_mode()?;
> +            Ok(json!({
> +                "encryption": crypt_mode,
> +                "raw_size": data_blob.raw_size(),
> +            }))
> +        }
> +        FIXED_SIZED_CHUNK_INDEX_1_0 => {
> +            let index = FixedIndexReader::new(file)?;
> +
> +            let mut ctime_str = index.ctime.to_string();
> +            if let Ok(s) = proxmox::tools::time::strftime_local("%c", index.ctime) {
> +                ctime_str = s;
> +            }
> +
> +            let mut chunk_digests = HashSet::new();
> +
> +            for pos in 0..index.index_count() {
> +                let digest = index.index_digest(pos).unwrap();
> +                chunk_digests.insert(proxmox::tools::digest_to_hex(digest));
> +            }
> +
> +            Ok(json!({
> +                "size": index.size,
> +                "ctime": ctime_str,
> +                "chunk-digests": chunk_digests
> +            }))
> +        }
> +        DYNAMIC_SIZED_CHUNK_INDEX_1_0 => {
> +            let index = DynamicIndexReader::new(file)?;
> +
> +            let mut ctime_str = index.ctime.to_string();
> +            if let Ok(s) = proxmox::tools::time::strftime_local("%c", index.ctime) {
> +                ctime_str = s;
> +            }
> +
> +            let mut chunk_digests = HashSet::new();
> +
> +            for pos in 0..index.index_count() {
> +                let digest = index.index_digest(pos).unwrap();
> +                chunk_digests.insert(proxmox::tools::digest_to_hex(digest));
> +            }
> +
> +            Ok(json!({
> +                "size": index.size,
> +                "ctime": ctime_str,
> +                "chunk-digests": chunk_digests
> +            }))
> +        }

nit: you could probably combine those two paths by returning a
trait object (Box<dyn IndexFile>), this would reduce the code
duplication a bit, but may be problematic if we ever want
to do different things for dynamic and fixed indexes

(to do this, we would have to add ctime and size functions to the
IndexFile trait, but this should not be a problem)

> +        _ => Err(format_err!(
> +            "Only .blob, .fidx and .didx files may be inspected"
> +        )),
> +    }?;
> +
> +    format_and_print_result(&val, &output_format);
> +
> +    Ok(())
> +}
> +
>   #[api(
>       input: {
>           properties: {
> @@ -156,7 +272,9 @@ fn inspect_chunk(
>   }
>   
>   pub fn inspect_commands() -> CommandLineInterface {
> -    let cmd_def = CliCommandMap::new().insert("chunk", CliCommand::new(&API_METHOD_INSPECT_CHUNK));
> +    let cmd_def = CliCommandMap::new()
> +        .insert("file", CliCommand::new(&API_METHOD_INSPECT_FILE))
> +        .insert("chunk", CliCommand::new(&API_METHOD_INSPECT_CHUNK));
>   
>       cmd_def.into()
>   }
>