Re: [PATCH proxmox v3 01/10] fix #7904: Add 'disable' to {sync,verify,tape,gc} job schema and datastore config

[Christian Ebner <c.ebner@proxmox.com>]

nit: this patch by itself does not fix the issue, so IMO the subject 
prefix is not warranted for this one. I would suggest to rather stick to 
pbs-api-types here.

On 10/9/26 4:53 PM, Jonas Theisen wrote:
> To be able to en-/disable the respective jobs this patch adds the
> "disable" field to the job config and the needed schema.
>
> This also adds the gc_disable parameter to the datastore config
> to allow this type of job to be disabled.

nit: The commit message might also include the reasoning for why 
`disable` was choosen here, i.e.:
```
`disable` was chosen as parameter name for the flag on all jobs for 
compatibility with the pre-existing flag for prune job config.

> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=7904
> Signed-off-by: Jonas Theisen <j.theisen@proxmox.com>
> ---
>   pbs-api-types/src/datastore.rs | 17 +++++++++++++++++
>   pbs-api-types/src/jobs.rs      | 27 +++++++++++++++++++++++++++
>   2 files changed, 44 insertions(+)
> 
> diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
> index 93ccaf07..b8ef7509 100644
> --- a/pbs-api-types/src/datastore.rs
> +++ b/pbs-api-types/src/datastore.rs
> @@ -460,6 +460,11 @@ pub const COUNTER_RESET_SCHEDULE_SCHEMA: Schema =
>               optional: true,
>               schema: SINGLE_LINE_COMMENT_SCHEMA,
>           },
> +        "gc-disable": {
> +            description: "Disable scheduled execution of garbage collection.",
> +            optional: true,
> +            type: Boolean,
> +        },
>           "gc-schedule": {
>               optional: true,
>               schema: GC_SCHEDULE_SCHEMA,
> @@ -524,6 +529,10 @@ pub struct DataStoreConfig {
>       #[serde(skip_serializing_if = "Option::is_none")]
>       pub comment: Option<String>,
>   
> +    #[serde(default, skip_serializing_if = "is_false")]
> +    #[updater(serde(skip_serializing_if = "Option::is_none"))]
> +    pub gc_disable: bool,
> +
>       #[serde(skip_serializing_if = "Option::is_none")]
>       pub gc_schedule: Option<String>,
>   
> @@ -605,6 +614,7 @@ impl DataStoreConfig {
>               name,
>               path,
>               comment: None,
> +            gc_disable: Default::default(),
>               gc_schedule: None,
>               gc_on_unmount: None,
>               prune_schedule: None,
> @@ -1704,6 +1714,9 @@ pub struct GarbageCollectionJobStatus {
>       pub store: String,
>       #[serde(flatten)]
>       pub status: GarbageCollectionStatus,
> +    /// Is this job disabled

nit: Above docstring sounds more like a question and should rather be 
aligned with the other descriptions. Given that, I wonder why not use 
the SCHEMA as suggested in the previous version of the patch series?

> +    #[serde(default)]
> +    pub disable: bool,
>       /// Schedule of the gc job
>       #[serde(skip_serializing_if = "Option::is_none")]
>       pub schedule: Option<String>,
> @@ -1721,6 +1734,10 @@ pub struct GarbageCollectionJobStatus {
>       pub duration: Option<i64>,
>   }
>   
> +fn is_false(b: &bool) -> bool {
> +    !b
> +}

comment: This is now already the 3rd implementation of this helper for 
skipping serde deserialization. It should rather move to lib.rs and made 
be accessible by all call sites. Ran into this myself lately, so the 
proposed fix is 
https://lore.proxmox.com/pbs-devel/20261014144357.771051-2-c.ebner@proxmox.com/

> +
>   #[api(
>       properties: {
>           "gc-status": {
> diff --git a/pbs-api-types/src/jobs.rs b/pbs-api-types/src/jobs.rs
> index 9a5d2b77..73d7c13f 100644
> --- a/pbs-api-types/src/jobs.rs
> +++ b/pbs-api-types/src/jobs.rs
> @@ -228,6 +228,11 @@ pub const VERIFICATION_OUTDATED_AFTER_SCHEMA: Schema =
>               optional: true,
>               schema: VERIFICATION_SCHEDULE_SCHEMA,
>           },
> +        disable: {
> +            type: Boolean,
> +            optional: true,
> +            default: false,
> +        },
>           ns: {
>               optional: true,
>               schema: BACKUP_NAMESPACE_SCHEMA,
> @@ -267,6 +272,10 @@ pub struct VerificationJobConfig {
>       #[serde(skip_serializing_if = "Option::is_none")]
>       /// when to schedule this job in calendar event notation
>       pub schedule: Option<String>,
> +    /// Disable this job.
> +    #[serde(default, skip_serializing_if = "is_false")]
> +    #[updater(serde(skip_serializing_if = "Option::is_none"))]
> +    pub disable: bool,
>       #[serde(skip_serializing_if = "Option::is_none", default)]
>       /// on which backup namespace to run the verification recursively
>       pub ns: Option<BackupNamespace>,
> @@ -403,6 +412,11 @@ pub struct TapeBackupJobSetup {
>               optional: true,
>               schema: SYNC_SCHEDULE_SCHEMA,
>           },
> +        disable: {
> +            type: Boolean,
> +            optional: true,
> +            default: false,
> +        },
>       }
>   )]
>   #[derive(Serialize, Deserialize, Clone, Updater, PartialEq)]
> @@ -417,6 +431,10 @@ pub struct TapeBackupJobConfig {
>       pub comment: Option<String>,
>       #[serde(skip_serializing_if = "Option::is_none")]
>       pub schedule: Option<String>,
> +    /// Disable this job.
> +    #[serde(default, skip_serializing_if = "is_false")]
> +    #[updater(serde(skip_serializing_if = "Option::is_none"))]
> +    pub disable: bool,
>   }
>   
>   #[api(
> @@ -642,6 +660,11 @@ pub const UNMOUNT_ON_SYNC_DONE_SCHEMA: Schema =
>               optional: true,
>               schema: SYNC_SCHEDULE_SCHEMA,
>           },
> +        disable: {
> +            type: Boolean,
> +            optional: true,
> +            default: false,
> +        },
>           "group-filter": {
>               schema: GROUP_FILTER_LIST_SCHEMA,
>               optional: true,
> @@ -717,6 +740,10 @@ pub struct SyncJobConfig {
>       pub comment: Option<String>,
>       #[serde(skip_serializing_if = "Option::is_none")]
>       pub schedule: Option<String>,
> +    /// Disable this job.
> +    #[serde(default, skip_serializing_if = "is_false")]
> +    #[updater(serde(skip_serializing_if = "Option::is_none"))]
> +    pub disable: bool,
>       #[serde(skip_serializing_if = "Option::is_none")]
>       pub group_filter: Option<Vec<GroupFilter>>,
>       #[serde(flatten)]