Commit Graph

163 Commits

Author SHA1 Message Date
Thomas Lamprecht
f1fe7a0733 file set contents: fix error handling with or-operator precedence
In perl the `or` and the `||` operator do mostly the same thing but
with a different precedence level [0].

A statement like:
`$foo += bar() or die "error"`
is basically equivalent to:
`($foo += bar()) or die "error"`

That means as long as bar only returns zero or positive integers the
`or die` can only happen the first time, as otherwise $foo is bigger
than zero and thus will never evaluate to false. This can be
reproduced by perl -we 'my $foo = 1; $foo += 0 or die "wont happen";'

While one could switch to the `||` operator, this is a bit to subtle,
so to fix this, separate tracking the total bytes written from getting
the bytes written by the current call, this avoids the error potential
completely.

[0]: https://perldoc.perl.org/perlop#Logical-or-and-Exclusive-Or

Reported-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2024-10-14 11:14:23 +02:00
Filip Schauer
ef0bcc98ad tools: file_set_contents: use syswrite instead of print
The use of `print` can be inefficient for writing larger files due to
its default buffering in 8 KiB blocks.

This is especially problematic on `pmxcfs` where files are written in
4 KiB blocks due to the defaults of `libfuse2`. This leads to
significant write amplification on files larger than 4 KiB.

Patch (fix #5728: pmxcfs: allow bigger writes than 4k for fuse) [1]
addresses this by enabling `big_writes`, allowing up to 128 KiB blocks.
But due to the use of `print` in `file_set_contents`, writes are still
only buffered in 8 KiB blocks.

To further address this, this commit switches to using `syswrite`
instead of `print` to mitigate the block size limit imposed by `print`.
Combined with patch [1], file writes to `/etc/pve/` are now buffered in
128 KiB blocks.

The table below illustrates the drastic reduction in write
amplification when writing files of different sizes to `/etc/pve/` using
`file_set_contents`:

           print                big_writes+print     big_writes+syswrite
file size  written     amplif.  written     amplif.  written    amplif.
    1 KiB      48 KiB     48.0      45 KiB     45.0     41 KiB     41.0
    2 KiB      48 KiB     24.0      45 KiB     22.5     62 KiB     31.0
    4 KiB      82 KiB     20.5      80 KiB     20.0     73 KiB     18.3
    8 KiB     121 KiB     15.1      90 KiB     11.3     89 KiB     11.1
   16 KiB     217 KiB     13.6     146 KiB      9.1    113 KiB      7.1
   32 KiB     506 KiB     15.8     314 KiB      9.8    158 KiB      4.9
   64 KiB    1472 KiB     23.0     826 KiB     12.9    259 KiB      4.0
  128 KiB    5585 KiB     43.6    3765 KiB     29.4    452 KiB      3.5
  256 KiB   20424 KiB     79.8   10743 KiB     42.0   2351 KiB      9.2
  512 KiB   86715 KiB    169.4   43650 KiB     85.3   3204 KiB      6.3
 1024 KiB  369568 KiB    360.9  187496 KiB    183.1  15845 KiB     15.5

Since `file_set_contents` also performs a `rename` after writing, the
following table shows the results when the file is written without
renaming it afterwards:

           print                big_writes+print     big_writes+syswrite
file size  written     amplif.  written     amplif.  written     amplif.
    1 KiB      29 KiB     29.0      29 KiB     29.0     25 KiB      25.0
    2 KiB      29 KiB     14.5      30 KiB     15.0     25 KiB      12.5
    4 KiB      37 KiB      9.3      44 KiB     11.0     41 KiB      10.3
    8 KiB      61 KiB      7.6      45 KiB      5.6     45 KiB       5.6
   16 KiB     143 KiB      8.9      86 KiB      5.4     57 KiB       3.6
   32 KiB     396 KiB     12.4     225 KiB      7.0     69 KiB       2.2
   64 KiB    1281 KiB     20.0     673 KiB     10.5    105 KiB       1.6
  128 KiB    4789 KiB     37.4    3478 KiB     27.2    169 KiB       1.3
  256 KiB   18868 KiB     73.7    9976 KiB     39.0    572 KiB       2.2
  512 KiB   79304 KiB    154.9   42714 KiB     83.4   2150 KiB       4.2
 1024 KiB  347929 KiB    339.8  182483 KiB    178.2  11133 KiB      10.9

[1] https://lists.proxmox.com/pipermail/pve-devel/2024-September/065396.html

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
2024-10-14 10:23:50 +02:00
Dominik Csapak
06f436f126 fix #5486: tools: encode_text: add '%' to list of encoded characters
all text that is going through encode_text will at a later point be
decoded by 'decode_text'. The latter is decoding all percent encoded
characters, even those not originally encoded by 'encode_text'.

This means, to preserve the original data, we first have to at least
percent encode the '%' itself, otherwise it's impossible to properly
store e.g. '%20' there.

It would get saved as '%20' directly, but on the next read, it gets
decoded to ' ', which is not the original data. instead we have to save
it as '%2520', which gets then correctly decoded to '%20' again

This is especially important for the vm/ct/node description, as there
users can store external links, which already include percent encoded
characters.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
2024-07-04 10:54:48 +02:00
Jing Luo via pve-devel
6dc7a73bd5 tools: fix syscall mknod()
b792e8df81 introduced a bug that can cause this:

Undefined subroutine &PVE::Syscall::SYS_mknod called at /usr/share/perl5/PVE/Syscall.pm line 11

It should be mknod, not SYS_mknod. This caused other pve perl lib failing
to build. I couldn't reproduce this on amd64 build, but I could reproduce this
on arm64 build; however this didn't seem to fix the issue, unless I revert
b792e8df81.

cf: b792e8df81
Signed-off-by: Jing Luo <jing@jing.rocks>
2024-07-03 10:25:21 +02:00
Dominik Csapak
15645af168 tools: add is_deeply
to compare nested hashes/lists and scalar values recursively.
Also includes some tests

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-11-17 09:24:09 +01:00
Filip Schauer
fe468fad74 tools: Add mount flag constants
Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
2023-11-13 15:08:58 +01:00
Filip Schauer
b792e8df81 tools: Add mknod syscall
Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
2023-11-13 15:08:58 +01:00
Gabriel Goller
a992ba134c fix #4162: added Auto-Submitted header to email body
`Auto-Submitted` is defined in the rfc 5436 [1] and describes how
an automatic response (f.e. ooo replies, etc.) should behave on the
emails. When using `Auto-Submitted: auto-generated` (or any value
other than `none`) automatic replies won't be triggered.

[1]: https://www.rfc-editor.org/rfc/rfc3834.html

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
2023-11-06 18:39:48 +01:00
Lukas Wagner
2943910ec3 tools: allow to force UTF-8 encoding for file_set_contents
Rationale: This is used from cfs_write_file, which is now also used to
write utf8-encoded strings that come from Rust. If no encoding is
specified while writing the file, we run into problems with certain
special characters (e.g. 'ü').

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
[FG: use flag parameter instead of encoding as a string
     use stricter 'UTF-8' instaed of 'utf8' (see 'perldoc Enocode')]
Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
[FE: implement changes suggested by Fabian
     move binmode call to where $fh is known to be set]
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-09-11 13:42:07 +02:00
Fiona Ebner
41ed439635 run fork with timeout: only special case timeout error in list context
run_with_timeout() will treat a timeout error differently when called
in list context and run_fork_with_timeout() should do the same. Ensure
this by calling run_with_timeout() in list context if and only if
run_fork_with_timeout() is called in list context too.

Fixes: a6aa0ae ("run with timeout: return if timeout happened in list context")
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-08-30 16:44:40 +02:00
Fiona Ebner
eac8b4b872 run with timeout: only special-case timeout error in list-context
and not other errors too.

Fixes: a6aa0ae ("run with timeout: return if timeout happened in list context")
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-08-30 16:44:40 +02:00
Philipp Hufnagl
4bb9bfe70b fix whitespaces
Signed-off-by: Philipp Hufnagl <p.hufnagl@proxmox.com>

FG: removed hunks that changed alignment..

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2023-08-04 13:47:10 +02:00
Fabian Grünbichler
0bf2e89a39 download file from url: improve cleanup
don't attempt cleanup if temp files don't exist (anymore, or not yet).

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2023-08-04 13:47:10 +02:00
Fabian Grünbichler
a4df83987b download file from url: simplify error handling
the top-level error handling ensures the temporary downloaded file gets
removed in case of an error, so there is no need to also handle that when
decompression fails..

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2023-08-04 12:50:40 +02:00
Fabian Grünbichler
bf8f0ca200 download file from url: fix indentation
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2023-08-04 12:48:23 +02:00
Philipp Hufnagl
7c3e155b28 fix #4849: download file from url: add opt parameter for a decompression command
Signed-off-by: Philipp Hufnagl <p.hufnagl@proxmox.com>
2023-08-04 12:45:35 +02:00
Thomas Lamprecht
a6aa0ae945 run with timeout: return if timeout happened in list context
This can be relevant info do differentiate if an undef return value
happened due to the closure returning it or if it happened due to a
timeout.

While for quite a few cases this could be handled by a
variable captured by the passed closure code reference, acting as
messenger, that might often require needless wrapping.

Also run_fork_with_timeout warned errors of execution, but any such
error handling for an actual timeout is better handled at the call
site, as a context-less "got timeout" at STDERR or journal is really
not helpful.

I checked all call sites of both, run_fork_with_timeout and
run_with_timeout most do not use the result at all, and the ones that
do are in scalar context.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-07-01 18:45:11 +02:00
Thomas Lamprecht
1ac0a30a0a read firstline: only map ENOENT to undef, raise error otherwise
Errors like permission denied or I/O ones should bubble up, otherwise
it might hide serious issues and seemingly continue to work, with a
wrong state or the like.

One could argue that the case for not existent should return undef,
while an empty file should return an empty string, but for that we
might want to check all use-sites first.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-13 07:16:40 +02:00
Wolfgang Bumiller
4cb946a81c fix #4671: use O_DIRECTORY from Fcntl
on ARM this flag has a different value, let's not hardcode
it.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2023-04-18 09:52:31 +02:00
Christian Ebner
12a0ec1888 tools: Add callback based filtering for logfile dump
This patch introduces callback based filtering functionality for logfile dumps.
Further, the `dump_logfile` function is split into a reusable part for dumps
generated based on a filehandle. The state parameter can be used to keep the
state for multiple consecutive function invocations.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
2023-01-18 11:13:35 +01:00
Thomas Lamprecht
7b1aa2e84a dump logfile: avoid boolean ternary if already boolean value
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-24 17:12:18 +01:00
Daniel Tschlatscher
f0c1b0c03c dump logfile: return whole log file if limit is 0
The dump_logfile now returns the whole log file if the limit
parameter is set to 0. This must be done explicitly though, as in the
case of 'limit' being undefined, the default as before, 50 will be
used.

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
2022-11-24 17:12:02 +01:00
Wolfgang Bumiller
6647801cb3 tools: use int() on all integer syscall parameters
this should fix an issue where users with custom id mappings
get bad ownership on intermediate directories caused by the
rootuid/gid being the string "100000" in perl instead of the
number 100000...

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2022-05-20 09:47:34 +02:00
Thomas Lamprecht
c663330d85 tools: add fixme comment for waiting on uninterruptible processes
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-05-20 09:47:27 +02:00
Thomas Lamprecht
a45a1df1ed small code/comment cleanups
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-01-13 14:50:14 +01:00
Thomas Lamprecht
d9339d016a getxattr: trim the returned buffer to the correct size
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-11-08 16:19:54 +01:00
Thomas Lamprecht
d94f7005ce safe_read_from: bump default size limit to 1 MiB to match pmxcfs
Done in a similar spirit as commit 8fb28ab914

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-10-21 11:40:27 +02:00
Thomas Lamprecht
c1e4c83ceb tools: getxattr: document how to get actual argument size
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-10-19 09:33:37 +02:00
Thomas Lamprecht
2e14735a84 tools: getxattr: drop debug statement
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-10-19 09:24:53 +02:00
Thomas Lamprecht
4c0c5c905d tools: add set/get xattr methods
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-10-19 08:18:00 +02:00
Thomas Lamprecht
85237c0b68 tempfile: add some comment
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-10-15 11:25:09 +02:00
Thomas Lamprecht
9cccad5e3e tempfile: improve base path selection
The path is not /that/ relevant privacy wise as we try to use
`O_TMPFILE` anyway and defaulting to /run generates trouble for calls
from non-root processes.

Try the user session run dir first, then /run if root or /tmp else.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-10-15 11:23:24 +02:00
Thomas Lamprecht
9915a41bb6 tools: sendmail: code cleanup, factor out some noise
Reduce by a few lines in general and move out checking the address to
avoid to much (repeated) inline noise..

no semantic change intended.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-10-15 11:22:41 +02:00
Thomas Lamprecht
7ac222d137 tools: fix some perlcritic lints
- Two-argument "open" used at line 462, column 3.  See page 207 of
  PBP. (Severity: 5)
- Subroutine "new" called using indirect syntax at line 487, column
  15. See page 349 of PBP.  (Severity: 5)
- Bareword file handle opened at line 1533, column 5. See pages 202,
  204 of PBP.  (Severity: 5)

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-10-15 10:46:06 +02:00
Thomas Lamprecht
b296c4dd81 tools: fix typo in comment
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-09-20 16:52:03 +02:00
Thomas Lamprecht
ae54eabff9 tools: followup: fix comment length and rename to upid_normalize_status_type
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-06-28 14:51:54 +02:00
Dominik Csapak
4e5360384c PVE/Tools: add 'upid_get_status_type'
as a single point where we get the type of upid status

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
2021-06-28 14:47:53 +02:00
Fabian Ebner
4cc5b13dfc tools: add upid_status_is_error function
There's also support for ending a task with warnings now, so the logic "status
not 'OK' means error" does not work anymore.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2021-06-17 15:22:56 +02:00
Thomas Lamprecht
06c1c13f1c tools: download from url: add option to allow overriding existing files
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-06-16 12:43:36 +02:00
Lorenz Stechauner
f52ecff957 tools: download_file_from_url: move check for existing file outside eval
it is not necessary to include this block in the eval which when it
fails tries to unlink $tmpdest, because in the check for the existing
file $tmpdest is not used.
2021-06-16 12:14:52 +02:00
Lorenz Stechauner
43cb80c5f2 tools: download_file_from_url: adapt error messages to start at new line
the front end expects the error message to be the first part of the
last line. putting the new line at the beginning of the die message
does not work, either.

https://lists.proxmox.com/pipermail/pve-devel/2021-June/048676.html
2021-06-16 12:14:52 +02:00
Lorenz Stechauner
eca898c0c1 tools: download_file_from_url: fix typo 2021-06-15 16:24:16 +02:00
Wolfgang Bumiller
bd9eb367a0 Syscalls/Tools: add renameat2
Mostly for the ability to atomically swap files.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2021-06-15 14:35:26 +02:00
Thomas Lamprecht
2531c455e8 tools, rest env: sort use statements
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-06-15 14:24:47 +02:00
Thomas Lamprecht
dc4bc96960 tools: get_file_hash: add use statements for Digest module
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-06-15 14:22:18 +02:00
Thomas Lamprecht
3a94648515 tools: download_file_from_url: handle interrupts
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-06-15 14:20:49 +02:00
Thomas Lamprecht
60fb1c2628 tools: download_file_from_url: improve UX and avoid cyclic dependencies
plus some refactoring

* drop worker, cannot be done here (RPCEnv is in pve-access-control)
* actually output the wrong "got" hash on mismatch
* die on existing file with mismatched
* drop double array for passing cmd
* drop `/usr/bin` prefix
* adapt rename error message
* add error handling for unlinking the temp. file

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-06-15 14:11:09 +02:00
Thomas Lamprecht
189f0321ca tools: cleanup usage line
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-06-15 13:12:58 +02:00
Lorenz Stechauner
a3327ea6fb tools: add download_file_from_url
adds a common function to download arbitrary files from urls.

code is based on
manager:PVE/API2/Nodes.pm:aplinfo

Security notice: this function does not perform any permission
checking. The callee has to make sure, that only authorized users may
use this function.

Caution: This function is able to download files from internal
networks (which would not be visible/accessible from outside), the
callee needs to ensure that unprivileged (e.g., non root@pam or the
like) can only pass OK URLs (e.g., resolving to public routable IPs)

Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-06-15 10:21:24 +02:00
Fabian Ebner
ff79ee6596 allow workers to count warnings and finish tasks in a WARNINGS state
as is already supported by the UI (and PBS).

A nice bonus is that warn() can be used by both workers and non-workers. For
workers, the output is redirected/duplicated as set up by {fork,tee}_worker(),
and non-erroring workers that issued a warning will end in a WARNINGS state.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2021-04-23 14:35:55 +02:00