Some systemd code got split out from proxmox-sys and left there
re-exported with a deprecation marker, use the newer crate, the
workspace already depends on proxmox-systemd anyway.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Some systemd code got split out from proxmox-sys and left there
re-exported with a deprecation marker, use the newer crate, the
workspace already depends on proxmox-systemd anyway.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Some systemd code got split out from proxmox-sys and left there
re-exported with a deprecation marker, use the newer crate, the
workspace already depends on proxmox-systemd anyway.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Dominik Csapak [Mon, 5 Aug 2024 09:24:13 +0000 (11:24 +0200)]
datastore: data blob: increase compression throughput
Increase the zstd compression throughput by not using the
`zstd::stream::copy_encode` method, because it seems it uses an
internal buffer size of 32 KiB [0], copies at least once extra in the
target buffer and might have some additional (allocation and/or
syscall) overhead. Due to the amount of wrappers and indirections it's
a bit hard to tell for sure. In anyway, there can be a reduced
throughput observed if all, the target and source storage and the
network are so fast that the operations from creating chunks, like
compressions, can become the bottleneck.
Instead use the lower-level `zstd_safe::compress` which avoids (big)
allocations, since we provide the target buffer.
In case of a compression error just return the uncompressed data,
there's nothing we can do and saving uncompressed data is better than
having none. Additionally, log any such error besides the one for the
target buffer being too small.
Some benchmarks on my machine (Intel i7-12700K with DDR5-4800 memory
using a ASUS Prime Z690-A motherboard) from a tmpfs to a datastore on
tmpfs:
Type without patches (MiB/s) with patches (MiB/s)
.img file ~614 ~767
pxar one big file ~657 ~807
pxar small files ~576 ~627
The new approach is faster by a factor of 1.19.
Note that the new approach should not have a measurable negative
impact, e.g. (peak) memory usage wise. That is because we always
reserved a vector with max-data-size (data length + header length) and
thus did not have to add a new buffer, rather we actually removed the
buffer that the high-level zstd wrapper crate used internally.
Dominik Csapak [Mon, 5 Aug 2024 09:24:12 +0000 (11:24 +0200)]
datastore: data blob: allow checking for zstd internal buffer-to-small error
We want to check the error code of zstd not to be 'Destination buffer
to small' (dstSize_tooSmall), but currently there is no practical API
that is also public. So we introduce a helper that uses the internal
logic of zstd to determine the error.
Since this is not guaranteed to be a stable api, add a test for that
so we catch that error early on build. This should be fine, as long as
the zstd behavior only changes with e.g. major debian upgrades, which
is normally the only time where the zstd version is updated.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
[ TL: re-order fn, rename test and reword comments ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
by dropping the print-per-chunk and making the input buffer size configurable
(8k is the default when using `new()`).
this allows benchmarking various input buffer sizes. basically the same code is
used for image-based backups in proxmox-backup-client, but just the
reading and chunking part. looking at the flame graphs the smaller input
buffer sizes clearly show most of time spent polling, instead of
reading+copying (or reading and scanning and copying).
for a fixed chunk size stream with a 16G input file on tmpfs:
fixed 1M ran
1.06 ± 0.17 times faster than fixed 4M
1.22 ± 0.11 times faster than fixed 16M
1.25 ± 0.09 times faster than fixed 512k
1.31 ± 0.10 times faster than fixed 256k
1.55 ± 0.13 times faster than fixed 128k
1.92 ± 0.15 times faster than fixed 64k
3.09 ± 0.31 times faster than fixed 32k
4.76 ± 0.32 times faster than fixed 16k
8.08 ± 0.59 times faster than fixed 8k
(from 15.275s down to 1.890s)
dynamic chunk stream, same input:
dynamic 4M ran
1.01 ± 0.03 times faster than dynamic 1M
1.03 ± 0.03 times faster than dynamic 16M
1.06 ± 0.04 times faster than dynamic 512k
1.07 ± 0.03 times faster than dynamic 128k
1.12 ± 0.03 times faster than dynamic 64k
1.15 ± 0.20 times faster than dynamic 256k
1.23 ± 0.03 times faster than dynamic 32k
1.47 ± 0.04 times faster than dynamic 16k
1.92 ± 0.05 times faster than dynamic 8k
(from 26.5s down to 13.772s)
same input file on ext4 on LVM on CT2000P5PSSD8 (with caches dropped for each run):
fixed 4M ran
1.06 ± 0.02 times faster than fixed 16M
1.10 ± 0.01 times faster than fixed 1M
1.12 ± 0.01 times faster than fixed 512k
1.15 ± 0.02 times faster than fixed 128k
1.17 ± 0.01 times faster than fixed 256k
1.22 ± 0.02 times faster than fixed 64k
1.55 ± 0.05 times faster than fixed 32k
2.00 ± 0.07 times faster than fixed 16k
3.01 ± 0.15 times faster than fixed 8k
(from 19.807s down to 6.574s)
dynamic 4M ran
1.04 ± 0.02 times faster than dynamic 512k
1.04 ± 0.02 times faster than dynamic 128k
1.04 ± 0.02 times faster than dynamic 16M
1.06 ± 0.02 times faster than dynamic 1M
1.06 ± 0.02 times faster than dynamic 256k
1.08 ± 0.02 times faster than dynamic 64k
1.16 ± 0.02 times faster than dynamic 32k
1.34 ± 0.03 times faster than dynamic 16k
1.70 ± 0.04 times faster than dynamic 8k
`cargo build` and `cargo install` pick up different config files, by symlinking
the wrapper config into a place with higher precedence than the one in the
top-level git repo dir, we ensure the package build actually picks up the
desired config instead of the one intended for quick dev builds.
Christian Ebner [Mon, 15 Jul 2024 10:15:43 +0000 (12:15 +0200)]
server: pull: fix sync info message for root namespace
The root namespace is displayed as empty string when used in the
format string. Distinguish and explicitly write out the root namespace
in the sync info message shown in the sync jobs task log.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
fix #3699: client: prefer xdg cache directory for tmp files
Adds a helper to create temporal files in XDG_CACHE_HOME. If we cannot
create a file there, we fallback to /tmp as before.
Note that the temporary files stored by the client might grow
arbitrarily in size, making XDG_RUNTIME_DIR a less desirable option.
Citing the Arch wiki [1]:
> Should not store large files as it may be mounted as a tmpfs.
While the cache directory is most often not backed up by an ephemeral
FS, using the `O_TMPFILE` flag avoids the need for potential cleanup,
e.g. on interruption of a command. As with this flag set the data will
be discarded when the last file descriptor is closed.
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
[ TL: mention TMPFILE flag for clarity ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Wed, 17 Jul 2024 09:44:40 +0000 (11:44 +0200)]
bump apt-api-types dependency to 1.0.1
to pull in the fix for restoring backwards compatibility due to the
digest from that crate using a u8 slice instead of our dedicated
ConfigDigest type, which would serialize to String.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Christian Ebner [Wed, 3 Jul 2024 10:11:00 +0000 (12:11 +0200)]
datastore: replace deprecated `archive_type` function
Commit ea584a75 "move more api types for the client" deprecated
the `archive_type` function in favor of the associated function
`ArchiveType::from_path`.
Replace all remaining callers of the deprecated function with its
replacement.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
[WB: and remove the deprecated function] Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
The protected status of the snapshot is retrieved twice. This is slow
because it stat's the .protected file multiple times.
Signed-off-by: Gabriel Goller <g.goller@proxmox.com> Tested-by: Christian Ebner <c.ebner@proxmox.com> Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
Gabriel Goller [Tue, 9 Jul 2024 14:20:13 +0000 (16:20 +0200)]
api: switch from task_log! macro to tracing
Import `proxmox-log` and substitute all `task_log!`
(and task_warn!, task_error!) invocations with tracing calls (info!,
warn!, etc..). Remove worker references where it isn't necessary
anymore.
Gabriel Goller [Tue, 9 Jul 2024 14:20:12 +0000 (16:20 +0200)]
switch from task_log! macro to tracing
Import `proxmox-log` and substitute all `task_log!`
(and task_warn!, task_error!) invocations with tracing calls (info!,
warn!, etc..). Remove worker references where it isn't necessary
anymore.
Gabriel Goller [Wed, 19 Jun 2024 10:15:44 +0000 (12:15 +0200)]
backup_manager: use confirmation helper in wipe-disk command
Use `Confirmation` helper in the wipe-disk command prompt.
Improves: 887d83cb (cli: add interactive confirmation for block device wipe, 2023-11-29) Cc: Markus Frank <m.frank@proxmox.com> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
Hannes Laimer [Thu, 25 Apr 2024 09:01:50 +0000 (11:01 +0200)]
datastore: fix problem with operations counting
... if `.chunks/` is not available(deleted/moved) ChunkStore::open
fails, but that would happen after updating the active operations on the
datastore, so no reference that could be dropped is returned. Leading to
the operations counter to always increase. This only updates the counter
when a reference is returned, not before.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
Hannes Laimer [Mon, 24 Jun 2024 07:54:38 +0000 (09:54 +0200)]
http_client: keep renewal future running on failed re-auth
The re-authentication request can also fail due to network instability,
and not necesarrily only due to an invalid ticket. In that case it makes
sense to retry refreshing the ticket in 15 minutes. Also, the future does
not depend on a failed re-authentication to be clean up properly, so that
happens already somewhere else, therefore we don't rely on this return
anyway. If the ticket is actually invalid or timed out, the main job
will fail and also terminate the renewal future, same applies if the
network is not just unstable but straight up not working.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com> Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Christian Ebner [Tue, 30 Apr 2024 15:37:14 +0000 (17:37 +0200)]
fix 5304: client: set process uid/gid for .pxarexclude-cli
The .pxarexclude-cli encodes the exclude patterns the client was
invoked with in the pxar archive as regular file entry. The current
behaviour of setting the uid and gid to default 0 (root) causes
however issues when trying to backup and restore the backup as
non-root user.
Opt for using the uid/gid of the user the executable was called as,
allowing the restore for this user to succeed. Root will succeed
to restore anyways.
Link to issue in bugtracker:
https://bugzilla.proxmox.com/show_bug.cgi?id=5304
Signed-off-by: Christian Ebner <c.ebner@proxmox.com> Tested-by: Gabriel Goller <g.goller@proxmox.com>
Gabriel Goller [Tue, 18 Jun 2024 14:03:33 +0000 (16:03 +0200)]
client: mount: wait for child to return before exiting
When using the `proxmox-backup-client mount` command, the parent sometimes
exits before we can print any error message. Most notably this happens
when no PBS_REPOSITORY is passed, as this is the first option checked.
If the underlying file descriptor has been closed, wait for the client
to complete and return the error message.
Reported-by: Friedrich Weber <f.weber@proxmox.com> Suggested-by: Christian Ebner <c.ebner@proxmox.com> Signed-off-by: Gabriel Goller <g.goller@proxmox.com> Tested-by: Friedrich Weber <f.weber@proxmox.com>
Christian Ebner [Tue, 2 Jul 2024 07:24:14 +0000 (09:24 +0200)]
close #5571: client: fix regression for `map` command
Commit 08fe5052 introduced functionality to mount split pxar archives
(sharing code with the map command), moving the manifest lookup
exclusive to fixed index archives.
However, the lookup now uses the incorrect archive name, not
containing the `.fidx` extension, which is however required for the
lookup in the manifest.
Fix the issue by calling the method with the correct server archive
name including the required extension.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com> Tested-by: Gabriel Goller <g.goller@proxmox.com> Reviewed-by: Gabriel Goller <g.goller@proxmox.com> Fixes: 08fe5052 ("client: mount: make split pxar archives mountable")
[FG: reworded, add proper "Fixes:" trailer.] Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
it builds about 1.5 times faster than regular `make deb` (shaving off a
whopping 100s on my machine). the resulting debs containing executables are of
course bigger (since the debug symbols are not split out into their own
package, and the ELF linkage stripping is also skipped), but other than the
associated file and memory mapping overhead there should be no difference in
behaviour or performance, and such debs are suitable for local testing (both of
the build process, and the built code).
HashMap::remove() returns the value it removes as an Option<>, so
instead of first checking if the key exists before removing it, just
try to remove it and use the returned Option<> to test whether we
should bail!().
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
warning: this multiplication by -1 can be written more succinctly
--> pbs-client/src/tools/mod.rs:700:58
|
700 | SignedDuration::Negative(val) => -1 * i64::try_from(val.as_secs())?,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `-i64::try_from(val.as_secs())?`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#neg_multiply
= note: `#[warn(clippy::neg_multiply)]` on by default
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
warning: unnecessary use of `get(&user2).is_none()`
--> pbs-config/src/acl.rs:1067:36
|
1067 | assert!(node.users.get(&user2).is_none());
| -----------^^^^^^^^^^^^^^^^^^^^^
| |
| help: replace it with: `!node.users.contains_key(&user2)`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_get_then_check
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
pbs2to3 was missing from the list of to-be-compiled binaries, and thus was only
compiled as a side-effect of running `cargo test` (which is skipped when the
build is using the `nocheck` build profile).
warning: Cargo.toml: `default_features` is deprecated in favor of `default-features` and will not work in the 2024 edition
(in the `proxmox-router` dependency)
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
else we don't pick up the options set by the wrapper, which include generation
of debug symbols. until rustc 1.77, this was not needed because compiled
binaries always included a non-stripped libstd. now, without this change, the
binaries built with `cargo build --release` have no debug symbols at all
trigger a warning. fix this and include debug symbols when building a package,
like was originally intended for release package builds.
Gabriel Goller [Mon, 3 Jun 2024 08:43:11 +0000 (10:43 +0200)]
close #4763: client: add command to forget backup group
Add the command `proxmox-backup-client group forget <group>` so
that we can forget (delete) whole groups with all the containing
snapshots.
To avoid printing full datastore paths (which are in the error messages)
we filter out the most common one (group not found) and rephrase it.
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
[WB: rebased & sorted import statements in client's main.rs]
[WB: replace extract_repository_from_value with
remove_repository_from_value since the parameter is rejected on
the remote side] Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
'extract_repository_from_value' takes an immutable reference and
doesn't remove the parsed parameter (whereas in contrast in our PVE
codebase, the 'extract_param' method does remove it).
This adds a variant that explicitly removes it called
'remove_repository_from_value'.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Gabriel Goller [Tue, 18 Jun 2024 14:22:03 +0000 (16:22 +0200)]
pxar: use anyhow::Error in PxarBackupStream
Instead of storing the error as a string in the PxarBackupStream, we
store it as an anyhow::Error. As we can't clone an anyhow::Error, we take
it out from the mutex and return it. This won't change anything as
the consumation of the stream will stop if it gets a Some(Err(..)).
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
Gabriel Goller [Tue, 18 Jun 2024 14:22:02 +0000 (16:22 +0200)]
pxar: add UniqueContext helper
To create a pxar archive, we recursively traverse the target folder.
If there is an error further down and we add a context using anyhow,
the context will be duplicated and we get an output like:
> Error: error at "xattr/xattr.txt": error at "xattr/xattr.txt": E2BIG [skip]
This is obviously not optimal, so in recursive contexts we can use the
UniqueContext, which quickly checks the context from the last item in
the error chain and only adds it if it is unique.
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
Gabriel Goller [Tue, 18 Jun 2024 14:22:01 +0000 (16:22 +0200)]
pxar: remove ArchiveError
The sole purpose of the ArchiveError was to add the file-path to the
error. Using anyhow::Error we can add this information using the context
and don't need this struct anymore.
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
Thomas Lamprecht [Tue, 18 Jun 2024 14:12:14 +0000 (16:12 +0200)]
ui: gc job edit: fix i18n gettext usage
String concatenating a variable with some static text as gettext
parameter cannot really work, and it also does not make sense to do
most of the time, as even if we'd use some overly generic format
string like '{0} (disabled)', it would be not easy to translate
correctly in all languages in such a generic way.
So just use the actual full string, which is already contained in our
translation catalogue anyway…
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Mon, 17 Jun 2024 15:06:55 +0000 (17:06 +0200)]
docs: drop blanket statement recommending against remote storage
This is basically semantic revert of e5c0d80c ("docs: add note for not
using remote storages") that, while well intended, has a few problems,
e.g.:
- This is the minimal/recommended requirements section, which should
list the rough basic specs a setup must/should have. Listing
everything that is not best to do would bloat this list
significantly and it's just the wrong place for it, i.e., it isn't a
recommended against list.
- while it's true that a remote storage will basically always have
_some_ overhead over using the same HW with a (modern) local storage
(file) system, that does **not** mean that the remote storage has
insufficient performance characteristics. We know of lots of fast
Ceph setups, even release benchmarks for them, or storages like
BlockBridge, that provide high performance while being remote.
So avoid this X-Y-problem style argumentation and focus on what is
actually important, even though I naturally get that there are some
users that use slow NFS attached storages, but breaking style here
won't cure them and I'm sure that they are capable of setting up such
a slow local storage that it won't make a real difference compared to
the NFS one.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Christian Ebner [Wed, 12 Jun 2024 13:17:13 +0000 (15:17 +0200)]
client: pxar: fix fuse mount performance for split archives
Adapt to the decoder/accessor method changes introduced in the pxar
library, which were introduced in order to move the consistency check
for metadata and payload data archives.
The new location of the checks allows to access the pxar archive via
a `Split` variant reader instance, without penalization when just
accessing the metadata, not reading any payload data.
This greatly improves performance when accessing fuse mounted
archives.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
bumped dependency after pxar version bump
projects / proxmox-backup.git / log