Re: [pbs-devel] [PATCH v4 proxmox{, -backup} 0/2] close #4763: client: added command to forget backup group

[Christian Ebner <c.ebner@proxmox.com>]

Hi,
thanks for tackling this issue.

The group forget command is something I missed quite a lot when doing 
testing on PBS with accumulating snapshots in a group an not wanting to 
fallback to the UI (I actually always went the route of deleting the 
snapshot folder).

Tested the following:
- Created a few backup snapshots, creating thereby a new backup group
- Tried to delete the group while backup is running
   result was as expected: delete failed, missing lock error
- Tried to delete the group while a restore is running
   result was as expected: delete failed, missing lock (although 
different from the previous one)
- Tried to remove without any other task running
   result was as expected: group deleted, including the group folder in 
the datastore
- Tried to delete empty group
   result was as expected: group deleted, including the group folder in 
the datastore

Two further things I noticed:

The confirmation dialog also allows me to type in `yolo` or any string 
starting with `yY` and nevertheless accepts this as valid confirmation 
input (similar for negation). Should we limit this further?

When one tries to delete a non existing group, the dialog asks me for 
confirmation, failing however afterwards with an error message, leaking 
also the datastore path to the client. While the former is not an issue 
and the intention is to be able to remove empty groups, the latter is 
not okay in my opinion.
So either check if the group even exists before asking for confirmation, 
or map the error to not leak the datastore path.

For the rest consider this:

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



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel