Re: [pve-devel] [PATCH v4 manager 1/5] api: backup: add endpoint to list included guests and volumes

[Aaron Lauterer <a.lauterer@proxmox.com>]

On 7/8/20 9:11 AM, Thomas Lamprecht wrote:
> On 07.07.20 11:48, Aaron Lauterer wrote:
>> This patch adds a new API endpoint that returns a list of included
>> guests, their volumes and whether they are included in a backup.
>>
>> The output is formatted to be used with the extJS tree panel.
>>
>> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
>> ---
>> The return types are `qemu`, `lxc` and `unknown`. The latter is there on
>> purpose because it is possible that a deleted but not purged VM is still
>> configured on a backup job. While the backup job itself will fail, I
>> think it is good to show it in the job detail view so users can react to
>> it.
>>
>> v3 -> v4:
>> * remove the "not all permissions" field as we never show such
>>    notifications anywhere else. This makes the returned data simpler
>> * define objects to be pushed in the return data directly in the push
>>    operation and not way ahead in the code.
>>
>> v2 -> v3 (hopefully I got them all):
>> * incorporate feedback from thomas
>>      * changed double negative for permissions `not_all_permissions` to
>>        `permissions_for_all`
>> * adapted to latest changes to return values from `get_included_guests`
>> * define $guest only once
>> * return VMID as int
>> * renamed some vars to be more descriptive
>>
>> v1 -> v2:
>> * simplified the code
>> * refactored according to feedback
>>
>>
>>   PVE/API2/Backup.pm | 174 +++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 174 insertions(+)
>>
>> diff --git a/PVE/API2/Backup.pm b/PVE/API2/Backup.pm
>> index 86377c0a..6fbe2106 100644
>> --- a/PVE/API2/<F2>Backup.pm
> 
> A <F2> slipped into here, git am -3 handles it like a champ though..

Oops :/
> 
> Further, when applying this series I get:
>> implement me - abstract method (500)
> 
> In the detail window, am I still missing some other part or package bump?

Turns out that pve-container hasn't gotten a version bump yet since the 
needed changes for this to work were applied. Should work if you build 
and install pve-container from the master branch.

Something to take a closer look at in the future :)

> 
>