From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 6787BE4A0 for ; Fri, 9 Dec 2022 10:14:26 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 4AC6221D1A for ; Fri, 9 Dec 2022 10:13:56 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Fri, 9 Dec 2022 10:13:54 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 933AB44109 for ; Fri, 9 Dec 2022 10:13:54 +0100 (CET) Date: Fri, 9 Dec 2022 10:13:52 +0100 From: Wolfgang Bumiller To: Lukas Wagner Cc: pbs-devel@lists.proxmox.com Message-ID: <20221209091352.qxph4w7hy6jfvjih@wobu-vie.proxmox.com> References: <20221207093819.75847-1-l.wagner@proxmox.com> <20221207093819.75847-2-l.wagner@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221207093819.75847-2-l.wagner@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.224 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pbs-devel] [PATCH v3 proxmox-backup 1/4] debug cli: show more file attributes for `diff archive` command X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 09 Dec 2022 09:14:26 -0000 On Wed, Dec 07, 2022 at 10:38:16AM +0100, Lukas Wagner wrote: > This commit enriches the output of the `diff archive` command, > showing pxar entry type, mode, uid, gid, size, mtime and filename. > Attributes that changed between both snapshots are prefixed > with a "*". > > For instance: > > $ proxmox-backup-debug diff archive ... > A f 644 10045 10000 0 B 2022-11-28 13:44:51 add.txt > M f 644 10045 10000 6 B *2022-11-28 13:45:05 content.txt > D f 644 10045 10000 0 B 2022-11-28 13:17:09 deleted.txt > M f 644 10045 *29 0 B 2022-11-28 13:16:20 gid.txt > M f *777 10045 10000 0 B 2022-11-28 13:42:47 mode.txt > M f 644 10045 10000 0 B *2022-11-28 13:44:33 mtime.txt > M f 644 10045 10000 *7 B *2022-11-28 13:44:59 *size.txt > M f 644 *64045 10000 0 B 2022-11-28 13:16:18 uid.txt > M *f 644 10045 10000 10 B 2022-11-28 13:44:59 type_changed.txt > > Also, this commit ensures that we always show the *new* type. > Previously, the command showed the old type if it was changed. > Signed-off-by: Lukas Wagner > --- > src/bin/proxmox_backup_debug/diff.rs | 221 ++++++++++++++++++++------- > 1 file changed, 167 insertions(+), 54 deletions(-) > > diff --git a/src/bin/proxmox_backup_debug/diff.rs b/src/bin/proxmox_backup_debug/diff.rs > index cb157ec2..d5a4a1fe 100644 > --- a/src/bin/proxmox_backup_debug/diff.rs > +++ b/src/bin/proxmox_backup_debug/diff.rs > @@ -11,7 +11,7 @@ use futures::FutureExt; > use proxmox_router::cli::{CliCommand, CliCommandMap, CommandLineInterface}; > use proxmox_schema::api; > > -use pbs_api_types::{BackupNamespace, BackupPart}; > +use pbs_api_types::{BackupNamespace, BackupPart, HumanByte}; > use pbs_client::tools::key_source::{ > crypto_parameters, format_key_source, get_encryption_key_password, KEYFD_SCHEMA, > }; > @@ -173,15 +173,18 @@ async fn diff_archive( > // If file is present in both snapshots, it *might* be modified, but does not have to be. > // If another, unmodified file resides in the same chunk as an actually modified one, > // it will also show up as modified here... > - let potentially_modified: HashMap<&OsStr, &FileEntry> = files_in_a > + let potentially_modified: HashMap<&OsStr, (&FileEntry, &FileEntry)> = files_in_a > .iter() > - .filter(|(path, _)| files_in_b.contains_key(*path)) > - .map(|(path, entry)| (path.as_os_str(), entry)) > + .filter_map(|(path, entry_a)| { > + files_in_b > + .get(path) > + .map(|entry_b| (path.as_os_str(), (entry_a, entry_b))) > + }) > .collect(); > > // ... so we compare the file metadata/contents to narrow the selection down to files > // which where *really* modified. > - let modified_files = compare_files(&files_in_a, &files_in_b, potentially_modified).await?; > + let modified_files = compare_files(potentially_modified).await?; > > show_file_list(&added_files, &deleted_files, &modified_files); > > @@ -335,7 +338,6 @@ fn visit_directory<'f, 'c>( > let digest = chunk_list.get(chunk_index).context("Invalid chunk index")?; > > if chunk_diff.get(digest).is_some() { > - // files.insert(file_path.clone().into_os_string()); > entries.insert(file_path.into_os_string(), entry); > break; > } > @@ -349,62 +351,177 @@ fn visit_directory<'f, 'c>( > > /// Check if files were actually modified > async fn compare_files<'a>( > - entries_a: &HashMap, > - entries_b: &HashMap, > - files: HashMap<&'a OsStr, &'a FileEntry>, > -) -> Result, Error> { > + files: HashMap<&'a OsStr, (&'a FileEntry, &'a FileEntry)>, > +) -> Result, Error> { > let mut modified_files = HashMap::new(); > > - for (path, entry) in files { > - let p = path.to_os_string(); > - let file_a = entries_a.get(&p).context("File entry not in map")?; > - let file_b = entries_b.get(&p).context("File entry not in map")?; > - > - if !compare_file(file_a, file_b).await { > - modified_files.insert(path, entry); > + for (path, (entry_a, entry_b)) in files { > + if let Some(changed) = compare_file(entry_a, entry_b).await? { > + modified_files.insert(path, (entry_b, changed)); > } > } > > Ok(modified_files) > } > > -async fn compare_file(file_a: &FileEntry, file_b: &FileEntry) -> bool { > - if file_a.metadata() != file_b.metadata() { > - // Check if mtime, permissions, ACLs, etc. have changed - if they have changed, we consider > - // the file as modified. > - return false; > - } > +async fn compare_file( > + file_a: &FileEntry, > + file_b: &FileEntry, > +) -> Result, Error> { > + let mut changed = ChangedProperties::default(); > + > + changed.set_from_metadata(file_a, file_b); > > match (file_a.kind(), file_b.kind()) { > (EntryKind::Symlink(a), EntryKind::Symlink(b)) => { > // Check whether the link target has changed. > - a.as_os_str() == b.as_os_str() > + changed.content = a.as_os_str() != b.as_os_str(); > } > (EntryKind::Hardlink(a), EntryKind::Hardlink(b)) => { > // Check whether the link target has changed. > - a.as_os_str() == b.as_os_str() > + changed.content = a.as_os_str() != b.as_os_str(); > + } > + (EntryKind::Device(a), EntryKind::Device(b)) => { > + changed.content = a.major != b.major || a.minor != b.minor Just noticed - we might be missing a change between blockdev/chardev here? Might need to include file_a.metadata().stat.is_blockdev() != file_b.metadata().stat.is_blockdev() as well? Or am I missing this being handled elsewhere already? > } > - (EntryKind::Device(a), EntryKind::Device(b)) => a.major == b.major && a.minor == b.minor, > - (EntryKind::Socket, EntryKind::Socket) => true, > - (EntryKind::Fifo, EntryKind::Fifo) => true, > (EntryKind::File { size: size_a, .. }, EntryKind::File { size: size_b, .. }) => { > - // At this point we know that all metadata including mtime is > - // the same. To speed things up, we consider the files as equal if they also have > - // the same size. > - // If one were completely paranoid, one could compare the actual file contents, > - // but this decreases performance drastically. > - size_a == size_b > + if size_a != size_b { > + changed.size = true; > + changed.content = true; > + }; > + } > + (EntryKind::Directory, EntryKind::Directory) => {} > + (EntryKind::Socket, EntryKind::Socket) => {} > + (EntryKind::Fifo, EntryKind::Fifo) => {} > + (_, _) => { > + changed.entry_type = true; > } > - (EntryKind::Directory, EntryKind::Directory) => true, > - (_, _) => false, // Kind has changed, so we of course consider it modified. > + } > + > + if changed.any() { > + Ok(Some(changed)) > + } else { > + Ok(None) > + } > +} > + > +#[derive(Copy, Clone, Default)] > +struct ChangedProperties { > + entry_type: bool, > + mtime: bool, > + acl: bool, > + xattrs: bool, > + fcaps: bool, > + quota_project_id: bool, > + mode: bool, > + flags: bool, > + uid: bool, > + gid: bool, > + size: bool, > + content: bool, > +} > + > +impl ChangedProperties { > + fn set_from_metadata(&mut self, file_a: &FileEntry, file_b: &FileEntry) { > + let a = file_a.metadata(); > + let b = file_b.metadata(); > + > + self.acl = a.acl != b.acl; > + self.xattrs = a.xattrs != b.xattrs; > + self.fcaps = a.fcaps != b.fcaps; > + self.quota_project_id = a.quota_project_id != b.quota_project_id; > + self.mode = a.stat.mode != b.stat.mode; > + self.flags = a.stat.flags != b.stat.flags; > + self.uid = a.stat.uid != b.stat.uid; > + self.gid = a.stat.gid != b.stat.gid; > + self.mtime = a.stat.mtime != b.stat.mtime; > + } > + > + fn any(&self) -> bool { > + self.entry_type > + || self.mtime > + || self.acl > + || self.xattrs > + || self.fcaps > + || self.quota_project_id > + || self.mode > + || self.flags > + || self.uid > + || self.gid > + || self.content > + } > +} > + > +fn change_indicator(changed: bool) -> &'static str { > + if changed { > + "*" > + } else { > + " " > + } > +} > + > +fn format_filesize(entry: &FileEntry, changed: bool) -> String { > + if let Some(size) = entry.file_size() { > + format!( > + "{}{:.1}", > + change_indicator(changed), > + HumanByte::new_decimal(size as f64) > + ) > + } else { > + String::new() > } > } > > +fn format_mtime(entry: &FileEntry, changed: bool) -> String { > + let mtime = &entry.metadata().stat.mtime; > + > + let format = if changed { "*%F %T" } else { " %F %T" }; > + > + proxmox_time::strftime_local(format, mtime.secs).unwrap_or_default() > +} > + > +fn format_mode(entry: &FileEntry, changed: bool) -> String { > + let mode = entry.metadata().stat.mode & 0o7777; > + format!("{}{:o}", change_indicator(changed), mode) > +} > + > +fn format_entry_type(entry: &FileEntry, changed: bool) -> String { > + let kind = match entry.kind() { > + EntryKind::Symlink(_) => "l", > + EntryKind::Hardlink(_) => "h", > + EntryKind::Device(_) if entry.metadata().stat.is_blockdev() => "b", > + EntryKind::Device(_) => "c", > + EntryKind::Socket => "s", > + EntryKind::Fifo => "p", > + EntryKind::File { .. } => "f", > + EntryKind::Directory => "d", > + _ => " ", > + }; > + > + format!("{}{}", change_indicator(changed), kind) > +} > + > +fn format_uid(entry: &FileEntry, changed: bool) -> String { > + format!("{}{}", change_indicator(changed), entry.metadata().stat.uid) > +} > + > +fn format_gid(entry: &FileEntry, changed: bool) -> String { > + format!("{}{}", change_indicator(changed), entry.metadata().stat.gid) > +} > + > +fn format_file_name(entry: &FileEntry, changed: bool) -> String { > + format!( > + "{}{}", > + change_indicator(changed), > + entry.file_name().to_string_lossy() > + ) > +} > + > /// Display a sorted list of added, modified, deleted files. > fn show_file_list( > added: &HashMap<&OsStr, &FileEntry>, > deleted: &HashMap<&OsStr, &FileEntry>, > - modified: &HashMap<&OsStr, &FileEntry>, > + modified: &HashMap<&OsStr, (&FileEntry, ChangedProperties)>, > ) { > let mut all: Vec<&OsStr> = Vec::new(); > > @@ -415,28 +532,24 @@ fn show_file_list( > all.sort(); > > for file in all { > - let (op, entry) = if let Some(entry) = added.get(file) { > - ("A", *entry) > + let (op, entry, changed) = if let Some(entry) = added.get(file) { > + ("A", entry, ChangedProperties::default()) > } else if let Some(entry) = deleted.get(file) { > - ("D", *entry) > - } else if let Some(entry) = modified.get(file) { > - ("M", *entry) > + ("D", entry, ChangedProperties::default()) > + } else if let Some((entry, changed)) = modified.get(file) { > + ("M", entry, *changed) > } else { > unreachable!(); > }; > > - let entry_kind = match entry.kind() { > - EntryKind::Symlink(_) => "l", > - EntryKind::Hardlink(_) => "h", > - EntryKind::Device(_) if entry.metadata().stat.is_blockdev() => "b", > - EntryKind::Device(_) => "c", > - EntryKind::Socket => "s", > - EntryKind::Fifo => "p", > - EntryKind::File { .. } => "f", > - EntryKind::Directory => "d", > - _ => " ", > - }; > + let entry_type = format_entry_type(entry, changed.entry_type); > + let uid = format_uid(entry, changed.uid); > + let gid = format_gid(entry, changed.gid); > + let mode = format_mode(entry, changed.mode); > + let size = format_filesize(entry, changed.size); > + let mtime = format_mtime(entry, changed.mtime); > + let name = format_file_name(entry, changed.content); > > - println!("{} {} {}", op, entry_kind, file.to_string_lossy()); > + println!("{op} {entry_type:>2} {mode:>5} {uid:>6} {gid:>6} {size:>10} {mtime:11} {name}"); > } > } > -- > 2.30.2