[pbs-devel] [PATCH v3 proxmox-backup 1/3] add chunk inspection to pb-debug

Wolfgang Bumiller w.bumiller at proxmox.com
Tue Feb 9 11:16:45 CET 2021


On Fri, Feb 05, 2021 at 08:58:04AM +0100, Hannes Laimer wrote:
> Adds possibility to inspect chunks and find indexes that reference the
> chunk. Options:
>  - chunk: path to the chunk file
>  - [opt] decode: path to a file or to stdout(-), if specified, the chunk will
>    be decoded into the specified location
>  - [opt] keyfile: path to a keyfile, needed if decode is specified and the
>    data was encrypted
>  - [opt] reference-filter: path in which indexes that reference the chunk
>    should be searched, can be a group, snapshot or the whole datastore,
>    if not specified no references will be searched
> 
> Signed-off-by: Hannes Laimer <h.laimer at proxmox.com>
> ---
> 
>  Makefile                                |   3 +-
>  src/bin/proxmox-backup-debug.rs         |  33 +++++
>  src/bin/proxmox_backup_debug/inspect.rs | 170 ++++++++++++++++++++++++
>  src/bin/proxmox_backup_debug/mod.rs     |   2 +
>  4 files changed, 207 insertions(+), 1 deletion(-)
>  create mode 100644 src/bin/proxmox-backup-debug.rs
>  create mode 100644 src/bin/proxmox_backup_debug/inspect.rs
>  create mode 100644 src/bin/proxmox_backup_debug/mod.rs
> 
> diff --git a/Makefile b/Makefile
> index a1af9d51..3c780bcf 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -13,7 +13,8 @@ USR_BIN := \
>  
>  # Binaries usable by admins
>  USR_SBIN := \
> -	proxmox-backup-manager
> +	proxmox-backup-manager \
> +	proxmox-backup-debug \
>  
>  # Binaries for services:
>  SERVICE_BIN := \
> diff --git a/src/bin/proxmox-backup-debug.rs b/src/bin/proxmox-backup-debug.rs
> new file mode 100644
> index 00000000..079b2bc8
> --- /dev/null
> +++ b/src/bin/proxmox-backup-debug.rs
> @@ -0,0 +1,33 @@
> +use anyhow::Error;

newline here

> +use proxmox::api::cli::*;
> +use proxmox::api::schema::{Schema, StringSchema};

no onewline here

> +use proxmox::sys::linux::tty;
> +
> +use proxmox_backup_debug::*;

no newline here, `mod` line first, and please expand the `*`.

> +mod proxmox_backup_debug;
> +
> +pub fn get_encryption_key_password() -> Result<Vec<u8>, Error> {
> +    tty::read_password("Encryption Key Password: ")
> +}
> +
> +pub const PATH_SCHEMA: Schema = StringSchema::new("Path to a file or a directory").schema();
> +
> +pub const KEYFILE_SCHEMA: Schema = StringSchema::new(
> +    "Path to encryption key. If the data was encrypted, this key will be used for decryption.",
> +)
> +.schema();

^ could probably move this from the client to `src/api2/types.rs` and
reuse for both binaries

> +
> +fn main() {
> +    proxmox_backup::tools::setup_safe_path_env();
> +
> +    let cmd_def = CliCommandMap::new().insert("inspect", inspect_commands());
> +
> +    let rpcenv = CliEnvironment::new();
> +    run_cli_command(
> +        cmd_def,
> +        rpcenv,
> +        Some(|future| proxmox_backup::tools::runtime::main(future)),
> +    );
> +}
> diff --git a/src/bin/proxmox_backup_debug/inspect.rs b/src/bin/proxmox_backup_debug/inspect.rs
> new file mode 100644
> index 00000000..a66818be
> --- /dev/null
> +++ b/src/bin/proxmox_backup_debug/inspect.rs
> @@ -0,0 +1,170 @@
> +use std::fs::File;
> +use std::io::Write;
> +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::*, RpcEnvironment};
> +use serde_json::{json, Value};
> +use walkdir::WalkDir;
> +
> +use proxmox_backup::backup::{
> +    load_and_decrypt_key, CryptConfig, DataBlob, DynamicIndexReader, FixedIndexReader, IndexFile,
> +};
> +use proxmox_backup::tools;
> +
> +use crate::{get_encryption_key_password, KEYFILE_SCHEMA, PATH_SCHEMA};
> +
> +/// Decodes a blob and writes its content either to stdout or into a file
> +fn decode_blob(
> +    output_path: Option<&Path>,
> +    key_file: Option<&Path>,
> +    digest: Option<&[u8; 32]>,
> +    blob: &DataBlob,
> +) -> Result<(), Error> {
> +    let mut crypt_conf_opt = None;
> +    let crypt_conf;
> +
> +    if blob.is_encrypted() && key_file.is_some() {
> +        let (key, _created, _fingerprint) =
> +            load_and_decrypt_key(&key_file.unwrap(), &get_encryption_key_password)?;
> +        crypt_conf = CryptConfig::new(key)?;
> +        crypt_conf_opt = Some(&crypt_conf);
> +    }
> +
> +    if output_path.is_some() {
> +        let mut file = File::create(output_path.unwrap())?;
> +        Ok(file.write_all(blob.decode(crypt_conf_opt, digest)?.as_slice())?)
> +    } else {
> +        Ok(println!(
> +            "{}",
> +            String::from_utf8_lossy(blob.decode(crypt_conf_opt, digest)?.as_slice())
> +        ))

Do we really want to use lossy utf8 here? We should at least warn when
it's invalid utf8, or maybe we just dump it to stdout as bytes.

In fact, the whole "output file or stdout" thing could probably use a
helper. If we don't have one already, maybe add to tools a
`pub fn output_or_stdout<P: AsRef<Path>>(path: Option<P>) -> File`

(or if we want to keep the 'locking' we get from the `std::io::Stdout`
type, this function would have to return a custom enum which is either a
`File` or a `Stdout` and just implements `io::Write` by forwarding)

> +    }
> +}
> +
> +#[api(
> +    input: {
> +        properties: {
> +            chunk: {
> +                schema: PATH_SCHEMA,
> +            },
> +            "reference-filter": {
> +                schema: PATH_SCHEMA,
> +                optional: true,
> +            },
> +            "decode": {
> +                schema: PATH_SCHEMA,
> +                optional: true,
> +            },
> +            "keyfile": {
> +                schema: KEYFILE_SCHEMA,
> +                optional: true,
> +            },
> +            "output-format": {
> +                schema: OUTPUT_FORMAT,
> +                optional: true,
> +            },
> +        }
> +    }
> +)]
> +/// Inspect a chunk
> +fn inspect_chunk(param: Value, _rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {

New code is much easier to read if the parameters are written out into
the `fn()`.
Also the `_rpcenv` can simply be dropped if you don't use it:

    fn inspect_chunk(
        chunk: String,
        reference_filter: Option<String>,
        decode: Option<String>,
        keyfile: Option<String>,
        param: Value, // for keep `output-format` in there for `get_output_format()`
    ) -> Result<Value, Error> {

> +    let chunk_path = Path::new(tools::required_string_param(&param, "chunk")?);
> +    let output_format = get_output_format(&param);
> +    let digest_str = chunk_path.file_name().unwrap().to_str().unwrap();
> +    let digest_raw = proxmox::tools::hex_to_digest(digest_str)
> +        .map_err(|e| format_err!("could not parse chunk - {}", e))?;
> +
> +    let reference_filter_param = param["reference-filter"].as_str();
> +    let decode_output_param = param["decode"].as_str();
> +    let key_file_param = param["keyfile"].as_str();
> +
> +    let search_path = reference_filter_param.map(|path| Path::new(path));
> +    let key_file_path = key_file_param.map(|path| Path::new(path));

^ hint: should be possible to shorten this to `.map(Path::new)`

> +
> +    let (decode_output_path, to_stdout) = match decode_output_param {
> +        Some(path) if path.eq("-") => (None, true),
> +        Some(path) => (Some(Path::new(path)), false),
> +        None => (None, false),
> +    };

Here we could call the `ouput_or_stdout()` helper to get an
`Option<File>` and the `to_stdout` bool could be dropped and the
`.is_some()` check would be sufficient.

> +
> +    let mut file = std::fs::File::open(&chunk_path)
> +        .map_err(|e| format_err!("could not open chunk file - {}", e))?;
> +    let blob = DataBlob::load_from_reader(&mut file)?;
> +
> +    let mut referenced_by = None;
> +    if let Some(search_path) = search_path {
> +        let mut references = Vec::new();
> +        for entry in WalkDir::new(search_path)
> +            .follow_links(false)
> +            .into_iter()
> +            .filter_map(|e| e.ok())
> +        {
> +            let file_name = entry.file_name().to_string_lossy();
> +            let mut index: Option<Box<dyn IndexFile>> = None;
> +
> +            if file_name.ends_with(".fidx") {
> +                index = match FixedIndexReader::open(entry.path()) {
> +                    Ok(index) => Some(Box::new(index)),
> +                    Err(_) => None,
> +                };
> +            }
> +
> +            if file_name.ends_with(".didx") {
> +                index = match DynamicIndexReader::open(entry.path()) {
> +                    Ok(index) => Some(Box::new(index)),
> +                    Err(_) => None,
> +                };
> +            }
> +
> +            if let Some(index) = index {
> +                for pos in 0..index.index_count() {
> +                    if let Some(index_chunk_digest) = index.index_digest(pos) {
> +                        let str = proxmox::tools::digest_to_hex(index_chunk_digest);
> +                        if str.eq(digest_str) {
> +                            references.push(entry.path().to_string_lossy().into_owned());
> +                            break;
> +                        }
> +                    }
> +                }
> +            }
> +        }
> +        referenced_by = Some(references);
> +    }
> +
> +    if decode_output_path.is_some() || to_stdout {
> +        decode_blob(decode_output_path, key_file_path, Some(&digest_raw), &blob)?;
> +    }
> +
> +    let crc_status = format!(
> +        "{}({})",
> +        blob.compute_crc(),
> +        blob.verify_crc().map_or("NOK", |_| "OK")
> +    );
> +
> +    let val = match referenced_by {
> +        Some(references) => json!({
> +            "digest": digest_str,
> +            "crc": crc_status,
> +            "encryption": blob.crypt_mode()?,
> +            "referenced-by": references
> +        }),
> +        None => json!({
> +             "digest": digest_str,
> +             "crc": crc_status,
> +             "encryption": blob.crypt_mode()?,
> +        }),
> +    };
> +
> +    format_and_print_result(&val, &output_format);
> +    Ok(Value::Null)
> +}
> +
> +pub fn inspect_commands() -> CommandLineInterface {
> +    let cmd_def = CliCommandMap::new().insert("chunk", CliCommand::new(&API_METHOD_INSPECT_CHUNK));
> +
> +    cmd_def.into()
> +}
> diff --git a/src/bin/proxmox_backup_debug/mod.rs b/src/bin/proxmox_backup_debug/mod.rs
> new file mode 100644
> index 00000000..644583db
> --- /dev/null
> +++ b/src/bin/proxmox_backup_debug/mod.rs
> @@ -0,0 +1,2 @@
> +mod inspect;
> +pub use inspect::*;
> -- 
> 2.20.1





More information about the pbs-devel mailing list