On 21.04.22 13:26, Fabian Ebner wrote:
> Namely, if there is a storage in the backup configuration that's not
> available on the current node.

Better than the status quo, but in the long run all the "force all volumes to a 
single storage"
on restore and also migrate isn't ideal for the case where one or more storages 
do not exist on
the target node. An per-volume override would be nicer, but may require some 
gui adaptions to
present that in a sensible way with good UX.

> 
> Signed-off-by: Fabian Ebner <f.eb...@proxmox.com>
> ---
> 
> New in v2.
> 
>  www/manager6/window/Restore.js | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/www/manager6/window/Restore.js b/www/manager6/window/Restore.js
> index e7b3e945..25babf89 100644
> --- a/www/manager6/window/Restore.js
> +++ b/www/manager6/window/Restore.js
> @@ -15,6 +15,40 @@ Ext.define('PVE.window.Restore', {
>               },
>           },
>       },
> +
> +     afterRender: function() {
> +         let view = this.getView();
> +
> +         Proxmox.Utils.API2Request({
> +             url: `/nodes/${view.nodename}/vzdump/extractconfig`,
> +             method: 'GET',
> +             params: {
> +                 volume: view.volid,
> +             },
> +             failure: (response) => Ext.Msg.alert('Error', 
> response.htmlStatus),

nit: no parenthesis required for single parameter arrow fns

> +             success: function(response, options) {
> +                 let storagesAvailable = true;
> +
> +                 response.result.data.split('\n').forEach(function(line) {

style nit: makes no big difference but for new code we prefer arrow fns for 
inline closures.

> +                     let match = line.match(
> +                         /^#qmdump#map:(\S+):(\S+):(\S*):(\S*):$/,
> +                     );

fits in 100 cc so would rather keep in in a single line

> +                     if (match) {
> +                         let currentAvailable = 
> !!PVE.data.ResourceStore.getById(
> +                             `storage/${view.nodename}/${match[3]}`,
> +                         );
> +                         storagesAvailable = storagesAvailable && 
> currentAvailable;



JS has a &&= operator, so could be:

storagesAvailable &&= !!PVE.data.ResourceStore.getById(
    `storage/${view.nodename}/${match[3]}`);

but as we only want to do one thing if any storage is not available we could do:

```
let allStoragesAvailable = response.result.data.split('\n').every(line => {
    let match = line.match(/^#qmdump#map:(\S+):(\S+):(\S*):(\S*):$/);
    if (match) {
        return 
!!PVE.data.ResourceStore.getById(`storage/${view.nodename}/${match[3]}`);
    }
    return true;
}

if (!allStoragesAvailable) {
    // ...
}
```

IMO a bit easier to follow.

> +                     }
> +                 });
> +
> +                 if (!storagesAvailable) {
> +                     let storagesel = 
> view.down('pveStorageSelector[name=storage]');
> +                     storagesel.allowBlank = false;
> +                     storagesel.setEmptyText('');
> +                 }
> +             },
> +         });
> +     },
>      },
>  
>      initComponent: function() {



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

Reply via email to