tyler
/
proxmox-backup
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

api: tape/restore: fix wrong datastore locking 

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>
This commit is contained in:
Dominik Csapak 2022-05-09 12:41:18 +02:00 committed by Thomas Lamprecht
parent 0b232f2edc
commit c94d2867c1
1 changed files with 13 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
src/api2/tape/restore.rs
View File

@ -88,17 +88,17 @@ impl TryFrom<String> for DataStoreMap {
} }
impl DataStoreMap { impl DataStoreMap {
fn used_datastores<'a>(&self) -> HashSet<&str> { fn used_datastores<'a>(&self) -> HashMap<&str, Arc<DataStore>> {
let mut set = HashSet::new(); let mut map = HashMap::new();
for store in self.map.values() { for (source, target) in self.map.iter() {
set.insert(store.name()); map.insert(source.as_str(), Arc::clone(target));
} }
if let Some(ref store) = self.default { 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>> { fn get_datastore(&self, source: &str) -> Option<Arc<DataStore>> {
@ -200,8 +200,8 @@ pub fn restore(
bail!("no datastores given"); bail!("no datastores given");
} }
for store in used_datastores.iter() { for (_, target) in used_datastores.iter() {
check_datastore_privs(&user_info, store, &auth_id, &owner)?; check_datastore_privs(&user_info, target.name(), &auth_id, &owner)?;
} }
let privs = user_info.lookup_privs(&auth_id, &["tape", "drive", &drive]); let privs = user_info.lookup_privs(&auth_id, &["tape", "drive", &drive]);
@ -233,7 +233,7 @@ pub fn restore(
let taskid = used_datastores let taskid = used_datastores
.iter() .iter()
.map(|s| s.to_string()) .map(|(_, t)| t.name().to_string())
.collect::<Vec<String>>() .collect::<Vec<String>>()
.join(", "); .join(", ");
@ -349,7 +349,7 @@ fn restore_full_worker(
store_map store_map
.used_datastores() .used_datastores()
.into_iter() .into_iter()
.map(String::from) .map(|(_, t)| String::from(t.name()))
.collect::<Vec<String>>() .collect::<Vec<String>>()
.join(", "), .join(", "),
); );
@ -366,12 +366,10 @@ fn restore_full_worker(
); );
let mut datastore_locks = Vec::new(); 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 // 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 = target.try_shared_chunk_store_lock()?;
let shared_store_lock = store.try_shared_chunk_store_lock()?; datastore_locks.push(shared_store_lock);
datastore_locks.push(shared_store_lock);
}
} }
let mut checked_chunks_map = HashMap::new(); let mut checked_chunks_map = HashMap::new();