From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id D3B5E1FF0AF for ; Thu, 08 Oct 2026 11:37:16 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 37F44212D2; Thu, 08 Oct 2026 11:37:16 +0200 (CEST) Message-ID: Date: Thu, 8 Oct 2026 11:37:09 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: Christian Ebner Subject: Re: [PATCH proxmox v2 1/6] fix #7904: Add 'enable' to {sync,verify,tape,gc} job schema and datastore config To: Jonas Theisen , pbs-devel@lists.proxmox.com References: <20261007134500.323872-1-j.theisen@proxmox.com> <20261007134500.323872-2-j.theisen@proxmox.com> Content-Language: en-US, de-DE In-Reply-To: <20261007134500.323872-2-j.theisen@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1791452230458 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.572 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: LDOZTOPRSV5VEUKCJR7JK6BLPY7VYST7 X-Message-ID-Hash: LDOZTOPRSV5VEUKCJR7JK6BLPY7VYST7 X-MailFrom: c.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 10/7/26 3:45 PM, Jonas Theisen wrote: > To be able to en-/disable the respective jobs this patch adds the > enable field to the job config and the needed schema. We do already have the `disable` field for prune jobs, so IMHO it is preferable to introduce the same fields and logic for the other jobs as well. This would allow to keep config and logic consistent. So rather define: ``` pub const DISABLE_JOB_SCHEMA: Schema = BooleanSchema::new( "Disable the scheduled execution of this task.", ) .default(false) .schema(); ``` And use that for all, including GC and prune job config. Avoiding serialization of default values as suggested by Maximiliano [0] would be preferable as well, especially to reduce config bloating with default values. [0] https://lore.proxmox.com/pbs-devel/s8old88mxnt.fsf@toolbox/ > This also adds the gc_enable parameter to the datastore config > to allow this type of job to be disabled. > > Signed-off-by: Jonas Theisen > --- > pbs-api-types/src/datastore.rs | 12 ++++++++++++ > pbs-api-types/src/jobs.rs | 24 ++++++++++++++++++++++++ > 2 files changed, 36 insertions(+) > > diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs > index 93ccaf07..50658b58 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-enable": { > + description: "Enable scheduled execution of garbage collection.", > + optional: true, > + type: bool, nit: It would be preferable to reuse the common schema definition for this case as well. > + }, > "gc-schedule": { > optional: true, > schema: GC_SCHEDULE_SCHEMA, > @@ -524,6 +529,9 @@ pub struct DataStoreConfig { > #[serde(skip_serializing_if = "Option::is_none")] > pub comment: Option, > > + #[serde(skip_serializing_if = "Option::is_none")] > + pub gc_enable: Option, > + > #[serde(skip_serializing_if = "Option::is_none")] > pub gc_schedule: Option, > > @@ -605,6 +613,7 @@ impl DataStoreConfig { > name, > path, > comment: None, > + gc_enable: None, > gc_schedule: None, > gc_on_unmount: None, > prune_schedule: None, > @@ -1704,6 +1713,9 @@ pub struct GarbageCollectionJobStatus { > pub store: String, > #[serde(flatten)] > pub status: GarbageCollectionStatus, > + /// Scheduled execution of the gc job > + #[serde(skip_serializing_if = "Option::is_none")] > + pub enable: Option, > /// Schedule of the gc job > #[serde(skip_serializing_if = "Option::is_none")] > pub schedule: Option, > diff --git a/pbs-api-types/src/jobs.rs b/pbs-api-types/src/jobs.rs > index 9a5d2b77..0ae93678 100644 > --- a/pbs-api-types/src/jobs.rs > +++ b/pbs-api-types/src/jobs.rs > @@ -62,6 +62,12 @@ pub const VERIFICATION_SCHEDULE_SCHEMA: Schema = > .type_text("") > .schema(); > > +pub const ENABLE_JOB_SCHEMA: Schema = BooleanSchema::new( > + "Enables the scheduled execution of this task.", > +) > +.default(true) > +.schema(); > + > pub const REMOVE_VANISHED_BACKUPS_SCHEMA: Schema = BooleanSchema::new( > "Delete vanished backups. This removes the local copy if the remote backup was deleted.", > ) > @@ -228,6 +234,10 @@ pub const VERIFICATION_OUTDATED_AFTER_SCHEMA: Schema = > optional: true, > schema: VERIFICATION_SCHEDULE_SCHEMA, > }, > + enable: { > + optional: true, > + schema: ENABLE_JOB_SCHEMA, > + }, > ns: { > optional: true, > schema: BACKUP_NAMESPACE_SCHEMA, > @@ -268,6 +278,8 @@ pub struct VerificationJobConfig { > /// when to schedule this job in calendar event notation > pub schedule: Option, > #[serde(skip_serializing_if = "Option::is_none", default)] > + pub enable: Option, > + #[serde(skip_serializing_if = "Option::is_none")] > /// on which backup namespace to run the verification recursively > pub ns: Option, > #[serde(skip_serializing_if = "Option::is_none", default)] > @@ -403,6 +415,10 @@ pub struct TapeBackupJobSetup { > optional: true, > schema: SYNC_SCHEDULE_SCHEMA, > }, > + enable: { > + optional: true, > + schema: ENABLE_JOB_SCHEMA, > + }, > } > )] > #[derive(Serialize, Deserialize, Clone, Updater, PartialEq)] > @@ -417,6 +433,8 @@ pub struct TapeBackupJobConfig { > pub comment: Option, > #[serde(skip_serializing_if = "Option::is_none")] > pub schedule: Option, > + #[serde(skip_serializing_if = "Option::is_none")] > + pub enable: Option, > } > > #[api( > @@ -642,6 +660,10 @@ pub const UNMOUNT_ON_SYNC_DONE_SCHEMA: Schema = > optional: true, > schema: SYNC_SCHEDULE_SCHEMA, > }, > + enable: { > + optional: true, > + schema: ENABLE_JOB_SCHEMA, > + }, > "group-filter": { > schema: GROUP_FILTER_LIST_SCHEMA, > optional: true, > @@ -718,6 +740,8 @@ pub struct SyncJobConfig { > #[serde(skip_serializing_if = "Option::is_none")] > pub schedule: Option, > #[serde(skip_serializing_if = "Option::is_none")] > + pub enable: Option, > + #[serde(skip_serializing_if = "Option::is_none")] > pub group_filter: Option>, > #[serde(flatten)] > pub limit: RateLimitConfig,