Re: [pve-devel] [PATCH manager v10 4/4] ui: add AMD SEV configuration to Options

[Dominik Csapak <d.csapak@proxmox.com>]

comments inline

On 5/10/24 13:47, Markus Frank wrote:
> By adding a new input panel with an AMD SEV technology selection combo
> box and checkboxes for the optional parameters in an advanced section,
> the user can configure the amd_sev option via the WebUI's Options tab.
> 
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
> changes v10:
> * this patch is new to v10
> 
>   www/manager6/Makefile        |  1 +
>   www/manager6/qemu/Options.js | 11 ++++
>   www/manager6/qemu/SevEdit.js | 98 ++++++++++++++++++++++++++++++++++++
>   3 files changed, 110 insertions(+)
>   create mode 100644 www/manager6/qemu/SevEdit.js
> 
> diff --git a/www/manager6/Makefile b/www/manager6/Makefile
> index 2c3a822b..801683a3 100644
> --- a/www/manager6/Makefile
> +++ b/www/manager6/Makefile
> @@ -264,6 +264,7 @@ JSSRC= 							\
>   	qemu/SSHKey.js					\
>   	qemu/ScsiHwEdit.js				\
>   	qemu/SerialEdit.js				\
> +	qemu/SevEdit.js					\
>   	qemu/Smbios1Edit.js				\
>   	qemu/SystemEdit.js				\
>   	qemu/USBEdit.js					\
> diff --git a/www/manager6/qemu/Options.js b/www/manager6/qemu/Options.js
> index 7b112400..6907699c 100644
> --- a/www/manager6/qemu/Options.js
> +++ b/www/manager6/qemu/Options.js
> @@ -338,6 +338,17 @@ Ext.define('PVE.qemu.Options', {
>   		    },
>   		} : undefined,
>   	    },
> +	    amd_sev: {
> +		header: gettext('AMD SEV'),
> +		editor: caps.vms['VM.Config.HWType'] ? 'PVE.qemu.SevEdit' : undefined,
> +		defaultValue: Proxmox.Utils.defaultText + ' (' + Proxmox.Utils.disabledText + ')',
> +		renderer: function(value, metaData, record, ri, ci, store, pending) {
> +		    let amd_sev = PVE.Parser.parsePropertyString(value, "type");
> +		    if (amd_sev.type === 'std') return 'AMD SEV (' + value + ')';
> +		    if (amd_sev.type === 'es') return 'AMD SEV-ES (' + value + ')';
> +		    return value;
> +		},
> +	    },
>   	    hookscript: {
>   		header: gettext('Hookscript'),
>   	    },
> diff --git a/www/manager6/qemu/SevEdit.js b/www/manager6/qemu/SevEdit.js
> new file mode 100644
> index 00000000..f0187cde
> --- /dev/null
> +++ b/www/manager6/qemu/SevEdit.js
> @@ -0,0 +1,98 @@
> +Ext.define('PVE.qemu.SevInputPanel', {
> +    extend: 'Proxmox.panel.InputPanel',
> +    xtype: 'pveSevInputPanel',
> +    onlineHelp: 'qm_memory_encryption',
> +
> +    viewModel: {
> +	data: {
> +	    type: '__default__',
> +	},
> +	formulas: {
> +	    sevEnabled: get => get('type') === 'std' || get('type') === 'es',

would'nt that be `get('type') !== '__default__'` ?

makes it shorter and more future proof should we add some other type there

> +	},
> +    },
> +
> +    onGetValues: function(values) {
> +	if (values.delete === 'type') {
> +	    values.delete = 'amd_sev';
> +	    return values;
> +	}
> +	let ret = {};
> +	ret.amd_sev = PVE.Parser.printPropertyString(values, 'type');
> +	return ret;
> +    },
> +
> +    items: {
> +	xtype: 'proxmoxKVComboBox',
> +	fieldLabel: gettext('AMD Secure Encrypted Virtualization (SEV)'),
> +	name: 'type',
> +	value: '__default__',
> +	comboItems: [
> +	    ['__default__', Proxmox.Utils.defaultText + ' (' + Proxmox.Utils.disabledText + ')'],
> +	    ['std', 'AMD SEV'],
> +	    ['es', 'AMD SEV-ES (highly experimental)'],
> +	],
> +	bind: {
> +	    value: '{type}',
> +	},
> +    },
> +
> +    advancedItems: [
> +	{
> +	    xtype: 'proxmoxcheckbox',
> +	    fieldLabel: gettext('no-debug'),

nit: i guess it probably make sense to expose the policy settings with their names
but i really disklike 'no-something' options that one have to enable

we could in the ui reverse it and make the default true?

also IMHO this text is a bit too short

e.g. 'allow debugging' would be nicer?

> +	    name: 'no-debug',
> +	    deleteDefaultValue: false,
> +	    bind: {
> +		hidden: '{!sevEnabled}',
> +		disabled: '{!sevEnabled}',
> +	    },
> +	},
> +	{
> +	    xtype: 'proxmoxcheckbox',
> +	    fieldLabel: gettext('no-key-sharing'),

same here...

> +	    name: 'no-key-sharing',
> +	    deleteDefaultValue: false,
> +	    bind: {
> +		hidden: '{!sevEnabled}',
> +		disabled: '{!sevEnabled}',
> +	    },
> +	},
> +	{
> +	    xtype: 'proxmoxcheckbox',
> +	    fieldLabel: gettext('kernel-hashes'),
> +	    name: 'kernel-hashes',
> +	    deleteDefaultValue: false,
> +	    bind: {
> +		hidden: '{!sevEnabled}',
> +		disabled: '{!sevEnabled}',
> +	    },
> +	},
> +    ],
> +});
> +
> +Ext.define('PVE.qemu.SevEdit', {
> +    extend: 'Proxmox.window.Edit',
> +
> +    subject: gettext('SEV'),
> +
> +    items: {
> +	xtype: 'pveSevInputPanel',
> +    },
> +
> +    width: 400,
> +
> +    initComponent: function() {
> +	let me = this;
> +
> +	me.callParent();
> +
> +	me.load({
> +	    success: function(response) {
> +		let conf = response.result.data;
> +		let amd_sev = conf.amd_sev || '__default__';
> +		me.setValues(PVE.Parser.parsePropertyString(amd_sev, 'type'));
> +	    },
> +	});
> +    },
> +});



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel