Re: [pve-devel] [PATCH qemu-server v10 1/6] enable VNC clipboard parameter in vga_fmt

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

Am 24/08/2023 um 10:03 schrieb Markus Frank:
> added option to use the qemu vdagent implementation to enable the VNC
> clipboard. When enabled with SPICE the spice-vdagent gets replaced with the QEMU
> implementation.
> 
> This patch does not solve #1406, but does allow copy and paste with
> a running X-session, when spice-vdagent is installed on the guest.
> 
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
>  PVE/API2/Qemu.pm  |  7 ++++++
>  PVE/QemuServer.pm | 61 +++++++++++++++++++++++++++++++++--------------
>  2 files changed, 50 insertions(+), 18 deletions(-)
> 

> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index bf1de17..9dc6e3c 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -195,8 +195,17 @@ my $vga_fmt = {
>  	minimum => 4,
>  	maximum => 512,
>      },
> +    'vnc-clipboard' => {
> +	description => "enable VNC clipboard (requires SPICE guest tools/"
> +	    ."SPICE vdagent installed on the guest OS).",
> +	type => 'boolean',
> +	optional => 1,
> +	default => 0,


I was about to apply this, but when replying to the UI patch I started to
question the boolean approach, especially w.r.t. that we might want to
decouple the chosen "display" and the added SPÌCE channel & features in
the future, so that one can explicitily add SPICE for any display type,
if wanted.

With that in mind it might be slightly more future proof if we use a
enum for the clipoard value, for now it would probably look something
like:

clipboard => {
    description => 'Enable a specific clipboard. If not set, depending on'
        .' the display type the SPICE one will be added.',
    type => 'string',
    enum => ['vnc'],
    optional => 1,
}

it might make sense to add the non-existing part explicitly already now,
e.g., by some 'auto' value in the enum, but no hard feelings here.


The current status API should then be changed accordingly to return a
"clipboard" key with the respective value, if set.

> +    },
>  };
>  
> +my $vnc_clipboard_regex = qr/^(std|cirrus|vmware|virtio|qxl)/;

I'd declare that inline in the "assert_vnc_clipboard_config" directly  as long
as it's not used by other helpers.


> +
>  my $ivshmem_fmt = {
>      size => {
>  	type => 'integer',
> @@ -1375,6 +1384,14 @@ sub pve_verify_hotplug_features {
>      die "unable to parse hotplug option\n";
>  }
>  
> +sub assert_vnc_clipboard_config {
> +    my ($vga) = @_;
> +
> +    if ($vga->{'vnc-clipboard'} && $vga->{type} && $vga->{type} !~ $vnc_clipboard_regex) {
> +	die "vga type $vga->{type} is not compatible with VNC clipboard\n";
> +    }
> +}
> +