[pbs-devel] [PATCH-SERIES proxmox/proxmox-backup v3 0/7] trigger sync jobs on mount

[pbs-devel] [PATCH-SERIES proxmox/proxmox-backup v3 0/7] trigger sync jobs on mount

[Hannes Laimer]

Sync jobs now have a run-on-mount flag, that, if set, runs the job whenever
a relevant removable datastore is mounted.

changes since v3, thanks @Chris:
- call run_sync_job endpoint so jobs run on the proxy process,
- use the UPID to keep track of job status
- log on the mount task that we'll run sync jobs
- changed some small things mentioned by @Chris 


proxmox:
Hannes Laimer (1):
  pbs-api-types: add run-on-mount flag to SyncJobConfig

 pbs-api-types/src/jobs.rs | 8 ++++++++
 1 file changed, 8 insertions(+)

proxmox-backup:
Hannes Laimer (6):
  api: config: sync: update run-on-mount correctly
  api: admin: run configured sync jobs when a datastore is mounted
  api: admin: trigger sync jobs only on datastore mount
  bin: manager: run uuid_mount/mount tasks on the proxy
  ui: add run-on-mount checkbox to SyncJob form
  ui: add task title for triggering sync jobs

 docs/storage.rst                                   |   4 +-
 pbs-datastore/Cargo.toml                           |   2 -
 pbs-datastore/src/backup_info.rs                   |  55 +++-----
 pbs-datastore/src/datastore.rs                     |  11 --
 .../src/proxmox_restore_daemon/disk.rs             |   2 +-
 src/api2/admin/datastore.rs                        | 138 ++++++++++++++++++---
 src/api2/config/sync.rs                            |   9 ++
 src/bin/proxmox_backup_manager/datastore.rs        |  41 ++++--
 www/Utils.js                                       |   1 +
 www/window/SyncJobEdit.js                          |  13 +-
 10 files changed, 195 insertions(+), 81 deletions(-)

-- 
2.39.5


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox v3 1/7] pbs-api-types: add run-on-mount flag to SyncJobConfig

[Hannes Laimer]

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 pbs-api-types/src/jobs.rs | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/pbs-api-types/src/jobs.rs b/pbs-api-types/src/jobs.rs
index 6ef13dc2..3eb61cde 100644
--- a/pbs-api-types/src/jobs.rs
+++ b/pbs-api-types/src/jobs.rs
@@ -536,6 +536,8 @@ pub const SYNC_ENCRYPTED_ONLY_SCHEMA: Schema =
     BooleanSchema::new("Only synchronize encrypted backup snapshots, exclude others.").schema();
 pub const SYNC_VERIFIED_ONLY_SCHEMA: Schema =
     BooleanSchema::new("Only synchronize verified backup snapshots, exclude others.").schema();
+pub const RUN_SYNC_ON_MOUNT_SCHEMA: Schema =
+    BooleanSchema::new("Run this job when a relevant datastore is mounted.").schema();
 
 #[api(
     properties: {
@@ -603,6 +605,10 @@ pub const SYNC_VERIFIED_ONLY_SCHEMA: Schema =
             schema: SYNC_VERIFIED_ONLY_SCHEMA,
             optional: true,
         },
+        "run-on-mount": {
+            schema: RUN_SYNC_ON_MOUNT_SCHEMA,
+            optional: true,
+        },
         "sync-direction": {
             type: SyncDirection,
             optional: true,
@@ -647,6 +653,8 @@ pub struct SyncJobConfig {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub verified_only: Option<bool>,
     #[serde(skip_serializing_if = "Option::is_none")]
+    pub run_on_mount: Option<bool>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub sync_direction: Option<SyncDirection>,
 }
 
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup v3 2/7] api: config: sync: update run-on-mount correctly

[Hannes Laimer]

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/config/sync.rs | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
index 6194d865..358409b5 100644
--- a/src/api2/config/sync.rs
+++ b/src/api2/config/sync.rs
@@ -339,6 +339,8 @@ pub enum DeletableProperty {
     EncryptedOnly,
     /// Delete the verified_only property,
     VerifiedOnly,
+    /// Delete the run_on_mount property,
+    RunOnMount,
     /// Delete the sync_direction property,
     SyncDirection,
 }
@@ -458,6 +460,9 @@ pub fn update_sync_job(
                 DeletableProperty::VerifiedOnly => {
                     data.verified_only = None;
                 }
+                DeletableProperty::RunOnMount => {
+                    data.run_on_mount = None;
+                }
                 DeletableProperty::SyncDirection => {
                     data.sync_direction = None;
                 }
@@ -507,6 +512,9 @@ pub fn update_sync_job(
     if let Some(verified_only) = update.verified_only {
         data.verified_only = Some(verified_only);
     }
+    if let Some(run_on_mount) = update.run_on_mount {
+        data.run_on_mount = Some(run_on_mount);
+    }
     if let Some(sync_direction) = update.sync_direction {
         data.sync_direction = Some(sync_direction);
     }
@@ -683,6 +691,7 @@ acl:1:/remote/remote1/remotestore1:write@pbs:RemoteSyncOperator
         transfer_last: None,
         encrypted_only: None,
         verified_only: None,
+        run_on_mount: None,
         sync_direction: None, // use default
     };
 
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup v3 3/7] api: admin: run configured sync jobs when a datastore is mounted

[Hannes Laimer]

When a datastore is mounted, spawn a new task to run all sync jobs
marked with `run-on-mount`. These jobs run sequentially and include
any job for which the mounted datastore is:

- The source or target in a local push/pull job
- The source in a push job to a remote datastore
- The target in a pull job from a remote datastore

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/admin/datastore.rs | 106 ++++++++++++++++++++++++++++++++++--
 1 file changed, 100 insertions(+), 6 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 39249448..68bb2a1f 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -42,8 +42,8 @@ use pbs_api_types::{
     DataStoreConfig, DataStoreListItem, DataStoreMountStatus, DataStoreStatus,
     GarbageCollectionJobStatus, GroupListItem, JobScheduleStatus, KeepOptions, MaintenanceMode,
     MaintenanceType, Operation, PruneJobOptions, SnapshotListItem, SnapshotVerifyState,
-    BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA, BACKUP_TIME_SCHEMA,
-    BACKUP_TYPE_SCHEMA, CATALOG_NAME, CLIENT_LOG_BLOB_NAME, DATASTORE_SCHEMA,
+    SyncJobConfig, BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA,
+    BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, CATALOG_NAME, CLIENT_LOG_BLOB_NAME, DATASTORE_SCHEMA,
     IGNORE_VERIFIED_BACKUPS_SCHEMA, MANIFEST_BLOB_NAME, MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_SCHEMA,
     PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE,
     PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, PRIV_SYS_MODIFY, UPID, UPID_SCHEMA,
@@ -66,7 +66,7 @@ use pbs_datastore::{
     DataStore, LocalChunkReader, StoreProgress,
 };
 use pbs_tools::json::required_string_param;
-use proxmox_rest_server::{formatter, WorkerTask};
+use proxmox_rest_server::{formatter, worker_is_active, WorkerTask};
 
 use crate::api2::backup::optional_ns_param;
 use crate::api2::node::rrd::create_value_from_rrd;
@@ -2510,6 +2510,63 @@ pub fn do_mount_device(datastore: DataStoreConfig) -> Result<(), Error> {
     Ok(())
 }
 
+async fn do_sync_jobs(
+    jobs_to_run: Vec<SyncJobConfig>,
+    worker: Arc<WorkerTask>,
+) -> Result<(), Error> {
+    let count = jobs_to_run.len();
+    info!(
+        "will run {} sync jobs: {}",
+        count,
+        jobs_to_run
+            .iter()
+            .map(|job| job.id.as_str())
+            .collect::<Vec<&str>>()
+            .join(", ")
+    );
+
+    for (i, job_config) in jobs_to_run.into_iter().enumerate() {
+        if worker.abort_requested() {
+            bail!("aborted due to user request");
+        }
+        let job_id = &job_config.id;
+        let client = crate::client_helpers::connect_to_localhost()?;
+        let Ok(result) = client
+            .post(format!("api2/json/admin/sync/{job_id}/run").as_str(), None)
+            .await
+        else {
+            warn!("unable to start sync job {job_id}");
+            continue;
+        };
+        info!("[{}/{count}] starting '{job_id}'...", i + 1);
+        let Some(upid_str) = &result["data"].as_str() else {
+            warn!(
+                "could not recieve UPID of started job (may be runnig, just can't track it here)"
+            );
+            continue;
+        };
+        let upid: UPID = upid_str.parse()?;
+
+        let sleep_duration = core::time::Duration::new(1, 0);
+        let mut status_retries = 1;
+        loop {
+            if worker.abort_requested() {
+                bail!("aborted due to user request, already started job will finish");
+            }
+            match worker_is_active(&upid).await {
+                Ok(true) => tokio::time::sleep(sleep_duration).await,
+                Ok(false) => break,
+                Err(_) if status_retries > 3 => break,
+                Err(err) => {
+                    warn!("could not get job status: {err} ({}/3)", status_retries);
+                    status_retries += 1;
+                }
+            }
+        }
+    }
+    Ok(())
+}
+
 #[api(
     protected: true,
     input: {
@@ -2541,12 +2598,49 @@ pub fn mount(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Er
     let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
     let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
 
-    let upid = WorkerTask::new_thread(
+    let upid = WorkerTask::spawn(
         "mount-device",
-        Some(store),
+        Some(store.clone()),
         auth_id.to_string(),
         to_stdout,
-        move |_worker| do_mount_device(datastore),
+        move |_worker| async move {
+            do_mount_device(datastore.clone())?;
+            let Ok((sync_config, _digest)) = pbs_config::sync::config() else {
+                warn!("unable to read sync job config, won't run any sync jobs");
+                return Ok(());
+            };
+            let Ok(list) = sync_config.convert_to_typed_array("sync") else {
+                warn!("unable to parse sync job config, won't run any sync jobs");
+                return Ok(());
+            };
+            let jobs_to_run: Vec<SyncJobConfig> = list
+                .into_iter()
+                .filter(|job: &SyncJobConfig| {
+                    // add job iff (running on mount is enabled and) any of these apply
+                    //   - the jobs is local and we are source or target
+                    //   - we are the source of a push to a remote
+                    //   - we are the target of a pull from a remote
+                    //
+                    // `job.store == datastore.name` iff we are the target for pull from remote or we
+                    // are the source for push to remote, therefore we don't have to check for the
+                    // direction of the job.
+                    job.run_on_mount.unwrap_or(false)
+                        && (job.remote.is_none() && job.remote_store == datastore.name
+                            || job.store == datastore.name)
+                })
+                .collect();
+            if !jobs_to_run.is_empty() {
+                info!("starting {} sync jobs", jobs_to_run.len());
+                let _ = WorkerTask::spawn(
+                    "mount-sync-jobs",
+                    Some(store),
+                    auth_id.to_string(),
+                    false,
+                    move |worker| async move { do_sync_jobs(jobs_to_run, worker).await },
+                );
+            }
+            Ok(())
+        },
     )?;
 
     Ok(json!(upid))
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup v3 3/7] api: admin: run configured sync jobs when a datastore is mounted

[Christian Ebner]

a few comments inline.

On 6/4/25 14:30, Hannes Laimer wrote:
> When a datastore is mounted, spawn a new task to run all sync jobs
> marked with `run-on-mount`. These jobs run sequentially and include
> any job for which the mounted datastore is:
> 
> - The source or target in a local push/pull job
> - The source in a push job to a remote datastore
> - The target in a pull job from a remote datastore
> 
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>   src/api2/admin/datastore.rs | 106 ++++++++++++++++++++++++++++++++++--
>   1 file changed, 100 insertions(+), 6 deletions(-)
> 
> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> index 39249448..68bb2a1f 100644
> --- a/src/api2/admin/datastore.rs
> +++ b/src/api2/admin/datastore.rs
> @@ -42,8 +42,8 @@ use pbs_api_types::{
>       DataStoreConfig, DataStoreListItem, DataStoreMountStatus, DataStoreStatus,
>       GarbageCollectionJobStatus, GroupListItem, JobScheduleStatus, KeepOptions, MaintenanceMode,
>       MaintenanceType, Operation, PruneJobOptions, SnapshotListItem, SnapshotVerifyState,
> -    BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA, BACKUP_TIME_SCHEMA,
> -    BACKUP_TYPE_SCHEMA, CATALOG_NAME, CLIENT_LOG_BLOB_NAME, DATASTORE_SCHEMA,
> +    SyncJobConfig, BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA,
> +    BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, CATALOG_NAME, CLIENT_LOG_BLOB_NAME, DATASTORE_SCHEMA,
>       IGNORE_VERIFIED_BACKUPS_SCHEMA, MANIFEST_BLOB_NAME, MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_SCHEMA,
>       PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE,
>       PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, PRIV_SYS_MODIFY, UPID, UPID_SCHEMA,
> @@ -66,7 +66,7 @@ use pbs_datastore::{
>       DataStore, LocalChunkReader, StoreProgress,
>   };
>   use pbs_tools::json::required_string_param;
> -use proxmox_rest_server::{formatter, WorkerTask};
> +use proxmox_rest_server::{formatter, worker_is_active, WorkerTask};
>   
>   use crate::api2::backup::optional_ns_param;
>   use crate::api2::node::rrd::create_value_from_rrd;
> @@ -2510,6 +2510,63 @@ pub fn do_mount_device(datastore: DataStoreConfig) -> Result<(), Error> {
>       Ok(())
>   }
>   
> +async fn do_sync_jobs(
> +    jobs_to_run: Vec<SyncJobConfig>,
> +    worker: Arc<WorkerTask>,
> +) -> Result<(), Error> {
> +    let count = jobs_to_run.len();
> +    info!(
> +        "will run {} sync jobs: {}",
> +        count,

nit, `count` can be in-lined

> +        jobs_to_run
> +            .iter()
> +            .map(|job| job.id.as_str())
> +            .collect::<Vec<&str>>()
> +            .join(", ")
> +    );
> +
> +    for (i, job_config) in jobs_to_run.into_iter().enumerate() {
> +        if worker.abort_requested() {
> +            bail!("aborted due to user request");
> +        }
> +        let job_id = &job_config.id;
> +        let client = crate::client_helpers::connect_to_localhost()?;

nit: Adding some error context here would be good so it is clear that an 
eventual connection error stems from the client.

nit: Further, the whole client instantiation can be moved outside of the 
sync jobs loop, so it does not need to be reconstructed for each loop.

> +        let Ok(result) = client
> +            .post(format!("api2/json/admin/sync/{job_id}/run").as_str(), None)
> +            .await
> +        else {
> +            warn!("unable to start sync job {job_id}");
> +            continue;
> +        };
> +        info!("[{}/{count}] starting '{job_id}'...", i + 1);
> +        let Some(upid_str) = &result["data"].as_str() else {

nit: there is no need for the dereference, as_str() already takes care 
of that

> +            warn!(
> +                "could not recieve UPID of started job (may be runnig, just can't track it here)"

nit: 2 typos: should be `receive` and `running`

> +            );
> +            continue;
> +        };
> +        let upid: UPID = upid_str.parse()?;
> +
> +        let sleep_duration = core::time::Duration::new(1, 0);

nit: this could use `Duration::from_secs(1)` instead, which inherently 
documents the arguments timeunit scale

> +        let mut status_retries = 1;
> +        loop {
> +            if worker.abort_requested() {
> +                bail!("aborted due to user request, already started job will finish");
> +            }
> +            match worker_is_active(&upid).await {
> +                Ok(true) => tokio::time::sleep(sleep_duration).await,
> +                Ok(false) => break,
> +                Err(_) if status_retries > 3 => break,
> +                Err(err) => {
> +                    warn!("could not get job status: {err} ({}/3)", status_retries);

nit: the max retry count could be defined as constant and the same 
constant used also for the warning output, to guarantee consistency. 
Further, `status_retries` can be in-lined.

> +                    status_retries += 1;
> +                }
> +            }
> +        }
> +    }
> +    Ok(())
> +}
> +
>   #[api(
>       protected: true,
>       input: {
> @@ -2541,12 +2598,49 @@ pub fn mount(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Er
>       let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
>       let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
>   
> -    let upid = WorkerTask::new_thread(
> +    let upid = WorkerTask::spawn(

question: already mentioned last time, but still not sure if I 
understand why spawn() instead of new_thread() is okay here. After all 
this would execute the whole mount code on the current thread, and 
potentially block that? So shouldn't do_mount_device() be non-blocking 
for this change to be okay?

>           "mount-device",
> -        Some(store),
> +        Some(store.clone()),
>           auth_id.to_string(),
>           to_stdout,
> -        move |_worker| do_mount_device(datastore),
> +        move |_worker| async move {
> +            do_mount_device(datastore.clone())?;
> +            let Ok((sync_config, _digest)) = pbs_config::sync::config() else {
> +                warn!("unable to read sync job config, won't run any sync jobs");
> +                return Ok(());
> +            };
> +            let Ok(list) = sync_config.convert_to_typed_array("sync") else {
> +                warn!("unable to parse sync job config, won't run any sync jobs");
> +                return Ok(());
> +            };
> +            let jobs_to_run: Vec<SyncJobConfig> = list
> +                .into_iter()
> +                .filter(|job: &SyncJobConfig| {
> +                    // add job iff (running on mount is enabled and) any of these apply
> +                    //   - the jobs is local and we are source or target
> +                    //   - we are the source of a push to a remote
> +                    //   - we are the target of a pull from a remote
> +                    //
> +                    // `job.store == datastore.name` iff we are the target for pull from remote or we
> +                    // are the source for push to remote, therefore we don't have to check for the
> +                    // direction of the job.
> +                    job.run_on_mount.unwrap_or(false)
> +                        && (job.remote.is_none() && job.remote_store == datastore.name
> +                            || job.store == datastore.name)
> +                })
> +                .collect();

note: it would make sense to sort the list of sync jobs by their sync id 
here, so that the execution ordering can be inferred. This should be 
documented in the user documentation as well.

> +            if !jobs_to_run.is_empty() {
> +                info!("starting {} sync jobs", jobs_to_run.len());
> +                let _ = WorkerTask::spawn(
> +                    "mount-sync-jobs",
> +                    Some(store),
> +                    auth_id.to_string(),
> +                    false,
> +                    move |worker| async move { do_sync_jobs(jobs_to_run, worker).await },
> +                );
> +            }
> +            Ok(())
> +        },
>       )?;
>   
>       Ok(json!(upid))



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup v3 4/7] api: admin: trigger sync jobs only on datastore mount

[Hannes Laimer]

Ensure sync jobs are triggered only when the datastore is actually
mounted. If the datastore is already mounted, we don't fail,
but sync jobs should not be re-triggered unnecessarily. This change
prevents redundant sync job execution.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/admin/datastore.rs | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 68bb2a1f..3f4957c0 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -2455,14 +2455,14 @@ fn setup_mounted_device(datastore: &DataStoreConfig, tmp_mount_path: &str) -> Re
 /// The reason for the randomized device mounting paths is to avoid two tasks trying to mount to
 /// the same path, this is *very* unlikely since the device is only mounted really shortly, but
 /// technically possible.
-pub fn do_mount_device(datastore: DataStoreConfig) -> Result<(), Error> {
+pub fn do_mount_device(datastore: DataStoreConfig) -> Result<bool, Error> {
     if let Some(uuid) = datastore.backing_device.as_ref() {
         if pbs_datastore::get_datastore_mount_status(&datastore) == Some(true) {
             info!(
                 "device is already mounted at '{}'",
                 datastore.absolute_path()
             );
-            return Ok(());
+            return Ok(false);
         }
         let tmp_mount_path = format!(
             "{}/{:x}",
@@ -2507,7 +2507,7 @@ pub fn do_mount_device(datastore: DataStoreConfig) -> Result<(), Error> {
             datastore.name
         )
     }
-    Ok(())
+    Ok(true)
 }
 
 async fn do_sync_jobs(
@@ -2604,7 +2604,9 @@ pub fn mount(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Er
         auth_id.to_string(),
         to_stdout,
         move |_worker| async move {
-            do_mount_device(datastore.clone())?;
+            if !do_mount_device(datastore.clone())? {
+                return Ok(());
+            }
             let Ok((sync_config, _digest)) = pbs_config::sync::config() else {
                 warn!("unable to read sync job config, won't run any sync jobs");
                 return Ok(());
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup v3 5/7] bin: manager: run uuid_mount/mount tasks on the proxy

[Hannes Laimer]

Use the API instead of running uuid_mount/mount directly in the CLI binary.
This ensures that all triggered tasks are handled by the proxy process.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/bin/proxmox_backup_manager/datastore.rs | 41 ++++++++++++++++-----
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/src/bin/proxmox_backup_manager/datastore.rs b/src/bin/proxmox_backup_manager/datastore.rs
index 1922a55a..3fbb5fe5 100644
--- a/src/bin/proxmox_backup_manager/datastore.rs
+++ b/src/bin/proxmox_backup_manager/datastore.rs
@@ -49,6 +49,10 @@ fn list_datastores(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Valu
             store: {
                 schema: DATASTORE_SCHEMA,
             },
+            "output-format": {
+                schema: OUTPUT_FORMAT,
+                optional: true,
+            },
             digest: {
                 optional: true,
                 schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
@@ -57,16 +61,23 @@ fn list_datastores(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Valu
     },
 )]
 /// Mount a removable datastore.
-async fn mount_datastore(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
-    param["node"] = "localhost".into();
+async fn mount_datastore(
+    store: String,
+    mut param: Value,
+    _rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+    let output_format = extract_output_format(&mut param);
 
-    let info = &api2::admin::datastore::API_METHOD_MOUNT;
-    let result = match info.handler {
-        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
-        _ => unreachable!(),
-    };
+    let client = connect_to_localhost()?;
+    let result = client
+        .post(
+            format!("api2/json/admin/datastore/{store}/mount").as_str(),
+            None,
+        )
+        .await?;
+
+    view_task_result(&client, result, &output_format).await?;
 
-    crate::wait_for_local_worker(result.as_str().unwrap()).await?;
     Ok(())
 }
 
@@ -260,7 +271,8 @@ async fn update_datastore(name: String, mut param: Value) -> Result<(), Error> {
     },
 )]
 /// Try mounting a removable datastore given the UUID.
-async fn uuid_mount(param: Value, _rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+async fn uuid_mount(mut param: Value, _rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+    let output_format = extract_output_format(&mut param);
     let uuid = param["uuid"]
         .as_str()
         .ok_or_else(|| format_err!("uuid has to be specified"))?;
@@ -282,7 +294,16 @@ async fn uuid_mount(param: Value, _rpcenv: &mut dyn RpcEnvironment) -> Result<Va
     }
 
     if let Some(store) = matching_stores.first() {
-        api2::admin::datastore::do_mount_device(store.clone())?;
+        let client = connect_to_localhost()?;
+        let result = client
+            .post(
+                format!("api2/json/admin/datastore/{}/mount", store.name).as_str(),
+                None,
+            )
+            .await?;
+
+        view_task_result(&client, result, &output_format).await?;
+        return Ok(Value::Null);
     }
 
     // we don't want to fail for UUIDs that are not associated with datastores, as that produces
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup v3 6/7] ui: add run-on-mount checkbox to SyncJob form

[Hannes Laimer]

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/window/SyncJobEdit.js | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/www/window/SyncJobEdit.js b/www/window/SyncJobEdit.js
index 4cef9a1d..73a489f1 100644
--- a/www/window/SyncJobEdit.js
+++ b/www/window/SyncJobEdit.js
@@ -195,7 +195,7 @@ Ext.define('PBS.window.SyncJobEdit', {
 			xtype: 'pbsCalendarEvent',
 			name: 'schedule',
 			fieldLabel: gettext('Sync Schedule'),
-			emptyText: gettext('none (disabled)'),
+			emptyText: gettext('none'),
 			cbind: {
 			    deleteEmpty: '{!isCreate}',
 			    value: '{scheduleValue}',
@@ -451,6 +451,17 @@ Ext.define('PBS.window.SyncJobEdit', {
 			uncheckedValue: false,
 			value: false,
 		    },
+		    {
+			xtype: 'proxmoxcheckbox',
+			name: 'run-on-mount',
+			fieldLabel: gettext('Run when mounted'),
+			autoEl: {
+			    tag: 'div',
+			    'data-qtip': gettext('Run this job when a relevant removable datastore is mounted.'),
+			},
+			uncheckedValue: false,
+			value: false,
+		    },
 		],
 	    },
 	    {
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup v3 6/7] ui: add run-on-mount checkbox to SyncJob form

[Christian Ebner]

On 6/4/25 14:30, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>   www/window/SyncJobEdit.js | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/www/window/SyncJobEdit.js b/www/window/SyncJobEdit.js
> index 4cef9a1d..73a489f1 100644
> --- a/www/window/SyncJobEdit.js
> +++ b/www/window/SyncJobEdit.js
> @@ -195,7 +195,7 @@ Ext.define('PBS.window.SyncJobEdit', {
>   			xtype: 'pbsCalendarEvent',
>   			name: 'schedule',
>   			fieldLabel: gettext('Sync Schedule'),
> -			emptyText: gettext('none (disabled)'),
> +			emptyText: gettext('none'),
>   			cbind: {
>   			    deleteEmpty: '{!isCreate}',
>   			    value: '{scheduleValue}',
> @@ -451,6 +451,17 @@ Ext.define('PBS.window.SyncJobEdit', {
>   			uncheckedValue: false,
>   			value: false,
>   		    },
> +		    {
> +			xtype: 'proxmoxcheckbox',
> +			name: 'run-on-mount',
> +			fieldLabel: gettext('Run when mounted'),
> +			autoEl: {
> +			    tag: 'div',
> +			    'data-qtip': gettext('Run this job when a relevant removable datastore is mounted.'),
> +			},
> +			uncheckedValue: false,
> +			value: false,
> +		    },
>   		],
>   	    },
>   	    {

nit: pre-existing for other boolean flags here as well, but this should 
make sure that it is set as `delete` value if set to it's default. 
Otherwise this is not removed from the sync job config, but set to the 
default instead. Removing it will help reduce the size of the config and 
improve it's readability.


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup v3 7/7] ui: add task title for triggering sync jobs

[Hannes Laimer]

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/Utils.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/www/Utils.js b/www/Utils.js
index 9dcde694..eb494301 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -422,6 +422,7 @@ Ext.define('PBS.Utils', {
 	    prunejob: (type, id) => PBS.Utils.render_prune_job_worker_id(id, gettext('Prune Job')),
 	    reader: (type, id) => PBS.Utils.render_datastore_worker_id(id, gettext('Read Objects')),
 	    'rewind-media': [gettext('Drive'), gettext('Rewind Media')],
+	    'mount-sync-jobs': [gettext('Datastore'), gettext('trigger sync jobs')],
 	    sync: ['Datastore', gettext('Remote Sync')],
 	    syncjob: [gettext('Sync Job'), gettext('Remote Sync')],
 	    'tape-backup': (type, id) => PBS.Utils.render_tape_backup_id(id, gettext('Tape Backup')),
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup v3 7/7] ui: add task title for triggering sync jobs

[Christian Ebner]

On 6/4/25 14:30, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>   www/Utils.js | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/www/Utils.js b/www/Utils.js
> index 9dcde694..eb494301 100644
> --- a/www/Utils.js
> +++ b/www/Utils.js
> @@ -422,6 +422,7 @@ Ext.define('PBS.Utils', {
>   	    prunejob: (type, id) => PBS.Utils.render_prune_job_worker_id(id, gettext('Prune Job')),
>   	    reader: (type, id) => PBS.Utils.render_datastore_worker_id(id, gettext('Read Objects')),
>   	    'rewind-media': [gettext('Drive'), gettext('Rewind Media')],
> +	    'mount-sync-jobs': [gettext('Datastore'), gettext('trigger sync jobs')],

nit: while not an issue, the text here is a bit unfitting, as this job 
will complete *after* all the sync jobs it's executing.
Therefore, this shows up in the task log after the sync jobs entries. So 
maybe instead of `trigger sync jobs`, this could state `sync jobs 
handler triggered by mount`... or something even better fitting...


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH-SERIES proxmox/proxmox-backup v3 0/7] trigger sync jobs on mount

[Christian Ebner]

On 6/4/25 14:30, Hannes Laimer wrote:
> Sync jobs now have a run-on-mount flag, that, if set, runs the job whenever
> a relevant removable datastore is mounted.

This is already in good shape, only some smaller nits and smaller 
comments. Also, this needs to be re-based onto current master and 
reformatted via proxmox-biome.

With these addressed, please consider:

Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
Tested-by: Christian Ebner <c.ebner@proxmox.com>


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel