[pve-devel] [PATCH manager] fix #3182 #3183: change backup retention mask logic

[pve-devel] [PATCH manager] fix #3182 #3183: change backup retention mask logic

[Dominik Csapak]

instead of relying on the contentTypeField (which does not need to
exists, e.g. for iscsi), explicitely write it into the panel/icon
mapping and check that

better would be if we query the backend about storage capabilities,
but such an api call does not exist yet, so this should be ok for now

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/Utils.js          | 41 +++++++++++++++++++++++-----------
 www/manager6/dc/StorageView.js |  1 +
 www/manager6/storage/Base.js   |  3 +--
 3 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 6e6498a2..2bc505f2 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -713,78 +713,93 @@ Ext.define('PVE.Utils', { utilities: {
 	dir: {
 	    name: Proxmox.Utils.directoryText,
 	    ipanel: 'DirInputPanel',
-	    faIcon: 'folder'
+	    faIcon: 'folder',
+	    backups: true,
 	},
 	lvm: {
 	    name: 'LVM',
 	    ipanel: 'LVMInputPanel',
-	    faIcon: 'folder'
+	    faIcon: 'folder',
+	    backups: false,
 	},
 	lvmthin: {
 	    name: 'LVM-Thin',
 	    ipanel: 'LvmThinInputPanel',
-	    faIcon: 'folder'
+	    faIcon: 'folder',
+	    backups: false,
 	},
 	nfs: {
 	    name: 'NFS',
 	    ipanel: 'NFSInputPanel',
-	    faIcon: 'building'
+	    faIcon: 'building',
+	    backups: true,
 	},
 	cifs: {
 	    name: 'CIFS',
 	    ipanel: 'CIFSInputPanel',
-	    faIcon: 'building'
+	    faIcon: 'building',
+	    backups: true,
 	},
 	glusterfs: {
 	    name: 'GlusterFS',
 	    ipanel: 'GlusterFsInputPanel',
-	    faIcon: 'building'
+	    faIcon: 'building',
+	    backups: true,
 	},
 	iscsi: {
 	    name: 'iSCSI',
 	    ipanel: 'IScsiInputPanel',
-	    faIcon: 'building'
+	    faIcon: 'building',
+	    backups: false,
 	},
 	cephfs: {
 	    name: 'CephFS',
 	    ipanel: 'CephFSInputPanel',
-	    faIcon: 'building'
+	    faIcon: 'building',
+	    backups: true,
 	},
 	pvecephfs: {
 	    name: 'CephFS (PVE)',
 	    ipanel: 'CephFSInputPanel',
 	    hideAdd: true,
-	    faIcon: 'building'
+	    faIcon: 'building',
+	    backups: true,
 	},
 	rbd: {
 	    name: 'RBD',
 	    ipanel: 'RBDInputPanel',
-	    faIcon: 'building'
+	    faIcon: 'building',
+	    backups: false,
 	},
 	pveceph: {
 	    name: 'RBD (PVE)',
 	    ipanel: 'RBDInputPanel',
 	    hideAdd: true,
-	    faIcon: 'building'
+	    faIcon: 'building',
+	    backups: false,
 	},
 	zfs: {
 	    name: 'ZFS over iSCSI',
 	    ipanel: 'ZFSInputPanel',
-	    faIcon: 'building'
+	    faIcon: 'building',
+	    backups: false,
 	},
 	zfspool: {
 	    name: 'ZFS',
 	    ipanel: 'ZFSPoolInputPanel',
-	    faIcon: 'folder'
+	    faIcon: 'folder',
+	    backups: false,
 	},
 	pbs: {
 	    name: 'Proxmox Backup Server',
 	    ipanel: 'PBSInputPanel',
 	    faIcon: 'floppy-o',
+	    backups: true,
 	},
 	drbd: {
 	    name: 'DRBD',
 	    hideAdd: true,
+	    backups: false,
 	},
     },
 
diff --git a/www/manager6/dc/StorageView.js b/www/manager6/dc/StorageView.js
index 83f86c47..74deb442 100644
--- a/www/manager6/dc/StorageView.js
+++ b/www/manager6/dc/StorageView.js
@@ -18,6 +18,7 @@ Ext.define('PVE.dc.StorageView', {
 	    paneltype: 'PVE.storage.' + schema.ipanel,
 	    type: type,
 	    storageId: sid,
+	    canDoBackups: schema.backups,
 	    autoShow: true,
 	    listeners: {
 		destroy: this.reloadStore
diff --git a/www/manager6/storage/Base.js b/www/manager6/storage/Base.js
index 13ba5b7b..45fa2bb3 100644
--- a/www/manager6/storage/Base.js
+++ b/www/manager6/storage/Base.js
@@ -211,8 +211,7 @@ Ext.define('PVE.storage.BaseEdit', {
 
 	me.callParent();
 
-	let contentTypeField = me.ipanel.down('pveContentTypeSelector');
-	if (contentTypeField && !contentTypeField.cts.includes('backup')) {
+	if (!me.canDoBackups) {
 	    // cannot mask now, not fully rendered until activated
 	    me.down('pmxPruneInputPanel').needMask = true;
 	}
-- 
2.20.1

Re: [pve-devel] [PATCH manager] fix #3182 #3183: change backup retention mask logic

[Thomas Lamprecht]

On 02.12.20 13:56, Dominik Csapak wrote:
> instead of relying on the contentTypeField (which does not need to
> exists, e.g. for iscsi), explicitely write it into the panel/icon
> mapping and check that

why not return it for iSCIS?

> 
> better would be if we query the backend about storage capabilities,
> but such an api call does not exist yet, so this should be ok for now

that's not true, the content type is exactly how the backend provides that,
that's why I used it. I'd like to avoid to further duplicating info all over
the place.

Re: [pve-devel] [PATCH manager] fix #3182 #3183: change backup retention mask logic

[Dominik Csapak]

On 12/2/20 2:11 PM, Thomas Lamprecht wrote:
> On 02.12.20 13:56, Dominik Csapak wrote:
>> instead of relying on the contentTypeField (which does not need to
>> exists, e.g. for iscsi), explicitely write it into the panel/icon
>> mapping and check that
> 
> why not return it for iSCIS? >

i don't understand what you mean? what should i return for iSCSI?
do you mean i should add a field to the iscsi panel?
(it has no content types to select, same as pbs/zfs over isci/etc.)


>>
>> better would be if we query the backend about storage capabilities,
>> but such an api call does not exist yet, so this should be ok for now
> 
> that's not true, the content type is exactly how the backend provides that,
> that's why I used it. I'd like to avoid to further duplicating info all over
> the place.
> 

what i meant was the only 'real' way is to ask the backend
(be it once or every time) what capabilities the storage has.

now we are simply querying what we hardcoded for each storage in
the frontend, my patch only adds a point where we save that specific
info (again), which is not ideal i know

Re: [pve-devel] [PATCH manager] fix #3182 #3183: change backup retention mask logic

[Thomas Lamprecht]

On 02.12.20 14:19, Dominik Csapak wrote:
> On 12/2/20 2:11 PM, Thomas Lamprecht wrote:
>> On 02.12.20 13:56, Dominik Csapak wrote:
>>> instead of relying on the contentTypeField (which does not need to
>>> exists, e.g. for iscsi), explicitely write it into the panel/icon
>>> mapping and check that
>>
>> why not return it for iSCIS? >
> 
> i don't understand what you mean? what should i return for iSCSI?
> do you mean i should add a field to the iscsi panel?
> (it has no content types to select, same as pbs/zfs over isci/etc.)
> 
> 
>>>
>>> better would be if we query the backend about storage capabilities,
>>> but such an api call does not exist yet, so this should be ok for now
>>
>> that's not true, the content type is exactly how the backend provides that,
>> that's why I used it. I'd like to avoid to further duplicating info all over
>> the place.
>>
> 
> what i meant was the only 'real' way is to ask the backend
> (be it once or every time) what capabilities the storage has.
> 
> now we are simply querying what we hardcoded for each storage in
> the frontend, my patch only adds a point where we save that specific
> info (again), which is not ideal i know
> 

hmm, true, we only have that info for existing storage - had wrongly the content
type split content view in mind... I'll rethink

[pve-devel] applied: [PATCH manager] fix #3182 #3183: change backup retention mask logic

[Thomas Lamprecht]

On 02.12.20 13:56, Dominik Csapak wrote:
> instead of relying on the contentTypeField (which does not need to
> exists, e.g. for iscsi), explicitely write it into the panel/icon
> mapping and check that
> 
> better would be if we query the backend about storage capabilities,
> but such an api call does not exist yet, so this should be ok for now
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  www/manager6/Utils.js          | 41 +++++++++++++++++++++++-----------
>  www/manager6/dc/StorageView.js |  1 +
>  www/manager6/storage/Base.js   |  3 +--
>  3 files changed, 30 insertions(+), 15 deletions(-)
> 
>

I gave it some thought to finally start the storage schema/capabilities API
endpoint we often talked about, but I did not want to rush it and this is
way less complicated for now..

applied, thanks!