all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Dominik Csapak <d.csapak@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup 1/2] api: tape/restore: fix wrong datastore locking
Date: Mon,  9 May 2022 12:41:18 +0200	[thread overview]
Message-ID: <20220509104119.1953106-1-d.csapak@proxmox.com> (raw)

used_datastores returned the 'target', but in the full_restore_worker,
we interpreted it as the source and searched for a mapping
(which we then locked)

since we cannot return a HashSet of Arc<T> (missing Hash trait on DataStore),
we have now a map of source -> target

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/tape/restore.rs | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/src/api2/tape/restore.rs b/src/api2/tape/restore.rs
index 93803f14..f24ae23c 100644
--- a/src/api2/tape/restore.rs
+++ b/src/api2/tape/restore.rs
@@ -88,17 +88,17 @@ impl TryFrom<String> for DataStoreMap {
 }
 
 impl DataStoreMap {
-    fn used_datastores<'a>(&self) -> HashSet<&str> {
-        let mut set = HashSet::new();
-        for store in self.map.values() {
-            set.insert(store.name());
+    fn used_datastores<'a>(&self) -> HashMap<&str, Arc<DataStore>> {
+        let mut map = HashMap::new();
+        for (source, target) in self.map.iter() {
+            map.insert(source.as_str(), Arc::clone(target));
         }
 
         if let Some(ref store) = self.default {
-            set.insert(store.name());
+            map.insert("", Arc::clone(store));
         }
 
-        set
+        map
     }
 
     fn get_datastore(&self, source: &str) -> Option<Arc<DataStore>> {
@@ -200,8 +200,8 @@ pub fn restore(
         bail!("no datastores given");
     }
 
-    for store in used_datastores.iter() {
-        check_datastore_privs(&user_info, store, &auth_id, &owner)?;
+    for (_, target) in used_datastores.iter() {
+        check_datastore_privs(&user_info, target.name(), &auth_id, &owner)?;
     }
 
     let privs = user_info.lookup_privs(&auth_id, &["tape", "drive", &drive]);
@@ -233,7 +233,7 @@ pub fn restore(
 
     let taskid = used_datastores
         .iter()
-        .map(|s| s.to_string())
+        .map(|(_, t)| t.name().to_string())
         .collect::<Vec<String>>()
         .join(", ");
 
@@ -349,7 +349,7 @@ fn restore_full_worker(
         store_map
             .used_datastores()
             .into_iter()
-            .map(String::from)
+            .map(|(_, t)| String::from(t.name()))
             .collect::<Vec<String>>()
             .join(", "),
     );
@@ -366,12 +366,10 @@ fn restore_full_worker(
     );
 
     let mut datastore_locks = Vec::new();
-    for store_name in store_map.used_datastores() {
+    for (_, target) in store_map.used_datastores() {
         // explicit create shared lock to prevent GC on newly created chunks
-        if let Some(store) = store_map.get_datastore(store_name) {
-            let shared_store_lock = store.try_shared_chunk_store_lock()?;
-            datastore_locks.push(shared_store_lock);
-        }
+        let shared_store_lock = target.try_shared_chunk_store_lock()?;
+        datastore_locks.push(shared_store_lock);
     }
 
     let mut checked_chunks_map = HashMap::new();
-- 
2.30.2





             reply	other threads:[~2022-05-09 10:41 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-09 10:41 Dominik Csapak [this message]
2022-05-09 10:41 ` [pbs-devel] [PATCH proxmox-backup 2/2] api: tape/restore: skip snapshot if owner check failed Dominik Csapak
2022-05-09 11:57 ` [pbs-devel] applied-series: Re: [PATCH proxmox-backup 1/2] api: tape/restore: fix wrong datastore locking Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220509104119.1953106-1-d.csapak@proxmox.com \
    --to=d.csapak@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal