From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <pbs-devel-bounces@lists.proxmox.com> Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 807E31FF18A for <inbox@lore.proxmox.com>; Mon, 26 May 2025 16:18:25 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 2AE2A3436A; Mon, 26 May 2025 16:18:38 +0200 (CEST) Message-ID: <e1a6415a-2555-41de-a231-1eef126afea7@proxmox.com> Date: Mon, 26 May 2025 16:18:34 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: pbs-devel@lists.proxmox.com References: <20240903123401.91513-1-h.laimer@proxmox.com> Content-Language: en-US From: Hannes Laimer <h.laimer@proxmox.com> In-Reply-To: <20240903123401.91513-1-h.laimer@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.025 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment 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] superseded: [PATCH proxmox-backup RFC 00/10] introduce typestate for datastore/chunkstore X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion <pbs-devel.lists.proxmox.com> List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe> List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/> List-Post: <mailto:pbs-devel@lists.proxmox.com> List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help> List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe> Reply-To: Proxmox Backup Server development discussion <pbs-devel@lists.proxmox.com> Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com> superseded by https://lore.proxmox.com/pbs-devel/20240903123401.91513-1-h.laimer@proxmox.com/ On 9/3/24 14:33, Hannes Laimer wrote: > This patch series introduces two traits, CanRead and CanWrite, to define whether > a datastore reference is readable, writable, or neither. Functions that read > or write are now implemented in `impl<T: CanRead>` or `impl<T: CanWrite>` blocks, ensuring > that they are only available to references that are supposed to read/write. > > Motivation: > Currently, we track the number of read/write references of a datastore but we don't > track Lookup operations as they don't read or write, they still need a chunkstore, so > eventhough they don't neccessarily directly do IO, they hold an open file handle. > This is a problem for things like unmounting, currently lookup operations are only really > short, so you'd need really unlucky timing to actually run into problems, but still, > if a datastore is in "offline" maintenance mode, we shouldn't open filehandles on it. > > By encoding state in the type: > 1. We can assign non-readable/writable references for lookup operations. > 2. The compiler ensures correct usage of references. Since it is easy to miss > what might happen a few function calls down the line, having the compiler > yell at you for easily missed things like this, is a really good thing > I think. > > Changes: > * Added CanRead and CanWrite traits. > * Separated functions into impl<T: CanRead> or impl<T: CanWrite>. > * Introduced three new datastore lookup functions that return concrete types implementing > CanRead, CanWrite, or neither. > * Renamed lookup_datastore() to open_datastore() and made it private. > > The main downside is needing separate datastore caches for read and write references due to > concrete type requirements in the cache HashMap. > > Almost all changes are either adding generics or moving functions into the appropriate > trait implementations. The logic itself is only touched twice, once in datastore_lookup() > and once check_privs_and_load_store() in /api/admin/datastore, this function now only checks > the privs, the datastore opening happens in the endpoint function directly. > > > > Hannes Laimer (10): > chunkstore: add CanRead and CanWrite trait > chunkstore: separate functions into impl block > datastore: add generics and new lookup functions > datastore: separate functions into impl block > backup_info: add generics and separate functions into impl blocks > pbs-datastore: add generics and separate functions into impl blocks > api: replace datastore_lookup with new, state-typed datastore > returning functions > server/bin: replace datastore_lookup with new, state-typed datastore > returning functions > api: add generics and separate functions into impl blocks > backup/server/tape: add generics and separate functions into impl > blocks > > pbs-datastore/src/backup_info.rs | 179 +- > pbs-datastore/src/chunk_store.rs | 228 ++- > pbs-datastore/src/datastore.rs | 1688 ++++++++++--------- > pbs-datastore/src/dynamic_index.rs | 22 +- > pbs-datastore/src/fixed_index.rs | 50 +- > pbs-datastore/src/hierarchy.rs | 92 +- > pbs-datastore/src/local_chunk_reader.rs | 13 +- > pbs-datastore/src/prune.rs | 19 +- > pbs-datastore/src/snapshot_reader.rs | 30 +- > src/api2/admin/datastore.rs | 170 +- > src/api2/admin/namespace.rs | 8 +- > src/api2/backup/environment.rs | 176 +- > src/api2/backup/mod.rs | 25 +- > src/api2/backup/upload_chunk.rs | 19 +- > src/api2/reader/environment.rs | 31 +- > src/api2/reader/mod.rs | 9 +- > src/api2/status.rs | 8 +- > src/api2/tape/backup.rs | 21 +- > src/api2/tape/drive.rs | 2 +- > src/api2/tape/restore.rs | 75 +- > src/backup/hierarchy.rs | 26 +- > src/backup/verify.rs | 53 +- > src/bin/proxmox-backup-proxy.rs | 4 +- > src/server/gc_job.rs | 8 +- > src/server/prune_job.rs | 9 +- > src/server/pull.rs | 29 +- > src/server/verify_job.rs | 4 +- > src/tape/file_formats/snapshot_archive.rs | 5 +- > src/tape/pool_writer/mod.rs | 11 +- > src/tape/pool_writer/new_chunks_iterator.rs | 7 +- > 30 files changed, 1585 insertions(+), 1436 deletions(-) > _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel