tree-wide: prefer api-type BackupGroup for logging

together with DatastoreWithNamespace where needed, to not forget
namespace information.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>

pbs-datastore/examples/ls-snapshots.rs

@@ -25,7 +25,7 @@ fn run() -> Result<(), Error> {
 
         for group in store.iter_backup_groups(ns)? {
             let group = group?;
-            println!("    found group {}", group);
+            println!("    found group {}", group.group());
 
             for snapshot in group.iter_snapshots()? {
                 let snapshot = snapshot?;

src/api2/admin/datastore.rs

@@ -225,16 +225,25 @@ pub fn list_groups(
     )?;
 
     let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
+    let store_with_ns = DatastoreWithNamespace {
+        store: store.to_owned(),
+        ns: ns.clone(),
+    };
 
     datastore
         .iter_backup_groups(ns.clone())? // FIXME: Namespaces and recursion parameters!
         .try_fold(Vec::new(), |mut group_info, group| {
             let group = group?;
+
             let owner = match datastore.get_owner(&ns, group.as_ref()) {
                 Ok(auth_id) => auth_id,
                 Err(err) => {
-                    let id = &store;
+                    eprintln!(
-                    eprintln!("Failed to get owner of group '{}/{}' - {}", id, group, err);
+                        "Failed to get owner of group '{}' in {} - {}",
+                        group.group(),
+                        store_with_ns,
+                        err
+                    );
                     return Ok(group_info);
                 }
             };
@@ -477,6 +486,10 @@ pub fn list_snapshots(
     )?;
 
     let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
+    let store_with_ns = DatastoreWithNamespace {
+        store: store.to_owned(),
+        ns: ns.clone(),
+    };
 
     // FIXME: filter also owner before collecting, for doing that nicely the owner should move into
     // backup group and provide an error free (Err -> None) accessor
@@ -575,8 +588,10 @@ pub fn list_snapshots(
             Ok(auth_id) => auth_id,
             Err(err) => {
                 eprintln!(
-                    "Failed to get owner of group '{}/{}' - {}",
+                    "Failed to get owner of group '{}' in {} - {}",
-                    &store, group, err
+                    &store_with_ns,
+                    group.group(),
+                    err
                 );
                 return Ok(snapshots);
             }
@@ -930,6 +945,10 @@ pub fn prune(
         Some(Operation::Write),
         &group,
     )?;
+    let store_with_ns = DatastoreWithNamespace {
+        store: store.to_owned(),
+        ns: ns.clone(),
+    };
 
     let group = datastore.backup_group(ns, group);
 
@@ -978,9 +997,9 @@ pub fn prune(
         );
         task_log!(
             worker,
-            "Starting prune on store \"{}\" group \"{}\"",
+            "Starting prune on {} group \"{}\"",
-            store,
+            store_with_ns,
-            group,
+            group.group(),
         );
     }
 
@@ -2175,7 +2194,7 @@ pub fn set_backup_owner(
                 UNAUTHORIZED,
                 "{} does not have permission to change owner of backup group '{}' to {}",
                 auth_id,
-                backup_group,
+                backup_group.group(),
                 new_owner,
             ));
         }

src/api2/tape/backup.rs

@@ -10,9 +10,9 @@ use proxmox_schema::api;
 use proxmox_sys::{task_log, task_warn, WorkerTaskContext};
 
 use pbs_api_types::{
-    print_ns_and_snapshot, Authid, GroupFilter, MediaPoolConfig, Operation, TapeBackupJobConfig,
+    print_ns_and_snapshot, Authid, DatastoreWithNamespace, GroupFilter, MediaPoolConfig, Operation,
-    TapeBackupJobSetup, TapeBackupJobStatus, Userid, JOB_ID_SCHEMA, PRIV_DATASTORE_READ,
+    TapeBackupJobConfig, TapeBackupJobSetup, TapeBackupJobStatus, Userid, JOB_ID_SCHEMA,
-    PRIV_TAPE_AUDIT, PRIV_TAPE_WRITE, UPID_SCHEMA,
+    PRIV_DATASTORE_READ, PRIV_TAPE_AUDIT, PRIV_TAPE_WRITE, UPID_SCHEMA,
 };
 
 use pbs_config::CachedUserInfo;
@@ -462,6 +462,11 @@ fn backup_worker(
     let mut need_catalog = false; // avoid writing catalog for empty jobs
 
     for (group_number, group) in group_list.into_iter().enumerate() {
+        let store_with_ns = DatastoreWithNamespace {
+            store: datastore_name.to_owned(),
+            ns: group.backup_ns().clone(),
+        };
+
         progress.done_groups = group_number as u64;
         progress.done_snapshots = 0;
         progress.group_snapshots = 0;
@@ -475,7 +480,12 @@ fn backup_worker(
             .collect();
 
         if snapshot_list.is_empty() {
-            task_log!(worker, "group {} was empty", group);
+            task_log!(
+                worker,
+                "{}, group {} was empty",
+                store_with_ns,
+                group.group()
+            );
             continue;
         }
 

src/backup/verify.rs

@@ -9,8 +9,8 @@ use anyhow::{bail, format_err, Error};
 use proxmox_sys::{task_log, WorkerTaskContext};
 
 use pbs_api_types::{
-    print_ns_and_snapshot, Authid, BackupNamespace, BackupType, CryptMode, SnapshotVerifyState,
+    print_ns_and_snapshot, Authid, BackupNamespace, BackupType, CryptMode, DatastoreWithNamespace,
-    VerifyState, UPID,
+    SnapshotVerifyState, VerifyState, UPID,
 };
 use pbs_datastore::backup_info::{BackupDir, BackupGroup, BackupInfo};
 use pbs_datastore::index::IndexFile;
@@ -453,11 +453,15 @@ pub fn verify_backup_group(
     let mut list = match group.list_backups() {
         Ok(list) => list,
         Err(err) => {
+            let store_with_ns = DatastoreWithNamespace {
+                store: verify_worker.datastore.name().to_owned(),
+                ns: group.backup_ns().clone(),
+            };
             task_log!(
                 verify_worker.worker,
-                "verify group {}:{} - unable to list backups: {}",
+                "verify {}, group {} - unable to list backups: {}",
-                verify_worker.datastore.name(),
+                store_with_ns,
-                group,
+                group.group(),
                 err,
             );
             return Ok(errors);
@@ -469,7 +473,7 @@ pub fn verify_backup_group(
         verify_worker.worker,
         "verify group {}:{} ({} snapshots)",
         verify_worker.datastore.name(),
-        group,
+        group.group(),
         snapshot_count
     );
 

src/server/pull.rs

@@ -1161,28 +1161,21 @@ pub async fn pull_ns(
         let result: Result<(), Error> = proxmox_lang::try_block!({
             for local_group in params.store.iter_backup_groups(target_ns.clone())? {
                 let local_group = local_group?;
-                if new_groups.contains(local_group.as_ref()) {
+                let local_group = local_group.group();
+                if new_groups.contains(local_group) {
                     continue;
                 }
-                let owner = params.store.get_owner(&target_ns, local_group.group())?;
+                let owner = params.store.get_owner(&target_ns, local_group)?;
                 if check_backup_owner(&owner, &params.owner).is_err() {
                     continue;
                 }
                 if let Some(ref group_filter) = &params.group_filter {
-                    if !apply_filters(local_group.as_ref(), group_filter) {
+                    if !apply_filters(local_group, group_filter) {
                         continue;
                     }
                 }
-                task_log!(
+                task_log!(worker, "delete vanished group '{local_group}'",);
-                    worker,
+                match params.store.remove_backup_group(&target_ns, local_group) {
-                    "delete vanished group '{}/{}'",
-                    local_group.backup_type(),
-                    local_group.backup_id()
-                );
-                match params
-                    .store
-                    .remove_backup_group(&target_ns, local_group.as_ref())
-                {
                     Ok(true) => {}
                     Ok(false) => {
                         task_log!(