drop exclusive lock for verify-after-complete

the backup is finished at that point, the only lock clash that is possible when
dropping the exclusive and attempting to obtain a shared lock would be

- the snapshot is pruned/removed
- the backup is in a pre-upgrade process, and the post-upgrade process opens a reader

the first case is OK, if the other invocation wins the race and removes the
snapshot verification is pointless anyway.

the second case means the snapshot is not verified directly after completion
(this fact would be logged in the backup task log), but usable immediately for
pulling/restoring/..

this should decrease the chances of triggering the issues described in #4523

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
This commit is contained in:
Fabian Grünbichler
2023-02-27 10:50:12 +01:00
committed by Wolfgang Bumiller
parent 20ecaad13b
commit 3df46e018b

View File

@ -7,7 +7,7 @@ use ::serde::Serialize;
use serde_json::{json, Value};
use proxmox_router::{RpcEnvironment, RpcEnvironmentType};
use proxmox_sys::fs::{replace_file, CreateOptions};
use proxmox_sys::fs::{lock_dir_noblock_shared, replace_file, CreateOptions};
use pbs_api_types::Authid;
use pbs_datastore::backup_info::{BackupDir, BackupInfo};
@ -634,7 +634,7 @@ impl BackupEnvironment {
/// If verify-new is set on the datastore, this will run a new verify task
/// for the backup. If not, this will return and also drop the passed lock
/// immediately.
pub fn verify_after_complete(&self, snap_lock: Dir) -> Result<(), Error> {
pub fn verify_after_complete(&self, excl_snap_lock: Dir) -> Result<(), Error> {
self.ensure_finished()?;
if !self.datastore.verify_new() {
@ -642,6 +642,14 @@ impl BackupEnvironment {
return Ok(());
}
// Downgrade to shared lock, the backup itself is finished
drop(excl_snap_lock);
let snap_lock = lock_dir_noblock_shared(
&self.backup_dir.full_path(),
"snapshot",
"snapshot is already locked by another operation",
)?;
let worker_id = format!(
"{}:{}/{}/{:08X}",
self.datastore.name(),