[pbs-devel] [PATCH proxmox-backup RFC 00/10] introduce typestate for datastore/chunkstore

[Hannes Laimer <h.laimer@proxmox.com>]

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(-)

-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel