Re: [pve-devel] [PATCH manager] ui: file-restore: start the file-restore on the selected node

[Stefan Reiter <s.reiter@proxmox.com>]

On 6/10/21 10:40 AM, Dominik Csapak wrote:
> On 6/10/21 10:23, Stefan Reiter wrote:
>> On 6/10/21 9:37 AM, Dominik Csapak wrote:
>>> and not the node where the browser connects.
>>> there are at least two good reasons for this:
>>> * it is confusing, since the user would expect it to start where
>>>    the ui is pointint to
>>> * the storage may not be available on the node the browser connects
>>>    to, but it must be available on the node selected in the ui
>>>
>>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>>> ---
>>
>> Does this work with the proxying code for downloading files? I believe 
>> my original reasoning behind this was because my implementation for 
>> quickly forwarding data between the unprivileged and privileged 
>> daemons uses a local Unix socket. Thus forwarding between nodes (as I 
>> understand this will do?) would be subject to caching the data at the 
>> node being contacted, before sending it to the browser.
> 
> well it does work, since i sucessfully downloaded some files from a backup
> 
> what exactly do you mean with
>  > would be subject to caching the data at the node being
>  > contacted, before sending it to the browser.
> 

Try downloading a larger file (multiple GB) - is it streamed directly to 
the browser or first transferred to node1 (from your example below), 
cached in memory there, and only once it's fully in RAM transferred to 
the client? I believe with this patch, the second variant will happen.

This whole idea was the reason for these shenanigans:
https://git.proxmox.com/?p=pve-http-server.git;a=commitdiff;h=51841e98fa5d4ad4d5b5250523c45f88769c577f

> yes the download api call goes afaiu
> 
> proxy@node1(the one the browser connects to) ->
> proxy@node2 (node selected from tree) ->
> daemon@node2 ->
> vm@node2
> 
>>
>>>   www/manager6/grid/BackupView.js    | 4 ++--
>>>   www/manager6/storage/BackupView.js | 4 ++--
>>>   2 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/www/manager6/grid/BackupView.js 
>>> b/www/manager6/grid/BackupView.js
>>> index 8825ed96..fbed4118 100644
>>> --- a/www/manager6/grid/BackupView.js
>>> +++ b/www/manager6/grid/BackupView.js
>>> @@ -247,8 +247,8 @@ Ext.define('PVE.grid.BackupView', {
>>>           let isVMArchive = 
>>> PVE.Utils.volume_is_qemu_backup(rec.data.volid, rec.data.format);
>>>           Ext.create('Proxmox.window.FileBrowser', {
>>>               title: gettext('File Restore') + " - " + rec.data.text,
>>> -            listURL: 
>>> `/api2/json/nodes/localhost/storage/${storage}/file-restore/list`,
>>> -            downloadURL: 
>>> `/api2/json/nodes/localhost/storage/${storage}/file-restore/download`,
>>> +            listURL: 
>>> `/api2/json/nodes/${nodename}/storage/${storage}/file-restore/list`,
>>> +            downloadURL: 
>>> `/api2/json/nodes/${nodename}/storage/${storage}/file-restore/download`,
>>>               extraParams: {
>>>               volume: rec.data.volid,
>>>               },
>>> diff --git a/www/manager6/storage/BackupView.js 
>>> b/www/manager6/storage/BackupView.js
>>> index 0613c94d..c287ec63 100644
>>> --- a/www/manager6/storage/BackupView.js
>>> +++ b/www/manager6/storage/BackupView.js
>>> @@ -114,8 +114,8 @@ Ext.define('PVE.storage.BackupView', {
>>>               let isVMArchive = 
>>> PVE.Utils.volume_is_qemu_backup(rec.data.volid, rec.data.format);
>>>               Ext.create('Proxmox.window.FileBrowser', {
>>>               title: gettext('File Restore') + " - " + rec.data.text,
>>> -            listURL: 
>>> `/api2/json/nodes/localhost/storage/${me.storage}/file-restore/list`,
>>> -            downloadURL: 
>>> `/api2/json/nodes/localhost/storage/${me.storage}/file-restore/download`, 
>>>
>>> +            listURL: 
>>> `/api2/json/nodes/${nodename}/storage/${me.storage}/file-restore/list`,
>>> +            downloadURL: 
>>> `/api2/json/nodes/${nodename}/storage/${me.storage}/file-restore/download`, 
>>>
>>>               extraParams: {
>>>                   volume: rec.data.volid,
>>>               },
>>>
> 
>