all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH v5 proxmox-backup 0/4] add proxmox-backup-debug binary
@ 2021-04-29 11:00 Hannes Laimer
  2021-04-29 11:00 ` [pbs-devel] [PATCH v5 proxmox-backup 1/4] add chunk inspection to pb-debug Hannes Laimer
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Hannes Laimer @ 2021-04-29 11:00 UTC (permalink / raw)
  To: pbs-devel

Adds a new proxmox-backup-debug binary for recovery or inspection of
index, blob and chunk files

v5:
  - improved some parts based on Dominik Csapak <d.csapak@proxmox.com>
    feedback on v4
  - changes are described in the patches themself

v4:
  - rewrote parts based on Wolfgang Bumiller <w.bumiller@proxmox.com>
    feedback on v3
  - check filetype mostly by magic number, except when
    looking for index files

v3:
  - move description into commit message

v2:
  - appiled suggestions + rewrote a few parts
  - renamed 'restore' to 'debug'<Paste>

Hannes Laimer (4):
  add chunk inspection to pb-debug
  add ctime and size function to IndexFile trait
  add file inspection to pb-debug
  add index recovery to pb-debug

 Makefile                                |   3 +-
 src/backup/dynamic_index.rs             |   8 +
 src/backup/fixed_index.rs               |   8 +
 src/backup/index.rs                     |   2 +
 src/bin/proxmox-backup-debug.rs         |  25 +++
 src/bin/proxmox_backup_debug/inspect.rs | 267 ++++++++++++++++++++++++
 src/bin/proxmox_backup_debug/mod.rs     |   4 +
 src/bin/proxmox_backup_debug/recover.rs | 115 ++++++++++
 src/tools.rs                            |  12 +-
 9 files changed, 442 insertions(+), 2 deletions(-)
 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
 create mode 100644 src/bin/proxmox_backup_debug/recover.rs

-- 
2.20.1





^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pbs-devel] [PATCH v5 proxmox-backup 1/4] add chunk inspection to pb-debug
  2021-04-29 11:00 [pbs-devel] [PATCH v5 proxmox-backup 0/4] add proxmox-backup-debug binary Hannes Laimer
@ 2021-04-29 11:00 ` Hannes Laimer
  2021-04-30  9:39   ` Wolfgang Bumiller
  2021-04-29 11:00 ` [pbs-devel] [PATCH v5 proxmox-backup 2/4] add ctime and size function to IndexFile trait Hannes Laimer
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Hannes Laimer @ 2021-04-29 11:00 UTC (permalink / raw)
  To: pbs-devel

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@proxmox.com>
---
v5:
 - compair digests directly
 - use KEYFILE_SCHEMA from proxmox_client_tools
 - outfile_out_stdout now returns something that is writebale, instead
   of writing directly

v4:
 - left the two schemas here since they are quite specific to this binary
 - output_or_stdout() directly outputs the data instead of returning
   stdout or an open file (could not find a type that allows to properly
   return either stdout or a file)

 Makefile                                |   3 +-
 src/bin/proxmox-backup-debug.rs         |  23 ++++
 src/bin/proxmox_backup_debug/inspect.rs | 162 ++++++++++++++++++++++++
 src/bin/proxmox_backup_debug/mod.rs     |   2 +
 src/tools.rs                            |  12 +-
 5 files changed, 200 insertions(+), 2 deletions(-)
 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 e5533561..f80e2dc4 100644
--- a/Makefile
+++ b/Makefile
@@ -17,7 +17,8 @@ USR_BIN := \
 
 # Binaries usable by admins
 USR_SBIN := \
-	proxmox-backup-manager
+q	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..fb548a31
--- /dev/null
+++ b/src/bin/proxmox-backup-debug.rs
@@ -0,0 +1,23 @@
+use proxmox::api::cli::*;
+use proxmox::api::schema::{Schema, StringSchema};
+
+mod proxmox_backup_debug;
+use proxmox_backup_debug::inspect_commands;
+
+pub mod proxmox_client_tools;
+use proxmox_client_tools::key_source::{get_encryption_key_password, KEYFILE_SCHEMA};
+
+pub const PATH_SCHEMA: Schema = StringSchema::new("Path to a file or a directory").schema();
+
+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..dd6ee287
--- /dev/null
+++ b/src/bin/proxmox_backup_debug/inspect.rs
@@ -0,0 +1,162 @@
+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 proxmox_backup::backup::{
+    load_and_decrypt_key, CryptConfig, DataBlob, DynamicIndexReader, FixedIndexReader, IndexFile,
+};
+
+use crate::{get_encryption_key_password, KEYFILE_SCHEMA, PATH_SCHEMA};
+
+use proxmox_backup::tools::outfile_or_stdout;
+
+/// 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);
+    }
+
+    outfile_or_stdout(output_path)?.write_all(blob.decode(crypt_conf_opt, digest)?.as_slice())?;
+    Ok(())
+}
+
+#[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(
+    chunk: String,
+    reference_filter: Option<String>,
+    decode: Option<String>,
+    keyfile: Option<String>,
+    param: Value,
+) -> Result<(), Error> {
+    let output_format = get_output_format(&param);
+    let chunk_path = Path::new(&chunk);
+    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 search_path = reference_filter.as_ref().map(Path::new);
+    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("-")),
+    );
+
+    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) {
+                        if digest_raw.eq(index_chunk_digest) {
+                            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(())
+}
+
+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::*;
diff --git a/src/tools.rs b/src/tools.rs
index 08af55e5..16ebf3fc 100644
--- a/src/tools.rs
+++ b/src/tools.rs
@@ -6,7 +6,7 @@ use std::borrow::Borrow;
 use std::collections::HashMap;
 use std::hash::BuildHasher;
 use std::fs::File;
-use std::io::{self, BufRead, Read, Seek, SeekFrom};
+use std::io::{self, BufRead, Read, Seek, SeekFrom, stdout, Write};
 use std::os::unix::io::RawFd;
 use std::path::Path;
 
@@ -571,3 +571,13 @@ pub fn create_run_dir() -> Result<(), Error> {
     let _: bool = proxmox::tools::fs::create_path(PROXMOX_BACKUP_RUN_DIR_M!(), None, None)?;
     Ok(())
 }
+
+/// Returns either a new file, if a path is given, or stdout, if no path is given.
+pub fn outfile_or_stdout<P: AsRef<Path>>(path: Option<P>) -> Result<Box<dyn Write>, Error> {
+    if let Some(path) = path {
+        let f = File::create(path)?;
+        Ok(Box::new(f) as Box<dyn Write>)
+    } else {
+        Ok(Box::new(stdout()) as Box<dyn Write>)
+    }
+}
-- 
2.20.1





^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pbs-devel] [PATCH v5 proxmox-backup 2/4] add ctime and size function to IndexFile trait
  2021-04-29 11:00 [pbs-devel] [PATCH v5 proxmox-backup 0/4] add proxmox-backup-debug binary Hannes Laimer
  2021-04-29 11:00 ` [pbs-devel] [PATCH v5 proxmox-backup 1/4] add chunk inspection to pb-debug Hannes Laimer
@ 2021-04-29 11:00 ` Hannes Laimer
  2021-04-30  9:41   ` [pbs-devel] applied: " Wolfgang Bumiller
  2021-04-29 11:00 ` [pbs-devel] [PATCH v5 proxmox-backup 3/4] add file inspection to pb-debug Hannes Laimer
  2021-04-29 11:00 ` [pbs-devel] [PATCH v5 proxmox-backup 4/4] add index recovery " Hannes Laimer
  3 siblings, 1 reply; 9+ messages in thread
From: Hannes Laimer @ 2021-04-29 11:00 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
needed for next patch in order to avoid duplicating code for fixed and
dynamic indexes

 src/backup/dynamic_index.rs | 8 ++++++++
 src/backup/fixed_index.rs   | 8 ++++++++
 src/backup/index.rs         | 2 ++
 3 files changed, 18 insertions(+)

diff --git a/src/backup/dynamic_index.rs b/src/backup/dynamic_index.rs
index 1619d8db..8321b295 100644
--- a/src/backup/dynamic_index.rs
+++ b/src/backup/dynamic_index.rs
@@ -233,6 +233,14 @@ impl IndexFile for DynamicIndexReader {
         })
     }
 
+    fn index_ctime(&self) -> i64 {
+        self.ctime
+    }
+
+    fn index_size(&self) -> usize {
+        self.size as usize
+    }
+
     fn chunk_from_offset(&self, offset: u64) -> Option<(usize, u64)> {
         let end_idx = self.index.len() - 1;
         let end = self.chunk_end(end_idx);
diff --git a/src/backup/fixed_index.rs b/src/backup/fixed_index.rs
index ceb6fe29..ebf64456 100644
--- a/src/backup/fixed_index.rs
+++ b/src/backup/fixed_index.rs
@@ -193,6 +193,14 @@ impl IndexFile for FixedIndexReader {
         })
     }
 
+    fn index_ctime(&self) -> i64 {
+        self.ctime
+    }
+
+    fn index_size(&self) -> usize {
+        self.size as usize
+    }
+
     fn compute_csum(&self) -> ([u8; 32], u64) {
         let mut csum = openssl::sha::Sha256::new();
         let mut chunk_end = 0;
diff --git a/src/backup/index.rs b/src/backup/index.rs
index c6bab56e..69788f80 100644
--- a/src/backup/index.rs
+++ b/src/backup/index.rs
@@ -22,6 +22,8 @@ pub trait IndexFile {
     fn index_digest(&self, pos: usize) -> Option<&[u8; 32]>;
     fn index_bytes(&self) -> u64;
     fn chunk_info(&self, pos: usize) -> Option<ChunkReadInfo>;
+    fn index_ctime(&self) -> i64;
+    fn index_size(&self) -> usize;
 
     /// Get the chunk index and the relative offset within it for a byte offset
     fn chunk_from_offset(&self, offset: u64) -> Option<(usize, u64)>;
-- 
2.20.1





^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pbs-devel] [PATCH v5 proxmox-backup 3/4] add file inspection to pb-debug
  2021-04-29 11:00 [pbs-devel] [PATCH v5 proxmox-backup 0/4] add proxmox-backup-debug binary Hannes Laimer
  2021-04-29 11:00 ` [pbs-devel] [PATCH v5 proxmox-backup 1/4] add chunk inspection to pb-debug Hannes Laimer
  2021-04-29 11:00 ` [pbs-devel] [PATCH v5 proxmox-backup 2/4] add ctime and size function to IndexFile trait Hannes Laimer
@ 2021-04-29 11:00 ` Hannes Laimer
  2021-04-30  9:44   ` Wolfgang Bumiller
  2021-04-29 11:00 ` [pbs-devel] [PATCH v5 proxmox-backup 4/4] add index recovery " Hannes Laimer
  3 siblings, 1 reply; 9+ messages in thread
From: Hannes Laimer @ 2021-04-29 11:00 UTC (permalink / raw)
  To: pbs-devel

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>
---
v5:
 - combine path for fixed and dynamic index fliles to avoid duplicate
   code

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 | 119 ++++++++++++++++++++++--
 1 file changed, 112 insertions(+), 7 deletions(-)

diff --git a/src/bin/proxmox_backup_debug/inspect.rs b/src/bin/proxmox_backup_debug/inspect.rs
index dd6ee287..2d95448e 100644
--- a/src/bin/proxmox_backup_debug/inspect.rs
+++ b/src/bin/proxmox_backup_debug/inspect.rs
@@ -1,19 +1,26 @@
+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::outfile_or_stdout;
 
 /// Decodes a blob and writes its content either to stdout or into a file
@@ -37,6 +44,102 @@ fn decode_blob(
     Ok(())
 }
 
+#[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 | DYNAMIC_SIZED_CHUNK_INDEX_1_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!("This is technically not possible")),
+            }?;
+
+            let mut ctime_str = index.index_ctime().to_string();
+            if let Ok(s) = proxmox::tools::time::strftime_local("%c", index.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.index_size(),
+                "ctime": ctime_str,
+                "chunk-digests": chunk_digests
+            }))
+        }
+        _ => 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 +259,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()
 }
-- 
2.20.1





^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pbs-devel] [PATCH v5 proxmox-backup 4/4] add index recovery to pb-debug
  2021-04-29 11:00 [pbs-devel] [PATCH v5 proxmox-backup 0/4] add proxmox-backup-debug binary Hannes Laimer
                   ` (2 preceding siblings ...)
  2021-04-29 11:00 ` [pbs-devel] [PATCH v5 proxmox-backup 3/4] add file inspection to pb-debug Hannes Laimer
@ 2021-04-29 11:00 ` Hannes Laimer
  2021-04-30  9:55   ` Wolfgang Bumiller
  3 siblings, 1 reply; 9+ messages in thread
From: Hannes Laimer @ 2021-04-29 11:00 UTC (permalink / raw)
  To: pbs-devel

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@proxmox.com>
---
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,
+            },
+            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;
+
+    if key_file_path.is_some() {
+        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





^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [pbs-devel] [PATCH v5 proxmox-backup 1/4] add chunk inspection to pb-debug
  2021-04-29 11:00 ` [pbs-devel] [PATCH v5 proxmox-backup 1/4] add chunk inspection to pb-debug Hannes Laimer
@ 2021-04-30  9:39   ` Wolfgang Bumiller
  0 siblings, 0 replies; 9+ messages in thread
From: Wolfgang Bumiller @ 2021-04-30  9:39 UTC (permalink / raw)
  To: Hannes Laimer; +Cc: pbs-devel

On Thu, Apr 29, 2021 at 01:00:13PM +0200, Hannes Laimer wrote:
> diff --git a/src/bin/proxmox_backup_debug/inspect.rs b/src/bin/proxmox_backup_debug/inspect.rs
> new file mode 100644
> index 00000000..dd6ee287
> --- /dev/null
> +++ b/src/bin/proxmox_backup_debug/inspect.rs
> @@ -0,0 +1,162 @@
> +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 proxmox_backup::backup::{
> +    load_and_decrypt_key, CryptConfig, DataBlob, DynamicIndexReader, FixedIndexReader, IndexFile,
> +};
> +
> +use crate::{get_encryption_key_password, KEYFILE_SCHEMA, PATH_SCHEMA};
> +
> +use proxmox_backup::tools::outfile_or_stdout;
> +
> +/// 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);
> +    }
> +
> +    outfile_or_stdout(output_path)?.write_all(blob.decode(crypt_conf_opt, digest)?.as_slice())?;
> +    Ok(())
> +}
> +
> +#[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(
> +    chunk: String,
> +    reference_filter: Option<String>,
> +    decode: Option<String>,
> +    keyfile: Option<String>,
> +    param: Value,
> +) -> Result<(), Error> {
> +    let output_format = get_output_format(&param);
> +    let chunk_path = Path::new(&chunk);
> +    let digest_str = chunk_path.file_name().unwrap().to_str().unwrap();

^ The first unwrap() fails with an ugly error on `/`, so please don't
use unwrap there.

Also I think for a debug helper it makes sense if it can also work on copied
and renamed files that aren't literally named after their digest, so I think
`digest_{str,raw}` should in fact be `Option<>`. (In the `decode` method the
digest and its verification are already optional after all.)

You can, however, output a warning that the digest will not be verified.

(Also I wonder if an additional optional `--digest` parameter should be added?)

> +    let digest_raw = proxmox::tools::hex_to_digest(digest_str)
> +        .map_err(|e| format_err!("could not parse chunk - {}", e))?;
> +
> +    let search_path = reference_filter.as_ref().map(Path::new);
> +    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("-")),

^ `to_stdout=true` now implies that `decode_output_path` was `Some`, so
I think you can drop that variable altogether.

> +    );
> +
> +    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)?;

^ If you're doing cleanups, I'd still like `file` to be dropped at this point.

> +
> +    let mut referenced_by = None;
> +    if let Some(search_path) = search_path {

Try to avoid `mut` variables where they're only assigned once anyway:

    let referenced_by = if let Some(search_path) = search_path {
        ...
        Some(references)
    } else {
        None
    };

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

^ Since you're just throwing away the produced string and only want to check
the suffix, just do:

    use std::os::unix::ffi::OsStrExt;

    let file_name = entry.file_name().as_bytes();

and prefix the strings with a `b` => `b".fidx"`

> +            let mut index: Option<Box<dyn IndexFile>> = None;

Same pattern as above:

    let index = if file_name.ends_with(b".fidx") {
        ...
    } else if file_name.ends_with(b".didx") {
        ...
    } else {
        continue;
    };

Also note the `continue`:

> +
> +            if file_name.ends_with(".fidx") {
> +                index = match FixedIndexReader::open(entry.path()) {
> +                    Ok(index) => Some(Box::new(index)),
> +                    Err(_) => None,

You can just drop the whole `Some`/`Option` part and use

   Err(_) => continue,

here

> +                };
> +            }
> +
> +            if file_name.ends_with(".didx") {
> +                index = match DynamicIndexReader::open(entry.path()) {
> +                    Ok(index) => Some(Box::new(index)),
> +                    Err(_) => None,

and here

> +                };
> +            }
> +
> +            if let Some(index) = index {

and then drop the condition above entirely
and get rid of a level of indentation on the block below

> +                for pos in 0..index.index_count() {
> +                    if let Some(index_chunk_digest) = index.index_digest(pos) {
> +                        if digest_raw.eq(index_chunk_digest) {
> +                            references.push(entry.path().to_string_lossy().into_owned());
> +                            break;
> +                        }
> +                    }
> +                }
> +            }
> +        }
> +        referenced_by = Some(references);
> +    }
> +
> +    if decode_output_path.is_some() || to_stdout {

... because this is now: `if X || <something which implies X>`

> +        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(())
> +}
> +
> +pub fn inspect_commands() -> CommandLineInterface {
> +    let cmd_def = CliCommandMap::new().insert("chunk", CliCommand::new(&API_METHOD_INSPECT_CHUNK));

^ I think this should also get a `.arg_param(["chunk"])`.

Otherwise it feels somewhat as if you'd have to write
  $ cat --file foo
instead of just
  $ cat foo

;-)

> +
> +    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::*;
> diff --git a/src/tools.rs b/src/tools.rs
> index 08af55e5..16ebf3fc 100644
> --- a/src/tools.rs
> +++ b/src/tools.rs
> @@ -6,7 +6,7 @@ use std::borrow::Borrow;
>  use std::collections::HashMap;
>  use std::hash::BuildHasher;
>  use std::fs::File;
> -use std::io::{self, BufRead, Read, Seek, SeekFrom};
> +use std::io::{self, BufRead, Read, Seek, SeekFrom, stdout, Write};
>  use std::os::unix::io::RawFd;
>  use std::path::Path;
>  
> @@ -571,3 +571,13 @@ pub fn create_run_dir() -> Result<(), Error> {
>      let _: bool = proxmox::tools::fs::create_path(PROXMOX_BACKUP_RUN_DIR_M!(), None, None)?;
>      Ok(())
>  }

Code below is fine but needs rebasing on current master.

> +
> +/// Returns either a new file, if a path is given, or stdout, if no path is given.
> +pub fn outfile_or_stdout<P: AsRef<Path>>(path: Option<P>) -> Result<Box<dyn Write>, Error> {
> +    if let Some(path) = path {
> +        let f = File::create(path)?;
> +        Ok(Box::new(f) as Box<dyn Write>)
> +    } else {
> +        Ok(Box::new(stdout()) as Box<dyn Write>)
> +    }
> +}
> -- 
> 2.20.1




^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pbs-devel] applied: [PATCH v5 proxmox-backup 2/4] add ctime and size function to IndexFile trait
  2021-04-29 11:00 ` [pbs-devel] [PATCH v5 proxmox-backup 2/4] add ctime and size function to IndexFile trait Hannes Laimer
@ 2021-04-30  9:41   ` Wolfgang Bumiller
  0 siblings, 0 replies; 9+ messages in thread
From: Wolfgang Bumiller @ 2021-04-30  9:41 UTC (permalink / raw)
  To: Hannes Laimer; +Cc: pbs-devel

On Thu, Apr 29, 2021 at 01:00:14PM +0200, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> needed for next patch in order to avoid duplicating code for fixed and
> dynamic indexes
> 
>  src/backup/dynamic_index.rs | 8 ++++++++
>  src/backup/fixed_index.rs   | 8 ++++++++
>  src/backup/index.rs         | 2 ++
>  3 files changed, 18 insertions(+)
> 
> diff --git a/src/backup/dynamic_index.rs b/src/backup/dynamic_index.rs
> index 1619d8db..8321b295 100644
> --- a/src/backup/dynamic_index.rs
> +++ b/src/backup/dynamic_index.rs
> @@ -233,6 +233,14 @@ impl IndexFile for DynamicIndexReader {
>          })
>      }
>  
> +    fn index_ctime(&self) -> i64 {
> +        self.ctime
> +    }
> +
> +    fn index_size(&self) -> usize {
> +        self.size as usize
> +    }
> +
>      fn chunk_from_offset(&self, offset: u64) -> Option<(usize, u64)> {
>          let end_idx = self.index.len() - 1;
>          let end = self.chunk_end(end_idx);
> diff --git a/src/backup/fixed_index.rs b/src/backup/fixed_index.rs
> index ceb6fe29..ebf64456 100644
> --- a/src/backup/fixed_index.rs
> +++ b/src/backup/fixed_index.rs
> @@ -193,6 +193,14 @@ impl IndexFile for FixedIndexReader {
>          })
>      }
>  
> +    fn index_ctime(&self) -> i64 {
> +        self.ctime
> +    }
> +
> +    fn index_size(&self) -> usize {
> +        self.size as usize
> +    }
> +
>      fn compute_csum(&self) -> ([u8; 32], u64) {
>          let mut csum = openssl::sha::Sha256::new();
>          let mut chunk_end = 0;
> diff --git a/src/backup/index.rs b/src/backup/index.rs
> index c6bab56e..69788f80 100644
> --- a/src/backup/index.rs
> +++ b/src/backup/index.rs
> @@ -22,6 +22,8 @@ pub trait IndexFile {
>      fn index_digest(&self, pos: usize) -> Option<&[u8; 32]>;
>      fn index_bytes(&self) -> u64;
>      fn chunk_info(&self, pos: usize) -> Option<ChunkReadInfo>;
> +    fn index_ctime(&self) -> i64;
> +    fn index_size(&self) -> usize;
>  
>      /// Get the chunk index and the relative offset within it for a byte offset
>      fn chunk_from_offset(&self, offset: u64) -> Option<(usize, u64)>;
> -- 
> 2.20.1




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [pbs-devel] [PATCH v5 proxmox-backup 3/4] add file inspection to pb-debug
  2021-04-29 11:00 ` [pbs-devel] [PATCH v5 proxmox-backup 3/4] add file inspection to pb-debug Hannes Laimer
@ 2021-04-30  9:44   ` Wolfgang Bumiller
  0 siblings, 0 replies; 9+ messages in thread
From: Wolfgang Bumiller @ 2021-04-30  9:44 UTC (permalink / raw)
  To: Hannes Laimer; +Cc: pbs-devel

On Thu, Apr 29, 2021 at 01:00:15PM +0200, 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>
> ---
> v5:
>  - combine path for fixed and dynamic index fliles to avoid duplicate
>    code
> 
> 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 | 119 ++++++++++++++++++++++--
>  1 file changed, 112 insertions(+), 7 deletions(-)
> 
> diff --git a/src/bin/proxmox_backup_debug/inspect.rs b/src/bin/proxmox_backup_debug/inspect.rs
> index dd6ee287..2d95448e 100644
> --- a/src/bin/proxmox_backup_debug/inspect.rs
> +++ b/src/bin/proxmox_backup_debug/inspect.rs
> @@ -1,19 +1,26 @@
> +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::outfile_or_stdout;
>  
>  /// Decodes a blob and writes its content either to stdout or into a file
> @@ -37,6 +44,102 @@ fn decode_blob(
>      Ok(())
>  }
>  
> +#[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) = (

Same reasoning about `to_stdout` here as in the other patch. I think it
can just be dropped.

> +                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!({

Please remove the `Ok` wrapping here.

> +                "encryption": crypt_mode,
> +                "raw_size": data_blob.raw_size(),
> +            }))
> +        }
> +        FIXED_SIZED_CHUNK_INDEX_1_0 | DYNAMIC_SIZED_CHUNK_INDEX_1_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!("This is technically not possible")),
> +            }?;
> +
> +            let mut ctime_str = index.index_ctime().to_string();
> +            if let Ok(s) = proxmox::tools::time::strftime_local("%c", index.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!({

and here

> +                "size": index.index_size(),
> +                "ctime": ctime_str,
> +                "chunk-digests": chunk_digests
> +            }))
> +        }
> +        _ => Err(format_err!(

and use `bail!` instead of `Err(format_err!(` followed by a `?` below.

> +            "Only .blob, .fidx and .didx files may be inspected"
> +        )),
> +    }?;

^ And just drop the `?` here.

> +
> +    format_and_print_result(&val, &output_format);
> +
> +    Ok(())
> +}
> +
>  #[api(
>      input: {
>          properties: {
> @@ -156,7 +259,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))

Here too, I think we should add:

    .arg_param(["file"])

> +        .insert("chunk", CliCommand::new(&API_METHOD_INSPECT_CHUNK));
>  
>      cmd_def.into()
>  }
> -- 
> 2.20.1




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [pbs-devel] [PATCH v5 proxmox-backup 4/4] add index recovery to pb-debug
  2021-04-29 11:00 ` [pbs-devel] [PATCH v5 proxmox-backup 4/4] add index recovery " Hannes Laimer
@ 2021-04-30  9:55   ` Wolfgang Bumiller
  0 siblings, 0 replies; 9+ messages in thread
From: Wolfgang Bumiller @ 2021-04-30  9:55 UTC (permalink / raw)
  To: Hannes Laimer; +Cc: pbs-devel

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 <h.laimer@proxmox.com>
> ---
> 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
    <snip>

    Usage: proxmox-backup-debug recover index --chunks <string> --file <string> [OPTIONS]
     --chunks   <string>
                 Path to a file or a directory

     --file     <string>
                 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<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>)

Please drop the `Ok()` wrapping here

> +        }
> +        DYNAMIC_SIZED_CHUNK_INDEX_1_0 => {
> +            Ok(Box::new(DynamicIndexReader::new(file)?) as Box<dyn IndexFile>)

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




^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-04-30  9:55 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-29 11:00 [pbs-devel] [PATCH v5 proxmox-backup 0/4] add proxmox-backup-debug binary Hannes Laimer
2021-04-29 11:00 ` [pbs-devel] [PATCH v5 proxmox-backup 1/4] add chunk inspection to pb-debug Hannes Laimer
2021-04-30  9:39   ` Wolfgang Bumiller
2021-04-29 11:00 ` [pbs-devel] [PATCH v5 proxmox-backup 2/4] add ctime and size function to IndexFile trait Hannes Laimer
2021-04-30  9:41   ` [pbs-devel] applied: " Wolfgang Bumiller
2021-04-29 11:00 ` [pbs-devel] [PATCH v5 proxmox-backup 3/4] add file inspection to pb-debug Hannes Laimer
2021-04-30  9:44   ` Wolfgang Bumiller
2021-04-29 11:00 ` [pbs-devel] [PATCH v5 proxmox-backup 4/4] add index recovery " Hannes Laimer
2021-04-30  9:55   ` Wolfgang Bumiller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal