[pve-devel] [PATCH v2 manager 1/3] ui: restore: disallow empty storage selection if it wouldn't work
Thomas Lamprecht
t.lamprecht at proxmox.com
Sat Apr 23 11:38:35 CEST 2022
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.ebner at 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() {
More information about the pve-devel
mailing list