Re: [pve-devel] [PATCH v3 manager] gui: expose content-dirs property in storage edit/create

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 24/03/2023 um 17:12 schrieb Leo Nunner:
> Add a separate tab for the storage edit/create panels to set the
> recently introduced "content-dirs" property which overrides the
> default directory locations. Analogous to the API implementation,
> the tab was added for Directory, CIFS and NFS storages.
> 
> Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
> ---
> Changes since v2:
>     - Factor out edit tab into separate component
>     - Add vtype for instant validation feedback to the user
>     - Introduce tab for CephFS
> 
>  www/manager6/Makefile              |   1 +
>  www/manager6/Toolkit.js            |   2 +
>  www/manager6/panel/ContentDirs.js  | 102 +++++++++++++++++++++++++++++
>  www/manager6/storage/CIFSEdit.js   |   9 +++
>  www/manager6/storage/CephFSEdit.js |   9 +++
>  www/manager6/storage/DirEdit.js    |   9 +++
>  www/manager6/storage/NFSEdit.js    |   9 +++
>  7 files changed, 141 insertions(+)
>  create mode 100644 www/manager6/panel/ContentDirs.js
> 
> diff --git a/www/manager6/Makefile b/www/manager6/Makefile
> index 2b577c8e..cb07b5b8 100644
> --- a/www/manager6/Makefile
> +++ b/www/manager6/Makefile
> @@ -96,6 +96,7 @@ JSSRC= 							\
>  	panel/GuestSummary.js				\
>  	panel/TemplateStatusView.js			\
>  	panel/MultiDiskEdit.js				\
> +	panel/ContentDirs.js				\
>  	tree/ResourceTree.js				\
>  	tree/SnapshotTree.js				\
>  	window/Backup.js				\
> diff --git a/www/manager6/Toolkit.js b/www/manager6/Toolkit.js
> index f69c376a..0c801d45 100644
> --- a/www/manager6/Toolkit.js
> +++ b/www/manager6/Toolkit.js
> @@ -15,6 +15,8 @@ Ext.apply(Ext.form.field.VTypes, {
>      IP64AddressListMask: /[A-Fa-f0-9,:.; ]/,
>      PciIdText: gettext('Example') + ': 0x8086',
>      PciId: v => /^0x[0-9a-fA-F]{4}$/.test(v),
> +    ContentDirText: gettext('Example') + ': custom/subdir/location',
> +    ContentDir: v => /([^.]*(?:\.?[^.]+)+)/.test(v),
>  });
>  
>  Ext.define('PVE.form.field.Display', {
> diff --git a/www/manager6/panel/ContentDirs.js b/www/manager6/panel/ContentDirs.js
> new file mode 100644
> index 00000000..57939101
> --- /dev/null
> +++ b/www/manager6/panel/ContentDirs.js
> @@ -0,0 +1,102 @@
> +Ext.define('PVE.panel.ContentDirsPanel', {
> +    extend: 'Proxmox.panel.InputPanel',
> +    xtype: 'pveContentDirsTab',
> +    mixins: ['Proxmox.Mixin.CBind'],
> +
> +    onGetValues: function(values) {
> +	let me = this;
> +	let ret = me.values ?? {};
> +	Object.keys(values || {}).forEach(function(name) {
> +	    ret[name] = values[name];
> +	});
> +	return { "content-dirs": PVE.Parser.printPropertyString(ret) };
> +    },
> +
> +    onSetValues: function(values) {
> +	let me = this;
> +	me.values = values?.["content-dirs"];
> +	return me.values;
> +    },
> +
> +    column1: [
> +	{
> +	    xtype: 'pmxDisplayEditField',
> +	    name: 'vztmpl',
> +	    fieldLabel: gettext('CT Template'),
> +	    allowBlank: true,
> +	    emptyText: "template/cache",
> +	    vtype: "ContentDir",
> +	    editable: true,
> +	},
> +	{
> +	    xtype: 'pmxDisplayEditField',
> +	    name: 'iso',
> +	    fieldLabel: gettext('ISO Image'),
> +	    allowBlank: true,
> +	    emptyText: "template/iso",
> +	    vtype: "ContentDir",
> +	    editable: true,
> +	},
> +	{
> +	    xtype: 'pmxDisplayEditField',
> +	    name: 'snippets',
> +	    fieldLabel: gettext('Snippets'),
> +	    allowBlank: true,
> +	    emptyText: "snippets",
> +	    vtype: "ContentDir",
> +	    editable: true,
> +	},

I'd add an advanced section  and move above into it.

> +    ],
> +    column2: [
> +	{
> +	    xtype: 'pmxDisplayEditField',
> +	    name: 'backup',
> +	    fieldLabel: gettext('Backup Files'),
> +	    allowBlank: true,
> +	    emptyText: "dump",
> +	    vtype: "ContentDir",
> +	    cbind: {
> +		editable: '{isCreate}',
> +	    },
> +	},
> +	{
> +	    xtype: 'pmxDisplayEditField',
> +	    name: 'rootdir',
> +	    fieldLabel: gettext('Container'),

this is misleading, it's not for Container volumes but rather expanded Container
root-directories, something from OpenVZ times, which we don't fully support anymore
IIRC. If, I'd place this into advanced and use a different name to convey that.

> +	    allowBlank: true,
> +	    emptyText: "private",
> +	    vtype: "ContentDir",
> +	    cbind: {
> +		editable: '{isCreate}',
> +	    },
> +	},
> +	{
> +	    xtype: 'pmxDisplayEditField',
> +	    name: 'images',
> +	    fieldLabel: gettext('Disk Image'),

Maybe call this 'Guest Volumes' or 'Guest Images' if it really has to be images.

> +	    allowBlank: true,
> +	    emptyText: "images",
> +	    vtype: "ContentDir",
> +	    cbind: {
> +		editable: '{isCreate}',
> +	    },
> +	},
> +    ],
> +    columnT: [
> +	{
> +	    xtype: 'displayfield',
> +	    userCls: 'pmx-hint',
> +	    value: gettext('Editing overrides for existing storages might affect guests. Proceed with caution.'),
> +	    cbind: {
> +		hidden: '{isCreate}',
> +	    },
> +	},
> +    ],
> +    columnB: [
> +	{
> +	    xtype: 'displayfield',
> +	    userCls: 'pmx-hint',
> +	    value: gettext('Paths are relative to the mountpoint of the storage.'),
> +	},> +    ],
> +});
> diff --git a/www/manager6/storage/CIFSEdit.js b/www/manager6/storage/CIFSEdit.js
> index 71415401..a3e7d3ea 100644
> --- a/www/manager6/storage/CIFSEdit.js
> +++ b/www/manager6/storage/CIFSEdit.js
> @@ -218,6 +218,15 @@ Ext.define('PVE.storage.CIFSInputPanel', {
>  	    },
>  	];
>  
> +	me.extraTabs = [
> +	    {
> +		xtype: 'pveContentDirsTab',
> +		title: gettext("Content Directories"),
> +		onlineHelp: me.onlineHelp,
> +		isCreate: me.isCreate,
> +	    },
> +	];
> +
>  	me.callParent();
>      },
>  });
> diff --git a/www/manager6/storage/CephFSEdit.js b/www/manager6/storage/CephFSEdit.js
> index 6a95a00a..7558a052 100644
> --- a/www/manager6/storage/CephFSEdit.js
> +++ b/www/manager6/storage/CephFSEdit.js
> @@ -129,6 +129,15 @@ Ext.define('PVE.storage.CephFSInputPanel', {
>  	    },
>  	];
>  
> +	me.extraTabs = [
> +	    {
> +		xtype: 'pveContentDirsTab',
> +		title: gettext("Content Directories"),
> +		onlineHelp: me.onlineHelp,
> +		isCreate: me.isCreate,
> +	    },
> +	];
> +
>  	me.callParent();
>      },
>  });
> diff --git a/www/manager6/storage/DirEdit.js b/www/manager6/storage/DirEdit.js
> index 7e9ec44d..d1ae46cc 100644
> --- a/www/manager6/storage/DirEdit.js
> +++ b/www/manager6/storage/DirEdit.js
> @@ -33,6 +33,15 @@ Ext.define('PVE.storage.DirInputPanel', {
>  	    },
>  	];
>  
> +	me.extraTabs = [
> +	    {
> +		xtype: 'pveContentDirsTab',
> +		title: gettext("Content Directories"),
> +		onlineHelp: me.onlineHelp,
> +		isCreate: me.isCreate,
> +	    },
> +	];
> +
>  	me.callParent();
>      },
>  });
> diff --git a/www/manager6/storage/NFSEdit.js b/www/manager6/storage/NFSEdit.js
> index 202c7de0..13a17e3b 100644
> --- a/www/manager6/storage/NFSEdit.js
> +++ b/www/manager6/storage/NFSEdit.js
> @@ -160,6 +160,15 @@ Ext.define('PVE.storage.NFSInputPanel', {
>  	    },
>  	];
>  
> +	me.extraTabs = [
> +	    {
> +		xtype: 'pveContentDirsTab',
> +		title: gettext("Content Directories"),
> +		onlineHelp: me.onlineHelp,
> +		isCreate: me.isCreate,
> +	    },
> +	];
> +
>  	me.callParent();
>      },
>  });