Re: [pve-devel] applied: Re: [PATCH v4 qemu-server 1/1] api2: add check_bridge_access for create/update/clone/restore vm

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On 09/06/2023 09:00, DERUMIER, Alexandre wrote:
> Le jeudi 08 juin 2023 à 18:02 +0200, Thomas Lamprecht a écrit :
>> On 07/06/2023 14:03, Alexandre Derumier wrote:
>>> Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
>>> ---
>>>  PVE/API2/Qemu.pm | 33 +++++++++++++++++++++++++++++----
>>>  1 file changed, 29 insertions(+), 4 deletions(-)
>>>
>>>
>>
>> applied, with Fabians R-b, thanks.
>>
>> Made a follow-up moving the checker method to QemuServer and
>> replacing getting
>> the config fromthe archive twice by checking after the config from
>> the backup
>> and the override pa<rameters passed on restore got merged into the
>> actual target
>> config, so this wasn't only a inefficiency thing IIUC, but actually
>> wrong, i.e.,
>> if one passed a override for a netX property the one from the backup
>> got checked,
>> not the effective one.
>>
> Thanks Thomas.
> 
> Just wonder, could it be done before disk restore ?  (That's what I was
> trying to do)> 
> it seem to be inefficiency too to check it after disk restore (if for
> example, user restore a big backup, taking hours)

yes, sure, but as mentioned in the commit message, if it's checked
to late other things happen to early, as doing stuff before having
the merged config seems odd.

And I did not wanted to re-work that part in a hurry, we can improve
that still in the next week(s).

> 
> I have done a test from the gui
> "
> ...
> progress 98% (read 21045379072 bytes, duration 14 sec)
> progress 99% (read 21260140544 bytes, duration 14 sec)
> progress 100% (read 21474836480 bytes, duration 14 sec)
> total bytes read 21474836480, sparse bytes 18656022528 (86.9%)
> space reduction due to 4K zero blocks 4.54%
> no lock found trying to remove 'create'  lock
> error before or during data restore, some or all disks were not
> completely restored. VM 249 state is NOT cleaned up.
> TASK ERROR: 403 Permission check failed
> (/sdn/zones/localnetwork/vmbr0/96, SDN.Use)
> 
> "
> 
> The vm config file is created, mostly empty:
> /etc/pve/qemu-server/<vmid>.conf
> memory:128
> 
> and the restored disk are not removed too
> 
> 


Yes, that's not ideal, but the check is now actually correct; the existing
order of restore and config merging needs the fixing.