* [pbs-devel] [PATCH proxmox-backup 1/2] verify: log/warn on invalid owner
@ 2020-11-10 12:52 Fabian Grünbichler
2020-11-10 12:52 ` [pbs-devel] [PATCH proxmox-backup 2/2] verify: cleanup logging order/messages Fabian Grünbichler
2020-11-10 13:14 ` [pbs-devel] applied-series: [PATCH proxmox-backup 1/2] verify: log/warn on invalid owner Thomas Lamprecht
0 siblings, 2 replies; 3+ messages in thread
From: Fabian Grünbichler @ 2020-11-10 12:52 UTC (permalink / raw)
To: pbs-devel
in order to trigger a notification/make the problem more visible than
just in syslog.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
Notes:
not filtering in case we don't have an explicit owner passed in to keep
backwards compat - we could also skip verification for them even in the
privileged case.. ?
-w --patience makes this easier to read
src/api2/admin/datastore.rs | 2 +-
src/backup/verify.rs | 42 ++++++++++++++++++++++---------------
2 files changed, 26 insertions(+), 18 deletions(-)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 8256f02f..e76867c7 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -648,7 +648,7 @@ pub fn verify(
verify_all_backups(datastore, worker.clone(), worker.upid(), owner, None)?
};
if failed_dirs.len() > 0 {
- worker.log("Failed to verify following snapshots:");
+ worker.log("Failed to verify following snapshots/groups:");
for dir in failed_dirs {
worker.log(format!("\t{}", dir));
}
diff --git a/src/backup/verify.rs b/src/backup/verify.rs
index b5bb85fc..512a3805 100644
--- a/src/backup/verify.rs
+++ b/src/backup/verify.rs
@@ -508,23 +508,31 @@ pub fn verify_all_backups(
}
let filter_by_owner = |group: &BackupGroup| {
- if let Some(owner) = &owner {
- match datastore.get_owner(group) {
- Ok(ref group_owner) => {
- group_owner == owner
- || (group_owner.is_token()
- && !owner.is_token()
- && group_owner.user() == owner.user())
- },
- Err(err) => {
- // intentionally not in task log
- // the task user might not be allowed to see this group!
- println!("Failed to get owner of group '{}' - {}", group, err);
- false
- },
- }
- } else {
- true
+ match (datastore.get_owner(group), &owner) {
+ (Ok(ref group_owner), Some(owner)) => {
+ group_owner == owner
+ || (group_owner.is_token()
+ && !owner.is_token()
+ && group_owner.user() == owner.user())
+ },
+ (Ok(_), None) => true,
+ (Err(err), Some(_)) => {
+ // intentionally not in task log
+ // the task user might not be allowed to see this group!
+ println!("Failed to get owner of group '{}' - {}", group, err);
+ false
+ },
+ (Err(err), None) => {
+ // we don't filter by owner, but we want to log the error
+ task_log!(
+ worker,
+ "Failed to get owner of group '{} - {}",
+ group,
+ err,
+ );
+ errors.push(group.to_string());
+ true
+ },
}
};
--
2.20.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 2/2] verify: cleanup logging order/messages
2020-11-10 12:52 [pbs-devel] [PATCH proxmox-backup 1/2] verify: log/warn on invalid owner Fabian Grünbichler
@ 2020-11-10 12:52 ` Fabian Grünbichler
2020-11-10 13:14 ` [pbs-devel] applied-series: [PATCH proxmox-backup 1/2] verify: log/warn on invalid owner Thomas Lamprecht
1 sibling, 0 replies; 3+ messages in thread
From: Fabian Grünbichler @ 2020-11-10 12:52 UTC (permalink / raw)
To: pbs-devel
otherwise we end up printing warnings before the start message..
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/backup/verify.rs | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/src/backup/verify.rs b/src/backup/verify.rs
index 512a3805..1eccdd67 100644
--- a/src/backup/verify.rs
+++ b/src/backup/verify.rs
@@ -498,13 +498,10 @@ pub fn verify_all_backups(
) -> Result<Vec<String>, Error> {
let mut errors = Vec::new();
+ task_log!(worker, "verify datastore {}", datastore.name());
+
if let Some(owner) = &owner {
- task_log!(
- worker,
- "verify datastore {} - limiting to backups owned by {}",
- datastore.name(),
- owner
- );
+ task_log!(worker, "limiting to backups owned by {}", owner);
}
let filter_by_owner = |group: &BackupGroup| {
@@ -545,8 +542,7 @@ pub fn verify_all_backups(
Err(err) => {
task_log!(
worker,
- "verify datastore {} - unable to list backups: {}",
- datastore.name(),
+ "unable to list backups: {}",
err,
);
return Ok(errors);
@@ -566,7 +562,7 @@ pub fn verify_all_backups(
// start with 64 chunks since we assume there are few corrupt ones
let corrupt_chunks = Arc::new(Mutex::new(HashSet::with_capacity(64)));
- task_log!(worker, "verify datastore {} ({} snapshots)", datastore.name(), snapshot_count);
+ task_log!(worker, "found {} snapshots", snapshot_count);
let mut done = 0;
for group in list {
--
2.20.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* [pbs-devel] applied-series: [PATCH proxmox-backup 1/2] verify: log/warn on invalid owner
2020-11-10 12:52 [pbs-devel] [PATCH proxmox-backup 1/2] verify: log/warn on invalid owner Fabian Grünbichler
2020-11-10 12:52 ` [pbs-devel] [PATCH proxmox-backup 2/2] verify: cleanup logging order/messages Fabian Grünbichler
@ 2020-11-10 13:14 ` Thomas Lamprecht
1 sibling, 0 replies; 3+ messages in thread
From: Thomas Lamprecht @ 2020-11-10 13:14 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Fabian Grünbichler
On 10.11.20 13:52, Fabian Grünbichler wrote:
> in order to trigger a notification/make the problem more visible than
> just in syslog.
>
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
>
> Notes:
> not filtering in case we don't have an explicit owner passed in to keep
> backwards compat - we could also skip verification for them even in the
> privileged case.. ?
>
> -w --patience makes this easier to read
>
> src/api2/admin/datastore.rs | 2 +-
> src/backup/verify.rs | 42 ++++++++++++++++++++++---------------
> 2 files changed, 26 insertions(+), 18 deletions(-)
>
>
applied both patches, thanks!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-11-10 13:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-10 12:52 [pbs-devel] [PATCH proxmox-backup 1/2] verify: log/warn on invalid owner Fabian Grünbichler
2020-11-10 12:52 ` [pbs-devel] [PATCH proxmox-backup 2/2] verify: cleanup logging order/messages Fabian Grünbichler
2020-11-10 13:14 ` [pbs-devel] applied-series: [PATCH proxmox-backup 1/2] verify: log/warn on invalid owner Thomas Lamprecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox