[PATCH proxmox{,-backup} v2 0/3] fixes #7465: add gc-on-unmount flag

[PATCH proxmox{,-backup} v2 0/3] fixes #7465: add gc-on-unmount flag

[Hannes Laimer]

So GC can run before unmount, without this its somewhat complicated to
run gc on a datastore that is configured to auto unmount after jobs
finish.

v2, thanks @Chris!:
 - include proxmox patch
 - don't lock config for just reading
 - improve comments/commit msg


proxmox:

Hannes Laimer (1):
  pbs-api-types: add gc-on-unmount flag for removable datastores

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


proxmox-backup:

Hannes Laimer (2):
  api: datastore: add option to run garbage collection before unmount
  ui: datastore: expose gc-on-unmount setting

 src/api2/admin/datastore.rs  | 23 ++++++++++++++++++++---
 src/api2/config/datastore.rs |  9 +++++++++
 www/datastore/OptionView.js  | 23 +++++++++++++++++++++++
 3 files changed, 52 insertions(+), 3 deletions(-)


Summary over all repositories:
  4 files changed, 61 insertions(+), 3 deletions(-)

-- 
Generated by murpp 0.11.0

[PATCH proxmox v2 1/3] pbs-api-types: add gc-on-unmount flag for removable datastores

[Hannes Laimer]

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

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index 9e85affc..098d2b7c 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -452,6 +452,11 @@ pub const COUNTER_RESET_SCHEDULE_SCHEMA: Schema =
             optional: true,
             schema: GC_SCHEDULE_SCHEMA,
         },
+        "gc-on-unmount": {
+            description: "Run garbage collection before unmounting a removable datastore.",
+            optional: true,
+            type: bool,
+        },
         "prune-schedule": {
             optional: true,
             schema: PRUNE_SCHEDULE_SCHEMA,
@@ -510,6 +515,9 @@ pub struct DataStoreConfig {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub gc_schedule: Option<String>,
 
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub gc_on_unmount: Option<bool>,
+
     #[serde(skip_serializing_if = "Option::is_none")]
     pub prune_schedule: Option<String>,
 
@@ -586,6 +594,7 @@ impl DataStoreConfig {
             path,
             comment: None,
             gc_schedule: None,
+            gc_on_unmount: None,
             prune_schedule: None,
             keep: Default::default(),
             verify_new: None,
-- 
2.47.3

applied: [PATCH proxmox v2 1/3] pbs-api-types: add gc-on-unmount flag for removable datastores

[Thomas Lamprecht]

Am 20.04.26 um 09:41 schrieb Hannes Laimer:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  pbs-api-types/src/datastore.rs | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
>

applied this one already for inclusion in next pbs-api-types bump, thanks!

[PATCH proxmox-backup v2 2/3] api: datastore: add option to run garbage collection before unmount

[Hannes Laimer]

Removable datastores set up for auto-unmount have no natural point at
which to run garbage collection, since the drive is unmounted right
after jobs finish. Expose a gc-on-unmount option so GC can be triggered
as part of the unmount for those setups.

Relies on the active write operation by garbage collection to block
the unmount task until GC completes (among possible other active
operations). No other active operation can occur in the meantime
since the datastore remains in 'unmount' maintenance mode.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/admin/datastore.rs  | 23 ++++++++++++++++++++---
 src/api2/config/datastore.rs |  9 +++++++++
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 757b3114..212a48da 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -2641,9 +2641,8 @@ fn do_unmount_device(
 }
 
 async fn do_unmount(store: String, auth_id: Authid, to_stdout: bool) -> Result<Value, Error> {
-    let _lock = pbs_config::datastore::lock_config()?;
-    let (mut section_config, _digest) = pbs_config::datastore::config()?;
-    let mut datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
+    let (section_config, _digest) = pbs_config::datastore::config()?;
+    let datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
 
     if datastore.backing_device.is_none() {
         bail!("datastore '{store}' is not removable");
@@ -2651,6 +2650,24 @@ async fn do_unmount(store: String, auth_id: Authid, to_stdout: bool) -> Result<V
 
     ensure_datastore_is_mounted(&datastore)?;
 
+    // Setting gc-on-unmount requires Datastore.Modify (or Datastore.Allocate at creation), the
+    // same level needed to start GC directly, so no privilege escalation from triggering it here.
+    if datastore.gc_on_unmount.unwrap_or(false) {
+        let client = crate::client_helpers::connect_to_localhost()
+            .context("failed to connect to localhost for starting GC")?;
+        match client
+            .post(&format!("api2/json/admin/datastore/{store}/gc"), None)
+            .await
+        {
+            Ok(_) => info!("started garbage collection, unmount will wait for it to finish"),
+            Err(err) => warn!("unable to start garbage collection before unmount: {err}"),
+        }
+    }
+
+    let _lock = pbs_config::datastore::lock_config()?;
+    let (mut section_config, _digest) = pbs_config::datastore::config()?;
+    let mut datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
+
     datastore.set_maintenance_mode(Some(MaintenanceMode {
         ty: MaintenanceType::Unmount,
         message: None,
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index c44d50d1..c0be0296 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -409,6 +409,8 @@ pub enum DeletableProperty {
     Comment,
     /// Delete the garbage collection schedule.
     GcSchedule,
+    /// Delete the gc-on-unmount property.
+    GcOnUnmount,
     /// Delete the prune job schedule.
     PruneSchedule,
     /// Delete the keep-last property
@@ -495,6 +497,9 @@ pub fn update_datastore(
                 DeletableProperty::GcSchedule => {
                     data.gc_schedule = None;
                 }
+                DeletableProperty::GcOnUnmount => {
+                    data.gc_on_unmount = None;
+                }
                 DeletableProperty::PruneSchedule => {
                     data.prune_schedule = None;
                 }
@@ -560,6 +565,10 @@ pub fn update_datastore(
         data.gc_schedule = update.gc_schedule;
     }
 
+    if update.gc_on_unmount.is_some() {
+        data.gc_on_unmount = update.gc_on_unmount;
+    }
+
     macro_rules! prune_disabled {
         ($(($param:literal, $($member:tt)+)),+) => {
             $(
-- 
2.47.3

Re: [PATCH proxmox-backup v2 2/3] api: datastore: add option to run garbage collection before unmount

[Shannon Sterz]

On Mon Apr 20, 2026 at 9:42 AM CEST, Hannes Laimer wrote:
> Removable datastores set up for auto-unmount have no natural point at
> which to run garbage collection, since the drive is unmounted right
> after jobs finish. Expose a gc-on-unmount option so GC can be triggered
> as part of the unmount for those setups.
>
> Relies on the active write operation by garbage collection to block
> the unmount task until GC completes (among possible other active
> operations). No other active operation can occur in the meantime
> since the datastore remains in 'unmount' maintenance mode.
>
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  src/api2/admin/datastore.rs  | 23 ++++++++++++++++++++---
>  src/api2/config/datastore.rs |  9 +++++++++
>  2 files changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> index 757b3114..212a48da 100644
> --- a/src/api2/admin/datastore.rs
> +++ b/src/api2/admin/datastore.rs
> @@ -2641,9 +2641,8 @@ fn do_unmount_device(
>  }
>
>  async fn do_unmount(store: String, auth_id: Authid, to_stdout: bool) -> Result<Value, Error> {
> -    let _lock = pbs_config::datastore::lock_config()?;
> -    let (mut section_config, _digest) = pbs_config::datastore::config()?;
> -    let mut datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
> +    let (section_config, _digest) = pbs_config::datastore::config()?;
> +    let datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
>
>      if datastore.backing_device.is_none() {
>          bail!("datastore '{store}' is not removable");
> @@ -2651,6 +2650,24 @@ async fn do_unmount(store: String, auth_id: Authid, to_stdout: bool) -> Result<V
>
>      ensure_datastore_is_mounted(&datastore)?;
>
> +    // Setting gc-on-unmount requires Datastore.Modify (or Datastore.Allocate at creation), the
> +    // same level needed to start GC directly, so no privilege escalation from triggering it here.
> +    if datastore.gc_on_unmount.unwrap_or(false) {
> +        let client = crate::client_helpers::connect_to_localhost()
> +            .context("failed to connect to localhost for starting GC")?;
> +        match client
> +            .post(&format!("api2/json/admin/datastore/{store}/gc"), None)
> +            .await
> +        {
> +            Ok(_) => info!("started garbage collection, unmount will wait for it to finish"),
> +            Err(err) => warn!("unable to start garbage collection before unmount: {err}"),

small question, any reason to do a round trip across the api here
instead of factoring out the logic needed here from the
`start_garbage_collection` function below and calling that directly?
something like this should to the trick:

```

fn init_garbage_collection_job(
    store: String,
    auth_id: &Authid,
    to_stdout: bool,
) -> Result<Value, Error> {
    let datastore = DataStore::lookup_datastore(lookup_with(&store, Operation::Write))?;

    let job = Job::new("garbage_collection", &store)
        .map_err(|_| format_err!("garbage collection already running"))?;

    let upid_str =
        crate::server::do_garbage_collection_job(job, datastore, &auth_id, None, to_stdout)
            .map_err(|err| {
                format_err!("unable to start garbage collection job on datastore {store} - {err:#}")
            })?;

    Ok(json!(upid_str))
}

you can then call that in `do_unmount` and `start_garbage_collection`.
or am i missing something? would also associate the gc task with user
starting the unmount operation instead of root@pam if im not mistaken?

> +        }
> +    }
> +
> +    let _lock = pbs_config::datastore::lock_config()?;
> +    let (mut section_config, _digest) = pbs_config::datastore::config()?;
> +    let mut datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
> +
>      datastore.set_maintenance_mode(Some(MaintenanceMode {
>          ty: MaintenanceType::Unmount,
>          message: None,
> diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
> index c44d50d1..c0be0296 100644
> --- a/src/api2/config/datastore.rs
> +++ b/src/api2/config/datastore.rs
> @@ -409,6 +409,8 @@ pub enum DeletableProperty {
>      Comment,
>      /// Delete the garbage collection schedule.
>      GcSchedule,
> +    /// Delete the gc-on-unmount property.
> +    GcOnUnmount,
>      /// Delete the prune job schedule.
>      PruneSchedule,
>      /// Delete the keep-last property
> @@ -495,6 +497,9 @@ pub fn update_datastore(
>                  DeletableProperty::GcSchedule => {
>                      data.gc_schedule = None;
>                  }
> +                DeletableProperty::GcOnUnmount => {
> +                    data.gc_on_unmount = None;
> +                }
>                  DeletableProperty::PruneSchedule => {
>                      data.prune_schedule = None;
>                  }
> @@ -560,6 +565,10 @@ pub fn update_datastore(
>          data.gc_schedule = update.gc_schedule;
>      }
>
> +    if update.gc_on_unmount.is_some() {
> +        data.gc_on_unmount = update.gc_on_unmount;
> +    }
> +
>      macro_rules! prune_disabled {
>          ($(($param:literal, $($member:tt)+)),+) => {
>              $(

Re: [PATCH proxmox-backup v2 2/3] api: datastore: add option to run garbage collection before unmount

[Hannes Laimer]

On 2026-04-20 10:31, Shannon Sterz wrote:
> On Mon Apr 20, 2026 at 9:42 AM CEST, Hannes Laimer wrote:
>> Removable datastores set up for auto-unmount have no natural point at
>> which to run garbage collection, since the drive is unmounted right
>> after jobs finish. Expose a gc-on-unmount option so GC can be triggered
>> as part of the unmount for those setups.
>>
>> Relies on the active write operation by garbage collection to block
>> the unmount task until GC completes (among possible other active
>> operations). No other active operation can occur in the meantime
>> since the datastore remains in 'unmount' maintenance mode.
>>
>> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
>> ---
>>  src/api2/admin/datastore.rs  | 23 ++++++++++++++++++++---
>>  src/api2/config/datastore.rs |  9 +++++++++
>>  2 files changed, 29 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
>> index 757b3114..212a48da 100644
>> --- a/src/api2/admin/datastore.rs
>> +++ b/src/api2/admin/datastore.rs
>> @@ -2641,9 +2641,8 @@ fn do_unmount_device(
>>  }
>>
>>  async fn do_unmount(store: String, auth_id: Authid, to_stdout: bool) -> Result<Value, Error> {
>> -    let _lock = pbs_config::datastore::lock_config()?;
>> -    let (mut section_config, _digest) = pbs_config::datastore::config()?;
>> -    let mut datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
>> +    let (section_config, _digest) = pbs_config::datastore::config()?;
>> +    let datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
>>
>>      if datastore.backing_device.is_none() {
>>          bail!("datastore '{store}' is not removable");
>> @@ -2651,6 +2650,24 @@ async fn do_unmount(store: String, auth_id: Authid, to_stdout: bool) -> Result<V
>>
>>      ensure_datastore_is_mounted(&datastore)?;
>>
>> +    // Setting gc-on-unmount requires Datastore.Modify (or Datastore.Allocate at creation), the
>> +    // same level needed to start GC directly, so no privilege escalation from triggering it here.
>> +    if datastore.gc_on_unmount.unwrap_or(false) {
>> +        let client = crate::client_helpers::connect_to_localhost()
>> +            .context("failed to connect to localhost for starting GC")?;
>> +        match client
>> +            .post(&format!("api2/json/admin/datastore/{store}/gc"), None)
>> +            .await
>> +        {
>> +            Ok(_) => info!("started garbage collection, unmount will wait for it to finish"),
>> +            Err(err) => warn!("unable to start garbage collection before unmount: {err}"),
> 
> small question, any reason to do a round trip across the api here
> instead of factoring out the logic needed here from the
> `start_garbage_collection` function below and calling that directly?
> something like this should to the trick:
> 
> ```
> 
> fn init_garbage_collection_job(
>     store: String,
>     auth_id: &Authid,
>     to_stdout: bool,
> ) -> Result<Value, Error> {
>     let datastore = DataStore::lookup_datastore(lookup_with(&store, Operation::Write))?;
> 
>     let job = Job::new("garbage_collection", &store)
>         .map_err(|_| format_err!("garbage collection already running"))?;
> 
>     let upid_str =
>         crate::server::do_garbage_collection_job(job, datastore, &auth_id, None, to_stdout)
>             .map_err(|err| {
>                 format_err!("unable to start garbage collection job on datastore {store} - {err:#}")
>             })?;
> 
>     Ok(json!(upid_str))
> }
> 
> you can then call that in `do_unmount` and `start_garbage_collection`.
> or am i missing something? would also associate the gc task with user
> starting the unmount operation instead of root@pam if im not mistaken?
> 

the reason is that the unmounting is running in the api process, so as
root. if we don't go through the api we would have the gc also running
in the privileged process. general datastore operations, like gc, do
assume they run as the `backup` user

hitting the (proxy) api endpoint is the simplest way to have the gc run
with the correct permissions

>> +        }
>> +    }
>> +
>> +    let _lock = pbs_config::datastore::lock_config()?;
>> +    let (mut section_config, _digest) = pbs_config::datastore::config()?;
>> +    let mut datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
>> +
>>      datastore.set_maintenance_mode(Some(MaintenanceMode {
>>          ty: MaintenanceType::Unmount,
>>          message: None,
>> diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
>> index c44d50d1..c0be0296 100644
>> --- a/src/api2/config/datastore.rs
>> +++ b/src/api2/config/datastore.rs
>> @@ -409,6 +409,8 @@ pub enum DeletableProperty {
>>      Comment,
>>      /// Delete the garbage collection schedule.
>>      GcSchedule,
>> +    /// Delete the gc-on-unmount property.
>> +    GcOnUnmount,
>>      /// Delete the prune job schedule.
>>      PruneSchedule,
>>      /// Delete the keep-last property
>> @@ -495,6 +497,9 @@ pub fn update_datastore(
>>                  DeletableProperty::GcSchedule => {
>>                      data.gc_schedule = None;
>>                  }
>> +                DeletableProperty::GcOnUnmount => {
>> +                    data.gc_on_unmount = None;
>> +                }
>>                  DeletableProperty::PruneSchedule => {
>>                      data.prune_schedule = None;
>>                  }
>> @@ -560,6 +565,10 @@ pub fn update_datastore(
>>          data.gc_schedule = update.gc_schedule;
>>      }
>>
>> +    if update.gc_on_unmount.is_some() {
>> +        data.gc_on_unmount = update.gc_on_unmount;
>> +    }
>> +
>>      macro_rules! prune_disabled {
>>          ($(($param:literal, $($member:tt)+)),+) => {
>>              $(
> 

Re: [PATCH proxmox-backup v2 2/3] api: datastore: add option to run garbage collection before unmount

[Shannon Sterz]

On Mon Apr 20, 2026 at 10:45 AM CEST, Hannes Laimer wrote:
> On 2026-04-20 10:31, Shannon Sterz wrote:
>> On Mon Apr 20, 2026 at 9:42 AM CEST, Hannes Laimer wrote:

-->8 snip 8<--

>>> +    // Setting gc-on-unmount requires Datastore.Modify (or Datastore.Allocate at creation), the
>>> +    // same level needed to start GC directly, so no privilege escalation from triggering it here.
>>> +    if datastore.gc_on_unmount.unwrap_or(false) {
>>> +        let client = crate::client_helpers::connect_to_localhost()
>>> +            .context("failed to connect to localhost for starting GC")?;
>>> +        match client
>>> +            .post(&format!("api2/json/admin/datastore/{store}/gc"), None)
>>> +            .await
>>> +        {
>>> +            Ok(_) => info!("started garbage collection, unmount will wait for it to finish"),
>>> +            Err(err) => warn!("unable to start garbage collection before unmount: {err}"),
>>
>> small question, any reason to do a round trip across the api here
>> instead of factoring out the logic needed here from the
>> `start_garbage_collection` function below and calling that directly?
>> something like this should to the trick:
>>
>> ```
>>
>> fn init_garbage_collection_job(
>>     store: String,
>>     auth_id: &Authid,
>>     to_stdout: bool,
>> ) -> Result<Value, Error> {
>>     let datastore = DataStore::lookup_datastore(lookup_with(&store, Operation::Write))?;
>>
>>     let job = Job::new("garbage_collection", &store)
>>         .map_err(|_| format_err!("garbage collection already running"))?;
>>
>>     let upid_str =
>>         crate::server::do_garbage_collection_job(job, datastore, &auth_id, None, to_stdout)
>>             .map_err(|err| {
>>                 format_err!("unable to start garbage collection job on datastore {store} - {err:#}")
>>             })?;
>>
>>     Ok(json!(upid_str))
>> }
>>
>> you can then call that in `do_unmount` and `start_garbage_collection`.
>> or am i missing something? would also associate the gc task with user
>> starting the unmount operation instead of root@pam if im not mistaken?
>>
>
> the reason is that the unmounting is running in the api process, so as
> root. if we don't go through the api we would have the gc also running
> in the privileged process. general datastore operations, like gc, do
> assume they run as the `backup` user
>
> hitting the (proxy) api endpoint is the simplest way to have the gc run
> with the correct permissions

ah yeah makes sense, thanks for clearing that up.

[PATCH proxmox-backup v2 3/3] ui: datastore: expose gc-on-unmount setting

[Hannes Laimer]

Shows the gc-on-unmount option in the datastore options and allows
editing for removable datastores.

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

diff --git a/www/datastore/OptionView.js b/www/datastore/OptionView.js
index 160edd44..2b35355a 100644
--- a/www/datastore/OptionView.js
+++ b/www/datastore/OptionView.js
@@ -104,6 +104,7 @@ Ext.define('PBS.Datastore.Options', {
         let record = listStore.findRecord('store', me.datastore, 0, false, true, true);
         return {
             s3Visible: record?.get('backend-type') === 's3' ? undefined : false,
+            removableVisible: record?.get('mount-status') !== 'nonremovable' ? undefined : false,
         };
     },
 
@@ -226,6 +227,28 @@ Ext.define('PBS.Datastore.Options', {
                 },
             },
         },
+        'gc-on-unmount': {
+            required: true,
+            header: gettext('GC on Unmount'),
+            defaultValue: false,
+            renderer: Proxmox.Utils.format_boolean,
+            cbind: {
+                visible: '{removableVisible}',
+            },
+            editor: {
+                xtype: 'proxmoxWindowEdit',
+                title: gettext('GC on Unmount'),
+                width: 350,
+                items: {
+                    xtype: 'proxmoxcheckbox',
+                    name: 'gc-on-unmount',
+                    boxLabel: gettext('Run garbage collection before unmounting'),
+                    defaultValue: false,
+                    deleteDefaultValue: true,
+                    deleteEmpty: true,
+                },
+            },
+        },
         'maintenance-mode': {
             required: true,
             header: gettext('Maintenance mode'),
-- 
2.47.3

Re: [PATCH proxmox{,-backup} v2 0/3] fixes #7465: add gc-on-unmount flag

[Christian Ebner]

On 4/20/26 9:41 AM, Hannes Laimer wrote:
> So GC can run before unmount, without this its somewhat complicated to
> run gc on a datastore that is configured to auto unmount after jobs
> finish.
> 
> v2, thanks @Chris!:
>   - include proxmox patch
>   - don't lock config for just reading
>   - improve comments/commit msg
> 

Thanks for addressing the comments from v1:

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