[pve-devel] applied: [PATCH v2 manager 1/1] Add 'type' option to AgentFeatureSelector

[Thomas Lamprecht]

On 11/19/19 3:25 PM, Matt Dunwoodie wrote:
> This adds an extra field to the AgentFeatureSelector that reflects the
> change in qemu-server.
> 
> Changes since previous version:
> 

note: those are really good, thanks for that, but normally the git history
doesn't has to care about review revision history, a commit should be rather
added as it was the "correct" (subjective) version from the beginning.
If you but these notes a bit below, under the "---" ...

>  * Use map rather than if/else if/else for type display string.
>  * Use Proxmox.Utils.unknownText for unknown type (should not occur with
>    regular use).
>  * Keep existing fields as boxLabel rather than fieldLabel, as they
>    look crammed with fieldLabel.
>  * Use __default__ for default option, to save space and replicate
>    behaviour in other places.
>  * Store option in advancedItems as it is a special case.
> 
> Even though the map only contains one item, it will be easily added to
> in the future. There is only one item as there is no need to have a
> string for "virtio" as it is not displayed because of __default__.
> 
> Signed-off-by: Matt Dunwoodie <ncon at noconroy.net>
> ---

... here, they won't get into git when one applies this mail with `git am`,
but are still very useful for reviewers. Just as a note, I rather have them
in the commit message than none at all :)

>  www/manager6/Utils.js                     | 13 ++++++++++---
>  www/manager6/form/AgentFeatureSelector.js | 14 ++++++++++++++
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 

applied, but re-added the "virtio" in the map and the selector, as else
it would get displayed as "Unknown" if someone set the type explicitly in
the config or through CLI, e.g., with:
# qm set VMID -agent 1,type=virtio
which could be slightly confusing for an user.

Much thanks for this!

> diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
> index 55b06e50..15cd4f4c 100644
> --- a/www/manager6/Utils.js
> +++ b/www/manager6/Utils.js
> @@ -276,10 +276,17 @@ Ext.define('PVE.Utils', { utilities: {
>  	    var keystring = '' ;
>  	    agentstring += ', ' + key + ': ';
>  
> -	    if (PVE.Parser.parseBoolean(value)) {
> -		agentstring += Proxmox.Utils.enabledText;
> +	    if (key === 'type') {
> +		let map = {
> +		    isa: "ISA",
> +		};
> +	        agentstring += map[value] || Proxmox.Utils.unknownText;
>  	    } else {
> -		agentstring += Proxmox.Utils.disabledText;
> +		if (PVE.Parser.parseBoolean(value)) {
> +		    agentstring += Proxmox.Utils.enabledText;
> +		} else {
> +		    agentstring += Proxmox.Utils.disabledText;
> +		}
>  	    }
>  	});
>  
> diff --git a/www/manager6/form/AgentFeatureSelector.js b/www/manager6/form/AgentFeatureSelector.js
> index 44ef2e57..4a968fdb 100644
> --- a/www/manager6/form/AgentFeatureSelector.js
> +++ b/www/manager6/form/AgentFeatureSelector.js
> @@ -31,6 +31,20 @@ Ext.define('PVE.form.AgentFeatureSelector', {
>  	},
>      ],
>  
> +    advancedItems: [
> +	{
> +	    xtype: 'proxmoxKVComboBox',
> +	    name: 'type',
> +	    value: '__default__',
> +	    deleteEmpty: false,
> +	    fieldLabel: 'Type',
> +	    comboItems: [
> +		['__default__', Proxmox.Utils.defaultText + " (VirtIO)"],
> +		['isa', 'ISA'],
> +	    ],
> +	}
> +    ],
> +
>      onGetValues: function(values) {
>  	var agentstr = PVE.Parser.printPropertyString(values, 'enabled');
>  	return { agent: agentstr };
>