5
0
mirror of git://git.proxmox.com/git/proxmox-backup.git synced 2025-01-03 01:18:02 +03:00
Commit Graph

247 Commits

Author SHA1 Message Date
Maximiliano Sandoval
d430b05ec3 datastore: docs: escape <uuid>
Fixes the cargo doc lint:

```
warning: unclosed HTML tag `uuid`
  --> pbs-datastore/src/datastore.rs:60:41
   |
60 | ///  - could not stat /dev/disk/by-uuid/<uuid>
   |                                         ^^^^^^
   |
   = note: `#[warn(rustdoc::invalid_html_tags)]` on by default

warning: unclosed HTML tag `uuid`
  --> pbs-datastore/src/datastore.rs:61:26
   |
61 | ///  - /dev/disk/by-uuid/<uuid> is not a block device
```

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
2024-12-04 14:40:42 +01:00
Maximiliano Sandoval
77c81bcb31 datastore: docs: turn uri into hyperlink
Fixes the cargo doc lint:

```
warning: this URL is not a hyperlink
   --> pbs-datastore/src/data_blob.rs:555:5
    |
555 | /// https://github.com/facebook/zstd/blob/dev/lib/common/error_private.h
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: bare URLs are not automatically turned into clickable links
    = note: `#[warn(rustdoc::bare_urls)]` on by default
help: use an automatic link instead
    |
555 | /// <https://github.com/facebook/zstd/blob/dev/lib/common/error_private.h>
    |     +                                                                    +
```

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
2024-12-04 14:40:42 +01:00
Dominik Csapak
8eaeedf31e tree-wide: add missing O_CLOEXEC flags to openat calls
Since we don't want to have lingering file descriptors on any fork +
exec, like the reload code from the proxmox-daemon crate we're using
for the rest-server(s) does, as that can have serious side effects and
even cause hangs.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
 [ TL: Reword commit message ]}
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2024-12-03 16:48:23 +01:00
Maximiliano Sandoval
109e063a7e chunker: do not reassign context's total field
```
warning: field assignment outside of initializer for an instance created with Default::default()
   --> pbs-datastore/src/chunker.rs:431:5
    |
431 |     ctx.total = buffer.len() as u64;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
note: consider initializing the variable with `chunker::Context { total: buffer.len() as u64, ..Default::default() }` and removing relevant reassignments
   --> pbs-datastore/src/chunker.rs:430:5
    |
430 |     let mut ctx = Context::default();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#field_reassign_with_default
    = note: `#[warn(clippy::field_reassign_with_default)]` on by default
```

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
2024-12-03 11:24:37 +01:00
Maximiliano Sandoval
47a29b1896 docs: remove empty lines in doc strings
Fixes the clippy lint:

```
warning: empty line after doc comment
   --> src/tape/pool_writer/mod.rs:441:5
    |
441 | /     /// updated.
442 | |
    | |_
...
448 | /     pub fn append_snapshot_archive(
449 | |         &mut self,
450 | |         snapshot_reader: &SnapshotReader,
451 | |     ) -> Result<(bool, usize), Error> {
    | |_____________________________________- the comment documents this method
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#empty_line_after_doc_comments
    = help: if the empty line is unintentional remove it
help: if the documentation should include the empty line include it in the comment
    |
442 |     ///
    |
```

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
2024-12-03 11:24:37 +01:00
Fabian Grünbichler
77fd1853b3 clippy: use div_ceil to calculate fixed index length
no semantic changes intended

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2024-12-02 11:37:06 +01:00
Fabian Grünbichler
a50e0014df clippy: elide more lifetimes
these were detected with 1.83.0

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2024-12-02 11:34:05 +01:00
Maximiliano Sandoval
cdeed5e440 datastore: simplify let-else block with ? operator
Fixes the question_mark clippy lint:

```
warning: this `let...else` may be rewritten with the `?` operator
   --> pbs-datastore/src/datastore.rs:101:5
    |
101 | /     let Some(ref device_uuid) = config.backing_device else {
102 | |         return None;
103 | |     };
    | |______^ help: replace it with: `let ref device_uuid = config.backing_device?;`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#question_mark
    = note: `#[warn(clippy::question_mark)]` on by default
```

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
2024-12-02 11:15:18 +01:00
Maximiliano Sandoval
f36e8fea91 remove needless borrows
Fixes the needless_borrow lint.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
2024-12-02 11:15:18 +01:00
Fabian Grünbichler
1b4426feec GC: add check for nested datastore
these are particularly problematic since GC will walk the whole datastore tree
on the file system, and will thus pick up indices (but not chunks!) from nested
directories that are ignored in other code paths that use our regular
iterators..

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2024-11-27 15:26:56 +01:00
Shannon Sterz
963401348a datastore: re-phrase error message when datastore is unavailable
the current phrase leads to clumsy log messages such as:

> datastore 'store' is in datastore is being unmounted

this commit re-phrases that too:

> datastore 'store' is unavailable: datastore is being unmounted

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
2024-11-26 16:44:26 +01:00
Hannes Laimer
b17ebd5c2c datastore: handle deletion of removable datastore properly
Data deletion is only possible if the datastore is mounted, won't attempt
mounting it for the purpose of deleting data.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
2024-11-25 21:34:22 +01:00
Hannes Laimer
46d7e573a9 datastore: add helper for checking if a datastore is mounted
... at a specific location. Also adds two additional functions to
get the mount status, and ensuring a removable datastore is mounted.

Co-authored-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
2024-11-25 21:34:22 +01:00
Hannes Laimer
652b774eb0 maintenance: make is_offline more generic
... and add MaintenanceType::Delete to it. We also want to clear any
cach entries if we are deleting the datastore, not just if it is marked
as offline.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
2024-11-25 21:34:22 +01:00
Gabriel Goller
b5be65cf8a snapshot: add helper function to retrieve verify_state
Add helper functions to retrieve the verify_state from the manifest of a
snapshot. Replaced all the manual "verify_state" parsing with the helper
function.

Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
2024-11-25 10:52:40 +01:00
Christian Ebner
6771869cc1 client/server: use dedicated api type for all archive names
Instead of using the plain String or slices of it for archive names,
use the dedicated api type and its methods to parse and check for
archive type based on archive filename extension.

Thereby, keeping the checks and mappings in the api type and
resticting function parameters by the narrower wrapper type to reduce
potential misuse.

Further, instead of declaring and using the archive name constants
throughout the codebase, use the `BackupArchiveName` helpers to
generate the archive names for manifest, client logs and encryption
keys.

This allows for easy archive name comparisons using the same
`BackupArchiveName` type, at the cost of some extra allocations and
avoids the currently present double constant declaration of
`CATALOG_NAME`.

A positive ergonomic side effect of this is that commands now also
accept the archive type extension optionally, when passing the archive
name.

E.g.
```
proxmox-backup-client restore <snapshot> <name>.pxar.didx <target>
```
is equal to
```
proxmox-backup-client restore <snapshot> <name>.pxar <target>
```

The previously default mapping of any archive name extension to a blob
has been dropped in favor of consistent mapping by the api type
helpers.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>

FG: use LazyLock for constant archive names
FG: add missing import

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2024-11-22 13:47:05 +01:00
Christian Ebner
e932ec101e datastore: move ArchiveType to api types
Moving the `ArchiveType` to avoid crate dependencies on
`pbs-datastore`.

In preparation for introducing a dedicated `BackupArchiveName` api
type, allowing to set the corresponding archive type variant when
parsing the archive name based on it's filename.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
2024-11-22 11:45:43 +01:00
Hannes Laimer
cd933e9d69 chunk_store: fix problem with permission checking
Permissions are stored in the lower 9 bits (rwxrwxrwx),
so we have to mask `st_mode` with 0o777.
The datastore root dir is created with 755, the `.chunks` dir and its
contents with 750 and the `.lock` file with 644, this changes the
expected permissions accordingly.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
Fixes: 6e101ff757 ("fix #5439: allow to reuse existing datastore")
Reviewed-By: Gabriel Goller <g.goller@proxmox.com>
2024-11-22 08:49:32 +01:00
Christian Ebner
4e50ef5193 api: datastore/namespace: return backup groups delete stats on remove
Add and optionally expose the backup group delete statistics by adding the
return type to the corresponding REST API endpoints.

Clients can opt-into the new behaviour by setting the new `error-on-protected`
flag to `false` when calling the api endpoints, which results in removal not
erroring out when encountering protected snapshots.

The default value for the flag remains `true` for now, to remain backwards
compatible with older clients expecting this behaviour.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
FG: reworded commit message slightly
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2024-11-21 10:57:09 +01:00
Christian Ebner
0be5b147d5 datastore: increment deleted group counter when removing group
To correctly account also for the number of deleted backup groups, in
preparation to correctly return the delete statistics when removing
contents via the REST API.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
2024-11-21 10:14:53 +01:00
Christian Ebner
d1e5e4533c datastore: move BackupGroupDeleteStats to api types
In preparation for the delete stats to be exposed as return type to
the backup group delete api endpoint.

Also, rename the private field `unremoved_protected` to a better
fitting `protected_snapshots` to be in line with the method names.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
2024-11-21 10:14:53 +01:00
Dietmar Maurer
01bbaef7fa config: factor out method to get the absolute datastore path
removable datastores will have a PBS-managed mountpoint as path, direct
access to the field needs to be replaced with a helper that can account
for this.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
2024-11-17 19:57:33 +01:00
Gabriel Goller
6e101ff757 fix #5439: allow to reuse existing datastore
Disallow creating datastores in non-empty directories. Allow adding
existing datastores via a 'reuse-datastore' checkmark. This only checks
if all the necessary directories (.chunks + subdirectories and .lock)
exist and have the correct permissions. Note that the reuse-datastore
path does not open the datastore, so that we don't drop the
ProcessLocker of an existing datastore.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
2024-11-11 23:51:06 +01:00
Christian Ebner
0a32544585 client: tools: factor out pxar entry to dir entry mapping
Perform the conversion from pxar file entries to catalog entry
attributes by implementing `TryFrom<&FileEntry<T>>` for
`DirEntryAttribute` and use that.

Allows the reuse for the catalog shell, when using the split pxar
archive instead of the catalog.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
2024-10-23 16:10:41 +02:00
Dominik Csapak
8f27262d42 data_blob: add TODO comment for zstd api
we currently use the behavior of zstd that is not part of the public
api, so this is at risk to be changed without notice.

There is a public api that we could use, but it's only available
with zstd_sys >= 2.0.9, which at this time, is not yet packaged for/by
us.

Add a comment that we can use the public api for this when the
new version of the crate gets available.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
2024-08-30 09:57:32 +02:00
Maximiliano Sandoval
a637e7f490 datastore: remove lazy_static dependency
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
2024-08-14 12:08:01 +02:00
Thomas Lamprecht
8caf3f9f57 cargo fmt
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2024-08-07 20:50:28 +02:00
Dominik Csapak
98c4056eaa datastore: data blob encode: simplify code
by combining the compression call from both encrypted and unencrypted
paths and deciding on the header magic at one site.

No functional changes intended, besides reusing the same buffer for
compression.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
2024-08-07 18:58:11 +02:00
Dominik Csapak
aae596ee18 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.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
2024-08-07 18:57:39 +02:00
Dominik Csapak
e1d92bce57 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>
2024-08-07 18:54:04 +02:00
Dominik Csapak
69b8b4b02f datastore: test DataBlob encode/decode roundtrip
so that we can be sure we can decode an encoded blob again

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
2024-08-07 18:52:06 +02:00
Dominik Csapak
b43845aa07 datastore: remove unused data blob writer
This is leftover code that is not currently used outside of its own
tests.

Should we need it again, we can just revert this commit.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
2024-08-07 18:51:09 +02:00
Wolfgang Bumiller
3f325047dc remove use of proxmox_lang::error::io_err_other
by now its functionality is provided by std::io::Error::other

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2024-07-26 13:05:20 +02:00
Christian Ebner
077c1a9979 datastore: data blob: fix typos in comments
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
2024-07-17 14:41:58 +02:00
Christian Ebner
6e3f844f9a 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>
2024-07-12 13:57:59 +02:00
Gabriel Goller
c052040028 datastore: fix typo in comment
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
2024-07-12 13:50:30 +02:00
Gabriel Goller
c1689192d9 datastore: use cached snapshot time string in path
When getting the `full_path` of a snapshot we did not use the cached
time string. By using it we avoid a call to the super-slow libc strftime.

This has some minor performance improvements of circa 7%. That is ~100ms
on my datastore with ~5000 snapshots.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
2024-07-12 13:41:04 +02:00
Wolfgang Bumiller
c4f2bb70da bump sys and rest-server dependencies to 0.6
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2024-07-11 16:02:38 +02:00
Gabriel Goller
7e2486e800 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.

Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
2024-07-11 15:56:04 +02:00
Hannes Laimer
0020601d52 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>
2024-07-02 17:02:38 +02:00
Maximiliano Sandoval
cef764ff85 chunk_store: do not explicitly write implied trait
Fixes the clippy warning:

warning: this bound is already specified as the supertrait of `std::iter::FusedIterator`
   --> pbs-datastore/src/chunk_store.rs:254:14
    |
254 |         impl Iterator<Item = (Result<proxmox_sys::fs::ReadDirEntry, Error>, usize, bool)>
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#implied_bounds_in_impls
    = note: `#[warn(clippy::implied_bounds_in_impls)]` on by default
help: try removing this bound
    |
254 -         impl Iterator<Item = (Result<proxmox_sys::fs::ReadDirEntry, Error>, usize, bool)>
255 -             + std::iter::FusedIterator,
254 +         impl std::iter::FusedIterator<Item = (Result<proxmox_sys::fs::ReadDirEntry, Error>, usize, bool)>,

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
2024-06-28 09:21:06 +02:00
Maximiliano Sandoval
f3f3c67267 replace get(key).is_some() with contains_key()
Fixes the clippy warning:

warning: unnecessary use of `get(realm).is_some()`
  --> pbs-config/src/domains.rs:68:58
   |
68 |     realm == "pbs" || realm == "pam" || domains.sections.get(realm).is_some()
   |                                                          ^^^^^^^^^^^^^^^^^^^^ help: replace it with: `contains_key(realm)`
   |
   = 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>
2024-06-28 09:20:56 +02:00
Wolfgang Bumiller
6359e6d4d4 replace c_str! macro with c"literals"
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2024-06-20 11:07:11 +02:00
Christian Ebner
e11ee319ce chunker: tests: add regression tests for payload chunker
Test chunking of a payload stream with suggested chunk boundaries.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
2024-06-05 16:39:42 +02:00
Christian Ebner
88ef759cc4 datastore: chunker: implement chunker for payload stream
Implement the Chunker trait for a dedicated payload stream chunker,
which extends the regular chunker by the option to suggest boundaries
to be used over the hast based boundaries whenever possible.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
2024-06-05 16:39:42 +02:00
Christian Ebner
e321815635 datastore: chunker: add Chunker trait
Add the Chunker trait and move the current Chunker to ChunkerImpl to
implement the trait instead. This allows to use different chunker
implementations by dynamic dispatch and is in preparation for
implementing a dedicated payload chunker.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
2024-06-05 16:39:42 +02:00
Christian Ebner
77fdae28cf chunker: add method to reset chunker state
When forcing a boundary, the internal chunker state is not in sync
with the chunk stream anymore. The reset method therefore allows
to reset the internal state.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
2024-06-05 16:39:41 +02:00
Christian Ebner
9978f6934b datastore: dynamic index: add method to get digest
In preparation for injecting reused payload chunks in payload streams
for regular files with unchanged metaddata. Allows to get the digest
of a dynamic index entry to construct a reusable dynamic entry from
it.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
2024-06-05 10:47:36 +02:00
Dietmar Maurer
50c0246a89 api: assert that maintenance mode transitions are valid
Maintenance mode Delete locks the datastore. It must not be possible to go
back to normal modes, because the datastore may be in undefined state.

Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
2024-04-22 12:19:22 +02:00
Christian Ebner
524ed4048b datastore: group: return basic stats on backup group destroy
No functional change intended: In preparation for including the
removed vanished groups and snapshots statistics in a sync jobs task
log output.

Instead of returning a boolean value showing whether all of the
snapshots of the group have been removed, return an instance of
`BackupGroupDeleteStats`, containing the count of deleted and
protected snapshots, the latter not having been removed from the
group.

The `removed_all` method is introduced as replacement for the previous
boolean return value and can be used to check if all snapshots have
been removed. If there are no protected snapshots, the group is
considered to be deleted.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2024-03-25 17:17:12 +01:00