[pve-devel] [PATCH proxmox-offline-mirror] Add possibility to create named snapshots

Laurențiu Leahu-Vlăducu l.leahu-vladucu at proxmox.com
Tue May 6 11:09:26 CEST 2025


In addition to the existing timed snapshots, it is now possible to
create named snapshots. For some use cases, it makes sense to give
snapshots a specific name, e.g. to specify the purpose of the snapshot
(instead of a timestamp). This can be useful when deploying snapshots
created for a certain purpose (for example, further testing).

Partially fixes #6252

Signed-off-by: Laurențiu Leahu-Vlăducu <l.leahu-vladucu at proxmox.com>
---
 src/bin/proxmox-offline-mirror-helper.rs      |  6 +-
 src/bin/proxmox_offline_mirror_cmds/mirror.rs | 32 ++++++++---
 src/medium.rs                                 |  4 +-
 src/mirror.rs                                 | 56 +++++++++++++++----
 src/types.rs                                  | 52 +++++++++++++----
 5 files changed, 117 insertions(+), 33 deletions(-)

diff --git a/src/bin/proxmox-offline-mirror-helper.rs b/src/bin/proxmox-offline-mirror-helper.rs
index 7911d52..a00d26d 100644
--- a/src/bin/proxmox-offline-mirror-helper.rs
+++ b/src/bin/proxmox-offline-mirror-helper.rs
@@ -151,8 +151,8 @@ async fn setup(_param: Value) -> Result<(), Error> {
                 let selected_mirror = read_selection_from_tty("Select mirror", &mirrors, None)?;
                 let snapshots: Vec<(Snapshot, String)> =
                     medium::list_snapshots(mountpoint, selected_mirror)?
-                        .into_iter()
-                        .map(|s| (s, s.to_string()))
+                        .iter()
+                        .map(|s| (s.clone(), s.to_string()))
                         .collect();
                 if snapshots.is_empty() {
                     println!("Mirror doesn't have any synced snapshots.");
@@ -173,7 +173,7 @@ async fn setup(_param: Value) -> Result<(), Error> {
                     selected_mirror.to_string(),
                     (
                         state.mirrors.get(*selected_mirror).unwrap(),
-                        **selected_snapshot,
+                        (**selected_snapshot).clone(),
                     ),
                 );
             }
diff --git a/src/bin/proxmox_offline_mirror_cmds/mirror.rs b/src/bin/proxmox_offline_mirror_cmds/mirror.rs
index 31a565e..7eba04d 100644
--- a/src/bin/proxmox_offline_mirror_cmds/mirror.rs
+++ b/src/bin/proxmox_offline_mirror_cmds/mirror.rs
@@ -17,7 +17,7 @@ use proxmox_schema::api;
 use proxmox_offline_mirror::{
     config::{MirrorConfig, SubscriptionKey},
     mirror,
-    types::{MIRROR_ID_SCHEMA, Snapshot},
+    types::{MIRROR_ID_SCHEMA, NAMED_SNAPSHOT_SCHEMA, Snapshot},
 };
 
 use super::get_config_path;
@@ -60,6 +60,10 @@ fn get_subscription_key(
             id: {
                 schema: MIRROR_ID_SCHEMA,
             },
+            name: {
+                schema: NAMED_SNAPSHOT_SCHEMA,
+                optional: true,
+            },
             "dry-run": {
                 type: bool,
                 optional: true,
@@ -73,6 +77,7 @@ fn get_subscription_key(
 async fn create_snapshot(
     config: Option<String>,
     id: String,
+    name: Option<String>,
     dry_run: bool,
     _param: Value,
 ) -> Result<(), Error> {
@@ -83,12 +88,12 @@ async fn create_snapshot(
 
     let subscription = get_subscription_key(&section_config, &config)?;
 
-    proxmox_offline_mirror::mirror::create_snapshot(
-        config,
-        &Snapshot::now(),
-        subscription,
-        dry_run,
-    )?;
+    let snapshot = match &name {
+        Some(name) => Snapshot::with_name(&name),
+        None => Snapshot::now(),
+    };
+
+    proxmox_offline_mirror::mirror::create_snapshot(config, &snapshot, subscription, dry_run)?;
 
     Ok(())
 }
@@ -101,6 +106,10 @@ async fn create_snapshot(
                 optional: true,
                 description: "Path to mirroring config file.",
             },
+            name: {
+                schema: NAMED_SNAPSHOT_SCHEMA,
+                optional: true,
+            },
             "dry-run": {
                 type: bool,
                 optional: true,
@@ -114,6 +123,7 @@ async fn create_snapshot(
 /// from original repository.
 async fn create_snapshots(
     config: Option<String>,
+    name: Option<String>,
     dry_run: bool,
     _param: Value,
 ) -> Result<(), Error> {
@@ -135,9 +145,15 @@ async fn create_snapshots(
                 continue;
             }
         };
+
+        let snapshot = match &name {
+            Some(name) => Snapshot::with_name(&name),
+            None => Snapshot::now(),
+        };
+
         let res = proxmox_offline_mirror::mirror::create_snapshot(
             mirror,
-            &Snapshot::now(),
+            &snapshot,
             subscription,
             dry_run,
         );
diff --git a/src/medium.rs b/src/medium.rs
index bef6cc7..a2e2ee4 100644
--- a/src/medium.rs
+++ b/src/medium.rs
@@ -18,7 +18,7 @@ use crate::{
     generate_repo_file_line,
     mirror::pool,
     pool::Pool,
-    types::{Diff, SNAPSHOT_REGEX, Snapshot},
+    types::{COMBINED_SNAPSHOT_REGEX, Diff, Snapshot},
 };
 #[derive(Clone, Debug, Serialize, Deserialize)]
 #[serde(rename_all = "kebab-case")]
@@ -162,7 +162,7 @@ pub fn list_snapshots(medium_base: &Path, mirror: &str) -> Result<Vec<Snapshot>,
     proxmox_sys::fs::scandir(
         libc::AT_FDCWD,
         &mirror_base,
-        &SNAPSHOT_REGEX,
+        &COMBINED_SNAPSHOT_REGEX,
         |_l2_fd, snapshot, file_type| {
             if file_type != nix::dir::Type::Directory {
                 return Ok(());
diff --git a/src/mirror.rs b/src/mirror.rs
index c88f81d..2aef691 100644
--- a/src/mirror.rs
+++ b/src/mirror.rs
@@ -11,14 +11,14 @@ use globset::{Glob, GlobSet, GlobSetBuilder};
 use nix::libc;
 use proxmox_http::{HttpClient, HttpOptions, ProxyConfig, client::sync::Client};
 use proxmox_schema::{ApiType, Schema};
-use proxmox_sys::fs::file_get_contents;
+use proxmox_sys::fs::{CreateOptions, file_get_contents};
 
 use crate::{
     FetchResult, Progress,
     config::{MirrorConfig, SkipConfig, SubscriptionKey, WeakCryptoConfig},
     convert_repo_line,
     pool::Pool,
-    types::{Diff, SNAPSHOT_REGEX, Snapshot},
+    types::{COMBINED_SNAPSHOT_REGEX, Diff, Snapshot},
 };
 
 use proxmox_apt::deb822::{
@@ -476,7 +476,7 @@ pub fn list_snapshots(config: &MirrorConfig) -> Result<Vec<Snapshot>, Error> {
     proxmox_sys::fs::scandir(
         libc::AT_FDCWD,
         &path,
-        &SNAPSHOT_REGEX,
+        &COMBINED_SNAPSHOT_REGEX,
         |_l2_fd, snapshot, file_type| {
             if file_type != nix::dir::Type::Directory {
                 return Ok(());
@@ -793,12 +793,6 @@ pub fn create_snapshot(
         None
     };
 
-    let mut config: ParsedMirrorConfig = config.try_into()?;
-    config.auth = auth;
-
-    let prefix = format!("{snapshot}.tmp");
-    let prefix = Path::new(&prefix);
-
     let mut progress = MirrorProgress {
         warnings: Vec::new(),
         skip_count: 0,
@@ -807,6 +801,42 @@ pub fn create_snapshot(
         total: Progress::new(),
     };
 
+    let mut config: ParsedMirrorConfig = config.try_into()?;
+    config.auth = auth;
+
+    let snapshot_relative_path = snapshot.to_string();
+    let snapshot_relative_path = Path::new(&snapshot_relative_path);
+    let snapshot_absolute_path = config.pool.get_path(snapshot_relative_path);
+
+    if snapshot_absolute_path.is_ok_and(|path| path.exists()) {
+        if dry_run {
+            let msg = "WARNING: snapshot {snapshot} already exists! Continuing due to dry run...";
+            eprintln!("{msg}");
+            progress.warnings.push(msg.into());
+        } else {
+            bail!("Snapshot {snapshot} already exists!");
+        }
+    }
+
+    let lockfile_path = config
+        .pool
+        .get_path(Path::new(&format!(".{snapshot}.lock")));
+
+    if let Err(lockfile_err) = lockfile_path {
+        bail!("cannot create lockfile for snapshot {snapshot}: {lockfile_err}");
+    }
+    let lockfile_path = lockfile_path.unwrap();
+
+    let _snapshot_lock = proxmox_sys::fs::open_file_locked(
+        &lockfile_path,
+        std::time::Duration::new(10, 0),
+        true,
+        CreateOptions::default(),
+    )?;
+
+    let prefix = format!("{snapshot}.tmp");
+    let prefix = Path::new(&prefix);
+
     let parse_release = |res: FetchResult, name: &str| -> Result<ReleaseFile, Error> {
         println!("Parsing {name}..");
         let parsed: ReleaseFile = res.data[..].try_into()?;
@@ -1078,9 +1108,15 @@ pub fn create_snapshot(
     if !dry_run {
         println!("\nRotating temp. snapshot in-place: {prefix:?} -> \"{snapshot}\"");
         let locked = config.pool.lock()?;
-        locked.rename(prefix, Path::new(&format!("{snapshot}")))?;
+
+        if snapshot_relative_path.exists() {
+            bail!("Snapshot {snapshot} already exists!");
+        }
+        locked.rename(prefix, snapshot_relative_path)?;
     }
 
+    std::fs::remove_file(lockfile_path)?;
+
     Ok(())
 }
 
diff --git a/src/types.rs b/src/types.rs
index 10dde65..49dc374 100644
--- a/src/types.rs
+++ b/src/types.rs
@@ -1,6 +1,6 @@
 use std::{fmt::Display, path::PathBuf, str::FromStr};
 
-use anyhow::Error;
+use anyhow::{Error, bail};
 use proxmox_schema::{ApiStringFormat, Schema, StringSchema, api, const_regex};
 use proxmox_serde::{forward_deserialize_to_from_str, forward_serialize_to_display};
 use proxmox_time::{epoch_i64, epoch_to_rfc3339_utc, parse_rfc3339};
@@ -32,6 +32,13 @@ pub const MEDIA_ID_SCHEMA: Schema = StringSchema::new("Medium name.")
     .max_length(32)
     .schema();
 
+/// Schema for named snapshots
+pub const NAMED_SNAPSHOT_SCHEMA: Schema = StringSchema::new("Custom name for snapshot.")
+    .format(&PROXMOX_SAFE_ID_FORMAT)
+    .min_length(3)
+    .max_length(32)
+    .schema();
+
 #[rustfmt::skip]
 #[macro_export]
 macro_rules! PROXMOX_SUBSCRIPTION_KEY_REGEX_STR { () => { r"(?:pom-|pve\d+[a-z]-|pbs[a-z]-|pmg[a-z]-).*" }; }
@@ -62,32 +69,51 @@ pub const PROXMOX_SERVER_ID_SCHEMA: Schema = StringSchema::new("Server ID.")
 
 #[rustfmt::skip]
 #[macro_export]
-macro_rules! SNAPSHOT_RE { () => (r"[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z") }
+macro_rules! TIMED_SNAPSHOT_RE { () => (r"[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z") }
 const_regex! {
-    pub(crate) SNAPSHOT_REGEX = concat!(r"^", SNAPSHOT_RE!() ,r"$");
+    pub(crate) TIMED_SNAPSHOT_REGEX = concat!(r"^", TIMED_SNAPSHOT_RE!() ,r"$");
+}
+
+#[rustfmt::skip]
+const_regex! {
+    pub(crate) COMBINED_SNAPSHOT_REGEX = concat!(r"^((", TIMED_SNAPSHOT_RE!() ,r")|", PROXMOX_SAFE_ID_REGEX_STR!(), ")$");
+}
+
+#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord)]
+enum SnapshotType {
+    TimedSnapshot(i64),
+    NamedSnapshot(String),
 }
 
 #[api(
     type: String,
-    format: &ApiStringFormat::Pattern(&SNAPSHOT_REGEX),
 )]
-#[derive(Debug, Clone, Copy, PartialEq, PartialOrd, Eq, Ord)]
+#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord)]
 /// Mirror snapshot
-pub struct Snapshot(i64);
+pub struct Snapshot(SnapshotType);
 
 forward_serialize_to_display!(Snapshot);
 forward_deserialize_to_from_str!(Snapshot);
 
 impl Snapshot {
     pub fn now() -> Self {
-        Self(epoch_i64())
+        Self(SnapshotType::TimedSnapshot(epoch_i64()))
+    }
+
+    pub fn with_name(name: &str) -> Self {
+        Self(SnapshotType::NamedSnapshot(name.into()))
     }
 }
 
 impl Display for Snapshot {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        let formatted = epoch_to_rfc3339_utc(self.0).map_err(|_| std::fmt::Error)?;
-        f.write_str(&formatted)
+        match &self.0 {
+            SnapshotType::TimedSnapshot(time) => {
+                let formatted = epoch_to_rfc3339_utc(*time).map_err(|_| std::fmt::Error)?;
+                f.write_str(&formatted)
+            }
+            SnapshotType::NamedSnapshot(name) => f.write_str(&name),
+        }
     }
 }
 
@@ -95,7 +121,13 @@ impl FromStr for Snapshot {
     type Err = Error;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        Ok(Self(parse_rfc3339(s)?))
+        if TIMED_SNAPSHOT_REGEX.is_match(s) {
+            Ok(Self(SnapshotType::TimedSnapshot(parse_rfc3339(s)?)))
+        } else if PROXMOX_SAFE_ID_REGEX.is_match(s) {
+            Ok(Self(SnapshotType::NamedSnapshot(s.into())))
+        } else {
+            bail!("invalid snapshot name")
+        }
     }
 }
 
-- 
2.39.5





More information about the pve-devel mailing list