157 Commits

Author SHA1 Message Date
Fabian Ebner
b6cf9f1576 vzdump: pbs: suppress output from upload-log command
which ended up in the backup task log and can be confusing to users:

> INFO: Backup finished at 2021-10-11 08:40:38
> Result: {
>       "data": null
> }
> INFO: Backup job finished successfully

For proxmox-backup-server < 2.0.11-1, there was no output for the
upload-log command.

Reported in the community forum:
https://forum.proxmox.com/threads/backup-finishes-ok-but-data-null-info-on-finish.97740/

(cherry picked from commit 76897db1466870b03ed2f2d06db5164fee0b305f)
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2022-03-04 09:56:59 +01:00
Fabian Grünbichler
58f763e1aa fix #3430: handle hook script paths better
if passing the hook script command as string, it might get interpreted
as shell command with side-effects. this is pretty harmless, since only
root is allowed to set the script parameter anyway, but making it more
robust and future-proof does not hurt.

tested with a reproducer of "/bin/echo $(touch $(whoami))" as script
parameter, with a file with that name existing, being executable and
having the following contents:

----8<----
echo "hello from hook script"
---->8----

without this change, the hookscript itself is not executed, but
'/bin/sh -c "/bin/echo $(touch $(whoami)) job"' and similar calls are,
which cause the file 'root' to be touched in the current working
directory of the vzdump process (or task worker).

with this change, the file is executed as is without any side-effects of
shell commands in the file name, and the 'hello from hook script' lines
are printed whenever the hook script is called by vzdump.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2021-05-17 09:49:34 +02:00
Fabian Ebner
875c2e5aae vzdump: getlock: return lock file handle and let the caller close it
so it doesn't get out of scope too early.

Regression introduced by 5620e5761efc6617ee8e8b004cfdd604547125fc as pointed
out by Fabian Grünbichler.

Reported in the community forum:
https://forum.proxmox.com/threads/limit-simultaneous-backup-jobs.87489

Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2021-04-12 14:36:08 +02:00
Fabian Ebner
164651dab6 vzdump: storage info: move out activate storage call
Otherwise storage_info() cannot be used for (at least) PBS storages from an API
call without 'protected => 1', because the password cannot be read from
'/etc/pve/priv'. Note that the function itself does not need the storage to be
active, because it only uses storage_config() and get_backup_dir().

AFAICT new() is the only existing user of this function and can be responsible
for activating the storage itself.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2021-03-31 15:40:13 +02:00
Fabian Ebner
914e86ccbd partially fix #2745: vzdump: use default for remove parameter
The initial default from the $confdesc is 1 anyways, and like
this changing the default in /etc/vzdump.conf to 0 actually works.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-03-05 21:31:32 +01:00
Thomas Lamprecht
1353489ed2 vzdump: indentation and whitespace fixes
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-02-25 19:21:18 +01:00
Thomas Lamprecht
94009776df vzdump: avoid single argument bless
reported by perlcritic.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-02-25 19:19:15 +01:00
Thomas Lamprecht
5620e5761e vzdump: avoid bareword file handles and two argument open
reported by perlcritic. The bareword file handles will be depreacated
in the future in perl, so start replacing now..

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-02-25 19:17:42 +01:00
Thomas Lamprecht
e8d9e1ed17 vzdump: PBS: make task log upload error non-fatal
We want to continue in that case, this is not really essential so
only warn about it.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-02-25 13:59:45 +01:00
Fabian Ebner
f8ed6af80d vzdump: refactor parsing mailto so it can be mocked
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
Reviewed-By: Dominik Csapak <d.csapak@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-02-19 16:33:33 +01:00
Fabian Ebner
ecc08c34ea vzdump: avoid parsing already parsed option
When a job is updated, verify_vzdump_parameters() is called twice. This led to
parse_property_string being called with the already parsed option.

Reported on the pve-user mailing list:
https://lists.proxmox.com/pipermail/pve-user/2021-January/172258.html

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2021-01-26 18:47:48 +01:00
Fabian Ebner
5a9b791b47 vzdump: fix error format for errors coming from storage_info
Errors from storage_info() are newline-terminated, because perl would append
the line number otherwise. Chomp those errors, because sendmail() relies
on the presence of a newline to decide if it's multiple problems or only one.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-12-21 15:29:02 +01:00
Fabian Ebner
213e0eb263 vzdump: sendmail: fix html by closing the tags
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-12-21 15:29:02 +01:00
Fabian Ebner
dd5d80f56a vzdump: defaults: convert to prune-backups early enough
Fixes the case where reading from /etc/vzdump.conf fails.

Also convert the options read from /etc/vzdump.conf before the loop. That
avoids showing a wrong warning when 'prune-backups' is configured in
/etc/vzdump.conf, and maxfiles isn't. Previously, because 'maxfiles' from the
schema defaults was automatically set, the call to parse_prune_backups_maxfiles
after the loop threw the warning that both options are defined.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-12-21 15:29:02 +01:00
Fabian Ebner
fb6871fc7f vzdump: use debugmsg instead of warn
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-12-07 09:31:12 +01:00
Fabian Ebner
f1dd762969 vzdump: warn when both storage and dumpdir are defined in vzdump.conf
and prefer storage, because the storage configuration might contain more
settings. Warning is preferable over dying, because all backups would be
affected (even if they don't use the vzdump.conf parameters) and the settings
could've been compatible (i.e. dumpdir being the storage's dump dir). Previously
one of the two options would randomly be chosen in the loop in new(), because of
perl hash iteration.

Reported here: https://forum.proxmox.com/threads/vzdump-times-out-very-often-on-zfs-storage-pool.80035/post-354277

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-12-07 09:31:12 +01:00
Fabian Ebner
acc963c3e5 vzdump: defaults: correctly parse prune-backups and convert maxfiles
Also simplify handling in new(), now that we never have maxfiles there anymore.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-12-02 13:15:52 +01:00
Fabian Ebner
1c56bcf39d vzdump: convert maxfiles CLI parameter to prune-backups
in preparation to clean up handling in new().

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-12-02 13:15:52 +01:00
Fabian Ebner
e2aac8cc8c vzdump: storage_info: adapt to new maxfiles backend behavior
It's automatically converted to prune-backups when using storage_config() now.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-12-02 13:15:52 +01:00
Fabian Ebner
b63baa391c cleanup keep-all handling
If keep-all is set to 0, adding it doesn't make a difference,
if the key is not in the hash, it won't show up in the 'values'.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-11-23 16:03:08 +01:00
Fabian Ebner
1db8529eb2 vzdump: adapt to new keep-all prune option
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-11-23 16:03:08 +01:00
Dominic Jäger
2cc64d0e42 vzdump mail: Refactor text part
Less lines exeeding the character limit, less nesting, less duplicate code,
more readable sprintf arguments.

Co-authored-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
2020-11-17 14:13:11 +01:00
Fabian Ebner
b15bdd864e fix maxfiles behavior
Commit b64dd8c8a56c424b4cfe8cb8ef0841e49ba38c03 hard-coded 0 as the default
for maxfiles in the --storage case, but the actual default should be the
value from read_vzdump_defaults(), which obtains the value from
/etc/vzdump.conf or the VZDump schema if the value has not been modified in
that file. The initial default from the schema is 1, not 0.
Tested on PVE 6.1 to verify that behavior.

Move the sanity check for zero-ness to where we have the final value for
maxfiles. Like this, we also have an implicit definedness check and more
importantly, it is more future-proof in case we ever allow maxfiles 0 in the
VZDump schema itself.

Also, force conversion to int to be extra safe.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-11-16 18:18:32 +01:00
Dominic Jäger
d0610a6b4f vzdump mail: fix #3136: Add name to plain/text part
The html/text part already has VMID NAME STATUS TIME..., but the text part only
had VMID STATUS TIME... so far. Therefore, add the missing "name" column.

Limit the length of names so that the content of the following columns remains
aligned to the headings. Note that (like before, too) this only works with
monospaced fonts.

Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
2020-11-16 18:06:12 +01:00
Stefan Reiter
b64dd8c8a5 restore default value of 0 for remove/maxfiles
If neither the 'remove' option of vzdump nor the 'maxfiles' option in
the storage config are set, assume a value of 0, i.e. do not delete
anything and allow unlimited backups.

Restores previous behaviour that was broken in e6946086e3.

Also fixes a warning about using '== 0' on a non-number type.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
2020-11-05 16:29:00 +01:00
Thomas Lamprecht
33b8ed2ed0 vzdump: fix regression with keep forever logic
fixes commit f354fe47fcaa51002e385ab132760e202a81279c, which changed
the logic to the newer storage prune helpers, but those are designed
in the spirits of PBS, with a keep-option not set meaning to keep
none.

This does not respects the storage special handling of maxfiles.
While in the API/CLI that option must be > 0m in can be zero when set
in a storage configuration entry, and then it means keep all. So, set
the internal remove option to false if that special condition is met.

This would have been a clearer, and less prone to changes,
implementation to begin with.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2020-11-01 20:11:03 +01:00
Thomas Lamprecht
77aea6ebcc vzdump: avoid passing zero to prune in legacy handling of maxfiles
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2020-10-23 10:05:28 +02:00
Fabian Ebner
f647d0fc01 sort the skip list numerically
The skip list was not always sorted if there were external IDs for multiple
external nodes.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-10-22 16:29:26 +02:00
Fabian Ebner
4d73f0e254 order guest IDs numerically in exec_backup
The assumption that they already are sorted is no longer valid,
because of the IDs for non-existent guests.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-10-22 16:29:26 +02:00
Fabian Ebner
7f87414894 backup: include IDs for non-existent guests
Like this, there will be a backup task (within the big worker task)
for such IDs, which will then visibly (i.e. also visible in the
notification mail) fail with, e.g.:
unable to find VM '123'

In get_included_guests, the key '' was chosen for the orphaned IDs,
because it cannot possibly denote a nodename.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-10-22 16:29:26 +02:00
Fabian Ebner
d51f5a1e8d only use plugin after truthiness check
Commit 1a87db9e566599d02e62d9daf4a248ef54e00469 introduced
a usage of plugin before the truthiness check for plugin.

At the moment it might not be possible to trigger this anymore,
because of the guest inclusion rework that happened later on.
But to make tasks for inexistent guest IDs visibly fail again,
this check will be necessary. Also, to get the error message in
the mail, it needs to fail inside the eval block.

Thus, keep the check in the eval block and move the block of code
using the plugin to below the check.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-10-22 16:29:26 +02:00
Fabian Ebner
b03093fedb remove unused variable
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-10-22 16:29:26 +02:00
Dominik Csapak
dfb1611534 partially fix #3056: namespace vzdump tmpdir with vmid
this fixes an issue where a rogue running backup would upload the vm
config of a later backup in a backup job

instead now that directory gets deleted and the config is not
available anymore

we cannot really keep those directories around until the end of the
backup job, since we temporarily save ct contents there, which could get
large very fast

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
2020-10-22 16:29:26 +02:00
Fabian Ebner
f354fe47fc Always use prune-backups instead of maxfiles internally
For the use case with '--dumpdir', it's not possible to call prune_backups
directly, so a little bit of special handling is required there.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-09-30 10:56:30 +02:00
Fabian Ebner
e6946086e3 Allow prune-backups as an alternative to maxfiles
and make the two options mutally exclusive as long
as they are specified on the same level (e.g. both
from the storage configuration). Otherwise prefer
option > storage config > default (only maxfiles has a default currently).

Defines the backup limit for prune-backups as the sum of all
keep-values.

There is no perfect way to determine whether a
new backup would trigger a removal with prune later:
1. we would need a way to include the not yet existing backup
   in a 'prune --dry-run' check.
2. even if we had that check, if it's executed right before
   a full hour, and the actual backup happens after the full
   hour, the information from the check is not correct.

So in some cases, we allow backup jobs with remove=0, that
will lead to a removal when the next prune is executed.
Still, the job with remove=0 does not execute a prune, so:
1. There is a well-defined limit.
2. A job with remove=0 never removes an old backup.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-09-30 10:56:30 +02:00
Dominic Jäger
dc6e0a523a vzdump: Fix typo in UPID error message
Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
2020-09-29 11:09:38 +02:00
Fabian Ebner
de2d177ec6 make use of archive_info and archive_remove
to avoid some code duplication.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-08-20 17:41:04 +02:00
Aaron Lauterer
eb0021502a backup: fix #2913 order jobs numerically by VMID
At this point, the VMIDs are already numerically sorted by the
PVE::VZDump::check_vmids method. Calling another sort on the array,
especially without `{$a <=> $b}`, resulted in reordering the array
alphabetically.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
2020-08-11 13:48:19 +02:00
Fabian Grünbichler
01fc36722d vzdump: use configured tmpdir for PBS
instead of always using the default hard-coded one.

otherwise, suspend mode container backups might run out of space even though the admin configured a big enough tmpdir.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2020-08-06 16:03:05 +02:00
Stoiko Ivanov
5d0d14f179 VZDump: add TARFILE to environment for hookscripts
The renaming of tarfile to target in 6cba1855d801545df3f3f81619dfa11cf2501858
can break existing vzdump hook scripts of users.
by setting the TARFILE variable in addition to TARGET the scripts will continue
to work.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
2020-07-14 10:27:18 +02:00
Thomas Lamprecht
ef7d3fdd04 vzdump: use more general 'guest' instead of 'vm'
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2020-07-14 10:26:50 +02:00
Thomas Lamprecht
310cde8bab vzdump: render a duration of 0s as <1s
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2020-07-14 10:26:14 +02:00
Aaron Lauterer
da7cb515c0 vzdump: included_guest: return empty hash if no guests selected
This will fix the behaviour when calling `vzdump --stop` to cause all
local guests to be backed up.

When refactoring this logic in commit df5875b4, the assumption was that
every call will have one of the following parameters set: pool, list of
VMIDs or all (intentional or when exclude is used).

There is an addtional possibility, that vzdump is called with only
--stop. Thus there are no other parameters that would indicate which
VMIDs to include.

In this case we want to return the empty hash.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
2020-07-09 13:30:47 +02:00
Thomas Lamprecht
e21a31a71f vzdump: drop usless pbs upload finished log message
this is implied, if not there should be an error.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2020-07-06 22:03:42 +02:00
Thomas Lamprecht
b3c3304ffc vzdump: set target key for PBS based backups
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2020-07-03 16:49:19 +02:00
Thomas Lamprecht
6cba1855d8 vzdump: rename tasks 'tarfile' key to 'target'
Even now we can have plain vma files which, while an archive, are not
a TARfile.
Use the generic (backup) target as key instead. Makes it less
confusing to be reused for PBS in a later patch.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2020-07-03 16:45:22 +02:00
Fabian Ebner
5b6b72e6ee Simplify how maxfiles is determined
No functional change is intended.
The preference order is: option, then storage config, then vzdump defaults.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-06-30 13:55:11 +02:00
Fabian Ebner
3a805f8d68 storage_info: avoid duplication
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-06-30 13:54:32 +02:00
Fabian Ebner
a6fcd7d9d4 Die if dumpdir and storage are both defined
dumpdir will be overwritten if a storage is specified

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-06-30 13:54:16 +02:00
Thomas Lamprecht
50ba40ec59 vzdump: move VMID sorting to check_vmids
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2020-06-17 15:45:56 +02:00