Re: [pve-devel] [PATCH 5/5] fix #3593: Added vm core pinning pve-manager

Re: [pve-devel] [PATCH 5/5] fix #3593: Added vm core pinning pve-manager

[Daniel Bowder]

> Is a string the best way of entering the CPUs in the GUI? Maybe a
> dropdown where you can (un)select the cores?

It would be a nice gui element to have a list of CPU cores with the ability
to select individual ones. Such a feature would require the Options.js to
know how many CPU cores the system had to dynamically generate the
list I do not know how one would get that information to Options.js, but it
certainly seems feasible. The cpuset list must be created eventually so
that the cpuset can be used by taskset, so the underlying data structure
remains this string.

I do not yet have the required knowledge to add a dynamically generated
list of CPU cores in the Options.js file. I do have the required knowledge
to add this string formatted list.

I would argue that having the feature in the GUI as a string is acceptable
as a version 1 implementation. The string format does not lock us into
anything and the gui can be upgraded to a fancy list in the future with no
hindrances. The cpuset format in $vmid.conf is the way that this variable
should be stored, so on the backend, there's nothing to change.

Re: [pve-devel] [PATCH 5/5] fix #3593: Added vm core pinning pve-manager

[Matthias Heiserer]

On 08.06.2022 13:54, Daniel Bowder wrote:
> Signed-off-by: Daniel Bowder <daniel@bowdernet.com>
> ---
>   The fifth patch adds the cpuset value to the GUI under the VM.Config.Options panel. The cpuset is set as a vtype of CPUSet so that it can be checked by the regex match in the fouth patch. This was modeled after the existing 'name' option, 
typo: fouth -> fourth

Don't think you should be referencing other patches by number in the 
commit message though, so the comment is a fitting location for this remark.
where the labels and textfield type have been changed.
>   www/manager6/qemu/Options.js | 30 ++++++++++++++++++++++++++++++
>   1 file changed, 30 insertions(+)
> 
> diff --git a/www/manager6/qemu/Options.js b/www/manager6/qemu/Options.js
> index a1def4bb..ce356130 100644
> --- a/www/manager6/qemu/Options.js
> +++ b/www/manager6/qemu/Options.js
> @@ -341,6 +341,36 @@ Ext.define('PVE.qemu.Options', {
>   	    hookscript: {
>   		header: gettext('Hookscript'),
>   	    },
> +		cpuset: {
> +		required: false,
> +		header: gettext('CPU Set'),
> +		defaultValue: "",
> +		editor: caps.vms['VM.Config.Options'] ? {
> +		    xtype: 'proxmoxWindowEdit',
> +		    subject: gettext('CPU Set'),
> +		    items: {
> +			xtype: 'inputpanel',
> +			items: {
> +			    xtype: 'textfield',
> +			    name: 'cpuset',
> +				vtype: 'CPUSet',
> +			    value: '',
> +			    fieldLabel: gettext('cpuset'),
> +			    allowBlank: true,
> +				emptyText: gettext("Pin to cores (e.g. 0,2-6,8)"),
> +			},
> +			onGetValues: function(values) {
> +			    var params = values;
> +			    if (values.cpuset === undefined ||
> +				values.cpuset === null ||
> +				values.cpuset === '') {
> +				params = { 'delete': 'cpuset' };
> +			    }
> +			    return params;
> +			},
> +		    },
> +		} : undefined,
> +		},
>   	};
>   
>   	var baseurl = 'nodes/' + nodename + '/qemu/' + vmid + '/config';

Is a string the best way of entering the CPUs in the GUI? Maybe a 
dropdown where you can (un)select the cores?

[pve-devel] [PATCH 5/5] fix #3593: Added vm core pinning pve-manager

[Daniel Bowder]

Signed-off-by: Daniel Bowder <daniel@bowdernet.com>
---
 The fifth patch adds the cpuset value to the GUI under the VM.Config.Options panel. The cpuset is set as a vtype of CPUSet so that it can be checked by the regex match in the fouth patch. This was modeled after the existing 'name' option, where the labels and textfield type have been changed.
 www/manager6/qemu/Options.js | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/www/manager6/qemu/Options.js b/www/manager6/qemu/Options.js
index a1def4bb..ce356130 100644
--- a/www/manager6/qemu/Options.js
+++ b/www/manager6/qemu/Options.js
@@ -341,6 +341,36 @@ Ext.define('PVE.qemu.Options', {
 	    hookscript: {
 		header: gettext('Hookscript'),
 	    },
+		cpuset: {
+		required: false,
+		header: gettext('CPU Set'),
+		defaultValue: "",
+		editor: caps.vms['VM.Config.Options'] ? {
+		    xtype: 'proxmoxWindowEdit',
+		    subject: gettext('CPU Set'),
+		    items: {
+			xtype: 'inputpanel',
+			items: {
+			    xtype: 'textfield',
+			    name: 'cpuset',
+				vtype: 'CPUSet',
+			    value: '',
+			    fieldLabel: gettext('cpuset'),
+			    allowBlank: true,
+				emptyText: gettext("Pin to cores (e.g. 0,2-6,8)"),
+			},
+			onGetValues: function(values) {
+			    var params = values;
+			    if (values.cpuset === undefined ||
+				values.cpuset === null ||
+				values.cpuset === '') {
+				params = { 'delete': 'cpuset' };
+			    }
+			    return params;
+			},
+		    },
+		} : undefined,
+		},
 	};
 
 	var baseurl = 'nodes/' + nodename + '/qemu/' + vmid + '/config';
-- 
2.30.2