Re: [pve-devel] [PATCH-SERIES storage/manager/guest-common/docs] improvements for protected backups

[Dominik Csapak <d.csapak@proxmox.com>]

one question:

either it's not explained, or i missed it:
what do we gain by having a limit on the number of protected backups?

storage 2/2 mentions that protection broke some assumption of vzdump
which is (somehow? not really explained imho) fixing it?

if it's not fixing it, what is the relation between those things?

also, why have a 'magic' -1 value that means indefinite, we could
simply always have that behavior?

in my opinion, it makes no sense to limit the number of protected
backups..

if it is necessary for some reason, it would be good to include
that reason either in the commit message, or at least in the cover
letter...

On 12/16/21 13:12, Fabian Ebner wrote:
> The series consists of two parts:
> 
> First part (up to docs 2/2) is for introducing a storage property
> to limit the number of protected backups and only count unprotected
> backups for the limit check in vzdump.
> 
> Second part is a convenience feature allowing 'protected' and 'notes'
> to be set directly with vzdump and for manual backups in the UI.
> 
> 
> storage:
> 
> Fabian Ebner (2):
>    list volumes: also return backup type for backups
>    plugins: allow limiting the number of protected backups per guest
> 
>   PVE/Storage.pm                 | 24 ++++++++++++++++++++++++
>   PVE/Storage/BTRFSPlugin.pm     |  3 ++-
>   PVE/Storage/CIFSPlugin.pm      |  1 +
>   PVE/Storage/CephFSPlugin.pm    |  1 +
>   PVE/Storage/DirPlugin.pm       |  1 +
>   PVE/Storage/GlusterfsPlugin.pm |  1 +
>   PVE/Storage/NFSPlugin.pm       |  1 +
>   PVE/Storage/PBSPlugin.pm       |  2 ++
>   PVE/Storage/Plugin.pm          |  8 ++++++++
>   test/list_volumes_test.pm      | 10 ++++++++++
>   10 files changed, 51 insertions(+), 1 deletion(-)
> 
> 
> manager:
> 
> Fabian Ebner (6):
>    vzdump: backup file list: drop unused parameter
>    vzdump: backup limit: only count unprotected backups
>    ui: storage edit: retention: add max-protected-backups setting
>    ui: storage creation: retention: dynamically adapt
>      max-protected-backups
>    vzdump: support setting protected status and notes
>    ui: backup: allow setting protected and notes for manual backup
> 
>   PVE/VZDump.pm                        | 69 ++++++++++++++++++---------
>   www/manager6/panel/BackupJobPrune.js | 71 ++++++++++++++++++++++++----
>   www/manager6/storage/Base.js         |  1 +
>   www/manager6/window/Backup.js        | 25 +++++++++-
>   4 files changed, 132 insertions(+), 34 deletions(-)
> 
> 
> guest-common:
> 
> Fabian Ebner (1):
>    vzdump: schema: add 'notes' and 'protected' properties
> 
>   src/PVE/VZDump/Common.pm | 14 +++++++++++++-
>   1 file changed, 13 insertions(+), 1 deletion(-)
> 
> 
> docs:
> 
> Fabian Ebner (2):
>    storage: switch to prune-backups in examples
>    vzdump/storage: mention protected backups limit and give an example
> 
>   pve-storage-dir.adoc | 9 +++++----
>   pve-storage-pbs.adoc | 2 +-
>   vzdump.adoc          | 3 +++
>   3 files changed, 9 insertions(+), 5 deletions(-)
>