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

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On March 23, 2026 11:52 am, Maximiliano Sandoval wrote:
> 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.

I think both this here and the backend description should contain the
words "discard" and "trim". That the end result is freeing capacity on
the backing device is a (desired) implementation detail.

If you don't know what discard means, you should not enable this option
anyway.

"when VM is deleted" is wrong in any case, the option affects volumes
which are destroyed (which can happen as part of destroying a guest, of
course, but is not limited to those tasks).

What we want to convey:

- destroying a volume always logically removes it
- saferemove wipes the volume before removing it (prevents data leakage)
- issue_blockdiscard discards the volume's data before removing it
  (increases lifespan of flash media, allows better thin provisioning,
  requires support across the whole stack to work)

"free up space" is too generic, since it's not clear whether this refers
to logical space usage or physical space usage, IMHO.

> 
> 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
> 
> 
> 
> 
>