qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ui/vdagent: fix windows agent regression
@ 2025-10-27 13:07 marcandre.lureau
  2025-10-29 13:50 ` Lucas Kornicki
  2025-11-17 12:00 ` Fiona Ebner
  0 siblings, 2 replies; 3+ messages in thread
From: marcandre.lureau @ 2025-10-27 13:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: lucas.kornicki, Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Since commit f626116f ("ui/vdagent: factor out clipboard peer
registration"), the QEMU clipboard serial is reset whenever the vdagent
chardev receives the guest caps. This triggers a CHR_EVENT_CLOSED which
is handled by virtio_serial_close() to notify the guest.

The "reconnection logic" is there to reset the agent when a
client (dbus, spice etc) reconnects, or the agent is restarted.
It is required to sync the clipboard serials and to prevent races or
loops due to clipboard managers on both ends (but this is not
implemented by windows vdagent).

The Unix agent has been reconnecting without resending caps, thus
working with this approach.

However, the Windows agent does not seem to have a way to handle
VIRTIO_CONSOLE_PORT_OPEN=0 event and do not receive further data...

Let's not trigger this disconnection/reset logic if the agent does not
support VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL.

Fixes: f626116f ("ui/vdagent: factor out clipboard peer registration")
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reported-by: Lucas Kornicki <lucas.kornicki@nutanix.com>
---
 ui/vdagent.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/ui/vdagent.c b/ui/vdagent.c
index ddb91e75c6..660686c9c0 100644
--- a/ui/vdagent.c
+++ b/ui/vdagent.c
@@ -316,6 +316,15 @@ static bool have_selection(VDAgentChardev *vd)
     return vd->caps & (1 << VD_AGENT_CAP_CLIPBOARD_SELECTION);
 }
 
+static bool have_clipboard_serial(VDAgentChardev *vd)
+{
+#if CHECK_SPICE_PROTOCOL_VERSION(0, 14, 1)
+    return vd->caps & (1 << VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL);
+#else
+    return false;
+#endif
+}
+
 static uint32_t type_qemu_to_vdagent(enum QemuClipboardType type)
 {
     switch (type) {
@@ -345,8 +354,7 @@ static void vdagent_send_clipboard_grab(VDAgentChardev *vd,
         return;
     }
 
-#if CHECK_SPICE_PROTOCOL_VERSION(0, 14, 1)
-    if (vd->caps & (1 << VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL)) {
+    if (have_clipboard_serial(vd)) {
         if (!info->has_serial) {
             /* client should win */
             info->serial = vd->last_serial[info->selection]++;
@@ -356,7 +364,6 @@ static void vdagent_send_clipboard_grab(VDAgentChardev *vd,
         data++;
         msg->size += sizeof(uint32_t);
     }
-#endif
 
     for (q = 0; q < QEMU_CLIPBOARD_TYPE__COUNT; q++) {
         type = type_qemu_to_vdagent(q);
@@ -464,6 +471,9 @@ static void vdagent_clipboard_reset_serial(VDAgentChardev *vd)
 {
     Chardev *chr = CHARDEV(vd);
 
+    if (!have_clipboard_serial(vd)) {
+        return;
+    }
     /* reopen the agent connection to reset the serial state */
     qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
     /* OPENED again after the guest disconnected, see set_fe_open */
@@ -518,8 +528,7 @@ static void vdagent_clipboard_recv_grab(VDAgentChardev *vd, uint8_t s, uint32_t
 
     trace_vdagent_cb_grab_selection(GET_NAME(sel_name, s));
     info = qemu_clipboard_info_new(&vd->cbpeer, s);
-#if CHECK_SPICE_PROTOCOL_VERSION(0, 14, 1)
-    if (vd->caps & (1 << VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL)) {
+    if (have_clipboard_serial(vd)) {
         if (size < sizeof(uint32_t)) {
             /* this shouldn't happen! */
             return;
@@ -537,7 +546,6 @@ static void vdagent_clipboard_recv_grab(VDAgentChardev *vd, uint8_t s, uint32_t
         data += sizeof(uint32_t);
         size -= sizeof(uint32_t);
     }
-#endif
     if (size > sizeof(uint32_t) * 10) {
         /*
          * spice has 6 types as of 2021. Limiting to 10 entries
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] ui/vdagent: fix windows agent regression
  2025-10-27 13:07 [PATCH] ui/vdagent: fix windows agent regression marcandre.lureau
@ 2025-10-29 13:50 ` Lucas Kornicki
  2025-11-17 12:00 ` Fiona Ebner
  1 sibling, 0 replies; 3+ messages in thread
From: Lucas Kornicki @ 2025-10-29 13:50 UTC (permalink / raw)
  To: marcandre.lureau; +Cc: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 4056 bytes --]

I've tested this patch on windows 11 and windows server 2022. In both 
cases the device stays open after serial reset and clipboard operations 
work. Thanks a lot

Tested-by: Lucas Kornicki <lucas.kornicki@nutanix.com>

On 10/27/25 14:07, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau<marcandre.lureau@redhat.com>
>
> Since commit f626116f ("ui/vdagent: factor out clipboard peer
> registration"), the QEMU clipboard serial is reset whenever the vdagent
> chardev receives the guest caps. This triggers a CHR_EVENT_CLOSED which
> is handled by virtio_serial_close() to notify the guest.
>
> The "reconnection logic" is there to reset the agent when a
> client (dbus, spice etc) reconnects, or the agent is restarted.
> It is required to sync the clipboard serials and to prevent races or
> loops due to clipboard managers on both ends (but this is not
> implemented by windows vdagent).
>
> The Unix agent has been reconnecting without resending caps, thus
> working with this approach.
>
> However, the Windows agent does not seem to have a way to handle
> VIRTIO_CONSOLE_PORT_OPEN=0 event and do not receive further data...
>
> Let's not trigger this disconnection/reset logic if the agent does not
> support VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL.
>
> Fixes: f626116f ("ui/vdagent: factor out clipboard peer registration")
> Signed-off-by: Marc-André Lureau<marcandre.lureau@redhat.com>
> Reported-by: Lucas Kornicki<lucas.kornicki@nutanix.com>
> ---
>   ui/vdagent.c | 20 ++++++++++++++------
>   1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/ui/vdagent.c b/ui/vdagent.c
> index ddb91e75c6..660686c9c0 100644
> --- a/ui/vdagent.c
> +++ b/ui/vdagent.c
> @@ -316,6 +316,15 @@ static bool have_selection(VDAgentChardev *vd)
>       return vd->caps & (1 << VD_AGENT_CAP_CLIPBOARD_SELECTION);
>   }
>   
> +static bool have_clipboard_serial(VDAgentChardev *vd)
> +{
> +#if CHECK_SPICE_PROTOCOL_VERSION(0, 14, 1)
> +    return vd->caps & (1 << VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL);
> +#else
> +    return false;
> +#endif
> +}
> +
>   static uint32_t type_qemu_to_vdagent(enum QemuClipboardType type)
>   {
>       switch (type) {
> @@ -345,8 +354,7 @@ static void vdagent_send_clipboard_grab(VDAgentChardev *vd,
>           return;
>       }
>   
> -#if CHECK_SPICE_PROTOCOL_VERSION(0, 14, 1)
> -    if (vd->caps & (1 << VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL)) {
> +    if (have_clipboard_serial(vd)) {
>           if (!info->has_serial) {
>               /* client should win */
>               info->serial = vd->last_serial[info->selection]++;
> @@ -356,7 +364,6 @@ static void vdagent_send_clipboard_grab(VDAgentChardev *vd,
>           data++;
>           msg->size += sizeof(uint32_t);
>       }
> -#endif
>   
>       for (q = 0; q < QEMU_CLIPBOARD_TYPE__COUNT; q++) {
>           type = type_qemu_to_vdagent(q);
> @@ -464,6 +471,9 @@ static void vdagent_clipboard_reset_serial(VDAgentChardev *vd)
>   {
>       Chardev *chr = CHARDEV(vd);
>   
> +    if (!have_clipboard_serial(vd)) {
> +        return;
> +    }
>       /* reopen the agent connection to reset the serial state */
>       qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
>       /* OPENED again after the guest disconnected, see set_fe_open */
> @@ -518,8 +528,7 @@ static void vdagent_clipboard_recv_grab(VDAgentChardev *vd, uint8_t s, uint32_t
>   
>       trace_vdagent_cb_grab_selection(GET_NAME(sel_name, s));
>       info = qemu_clipboard_info_new(&vd->cbpeer, s);
> -#if CHECK_SPICE_PROTOCOL_VERSION(0, 14, 1)
> -    if (vd->caps & (1 << VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL)) {
> +    if (have_clipboard_serial(vd)) {
>           if (size < sizeof(uint32_t)) {
>               /* this shouldn't happen! */
>               return;
> @@ -537,7 +546,6 @@ static void vdagent_clipboard_recv_grab(VDAgentChardev *vd, uint8_t s, uint32_t
>           data += sizeof(uint32_t);
>           size -= sizeof(uint32_t);
>       }
> -#endif
>       if (size > sizeof(uint32_t) * 10) {
>           /*
>            * spice has 6 types as of 2021. Limiting to 10 entries

[-- Attachment #2: Type: text/html, Size: 4784 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ui/vdagent: fix windows agent regression
  2025-10-27 13:07 [PATCH] ui/vdagent: fix windows agent regression marcandre.lureau
  2025-10-29 13:50 ` Lucas Kornicki
@ 2025-11-17 12:00 ` Fiona Ebner
  1 sibling, 0 replies; 3+ messages in thread
From: Fiona Ebner @ 2025-11-17 12:00 UTC (permalink / raw)
  To: marcandre.lureau, qemu-devel; +Cc: lucas.kornicki, qemu-stable

Am 27.10.25 um 2:09 PM schrieb marcandre.lureau@redhat.com:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Since commit f626116f ("ui/vdagent: factor out clipboard peer
> registration"), the QEMU clipboard serial is reset whenever the vdagent
> chardev receives the guest caps. This triggers a CHR_EVENT_CLOSED which
> is handled by virtio_serial_close() to notify the guest.
> 
> The "reconnection logic" is there to reset the agent when a
> client (dbus, spice etc) reconnects, or the agent is restarted.
> It is required to sync the clipboard serials and to prevent races or
> loops due to clipboard managers on both ends (but this is not
> implemented by windows vdagent).
> 
> The Unix agent has been reconnecting without resending caps, thus
> working with this approach.
> 
> However, the Windows agent does not seem to have a way to handle
> VIRTIO_CONSOLE_PORT_OPEN=0 event and do not receive further data...
> 
> Let's not trigger this disconnection/reset logic if the agent does not
> support VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL.
> 
> Fixes: f626116f ("ui/vdagent: factor out clipboard peer registration")
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reported-by: Lucas Kornicki <lucas.kornicki@nutanix.com>

Thank you for the fix!

Tested-by: Fiona Ebner <f.ebner@proxmox.com>

and with my very limited knowledge in the area:

Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>

CC: qemu-stable, because it affects 10.1



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-11-17 12:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-27 13:07 [PATCH] ui/vdagent: fix windows agent regression marcandre.lureau
2025-10-29 13:50 ` Lucas Kornicki
2025-11-17 12:00 ` Fiona Ebner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).