From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 7AC0789715 for ; Fri, 29 Jul 2022 10:26:14 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 6FD451A7CD for ; Fri, 29 Jul 2022 10:26:14 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Fri, 29 Jul 2022 10:26:13 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 8E47C42C2C for ; Fri, 29 Jul 2022 10:26:13 +0200 (CEST) Message-ID: Date: Fri, 29 Jul 2022 10:26:11 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:104.0) Gecko/20100101 Thunderbird/104.0 Content-Language: en-GB To: Proxmox Backup Server development discussion , Hannes Laimer , Wolfgang Bumiller References: <20220705130834.14285-1-h.laimer@proxmox.com> <20220705130834.14285-5-h.laimer@proxmox.com> <20220706113356.y2owpzmxtp4k6loc@casey.proxmox.com> <0c2809af-aeea-77e2-aa46-7eccc248d3de@proxmox.com> From: Thomas Lamprecht In-Reply-To: <0c2809af-aeea-77e2-aa46-7eccc248d3de@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.001 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pbs-devel] [PATCH proxmox-backup 02/26] config: make RemovableDeviceConfig savable to config file X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Jul 2022 08:26:14 -0000 On 07/07/2022 10:35, Hannes Laimer wrote: > Am 06.07.22 um 13:44 schrieb Thomas Lamprecht: >> On 06/07/2022 13:33, Wolfgang Bumiller wrote: >>> Do we need this to be a separate config file though? Can this not simply >>> be part of the datastore directly? We already "link" them by having to >>> define the datastore as `removable`, so can we not just put all the >>> values in there? >> >> IIRC we talked about adding just a "backing-device", or the like (probably >> something a bit more explicit w.r.t. to removable), property to existing >> datastores, and then pretty much handle them like existing ones. >> >> That way we can reuse most of existing infrastructure and functionality, >> what changes is a different (or no) error on sync, GC, etc. (or repeat skipped >> jobs when plugged in) and the "auto-mount + activate in PBS" via udev helper. > > Yes, we could put all the removable-device data directly into the > datastore config. But I think this is a cleaner and more flexible > approach, I guess you could argue similarly for why sync and prune jobs > have their own configs. Having less config files def. reduces complexity, that is for both, us _and_ the user/admins to maintain. Having two separate API and possible GUI endpoints makes whole PBS more complex to manage, especially for new users (IME the perceived complexity grows over-proportionally with the amount of UI/API knobs exposed), so reducing that has more weight than the complexity of a potential implementation, or better the change to one, as most of the time simple UI/UX can mean simpler end result (even if the way to get there definitively can be more complex). > What do you mean with we can't reuse > infrastructure with the own config approach? Sync/Prune/GC work like on > a normal datastore, just with the possibility of failing with "no > device present". Yeah, exactly, a separate config or section type doesn't provides much value there, at least fwict with having only a short look now and relying more on my previous experience with that parts of the code base. > > I think by having a removable-device as its own thing we actually end up > reusing more of the already existing API/UI functionality, because > having it directly in the datastore config would mean a lot of "manual" > parsing of property strings and a "custom" update/add implementation. hmm, not sure why that should be, albeit wouldn't it be just a normal, optional property? And updater support probably isn't required as we don't want to expose altering this after creation, at least not initially. W.r.t. "reuse more": is this meant as "we can then factor out some stuff and use it twice with the special logic for each case upfront"? As while I'm not having all that code base in my head atm., I'd think that adding some simple special function call to the respective run job handlers (either in the task if we want to still log a skipped run as task log, or upfront, in the "should a job run now" logic, if we don't want a task log entry) would seem fine and def. less code churn without duplicating config (or section types).