When re-opening a datastore due to the cached entry being stale
(config change) but also if the last re-open was >60s ago). On
datastore open the chunk store was also re-opened, which in turn
creates a new ProcessLocker, loosing any existing shared lock which
can cause conflicts between long running (24h+) backups and GC.
To fix this, reuse the existing ChunkStore, and thus its
ProcessLocker, when creating a up-to-date datastore instance on
lookup, since only the datastore config should be reloaded. This is
fine as the ChunkStore path is not updatable over our API.
This was always a potential issue but got exposed in practice by
commit 118deb4db8 which introduced the
unconditional "re-open after 60s" mechanism.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
[ T: reword commit message a bit and reference commit that made the
issue much more likely ]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
which is stable since rustc 1.32 but wasn't available in out
toolchain when this was originally written in commit 7bc1d7277
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
instead move the acl_path helper to BackupNamespace, and introduce a new
helper for printing a store+ns when logging/generating error messages.
Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
this shouldn't be printed/logged - use DatastoreWithNamespace /
pbs_api_types::BackupGroup instead.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
the api type implements it already, all call sites should rather use
DatastoreWithName and pbs_api_types::BackupDir for logging/..
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
causes trouble with UI and is inconsistent as its still missing in
file restore (daemon)
We probably want to use a separate property to safe this to avoid
confusion with size.
This reverts commit 66ad63bac2.
after all we couldn't delete all that got requested, ideally this
should become a task where we can log what got deleted and what
not...
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
no changes in interface for users of the crate as we re-export
anyway, so more for avoiding to crowd the datastore module to much
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
And make that opt-in in the API endpoint, to avoid bad surprises by
default.
If not set we'll only prune empty namespaces.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
When listing the content of a catalog, add the number of files
contained in the directory as its size. Also removes redundant code,
the `mtime` and the `size` of a file is already set when creating the
archive entry, but we naturally need to override the size now for
directories.
Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
checks the privilegs for the target namespace. If that does not exist,
try to recursively create them while checking the privileges.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
other sizes can happen in legitimate and illegitimate ways:
- illegitimate: encryped chunks and bad actor client
- legitimate: same chunk but newer zstd version (or compression
level) can compress it better (or worse) so the
Ideally we could take the actual smaller chunk so that improved zstd
tech gets leveraged, but we could only allow to do that for
un-encrypted ones.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
We sometimes need to do some in-memory only stuff, e.g., to check if
GC is already running for a datastore, which is a try_lock on a mutex
that is in-memory.
Actually the whole thing would be nicer if we could guarantee to hold
the correct contract statically, e.g., like
https://docs.rust-embedded.org/book/static-guarantees/design-contracts.html
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
We decided to go this route because it'll most likely be
safer in the API as we need to explicitly add namespaces
support to the various API endpoints this way.
For example, 'pull' should have 2 namespaces: local and
remote, and the GroupFilter (which would otherwise contain
exactly *one* namespace parameter) needs to be applied for
both sides (to decide what to pull from the remote, and what
to *remove* locally as cleanup).
The *datastore* types still contain the namespace and have a
`.backup_ns()` getter.
Note that the datastore's `Display` implementations are no
longer safe to use as a deserializable string.
Additionally, some datastore based methods now have been
exposed via the BackupGroup/BackupDir types to avoid a
"round trip" in code.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
on depth == 0 we only yield the anchor ns, this simplifies usage in
the API as for, e.g. list-snapthos the depth == 0 means only the
snapshots from the passed namespace, nothing below.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
The behavior on "any snapshot was protected" isn't yet ideal, as we
then do not cleanup any (sub) namespace, even if some of them where
cleaned from groups & snapshots completely. But that isn't easy to do
with our current depth-first pre-order iterator behavior, and it's
also not completely wrong either, the user can re-do the removal on
the sub-namespaces, so leave that for later.
Should get moved to a datastore::BackupNamespace type once/if we get
one
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Make it easier by adding an helper accepting either group or
directory
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
and issue a warning. We can do this, because we know an empty chunk
cannot be valid, and we (assumedly) have a valid chunk in memory.
Having empty chunks on disk is currently possible when PBS crashes,
but the rename of the chunk was flushed to disk, when the actual data
was not.
If it's not empty but there is a size mismatch, return an error.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
data blobs can only appear in a BackupDir (snapshot) in the backup
hierachy, so makes more sense that it lives in there.
As it wasn't widely used anyway it's easy to move the single
non-package call site over to the new one directly and drop the
implementation from Datastore completely.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
the manifest is owned by the backup dir (snapshot) so it should also
handle locking, makes no sense to have the implementation somewhere
higher up.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
move the check for directory before doing the OSString -> String
conversion, which should be a bit more efficient.
Also let the match return the entry in the non-skip/return case to
reduce indentation level for the inner "yield element" part, making
it slightly easier to follow.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht
Dominik Csapak
Fabian Grünbichler
Wolfgang Bumiller
Stefan Sterz