IF YOU WOULD LIKE TO GET AN ACCOUNT, please write an
email to Administrator. User accounts are meant only to access repo
and report issues and/or generate pull requests.
This is a purpose-specific Git hosting for
BaseALT
projects. Thank you for your understanding!
Только зарегистрированные пользователи имеют доступ к сервису!
Для получения аккаунта, обратитесь к администратору.
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>