* [pbs-devel] [PATCH proxmox-backup 1/2] backup/snapshot reader: improve error message for ENOENT
@ 2023-02-02 13:04 Fabian Grünbichler
2023-02-02 13:04 ` [pbs-devel] [PATCH proxmox-backup 2/2] verify/protect: improve error on disappearing snapshots Fabian Grünbichler
2023-02-08 14:22 ` [pbs-devel] applied-series: [PATCH proxmox-backup 1/2] backup/snapshot reader: improve error message for ENOENT Wolfgang Bumiller
0 siblings, 2 replies; 3+ messages in thread
From: Fabian Grünbichler @ 2023-02-02 13:04 UTC (permalink / raw)
To: pbs-devel
instead of
Error: unable to open snapshot directory "/full/path/to/snapshot" for locking - ENOENT: No such file or directory
this will now print
Error: Snapshot vm/800/2023-01-16T12:28:11Z does not exist.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
Notes:
we could also include the datastore/namespace, although we are not that
consistent in general in that regard, and the caller should technically
know since they provided it ;).
pbs-datastore/src/snapshot_reader.rs | 4 ++++
src/api2/reader/mod.rs | 4 ++++
2 files changed, 8 insertions(+)
diff --git a/pbs-datastore/src/snapshot_reader.rs b/pbs-datastore/src/snapshot_reader.rs
index 899c3bce..ec7a48e5 100644
--- a/pbs-datastore/src/snapshot_reader.rs
+++ b/pbs-datastore/src/snapshot_reader.rs
@@ -41,6 +41,10 @@ impl SnapshotReader {
let datastore = snapshot.datastore();
let snapshot_path = snapshot.full_path();
+ if !snapshot_path.exists() {
+ bail!("snapshot {} does not exist!", snapshot.dir());
+ }
+
let locked_dir =
lock_dir_noblock_shared(&snapshot_path, "snapshot", "locked by another operation")?;
diff --git a/src/api2/reader/mod.rs b/src/api2/reader/mod.rs
index e2a10da3..1ac4ac40 100644
--- a/src/api2/reader/mod.rs
+++ b/src/api2/reader/mod.rs
@@ -125,6 +125,10 @@ fn upgrade_to_backup_reader_protocol(
}
}
+ if !backup_dir.full_path().exists() {
+ bail!("snapshot {} does not exist.", backup_dir.dir());
+ }
+
let _guard = lock_dir_noblock_shared(
&backup_dir.full_path(),
"snapshot",
--
2.30.2
^ permalink raw reply [flat|nested] 3+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 2/2] verify/protect: improve error on disappearing snapshots
2023-02-02 13:04 [pbs-devel] [PATCH proxmox-backup 1/2] backup/snapshot reader: improve error message for ENOENT Fabian Grünbichler
@ 2023-02-02 13:04 ` Fabian Grünbichler
2023-02-08 14:22 ` [pbs-devel] applied-series: [PATCH proxmox-backup 1/2] backup/snapshot reader: improve error message for ENOENT Wolfgang Bumiller
1 sibling, 0 replies; 3+ messages in thread
From: Fabian Grünbichler @ 2023-02-02 13:04 UTC (permalink / raw)
To: pbs-devel
or clients passing in a non-existent snapshot.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
pbs-datastore/src/datastore.rs | 4 ++++
src/backup/verify.rs | 10 ++++++++++
2 files changed, 14 insertions(+)
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 6f37e886..bf951305 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -1180,6 +1180,10 @@ impl DataStore {
pub fn update_protection(&self, backup_dir: &BackupDir, protection: bool) -> Result<(), Error> {
let full_path = backup_dir.full_path();
+ if !full_path.exists() {
+ bail!("snapshot {} does not exist!", backup_dir.dir());
+ }
+
let _guard = lock_dir_noblock(&full_path, "snapshot", "possibly running or in use")?;
let protected_path = backup_dir.protected_file();
diff --git a/src/backup/verify.rs b/src/backup/verify.rs
index 3984e28d..c972e532 100644
--- a/src/backup/verify.rs
+++ b/src/backup/verify.rs
@@ -328,6 +328,16 @@ pub fn verify_backup_dir(
upid: UPID,
filter: Option<&dyn Fn(&BackupManifest) -> bool>,
) -> Result<bool, Error> {
+ if !backup_dir.full_path().exists() {
+ task_log!(
+ verify_worker.worker,
+ "SKIPPED: verify {}:{} - snapshot does not exist (anymore).",
+ verify_worker.datastore.name(),
+ backup_dir.dir(),
+ );
+ return Ok(true);
+ }
+
let snap_lock = lock_dir_noblock_shared(
&backup_dir.full_path(),
"snapshot",
--
2.30.2
^ permalink raw reply [flat|nested] 3+ messages in thread
* [pbs-devel] applied-series: [PATCH proxmox-backup 1/2] backup/snapshot reader: improve error message for ENOENT
2023-02-02 13:04 [pbs-devel] [PATCH proxmox-backup 1/2] backup/snapshot reader: improve error message for ENOENT Fabian Grünbichler
2023-02-02 13:04 ` [pbs-devel] [PATCH proxmox-backup 2/2] verify/protect: improve error on disappearing snapshots Fabian Grünbichler
@ 2023-02-08 14:22 ` Wolfgang Bumiller
1 sibling, 0 replies; 3+ messages in thread
From: Wolfgang Bumiller @ 2023-02-08 14:22 UTC (permalink / raw)
To: Fabian Grünbichler; +Cc: pbs-devel
While I really do not like `.exists()` on a path that is used right
afterwards and our "helpers" there need to switch to `io::Error` instead
of useless `anyhow::Error` strings...
expecting our locking to change and the paths don't matching up
afterwards this series should be fine this way, too.
applied, thanks
On Thu, Feb 02, 2023 at 02:04:23PM +0100, Fabian Grünbichler wrote:
> instead of
>
> Error: unable to open snapshot directory "/full/path/to/snapshot" for locking - ENOENT: No such file or directory
>
> this will now print
>
> Error: Snapshot vm/800/2023-01-16T12:28:11Z does not exist.
>
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
>
> Notes:
> we could also include the datastore/namespace, although we are not that
> consistent in general in that regard, and the caller should technically
> know since they provided it ;).
>
> pbs-datastore/src/snapshot_reader.rs | 4 ++++
> src/api2/reader/mod.rs | 4 ++++
> 2 files changed, 8 insertions(+)
>
> diff --git a/pbs-datastore/src/snapshot_reader.rs b/pbs-datastore/src/snapshot_reader.rs
> index 899c3bce..ec7a48e5 100644
> --- a/pbs-datastore/src/snapshot_reader.rs
> +++ b/pbs-datastore/src/snapshot_reader.rs
> @@ -41,6 +41,10 @@ impl SnapshotReader {
> let datastore = snapshot.datastore();
> let snapshot_path = snapshot.full_path();
>
> + if !snapshot_path.exists() {
> + bail!("snapshot {} does not exist!", snapshot.dir());
> + }
> +
> let locked_dir =
> lock_dir_noblock_shared(&snapshot_path, "snapshot", "locked by another operation")?;
>
> diff --git a/src/api2/reader/mod.rs b/src/api2/reader/mod.rs
> index e2a10da3..1ac4ac40 100644
> --- a/src/api2/reader/mod.rs
> +++ b/src/api2/reader/mod.rs
> @@ -125,6 +125,10 @@ fn upgrade_to_backup_reader_protocol(
> }
> }
>
> + if !backup_dir.full_path().exists() {
> + bail!("snapshot {} does not exist.", backup_dir.dir());
> + }
> +
> let _guard = lock_dir_noblock_shared(
> &backup_dir.full_path(),
> "snapshot",
> --
> 2.30.2
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-02-08 14:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-02 13:04 [pbs-devel] [PATCH proxmox-backup 1/2] backup/snapshot reader: improve error message for ENOENT Fabian Grünbichler
2023-02-02 13:04 ` [pbs-devel] [PATCH proxmox-backup 2/2] verify/protect: improve error on disappearing snapshots Fabian Grünbichler
2023-02-08 14:22 ` [pbs-devel] applied-series: [PATCH proxmox-backup 1/2] backup/snapshot reader: improve error message for ENOENT Wolfgang Bumiller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox