From 7efc3de78f1e4e8afd83217a71ffaf24d8d406e7 Mon Sep 17 00:00:00 2001 From: Dennis Schwerdel Date: Mon, 10 Apr 2017 19:28:17 +0200 Subject: [PATCH] Better control over what is checked in `check` subcommand --- TODO.md | 1 - docs/man/zvault-check.1.md | 33 ++++++++++++++++++------- src/bundledb/db.rs | 2 +- src/cli/args.rs | 12 ++++++--- src/cli/mod.rs | 11 +++++++-- src/repository/integrity.rs | 49 +++++++++++++++++-------------------- 6 files changed, 65 insertions(+), 43 deletions(-) diff --git a/TODO.md b/TODO.md index 2c66ce2..7602690 100644 --- a/TODO.md +++ b/TODO.md @@ -10,7 +10,6 @@ ## Usability * Backup directories as a thing (list, remove) -* Better control over what is checked in `check` subcommand * Man pages for all minor subcommands ## Code quality diff --git a/docs/man/zvault-check.1.md b/docs/man/zvault-check.1.md index c8b8eb4..7fb0866 100644 --- a/docs/man/zvault-check.1.md +++ b/docs/man/zvault-check.1.md @@ -15,9 +15,9 @@ The repository, backup, of subtree given by `PATH` must be in the format `[repository][::backup_name[::subtree]]` as described in _zvault(1)_. The command will perform the following checks in order: -- Bundle integrity +- Bundle integrity (optional) - Full bundle contents (optional) -- Index integrity +- Index integrity (optional) - Backup integrity - Filesystem integrity @@ -28,22 +28,37 @@ in the filesystem integrity check. If a subtree is specified in `PATH`, no backups will be checked and only the given subtree will be checked in the filesystem integrity check. -Unless `--full` is set, the bundles will only be checked without actually -fetching them fully. This means that their contents can only be read from their -header and this information is not verified. If `--full` is set, the full -bundles are fetched and their contents are compared to what their header claims. -This check takes a long time since all bundles need to fetched, decrypted and -decompressed fully to read their contents. +If `--bundles` is set, the integrity of the bundles will be checked before +checking any backups. +If `--bundle-data` is also set, the full bundles are fetched and their contents +are compared to what their header claims. This check takes a long time since all +bundles need to fetched, decrypted and decompressed fully to read their +contents. If this flag is not set, the bundles will only be checked without +actually fetching them fully. This means that their contents can only be read +from their header and this information is not verified. + +If `--index` is set, the integrity of the index and its contents will be checked +before checking any backups. ## OPTIONS - * `--full`: + * `-b`, `--bundles`: + + Check the integrity of the bundles too. + + + * `--bundle-data`: Also check the contents of the bundles by fetching and decompressing them. Note: This flag causes the check to be much slower. + * `-i`, `--index`: + + Also check the integrity of the index and its contents. + + * `-h`, `--help`: Prints help information diff --git a/src/bundledb/db.rs b/src/bundledb/db.rs index 51f2f1b..5213c16 100644 --- a/src/bundledb/db.rs +++ b/src/bundledb/db.rs @@ -281,7 +281,7 @@ impl BundleDb { #[inline] pub fn get_bundle_info(&self, bundle: &BundleId) -> Option<&StoredBundle> { - self.get_stored_bundle(bundle).ok() + self.remote_bundles.get(bundle) } #[inline] diff --git a/src/cli/args.rs b/src/cli/args.rs index 4ec5517..5e33765 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -57,7 +57,9 @@ pub enum Arguments { repo_path: String, backup_name: Option, inode: Option, - full: bool + bundles: bool, + bundle_data: bool, + index: bool }, List { repo_path: String, @@ -337,7 +339,9 @@ pub fn parse() -> Result<(LogLevel, Arguments), ErrorCode> { .arg(Arg::from_usage(" 'Path of the repository'") .validator(|val| validate_repo_path(val, true, Some(false), Some(false))))) .subcommand(SubCommand::with_name("check").about("Check the repository, a backup or a backup subtree") - .arg(Arg::from_usage("--full 'Also check file contents (slow)'")) + .arg(Arg::from_usage("-b --bundles 'Check the bundles'")) + .arg(Arg::from_usage("[bundle_data] --bundle-data 'Check bundle contents (slow)'").requires("bundles").alias("--data")) + .arg(Arg::from_usage("-i --index 'Check the chunk index'")) .arg(Arg::from_usage(" 'Path of the repository/backup/subtree, [repository][::backup[::subtree]]'") .validator(|val| validate_repo_path(val, true, None, None)))) .subcommand(SubCommand::with_name("list").alias("ls").about("List backups or backup contents") @@ -491,7 +495,9 @@ pub fn parse() -> Result<(LogLevel, Arguments), ErrorCode> { repo_path: repository.to_string(), backup_name: backup.map(|v| v.to_string()), inode: inode.map(|v| v.to_string()), - full: args.is_present("full") + bundles: args.is_present("bundles"), + bundle_data: args.is_present("bundle_data"), + index: args.is_present("index") } }, ("list", Some(args)) => { diff --git a/src/cli/mod.rs b/src/cli/mod.rs index bd455da..1567bd3 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -418,8 +418,15 @@ pub fn run() -> Result<(), ErrorCode> { info!("Reclaimed {}", to_file_size(info_before.encoded_data_size - info_after.encoded_data_size)); } }, - Arguments::Check{repo_path, backup_name, inode, full} => { + Arguments::Check{repo_path, backup_name, inode, bundles, index, bundle_data} => { let mut repo = try!(open_repository(&repo_path)); + checked!(repo.check_repository(), "check repository", ErrorCode::CheckRun); + if bundles { + checked!(repo.check_bundles(bundle_data), "check bundles", ErrorCode::CheckRun); + } + if index { + checked!(repo.check_index(), "check index", ErrorCode::CheckRun); + } if let Some(backup_name) = backup_name { let backup = try!(get_backup(&repo, &backup_name)); if let Some(path) = inode { @@ -429,7 +436,7 @@ pub fn run() -> Result<(), ErrorCode> { checked!(repo.check_backup(&backup), "check backup", ErrorCode::CheckRun) } } else { - checked!(repo.check(full), "check repository", ErrorCode::CheckRun) + checked!(repo.check_backups(), "check repository", ErrorCode::CheckRun) } info!("Integrity verified") }, diff --git a/src/repository/integrity.rs b/src/repository/integrity.rs index fc3c140..b213cbf 100644 --- a/src/repository/integrity.rs +++ b/src/repository/integrity.rs @@ -64,19 +64,6 @@ impl Repository { }) } - fn check_repository(&self) -> Result<(), RepositoryError> { - if self.next_data_bundle == self.next_meta_bundle { - return Err(IntegrityError::InvalidNextBundleId.into()) - } - if self.bundle_map.get(self.next_data_bundle).is_some() { - return Err(IntegrityError::InvalidNextBundleId.into()) - } - if self.bundle_map.get(self.next_meta_bundle).is_some() { - return Err(IntegrityError::InvalidNextBundleId.into()) - } - Ok(()) - } - fn check_chunks(&self, checked: &mut Bitmap, chunks: &[Chunk]) -> Result { let mut new = false; for &(hash, _len) in chunks { @@ -132,11 +119,13 @@ impl Repository { } pub fn check_backup(&mut self, backup: &Backup) -> Result<(), RepositoryError> { + info!("Checking backup..."); let mut checked = Bitmap::new(self.index.capacity()); self.check_subtree(Path::new("").to_path_buf(), &backup.root, &mut checked) } pub fn check_inode(&mut self, inode: &Inode, path: &Path) -> Result<(), RepositoryError> { + info!("Checking inode..."); let mut checked = Bitmap::new(self.index.capacity()); try!(self.check_inode_contents(inode, &mut checked)); if let Some(ref children) = inode.children { @@ -147,7 +136,8 @@ impl Repository { Ok(()) } - fn check_backups(&mut self) -> Result<(), RepositoryError> { + pub fn check_backups(&mut self) -> Result<(), RepositoryError> { + info!("Checking backups..."); let mut checked = Bitmap::new(self.index.capacity()); let backup_map = match self.get_backups() { Ok(backup_map) => backup_map, @@ -164,7 +154,17 @@ impl Repository { Ok(()) } - fn check_bundle_map(&mut self) -> Result<(), RepositoryError> { + pub fn check_repository(&mut self) -> Result<(), RepositoryError> { + info!("Checking repository integrity..."); + if self.next_data_bundle == self.next_meta_bundle { + return Err(IntegrityError::InvalidNextBundleId.into()) + } + if self.bundle_map.get(self.next_data_bundle).is_some() { + return Err(IntegrityError::InvalidNextBundleId.into()) + } + if self.bundle_map.get(self.next_meta_bundle).is_some() { + return Err(IntegrityError::InvalidNextBundleId.into()) + } for (_id, bundle_id) in self.bundle_map.bundles() { if self.bundles.get_bundle_info(&bundle_id).is_none() { return Err(IntegrityError::MissingBundle(bundle_id).into()) @@ -179,20 +179,15 @@ impl Repository { Ok(()) } - pub fn check(&mut self, full: bool) -> Result<(), RepositoryError> { - try!(self.flush()); - info!("Checking bundle integrity..."); - try!(self.bundles.check(full)); + pub fn check_index(&mut self) -> Result<(), RepositoryError> { info!("Checking index integrity..."); try!(self.index.check()); - info!("Checking bundle map..."); - try!(self.check_bundle_map()); info!("Checking index entries..."); - try!(self.check_index_chunks()); - info!("Checking backup integrity..."); - try!(self.check_backups()); - info!("Checking repository integrity..."); - try!(self.check_repository()); - Ok(()) + self.check_index_chunks() + } + + pub fn check_bundles(&mut self, full: bool) -> Result<(), RepositoryError> { + info!("Checking bundle integrity..."); + Ok(try!(self.bundles.check(full))) } }