I made some comparision with bombardier[0], the one listed here are
30s looped requests with two concurrent clients:
[ static download of ext-all.js ]:
lvl avg / stdev / max
none 1.98 MiB 100 % 5.17ms / 1.30ms / 32.38ms
fastest 813.14 KiB 42 % 20.53ms / 2.85ms / 58.71ms
default 626.35 KiB 30 % 39.70ms / 3.98ms / 85.47ms
[ deterministic (pre-defined data), but real API call ]:
lvl avg / stdev / max
none 129.09 KiB 100 % 2.70ms / 471.58us / 26.93ms
fastest 42.12 KiB 33 % 3.47ms / 606.46us / 32.42ms
default 34.82 KiB 27 % 4.28ms / 737.99us / 33.75ms
The reduction is quite better with default, but it's also slower, but
only when testing over unconstrained network. For real world
scenarios where compression actually matters, e.g., when using a
spotty train connection, we will be faster again with better
compression.
A GPRS limited connection (Firefox developer console) requires the
following load (until the DOMContentLoaded event triggered) times:
lvl t x faster
none 9m 18.6s x 1.0
fastest 3m 20.0s x 2.8
default 2m 30.0s x 3.7
So for worst case using sligthly more CPU time on the server has a
tremendous effect on the client load time.
Using a more realistical example and limiting for "Good 2G" gives:
none 1m 1.8s x 1.0
fastest 22.6s x 2.7
default 16.6s x 3.7
16s is somewhat OK, >1m just isn't...
So, use default level to ensure we get bearable load times on
clients, and if we want to improve transmission size AND speed then
we could always use a in-memory cache, only a few MiB would be
required for the compressable static files we server.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
by using our DeflateEncoder
for this to work, we have to create wrapper reader that generates the crc32
checksum while reading.
also we need to put the target writer in an Option, so that we can take
it out of self and move it into the DeflateEncoder while writing
compressed
we can drop the internal buffer then, since that is managed by the
deflate encoder now
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
implements a deflate encoder that can compress anything that implements
AsyncRead + Unpin into a file with the helper 'compress'
if the inner type is a Stream, it implements Stream itself, this way
some streaming data can be streamed compressed
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Previously we did not store GROUP_OBJ ACL entries for
directories, this means that these were lost which may
potentially elevate group permissions if they were masked
before via ACLs, so we also show a warning.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Don't override `group_obj` with `None` when handling
`ACL_TYPE_DEFAULT` entries for directories.
Reproducer: /var/log/journal ends up without a `MASK` type
entry making it invalid as it has `USER` and `GROUP`
entries.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Add a new module containing key-related functions and schemata from all
over, code moved is not changed as much as possible.
Requires adapting some 'use' statements across proxmox-backup-client and
putting the XDG helpers quite cozily into proxmox_client_tools/mod.rs
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Pass in an optional auth tag, which will be passed as an Authorization
header on every subsequent call.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
This way we get a better rendering in the api-viewer.
before:
[<string>, ... ]
after:
[(<source>=)?<target>, ... ]
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
by changing the 'store' parameter of the restore api call to a
list of mappings (or a single default datastore)
for example giving:
a=b,c=d,e
would restore
datastore 'a' from tape to local datastore 'b'
datastore 'c' from tape to local datastore 'e'
all other datastores to 'e'
this way, only a single datastore can also be restored, by only
giving a single mapping, e.g. 'a=b'
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
The text 'had to upload [KMG]iB' implies that this is the size we
actually had to send to the server, while in reality it is the
raw data size before compression.
Count the size of the compressed chunks and print it separately.
Split the average speed into its own line so they do not get too long.
Rename 'uploaded' into 'size_dirty' and 'vsize_h' into 'size'
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
in commit `asyncify pxar create_archive`, we changed from a
separate thread for creating a pxar to using async code, but the
StdChannelWriter used for both pxar and catalog can block, which
may block the tokio runtime for single (and probably dual) core
environments
this patch adds a wrapper struct for any writer that implements
'std::io::Write' and wraps the write calls with 'block_in_place'
so that if called in a tokio runtime, it knows that this code
potentially blocks
Fixes: 6afb60abf5 ("asyncify pxar create_archive")
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
This is just an optimization, avoiding to read the catalog into memory.
We also expose create_temporary_database_file() now (will be
used for catalog restore).
- new helper: lock_media_set()
- MediaPool: lock media set
- Expose Inventory::new() to avoid double loading
- do not lock pool on restore (only lock media-set)
- change pool lock name to ".pool-{name}"
so that a user can schedule multiple backup jobs onto a single
media pool without having to consider timing them apart
this makes sense since we can backup multiple datastores onto
the same media-set but can only specify one datastore per backup job
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
the default escape handler is handlebars::html_escape, but this are
plain text emails and we manually escape them for the html part, so
set the default escape handler to 'no_escape'
this avoids double html escape for the characters: '&"<>' in emails
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
instead print an error and continue, the rendering functions will error
out if one of the templates could not be registered
if we `.unwrap()` here, it can lead to problems if the templates are
not correct, i.e. we could panic while holding a lock, if something holds
a mutex while this is called for the first time
add a test to catch registration issues during package build
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
so that the tape backup can be restored as any user, given
the current logged in user has the correct permission.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
instead of always using the starttime, use the last timestamp from the log
this way, one can see when the task was aborted without having to read
the log
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
and reuse 'send_job_status_mail' there so that we get consistent
formatted mails from pbs (e.g. html part and author)
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
found and semi-manually replaced by using:
codespell -L mut -L crate -i 3 -w
Mostly in comments, but also email notification and two occurrences
of misspelled 'reserved' struct member, which where not used and
cargo build did not complain about the change, soo ...
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
when we do a prune during a tape backup, do not cancel the tape backup,
but continue with a warning
the task still fails and prompts the user to check the log
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Try once first to load the correct tape before sending an email to
insert the correct one. This way, the admin does not get a mail
if the correct tape is already inserted.
Also include the error we got that prompted the email to insert the
tape. This means that if the admin gets prompted to insert e.g.
"FOO" but inserts "BAR", he'll get an email that the wrong
tape is inserted.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
by adding the existing credential id to the 'excludeCredentials' list
this prevents the browser from registering a token twice, which
lets authentication fail on some browser/token combinations
(e.g. onlykey/solokey+chromium)
while is seems this is currently a bug in chromium, in a future spec
update the underlying behaviour should be better defined, making this
an authenticator bug
also explicitly catch registering errors and show appropriate error messages
0: https://bugs.chromium.org/p/chromium/issues/detail?id=1087642
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
the current default is 'Preferred', which is not really useful, as the
(web) client can simply change this to discouraged, since the
webauthn_rs crate does not verify the 'user_verified' bit of the
response in that case
setting this to 'Required' is not really useful either at the moment,
since a user can have a mix of different authenticators that may or
may not support user verification
there is ongoing discussion in the crate how to handle that[0]
we could probably expose this setting(discouraged/required) to the user/admin
and save it to the credential and allow only registering credentials
of the same type or filter them out on login (i.e. if there is an
authenticator that can handle userVerification, require it)
in any case, the current default is not helpful for security, but
makes loggin in harder, since the key will by default want to verify
the user
0: https://github.com/kanidm/webauthn-rs/pull/49
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
If the usage of a datastore did not change, we did not
return an estimate. The ui interpreted this as 'not enough data', but
it should actually be 'never'.
Fixing this by always setting the estimate first to 0 and overwriting
if we successfully calculated one, and checking for 'undefined' in the ui.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
we test for the config key in the API so it makes sense to have as
test here too. Actually it would be better if we'd have a expect
Value defined here and enforce that it matches, but better than
nothing.
Fix the input for test 1, where tabs got replaced by spaces, as else
it fails
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
As 20s is really not that high, especially for loaded setups one is
connected to through a spotty network (looking at you ÖBB railnet)
and gets latency spikes of 5 - 10s for some minutes at a time..
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
since the PUT api call is using the 'Updater', the 'id' parameter is
already encoded in there, tripping up the api verify tests with
'Duplicate keys found in AllOf schema: id'
"fixing" it by removing the explicit id from the api call and
taking it from the Updater (and failing if it does not exists there;
even though that should never happen)
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
a 'leaf' node is every file *except* directories, so we have
to reverse the logtic here
this fixes the pxar.didx browser in the web ui
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
instead of filling them with zeroes
this fixes an issue where we could not restore a container with large
sparse files in the backup (e.g. a 10GiB sparse file in a container
with a 8GiB disk)
if the last operation of the copy was a seek, we need to truncate
the file to the correct size (seek beyond filesize does not change it)
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
those calls could also block, so we have to run them in a blocking
tokio task, as to not block the current thread
nice side effect is that we now also update the state for that
drive in those instances
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
simply writes into/reads from a file in /run, we will use this
for writing the upid (or potential other states) per drive
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
...including common schemata, connect(), extract_*() and completion
functions.
For later use with proxmox-file-restore binary.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
...to take advantage of the aio::Encoder from the pxar create.
Rather straightforward conversion, but does require getting rid of
references in the Archiver struct, and thus has to be given the Mutex
for the catalog directly. The callback is boxed.
archive_dir_contents can call itself recursively, and thus needs to
return a boxed future.
Users are adjusted, namely PxarBackupStream is converted to use an
Abortable future instead of a thread so it supports async in its handler
function, and the pxar bin create_archive is converted to an async API
function. One test case is made to just use 'block_on'.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
to extract some subdirectory of a pxar into a given target
this will be used in the client
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
we will reuse that code in the client, so we need to move it to
where we can access it from the client
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
[clippy fixes]
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
we will reuse that later in the client, so we need it somewhere
we can use from there
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
[add strongly typed ArchiveEntry and put api code into helpers.rs]
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Currently useful only for single file restore, but kept generic enough
to use any compatible API endpoint over a virtio-vsock[0,1] interface.
VsockClient is adapted and slimmed down from HttpClient.
A tower-compatible VsockConnector is implemented, using a wrapped
UnixStream as transfer. The UnixStream has to be wrapped in a custom
struct to implement 'Connection', Async{Read,Write} are simply forwarded
directly to the underlying stream.
[0] https://www.man7.org/linux/man-pages/man7/vsock.7.html
[1] https://wiki.qemu.org/Features/VirtioVsock
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
This allows anything that can be represented as a UnixStream to be used
as transport for an API server (e.g. virtio sockets).
A tower service expects an IP address as it's peer, which we can't
reliably provide for unix socket based transports, so just fake one.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
if given, erases the tape only iff the inserted tape contains that label
used to safeguard tape erasing from ui for standalone drives
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
in most uses, we want to remove the drive from the param afterwards
where we don't, we already overwrite it with the result of this function
this fixes some commands (like 'proxmox-tape read-label --drive foo')
that failed with:
parameter 'drive': duplicate parameter.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
to avoid confusing messages about using encryption keys when restoring
plaintext backups, or about loading master keys when they are not
actually used for the current operation.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
pull out the crypt-mode to logically group arms and make the whole mess
a bit more "human-parsable".
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
it's needed for PVE's LXC integration, and might be interesting for
other more special usage scenarios as well.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
fixes connecting to hosts with valid certificates without a
pinned fingerprint
this was accidentally changed in the tokio-1.0 updates
apparently
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Fixes: 0f860f712f ("tokio 1.0: update to new tokio-openssl interface")
some users might want to store the plain version of their master key for
long-term storage and rely on physical security instead of a passphrase
to protect the paper key.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
with the fix for #2909 (improving handling missing chunks), we
changed from bailing to warning during a garbage collection when
updating the atime of a chunk.
but, updating the atime can not only fail when the chunk is missing,
but also on other occasions, e.g. no permissions or more importantly,
no space left on the device. in that case, the atime of a valid and used
chunk cannot be updated, and the second sweep of the gc will remove that chunk.
[0] is a real world example of that happening.
instead, only warn on really missin chunks, and bail on all other
errors.
0: https://forum.proxmox.com/threads/pbs-server-full-two-days-later-almost-empty.83274/
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
when executing this code as non-root, we use sg-tape-cmd (a setuid binary)
to execute various ioctls on the tape device
we give the command the open tape device fd as stdin, but did not
dup it, so the std::process:Stdio handle closed it on drop,
which let subsequent operation on that file fail (since it was closed)
fix it by dup'ing it before giving it to the command, and also refactor
the calling code, so that we do not forget to do this
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
we did this for 'mtx', but missed it for the sg_pt_changer code
refactor it into the MtxStatus strut, and call it from both
code paths
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
this check is not perfect since there are often multiple device
nodes per drive/changer, but from the scan api we should return always
the same, so for an api user this should be enough
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
so that an api user can get the drives belonging to a changer
without having to parse the config listing themselves
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
similar to the changers, create a listing at /tape/drive and put
the specific api calls below that
move the scan api call up one level
remove the status info from the config listing
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
so that an api user can see which drive belongs to which drivenum of a changer
for ones with multiple drives
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
add a changer listing here (copied from api2/config/changer)
and put the status and transfer api calls below that
puts the changer scan into the top level tape api
and removes the (now redundant) info from the config api path
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
improves upid_read_status with:
* ignore multiple newlines at the end
* remove all code that could panic (array index access)
the one place where we access with '[pos+1..]' is ok since
we explicitely test the len of the vector, this is done to
let rust optimize away the range checks, so it cannot panic
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
The options struct has no Drop handler and is passed by-move
so we can partially move out of it.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
all the verify methods pass along the following:
- task worker
- datastore
- corrupt and verified chunks
might as well pull that out into a common type, with the added bonus of
now having a single point for construction instead of copying the
default capacaties in three different modules..
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
it's needed to derive Hash, and we always compare Authids or their
Userid components, never just the Tokenname part anyway..
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
containing the CLI parameters that are mostly passed-through from the
client to our pxar archive creation wrapper in pxar::create
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Otherwise we run the drop handler for the scsi pt object AND
the box itself, which shouldn't even work as it should be
doing a double-free (unless the library does some kind of
reference counting in which case this should simply crash
later on?)
anyway, let's make a wrapper simply called `SgPt` containing
the pointer from `construct_scsi_pt_obj()`
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
which was even copy-pasted once without noticing.
found with clippy.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
the error message don't make sense with an empty default
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
chunk_stream one can be collapsed, since split == split_to with at set
to buffer.len() anyway.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
and allow it in the one case where the entry loop is intended, but the
code is not yet implemented fully.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
While the user chosen description is not allowed to be
empty, we do leave it empty for recovery keys, as a "dummy
description" makes little sense...
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
otherwise the user is confronted with a generic error like "permission
check failed" with no indication that it refers to a request made to the
remote PBS instance..
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Currently there's not yet a node config and the WA config is
somewhat "tightly coupled" to the user entries in that
changing it can lock them all out, so for now I opted for
fewer reorganization and just use a digest of the
canonicalized config here, and keep it all in the tfa.json
file.
Experimentally using the flatten feature on the methods with
an`Updater` struct similar to what the api macro is supposed
to be able to derive on its own in the future.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
modifying @pam users credentials should be only possible for root@pam,
otherwise it can have unintended consequences.
also enforce the same limit on user creation (except self_service check,
since it makes no sense during user creation)
Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
Signal does not yet re-implement Stream (and is not yet wrapped in
tokio-stream either).
see https://github.com/tokio-rs/tokio/pull/3383
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
to wrap a Receiver in a Stream. this will likely move back into tokio
proper once we have a std Stream..
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Child itself is no longer a Future, but it has a new wait() async fn
that does the same thing
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
enter() now returns a guard, and the builder got revamped to make the
choice between MT and current thread explicit.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
We always automatically unload tapes to free library slots,
so it should not happen that an ejected tape resides inside the drive.
This is just a safe guard to handle the situation in case it happens ...
You can manually produce the situation by ejecting a tape without unloading:
mt -f /dev/nst0 eject
Note: Our "proxmox-tape eject" does automatic unload
Try to provide generic implementation for complex operations:
- unload_to_free_slot
- load_media
- export media
- clean drive
- online_media_changer_ids
the old variant attempted to parse a tokenid as userid and returned the
cryptic parsing error to the client, which is rather confusing.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Use timeout futures for sections that might hang in certain error
conditions. This is mostly intended to be used as a safeguard, not a
first line of defense - i.e. best-effort avoidance of total hangs.
Not every future used for the HttpClient/H2Client is changed, only those
where a quick response is to be expected. For example, the response
reading futures are left alone, so data transfer is never capped with
timeout, only the initial server connect.
It is also used for upgrading to H2 connections, as that can take a long
time on overloaded servers.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
it seems that sometimes, the child process signal gets handled
before the parent process signal. Systemd then ignores the
childs signal (finished reloading) and only after going into
reloading state because of the parent. this will never finish.
Instead, wait for the state to change to 'reloading' after sending
that signal in the parent, an only fork afterwards. This way
we ensure that systemd knows about the reloading before actually trying
to do it.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-By: Fabian Ebner <f.ebner@proxmox.com>
of ProcessLockSharedGuard.
We use a counter to determine if we can unlock the file again, but
we never actually decremented the writer count, so we held the
lock forever.
This fixes the issue that we could not start a garbage collect after
a reload, as long as the old process is still running, even when that
process has no active backup anymore but another long running task
(e.g. file download, terminal, etc.).
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
document all public things, add some doc links and make some
previously-public things only available for test cases or within the
crate:
previously public, now private:
- AclTreeNode::extract_user_roles (we have extract_roles())
- AclTreeNode::extract_group_roles (same)
- AclTreeNode::delete_group_role (exists on AclTree)
- AclTreeNode::delete_user_role (same)
- AclTreeNode::insert_group_role (same)
- AclTreeNode::insert_user_role (same)
- AclTree::write_config (we have save_config())
- AclTree::load (we have config()/cached_config())
previously public, now crate-internal:
- AclTree::from_raw (only used by tests)
- split_acl_path (used by some test binaries)
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
instead of just logging the error. this should never happen in practice
unless someone is messing with the keyfile, in which case, it's better
to abort.
update tests accordingly (wrong fingerprint should fail, no fingerprint
should get the expected one).
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
the RSA key and the encryption key itself are hard-coded to avoid
stalling the test runs because of lack of entropy, they have no special
significance otherwise.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
when restoring an encrypted key, the original one is obviously not
available to check the fingerprint with.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
this fixes the issue that on some filesystems, you cannot recursively
remove a directory when you hold a lock on a file inside (e.g. nfs/cifs)
it is not really backwards compatible (so during an upgrade, there
could be two daemons have the lock), but since the locking was
broken before (see previous patch) it should not really matter
(also it seems very unlikely that someone will trigger this)
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
'lock_manifest' returns a Result<File, Error> so we always got the result,
even when we did not get the lock, but we acted like we had.
bubble the locking error up
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
if no groups were found, the task log was very confusing as it
contained no real information why nothing was synced, e.g.:
Starting datastore sync job 'remote:datastore:local-datastore:s-79412799-e6ee'
Sync datastore 'local-datastore' from 'remote/datastore'
sync job 'remote:datastore:local-datastore:s-79412799-e6ee' end
TASK OK
this patch simply logs how many groups were found and are about to be synced:
Starting datastore sync job 'remote:datastore:local-datastore:s-79412799-e6ee'
Sync datastore 'local-datastore' from 'remote/datastore'
found 0 groups to sync
sync job 'remote:datastore:local-datastore:s-79412799-e6ee' end
TASK OK
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
percentage of verified groups, interpolating based on snapshot count
within the group. in most cases, this will also be closer to 'real'
progress since added snapshots (those which will be verified) in active
backup groups will be roughly evenly distributed, while number of total
snapshots per group will be heavily skewed towards those groups which
have existed the longest, even though most of those old snapshots will
only be re-verified very infrequently.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
BackupInfo::list_backup_groups is identical code-wise, and makes more
sense as entry point for listing groups.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
WalkDir does not follow symlinks by default anyway, and this behaviour
is not documented anywhere. e.g., if a sysadmin mounts 'extra storage'
for some backup group or type (not knowing that only metadata is stored
in those directories), GC will ignore all the indices contained within
and happily garbage collect their chunks..
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
for safety reason, GC finds and marks all index files below the
datastore base path. as a result of regular operations, only index files
within the expected scheme of <TYPE>/<ID>/<TIMESTAMP> should exist.
add a small check + warning if the index list contains index files out
side of this expected scheme, so that an admin with shell access can
investigate.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
we have messages starting the phases anyway, and limit the number of
progress updates so that context remains available at all times.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
