Re: [PATCH manager 2/2] fix #7339: lvmthick: ui: add UI fields for option to free storage

[Maximiliano Sandoval <m.sandoval@proxmox.com>]

Lukas Sichert <l.sichert@proxmox.com> writes:

> In the previous commit the backend recieved the functionality to discard
> allocated space of a VM disk on a SAN, when a VM is deleted. The backend
> checks whether to use this option by parsing storage.cfg to see if
> 'issue_blkdiscard' is set to 1. This variable will automatically be
> stored into the config file if it is present in the 'PUT' API request.
> To be able to append this to the API call, the variable needs
> to be defined in the json-Schema, but this has also been added in the
> previous commit.
>
> To enable a option to free storage in the Gui, create a checkbox with
> the name 'issue_blkdiscard'. In the checkbox use cbind to evaluate
> 'iscreate' from the component, to only autoenable on new creations of
> LVM Storages. This checkbox also adds the 'issue_blkdiscard'
> variable and its value to the return call.
>
> Also add remappings for the description of the new worker tasks, to show
> in the Gui, which options are used for the current deletion.
>
> Signed-off-by: Lukas Sichert <l.sichert@proxmox.com>
> ---
>  www/manager6/Utils.js           |  3 +++
>  www/manager6/storage/LVMEdit.js | 13 +++++++++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
> index 77dae42e..1c70b159 100644
> --- a/www/manager6/Utils.js
> +++ b/www/manager6/Utils.js
> @@ -2133,6 +2133,9 @@ Ext.define('PVE.Utils', {
>              clusterjoin: ['', gettext('Join Cluster')],
>              dircreate: [gettext('Directory Storage'), gettext('Create')],
>              dirremove: [gettext('Directory'), gettext('Remove')],
> +            diskdiscard: ['', gettext('Discard disk')],
> +            diskzero: ['', gettext('Zero out disk')],
> +            diskzerodiscard: ['', gettext('Zero out and discard disk')],
>              download: [gettext('File'), gettext('Download')],
>              hamigrate: ['HA', gettext('Migrate')],
>              hashutdown: ['HA', gettext('Shutdown')],
> diff --git a/www/manager6/storage/LVMEdit.js b/www/manager6/storage/LVMEdit.js
> index 148f0601..35d2b329 100644
> --- a/www/manager6/storage/LVMEdit.js
> +++ b/www/manager6/storage/LVMEdit.js
> @@ -241,5 +241,18 @@ Ext.define('PVE.storage.LVMInputPanel', {
>              uncheckedValue: 0,
>              fieldLabel: gettext('Wipe Removed Volumes'),
>          },
> +        {
> +            xtype: 'proxmoxcheckbox',
> +            name: 'issue_blkdiscard',
> +            uncheckedValue: 0,
> +            cbind: {
> +                checked: '{isCreate}',
> +            },
> +            fieldLabel: gettext('Discard Removed Volumes'),
> +            autoEl: {
> +                tag: 'div',
> +                'data-qtip': gettext('Enable if storage space should be freed when VM is deleted.'),

I would recommend:

'Whether to free up storage space when deleting the VM.'

instead.

But this begs the question: How is this different from "Wipe Remove
Volumes" above? Imho it should be possible to tell them apart just with
the information available in the web UI.

> +            },
> +        },
>      ],
>  });

-- 
Maximiliano