[pve-devel] [PATCH manager v15 1/2] add clipboard comboBox to VM Options

[pve-devel] [PATCH manager v15 1/2] add clipboard comboBox to VM Options

[Markus Frank]

For SPICE and VNC, a different message is displayed.

Save config in DisplayEdit so that the clipboard setting persist.

Signed-off-by: Markus Frank <m.frank@proxmox.com>
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak <d.csapak@proxmox.com>
---
v15:
* changed style of line break in vncHint field

 www/manager6/qemu/DisplayEdit.js |  8 ++++
 www/manager6/qemu/Options.js     | 80 ++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+)

diff --git a/www/manager6/qemu/DisplayEdit.js b/www/manager6/qemu/DisplayEdit.js
index 9bb1763e..d7cd51a9 100644
--- a/www/manager6/qemu/DisplayEdit.js
+++ b/www/manager6/qemu/DisplayEdit.js
@@ -4,6 +4,9 @@ Ext.define('PVE.qemu.DisplayInputPanel', {
     onlineHelp: 'qm_display',
 
     onGetValues: function(values) {
+	if (typeof this.originalConfig.clipboard !== 'undefined') {
+	    values.clipboard = this.originalConfig.clipboard;
+	}
 	let ret = PVE.Parser.printPropertyString(values, 'type');
 	if (ret === '') {
 	    return { 'delete': 'vga' };
@@ -11,6 +14,11 @@ Ext.define('PVE.qemu.DisplayInputPanel', {
 	return { vga: ret };
     },
 
+    onSetValues: function(values) {
+	this.originalConfig = values;
+	return values;
+    },
+
     items: [{
 	name: 'type',
 	xtype: 'proxmoxKVComboBox',
diff --git a/www/manager6/qemu/Options.js b/www/manager6/qemu/Options.js
index 7b112400..53d0beac 100644
--- a/www/manager6/qemu/Options.js
+++ b/www/manager6/qemu/Options.js
@@ -154,6 +154,86 @@ Ext.define('PVE.qemu.Options', {
 		    },
 		} : undefined,
 	    },
+	    vga: {
+		header: gettext('Clipboard'),
+		defaultValue: false,
+		renderer: function(value) {
+		    let vga = PVE.Parser.parsePropertyString(value, 'type');
+		    if (vga.clipboard) {
+			return vga.clipboard.toUpperCase();
+		    } else {
+			return Proxmox.Utils.defaultText + ' (SPICE)';
+		    }
+		},
+		editor: caps.vms['VM.Config.HWType'] ? {
+		    xtype: 'proxmoxWindowEdit',
+		    subject: gettext('Clipboard'),
+		    onlineHelp: 'qm_display',
+		    items: {
+			xtype: 'pveDisplayInputPanel',
+			referenceHolder: true,
+			items: [
+			    {
+				xtype: 'proxmoxKVComboBox',
+				name: 'clipboard',
+				reference: 'clipboard',
+				itemId: 'clipboardBox',
+				fieldLabel: gettext('Clipboard'),
+				deleteDefaultValue: true,
+				listeners: {
+				    change: function(field, value) {
+					let inputpanel = field.up("inputpanel");
+					let isVnc = value === 'vnc';
+					inputpanel.lookup('vncHint').setVisible(isVnc);
+					inputpanel.lookup('defaultHint').setVisible(!isVnc);
+				    },
+				},
+				value: '__default__',
+				comboItems: [
+				    ['__default__', Proxmox.Utils.defaultText + ' (SPICE)'],
+				    ['vnc', 'VNC'],
+				],
+			    },
+			    {
+				itemId: 'vncHint',
+				name: 'vncHint',
+				reference: 'vncHint',
+				xtype: 'displayfield',
+				userCls: 'pmx-hint',
+				hidden: true,
+				value: gettext('You cannot use the default SPICE clipboard if the VNC Clipboard is selected.') + ' ' +
+				    gettext('VNC Clipboard requires spice-tools installed in the Guest-VM.'),
+			    },
+			    {
+				itemId: 'defaultHint',
+				name: 'defaultHint',
+				reference: 'defaultHint',
+				xtype: 'displayfield',
+				userCls: 'pmx-hint',
+				hidden: false,
+				value: gettext('This option depends on your display type.') + ' ' +
+				    gettext('If the display type uses SPICE you are able to use the default SPICE Clipboard.'),
+			    },
+			],
+			onGetValues: function(values) {
+			    values = Ext.apply(this.originalConfig, values);
+			    if (values.delete === "clipboard") {
+				delete values.clipboard;
+				delete values.delete;
+			    }
+			    let ret = PVE.Parser.printPropertyString(values, 'type');
+			    if (ret === "") {
+				return { 'delete': "vga" };
+			    }
+			    return { vga: ret };
+			},
+			onSetValues: function(values) {
+			    this.originalConfig = PVE.Parser.parsePropertyString(values.vga, 'type');
+			    return this.originalConfig;
+			},
+		    },
+		} : undefined,
+	    },
 	    hotplug: {
 		header: gettext('Hotplug'),
 		defaultValue: 'disk,network,usb',
-- 
2.39.2

[pve-devel] [PATCH docs v15 2/2] add VNC clipboard documentation

[Markus Frank]

Signed-off-by: Markus Frank <m.frank@proxmox.com>
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak <d.csapak@proxmox.com>
---
 qm.adoc | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/qm.adoc b/qm.adoc
index 55a4728..cd0d907 100644
--- a/qm.adoc
+++ b/qm.adoc
@@ -817,6 +817,24 @@ Selecting `serialX` as display 'type' disables the VGA output, and redirects
 the Web Console to the selected serial port. A configured display 'memory'
 setting will be ignored in that case.
 
+.VNC clipboard
+You can enable the VNC clipboard by setting `clipboard` to `vnc`.
+
+----
+# qm set <vmid> -vga <displaytype>,clipboard=vnc
+----
+
+In order to use the clipboard feature, you must first install the
+SPICE guest tools. On Debian-based distributions, this can be achieved
+by installing `spice-vdagent`. For other Operating Systems search for it
+in the offical repositories or see: https://www.spice-space.org/download.html
+
+Once you have installed the spice guest tools, you can use the VNC clipboard
+function (e.g. in the noVNC console panel). However, if you're using
+SPICE, virtio or virgl, you'll need to choose which clipboard to use.
+This is because the default *SPICE* clipboard will be replaced by the
+*VNC* clipboard, if `clipboard` is set to `vnc`.
+
 [[qm_usb_passthrough]]
 USB Passthrough
 ~~~~~~~~~~~~~~~
-- 
2.39.2

[pve-devel] applied: [PATCH docs v15 2/2] add VNC clipboard documentation

[Thomas Lamprecht]

Am 21/11/2023 um 13:39 schrieb Markus Frank:
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
> Tested-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  qm.adoc | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
>

applied, thanks!

Re: [pve-devel] [PATCH manager v15 1/2] add clipboard comboBox to VM Options

[Markus Frank]

Ping, the patch still applies.

On  2023-11-21 13:39, Markus Frank wrote:
> For SPICE and VNC, a different message is displayed.
> 
> Save config in DisplayEdit so that the clipboard setting persist.
> 
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
> Tested-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> v15:
> * changed style of line break in vncHint field
> 
>   www/manager6/qemu/DisplayEdit.js |  8 ++++
>   www/manager6/qemu/Options.js     | 80 ++++++++++++++++++++++++++++++++
>   2 files changed, 88 insertions(+)
> 
> diff --git a/www/manager6/qemu/DisplayEdit.js b/www/manager6/qemu/DisplayEdit.js
> index 9bb1763e..d7cd51a9 100644
> --- a/www/manager6/qemu/DisplayEdit.js
> +++ b/www/manager6/qemu/DisplayEdit.js
> @@ -4,6 +4,9 @@ Ext.define('PVE.qemu.DisplayInputPanel', {
>       onlineHelp: 'qm_display',
>   
>       onGetValues: function(values) {
> +	if (typeof this.originalConfig.clipboard !== 'undefined') {
> +	    values.clipboard = this.originalConfig.clipboard;
> +	}
>   	let ret = PVE.Parser.printPropertyString(values, 'type');
>   	if (ret === '') {
>   	    return { 'delete': 'vga' };
> @@ -11,6 +14,11 @@ Ext.define('PVE.qemu.DisplayInputPanel', {
>   	return { vga: ret };
>       },
>   
> +    onSetValues: function(values) {
> +	this.originalConfig = values;
> +	return values;
> +    },
> +
>       items: [{
>   	name: 'type',
>   	xtype: 'proxmoxKVComboBox',
> diff --git a/www/manager6/qemu/Options.js b/www/manager6/qemu/Options.js
> index 7b112400..53d0beac 100644
> --- a/www/manager6/qemu/Options.js
> +++ b/www/manager6/qemu/Options.js
> @@ -154,6 +154,86 @@ Ext.define('PVE.qemu.Options', {
>   		    },
>   		} : undefined,
>   	    },
> +	    vga: {
> +		header: gettext('Clipboard'),
> +		defaultValue: false,
> +		renderer: function(value) {
> +		    let vga = PVE.Parser.parsePropertyString(value, 'type');
> +		    if (vga.clipboard) {
> +			return vga.clipboard.toUpperCase();
> +		    } else {
> +			return Proxmox.Utils.defaultText + ' (SPICE)';
> +		    }
> +		},
> +		editor: caps.vms['VM.Config.HWType'] ? {
> +		    xtype: 'proxmoxWindowEdit',
> +		    subject: gettext('Clipboard'),
> +		    onlineHelp: 'qm_display',
> +		    items: {
> +			xtype: 'pveDisplayInputPanel',
> +			referenceHolder: true,
> +			items: [
> +			    {
> +				xtype: 'proxmoxKVComboBox',
> +				name: 'clipboard',
> +				reference: 'clipboard',
> +				itemId: 'clipboardBox',
> +				fieldLabel: gettext('Clipboard'),
> +				deleteDefaultValue: true,
> +				listeners: {
> +				    change: function(field, value) {
> +					let inputpanel = field.up("inputpanel");
> +					let isVnc = value === 'vnc';
> +					inputpanel.lookup('vncHint').setVisible(isVnc);
> +					inputpanel.lookup('defaultHint').setVisible(!isVnc);
> +				    },
> +				},
> +				value: '__default__',
> +				comboItems: [
> +				    ['__default__', Proxmox.Utils.defaultText + ' (SPICE)'],
> +				    ['vnc', 'VNC'],
> +				],
> +			    },
> +			    {
> +				itemId: 'vncHint',
> +				name: 'vncHint',
> +				reference: 'vncHint',
> +				xtype: 'displayfield',
> +				userCls: 'pmx-hint',
> +				hidden: true,
> +				value: gettext('You cannot use the default SPICE clipboard if the VNC Clipboard is selected.') + ' ' +
> +				    gettext('VNC Clipboard requires spice-tools installed in the Guest-VM.'),
> +			    },
> +			    {
> +				itemId: 'defaultHint',
> +				name: 'defaultHint',
> +				reference: 'defaultHint',
> +				xtype: 'displayfield',
> +				userCls: 'pmx-hint',
> +				hidden: false,
> +				value: gettext('This option depends on your display type.') + ' ' +
> +				    gettext('If the display type uses SPICE you are able to use the default SPICE Clipboard.'),
> +			    },
> +			],
> +			onGetValues: function(values) {
> +			    values = Ext.apply(this.originalConfig, values);
> +			    if (values.delete === "clipboard") {
> +				delete values.clipboard;
> +				delete values.delete;
> +			    }
> +			    let ret = PVE.Parser.printPropertyString(values, 'type');
> +			    if (ret === "") {
> +				return { 'delete': "vga" };
> +			    }
> +			    return { vga: ret };
> +			},
> +			onSetValues: function(values) {
> +			    this.originalConfig = PVE.Parser.parsePropertyString(values.vga, 'type');
> +			    return this.originalConfig;
> +			},
> +		    },
> +		} : undefined,
> +	    },
>   	    hotplug: {
>   		header: gettext('Hotplug'),
>   		defaultValue: 'disk,network,usb',

Re: [pve-devel] [PATCH manager v15 1/2] add clipboard comboBox to VM Options

[Thomas Lamprecht]

subject should be more in the style of:

ui: qemu: add clipboard selector to options

Am 21/11/2023 um 13:39 schrieb Markus Frank:
> For SPICE and VNC, a different message is displayed.
> 

possibly reference the backend work here,

> Save config in DisplayEdit so that the clipboard setting persist.

this sentence is not really that self-explaining, maybe extend
on the reasoning here.

> 
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
> Tested-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> v15:
> * changed style of line break in vncHint field
> 
>  www/manager6/qemu/DisplayEdit.js |  8 ++++
>  www/manager6/qemu/Options.js     | 80 ++++++++++++++++++++++++++++++++
>  2 files changed, 88 insertions(+)
> 


> diff --git a/www/manager6/qemu/Options.js b/www/manager6/qemu/Options.js
> index 7b112400..53d0beac 100644
> --- a/www/manager6/qemu/Options.js
> +++ b/www/manager6/qemu/Options.js
> @@ -154,6 +154,86 @@ Ext.define('PVE.qemu.Options', {
>  		    },
>  		} : undefined,
>  	    },
> +	    vga: {
> +		header: gettext('Clipboard'),
> +		defaultValue: false,
> +		renderer: function(value) {
> +		    let vga = PVE.Parser.parsePropertyString(value, 'type');
> +		    if (vga.clipboard) {
> +			return vga.clipboard.toUpperCase();
> +		    } else {
> +			return Proxmox.Utils.defaultText + ' (SPICE)';
> +		    }
> +		},
> +		editor: caps.vms['VM.Config.HWType'] ? {
> +		    xtype: 'proxmoxWindowEdit',
> +		    subject: gettext('Clipboard'),
> +		    onlineHelp: 'qm_display',
> +		    items: {
> +			xtype: 'pveDisplayInputPanel',
> +			referenceHolder: true,

It might be avoidable to make this a referenceHolder by using a viewModel,
see below.

> +			items: [
> +			    {
> +				xtype: 'proxmoxKVComboBox',
> +				name: 'clipboard',
> +				reference: 'clipboard',

do we really need a reference here? I see no use of it (i.e., through lookup)

> +				itemId: 'clipboardBox',

the itemId seems also unused, and those have to be globally unique, so best
to be avoided if unsure.


> +				fieldLabel: gettext('Clipboard'),
> +				deleteDefaultValue: true,
> +				listeners: {
> +				    change: function(field, value) {
> +					let inputpanel = field.up("inputpanel");
> +					let isVnc = value === 'vnc';
> +					inputpanel.lookup('vncHint').setVisible(isVnc);
> +					inputpanel.lookup('defaultHint').setVisible(!isVnc);
> +				    },

a viewModel would be nicer to do this, e.g at the input panel level add:

viewModel: {
    data: {
        clipboard: '__default__',
    },
    formulas: {
        isVnc: get => get('clipboard') === 'vnc',
    },
},

Then in the clipboard field bind the value to the viewModel:

bind: {
    value: {clipboard},
},

and in the hint fields bind the visibility state to the formula:

bind: {
    hidden: {isVNC},
    // or reverse: hidden: {!isVNC},
},


> +				},
> +				value: '__default__',
> +				comboItems: [
> +				    ['__default__', Proxmox.Utils.defaultText + ' (SPICE)'],
> +				    ['vnc', 'VNC'],
> +				],
> +			    },
> +			    {
> +				itemId: 'vncHint',
> +				name: 'vncHint',
> +				reference: 'vncHint',

view the viewModel you would avoid the explicit references and let ExtJS built-in
logic handle this.

> +				xtype: 'displayfield',

the xtype should be the first property in item object definitions. immediately
followed by the name.

> +				userCls: 'pmx-hint',
> +				hidden: true,
> +				value: gettext('You cannot use the default SPICE clipboard if the VNC Clipboard is selected.') + ' ' +
> +				    gettext('VNC Clipboard requires spice-tools installed in the Guest-VM.'),
> +			    },
> +			    {
> +				itemId: 'defaultHint',
> +				name: 'defaultHint',
> +				reference: 'defaultHint',

same here w.r.t. reference avoidance by using a viewModel.

> +				xtype: 'displayfield',

same here with xtype definition having to move up.

> +				userCls: 'pmx-hint',
> +				hidden: false,
> +				value: gettext('This option depends on your display type.') + ' ' +
> +				    gettext('If the display type uses SPICE you are able to use the default SPICE Clipboard.'),

Those hints are IMO a good indicator that it would be more fitting to have in
the advanced options of the display hardware.

As while it might be slightly more discoverable here, having to jump between
hardware and options to bring this in sync is  a bit of a nuisance, and docs
can improve discoverability wherever it is.


That said, I have no hard feelings for this, there are some arguments for either,
I'd say just decide on your own.

> +			    },
> +			],
> +			onGetValues: function(values) {
> +			    values = Ext.apply(this.originalConfig, values);
> +			    if (values.delete === "clipboard") {
> +				delete values.clipboard;
> +				delete values.delete;
> +			    }
> +			    let ret = PVE.Parser.printPropertyString(values, 'type');
> +			    if (ret === "") {
> +				return { 'delete': "vga" };
> +			    }
> +			    return { vga: ret };
> +			},
> +			onSetValues: function(values) {
> +			    this.originalConfig = PVE.Parser.parsePropertyString(values.vga, 'type');
> +			    return this.originalConfig;
> +			},
> +		    },
> +		} : undefined,
> +	    },
>  	    hotplug: {
>  		header: gettext('Hotplug'),
>  		defaultValue: 'disk,network,usb',