public inbox for pbs-devel@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal