[PATCH] ui/vnc: Update display update interval when VM state changes to RUNNING

[PATCH] ui/vnc: Update display update interval when VM state changes to RUNNING

[Juraj Marcin]

From: Juraj Marcin <jmarcin@redhat.com>

If a virtual machine is paused for an extended period time, for example,
due to an incoming migration, there are also no changes on the screen.
VNC in such case increases the display update interval by
VNC_REFRESH_INTERVAL_INC (50 ms). The update interval can then grow up
to VNC_REFRESH_INTERVAL_MAX (3000 ms).

When the machine resumes, it can then take up to 3 seconds for the first
display update. Furthermore, the update interval is then halved with
each display update with changes on the screen. If there are moving
elements on the screen, such as a video, this can be perceived as
freezing and stuttering for few seconds before the movement is smooth
again.

This patch resolves this issue, by adding a listener to VM state changes
and changing the update interval when the VM state changes to RUNNING.
The update_displaychangelistener() function updates the internal timer,
and the display is refreshed immediately if the timer is expired.

Signed-off-by: Juraj Marcin <jmarcin@redhat.com>
---
 ui/vnc.c | 12 ++++++++++++
 ui/vnc.h |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/ui/vnc.c b/ui/vnc.c
index 9e097dc4b4..32f8bfd1f9 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3384,6 +3384,16 @@ static const DisplayChangeListenerOps dcl_ops = {
     .dpy_cursor_define    = vnc_dpy_cursor_define,
 };
 
+static void vmstate_change_handler(void *opaque, bool running, RunState state)
+{
+    VncDisplay *vd = opaque;
+
+    if (state != RUN_STATE_RUNNING) {
+        return;
+    }
+    update_displaychangelistener(&vd->dcl, VNC_REFRESH_INTERVAL_BASE);
+}
+
 void vnc_display_init(const char *id, Error **errp)
 {
     VncDisplay *vd;
@@ -3420,6 +3430,8 @@ void vnc_display_init(const char *id, Error **errp)
     vd->dcl.ops = &dcl_ops;
     register_displaychangelistener(&vd->dcl);
     vd->kbd = qkbd_state_init(vd->dcl.con);
+    vd->vmstate_handler_entry = qemu_add_vm_change_state_handler(
+        &vmstate_change_handler, vd);
 }
 
 
diff --git a/ui/vnc.h b/ui/vnc.h
index acc53a2cc1..3bb23acd34 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -185,6 +185,8 @@ struct VncDisplay
 #endif
 
     AudioState *audio_state;
+
+    VMChangeStateEntry *vmstate_handler_entry;
 };
 
 typedef struct VncTight {
-- 
2.49.0

Re: [PATCH] ui/vnc: Update display update interval when VM state changes to RUNNING

[Daniel P. Berrangé]

On Wed, May 21, 2025 at 05:16:13PM +0200, Juraj Marcin wrote:
> From: Juraj Marcin <jmarcin@redhat.com>
> 
> If a virtual machine is paused for an extended period time, for example,
> due to an incoming migration, there are also no changes on the screen.
> VNC in such case increases the display update interval by
> VNC_REFRESH_INTERVAL_INC (50 ms). The update interval can then grow up
> to VNC_REFRESH_INTERVAL_MAX (3000 ms).
> 
> When the machine resumes, it can then take up to 3 seconds for the first
> display update. Furthermore, the update interval is then halved with
> each display update with changes on the screen. If there are moving
> elements on the screen, such as a video, this can be perceived as
> freezing and stuttering for few seconds before the movement is smooth
> again.
> 
> This patch resolves this issue, by adding a listener to VM state changes
> and changing the update interval when the VM state changes to RUNNING.
> The update_displaychangelistener() function updates the internal timer,
> and the display is refreshed immediately if the timer is expired.
> 
> Signed-off-by: Juraj Marcin <jmarcin@redhat.com>
> ---
>  ui/vnc.c | 12 ++++++++++++
>  ui/vnc.h |  2 ++
>  2 files changed, 14 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] ui/vnc: Update display update interval when VM state changes to RUNNING

[Peter Xu]

On Wed, May 21, 2025 at 05:16:13PM +0200, Juraj Marcin wrote:
> From: Juraj Marcin <jmarcin@redhat.com>
> 
> If a virtual machine is paused for an extended period time, for example,
> due to an incoming migration, there are also no changes on the screen.
> VNC in such case increases the display update interval by
> VNC_REFRESH_INTERVAL_INC (50 ms). The update interval can then grow up
> to VNC_REFRESH_INTERVAL_MAX (3000 ms).
> 
> When the machine resumes, it can then take up to 3 seconds for the first
> display update. Furthermore, the update interval is then halved with
> each display update with changes on the screen. If there are moving
> elements on the screen, such as a video, this can be perceived as
> freezing and stuttering for few seconds before the movement is smooth
> again.
> 
> This patch resolves this issue, by adding a listener to VM state changes
> and changing the update interval when the VM state changes to RUNNING.
> The update_displaychangelistener() function updates the internal timer,
> and the display is refreshed immediately if the timer is expired.
> 
> Signed-off-by: Juraj Marcin <jmarcin@redhat.com>

Thanks for looking into it!

Reviewed-by: Peter Xu <peterx@redhat.com>

One trivial comment (and partly, pure question) below,

> ---
>  ui/vnc.c | 12 ++++++++++++
>  ui/vnc.h |  2 ++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 9e097dc4b4..32f8bfd1f9 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -3384,6 +3384,16 @@ static const DisplayChangeListenerOps dcl_ops = {
>      .dpy_cursor_define    = vnc_dpy_cursor_define,
>  };
>  
> +static void vmstate_change_handler(void *opaque, bool running, RunState state)
> +{
> +    VncDisplay *vd = opaque;
> +
> +    if (state != RUN_STATE_RUNNING) {

Just to mention in vm_prepare_start() it's possible we migrate a VM that
used to be suspended, if so it'll keep suspended after migration:

    RunState state = vm_was_suspended ? RUN_STATE_SUSPENDED : RUN_STATE_RUNNING;

Here I'm not sure whether SUSPENDED would also like to update the display
freq.  I don't think it matters hugely, but just to say, if we want we can
simply check "running=true" instead of checking the state to cover both
RUNNING|SUSPENDED cases.

> +        return;
> +    }
> +    update_displaychangelistener(&vd->dcl, VNC_REFRESH_INTERVAL_BASE);
> +}
> +
>  void vnc_display_init(const char *id, Error **errp)
>  {
>      VncDisplay *vd;
> @@ -3420,6 +3430,8 @@ void vnc_display_init(const char *id, Error **errp)
>      vd->dcl.ops = &dcl_ops;
>      register_displaychangelistener(&vd->dcl);
>      vd->kbd = qkbd_state_init(vd->dcl.con);
> +    vd->vmstate_handler_entry = qemu_add_vm_change_state_handler(
> +        &vmstate_change_handler, vd);
>  }
>  
>  
> diff --git a/ui/vnc.h b/ui/vnc.h
> index acc53a2cc1..3bb23acd34 100644
> --- a/ui/vnc.h
> +++ b/ui/vnc.h
> @@ -185,6 +185,8 @@ struct VncDisplay
>  #endif
>  
>      AudioState *audio_state;
> +
> +    VMChangeStateEntry *vmstate_handler_entry;
>  };
>  
>  typedef struct VncTight {
> -- 
> 2.49.0
> 

-- 
Peter Xu

Re: [PATCH] ui/vnc: Update display update interval when VM state changes to RUNNING

[Juraj Marcin]

Hi Peter

On 2025-05-21 12:04, Peter Xu wrote:
> On Wed, May 21, 2025 at 05:16:13PM +0200, Juraj Marcin wrote:
> > From: Juraj Marcin <jmarcin@redhat.com>
> > 
> > If a virtual machine is paused for an extended period time, for example,
> > due to an incoming migration, there are also no changes on the screen.
> > VNC in such case increases the display update interval by
> > VNC_REFRESH_INTERVAL_INC (50 ms). The update interval can then grow up
> > to VNC_REFRESH_INTERVAL_MAX (3000 ms).
> > 
> > When the machine resumes, it can then take up to 3 seconds for the first
> > display update. Furthermore, the update interval is then halved with
> > each display update with changes on the screen. If there are moving
> > elements on the screen, such as a video, this can be perceived as
> > freezing and stuttering for few seconds before the movement is smooth
> > again.
> > 
> > This patch resolves this issue, by adding a listener to VM state changes
> > and changing the update interval when the VM state changes to RUNNING.
> > The update_displaychangelistener() function updates the internal timer,
> > and the display is refreshed immediately if the timer is expired.
> > 
> > Signed-off-by: Juraj Marcin <jmarcin@redhat.com>
> 
> Thanks for looking into it!
> 
> Reviewed-by: Peter Xu <peterx@redhat.com>
> 
> One trivial comment (and partly, pure question) below,
> 
> > ---
> >  ui/vnc.c | 12 ++++++++++++
> >  ui/vnc.h |  2 ++
> >  2 files changed, 14 insertions(+)
> > 
> > diff --git a/ui/vnc.c b/ui/vnc.c
> > index 9e097dc4b4..32f8bfd1f9 100644
> > --- a/ui/vnc.c
> > +++ b/ui/vnc.c
> > @@ -3384,6 +3384,16 @@ static const DisplayChangeListenerOps dcl_ops = {
> >      .dpy_cursor_define    = vnc_dpy_cursor_define,
> >  };
> >  
> > +static void vmstate_change_handler(void *opaque, bool running, RunState state)
> > +{
> > +    VncDisplay *vd = opaque;
> > +
> > +    if (state != RUN_STATE_RUNNING) {
> 
> Just to mention in vm_prepare_start() it's possible we migrate a VM that
> used to be suspended, if so it'll keep suspended after migration:
> 
>     RunState state = vm_was_suspended ? RUN_STATE_SUSPENDED : RUN_STATE_RUNNING;
> 
> Here I'm not sure whether SUSPENDED would also like to update the display
> freq.  I don't think it matters hugely, but just to say, if we want we can
> simply check "running=true" instead of checking the state to cover both
> RUNNING|SUSPENDED cases.

Thank you for the comment. I don't think it is necessary to update the
screen frequency if the machine is suspended. In case there is an
explicit request for that, we can change it then. The display frequency
is still updated when the machine is resumed, same as if it was just
suspended and then resumed without migration.

Best regards

Juraj Marcin

> 
> > +        return;
> > +    }
> > +    update_displaychangelistener(&vd->dcl, VNC_REFRESH_INTERVAL_BASE);
> > +}
> > +
> >  void vnc_display_init(const char *id, Error **errp)
> >  {
> >      VncDisplay *vd;
> > @@ -3420,6 +3430,8 @@ void vnc_display_init(const char *id, Error **errp)
> >      vd->dcl.ops = &dcl_ops;
> >      register_displaychangelistener(&vd->dcl);
> >      vd->kbd = qkbd_state_init(vd->dcl.con);
> > +    vd->vmstate_handler_entry = qemu_add_vm_change_state_handler(
> > +        &vmstate_change_handler, vd);
> >  }
> >  
> >  
> > diff --git a/ui/vnc.h b/ui/vnc.h
> > index acc53a2cc1..3bb23acd34 100644
> > --- a/ui/vnc.h
> > +++ b/ui/vnc.h
> > @@ -185,6 +185,8 @@ struct VncDisplay
> >  #endif
> >  
> >      AudioState *audio_state;
> > +
> > +    VMChangeStateEntry *vmstate_handler_entry;
> >  };
> >  
> >  typedef struct VncTight {
> > -- 
> > 2.49.0
> > 
> 
> -- 
> Peter Xu
> 

Re: [PATCH] ui/vnc: Update display update interval when VM state changes to RUNNING

[Marc-André Lureau]

On Wed, May 21, 2025 at 5:18 PM Juraj Marcin <jmarcin@redhat.com> wrote:
>
> From: Juraj Marcin <jmarcin@redhat.com>
>
> If a virtual machine is paused for an extended period time, for example,
> due to an incoming migration, there are also no changes on the screen.
> VNC in such case increases the display update interval by
> VNC_REFRESH_INTERVAL_INC (50 ms). The update interval can then grow up
> to VNC_REFRESH_INTERVAL_MAX (3000 ms).
>
> When the machine resumes, it can then take up to 3 seconds for the first
> display update. Furthermore, the update interval is then halved with
> each display update with changes on the screen. If there are moving
> elements on the screen, such as a video, this can be perceived as
> freezing and stuttering for few seconds before the movement is smooth
> again.
>
> This patch resolves this issue, by adding a listener to VM state changes
> and changing the update interval when the VM state changes to RUNNING.
> The update_displaychangelistener() function updates the internal timer,
> and the display is refreshed immediately if the timer is expired.
>
> Signed-off-by: Juraj Marcin <jmarcin@redhat.com>

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

> ---
>  ui/vnc.c | 12 ++++++++++++
>  ui/vnc.h |  2 ++
>  2 files changed, 14 insertions(+)
>
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 9e097dc4b4..32f8bfd1f9 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -3384,6 +3384,16 @@ static const DisplayChangeListenerOps dcl_ops = {
>      .dpy_cursor_define    = vnc_dpy_cursor_define,
>  };
>
> +static void vmstate_change_handler(void *opaque, bool running, RunState state)
> +{
> +    VncDisplay *vd = opaque;
> +
> +    if (state != RUN_STATE_RUNNING) {
> +        return;
> +    }
> +    update_displaychangelistener(&vd->dcl, VNC_REFRESH_INTERVAL_BASE);
> +}
> +
>  void vnc_display_init(const char *id, Error **errp)
>  {
>      VncDisplay *vd;
> @@ -3420,6 +3430,8 @@ void vnc_display_init(const char *id, Error **errp)
>      vd->dcl.ops = &dcl_ops;
>      register_displaychangelistener(&vd->dcl);
>      vd->kbd = qkbd_state_init(vd->dcl.con);
> +    vd->vmstate_handler_entry = qemu_add_vm_change_state_handler(
> +        &vmstate_change_handler, vd);
>  }
>
>
> diff --git a/ui/vnc.h b/ui/vnc.h
> index acc53a2cc1..3bb23acd34 100644
> --- a/ui/vnc.h
> +++ b/ui/vnc.h
> @@ -185,6 +185,8 @@ struct VncDisplay
>  #endif
>
>      AudioState *audio_state;
> +
> +    VMChangeStateEntry *vmstate_handler_entry;
>  };
>
>  typedef struct VncTight {
> --
> 2.49.0
>
>


-- 
Marc-André Lureau

Re: [PATCH] ui/vnc: Update display update interval when VM state changes to RUNNING

[Peter Xu]

On Wed, May 21, 2025 at 05:16:13PM +0200, Juraj Marcin wrote:
> From: Juraj Marcin <jmarcin@redhat.com>
> 
> If a virtual machine is paused for an extended period time, for example,
> due to an incoming migration, there are also no changes on the screen.
> VNC in such case increases the display update interval by
> VNC_REFRESH_INTERVAL_INC (50 ms). The update interval can then grow up
> to VNC_REFRESH_INTERVAL_MAX (3000 ms).
> 
> When the machine resumes, it can then take up to 3 seconds for the first
> display update. Furthermore, the update interval is then halved with
> each display update with changes on the screen. If there are moving
> elements on the screen, such as a video, this can be perceived as
> freezing and stuttering for few seconds before the movement is smooth
> again.
> 
> This patch resolves this issue, by adding a listener to VM state changes
> and changing the update interval when the VM state changes to RUNNING.
> The update_displaychangelistener() function updates the internal timer,
> and the display is refreshed immediately if the timer is expired.
> 
> Signed-off-by: Juraj Marcin <jmarcin@redhat.com>

[add Dan]

Ping - anyone is willing to pick this one up?

Thanks,

-- 
Peter Xu

Re: [PATCH] ui/vnc: Update display update interval when VM state changes to RUNNING

[Marc-André Lureau]

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

Hi

On Wed, Jun 11, 2025 at 4:34 PM Peter Xu <peterx@redhat.com> wrote:

> On Wed, May 21, 2025 at 05:16:13PM +0200, Juraj Marcin wrote:
> > From: Juraj Marcin <jmarcin@redhat.com>
> >
> > If a virtual machine is paused for an extended period time, for example,
> > due to an incoming migration, there are also no changes on the screen.
> > VNC in such case increases the display update interval by
> > VNC_REFRESH_INTERVAL_INC (50 ms). The update interval can then grow up
> > to VNC_REFRESH_INTERVAL_MAX (3000 ms).
> >
> > When the machine resumes, it can then take up to 3 seconds for the first
> > display update. Furthermore, the update interval is then halved with
> > each display update with changes on the screen. If there are moving
> > elements on the screen, such as a video, this can be perceived as
> > freezing and stuttering for few seconds before the movement is smooth
> > again.
> >
> > This patch resolves this issue, by adding a listener to VM state changes
> > and changing the update interval when the VM state changes to RUNNING.
> > The update_displaychangelistener() function updates the internal timer,
> > and the display is refreshed immediately if the timer is expired.
> >
> > Signed-off-by: Juraj Marcin <jmarcin@redhat.com>
>
> [add Dan]
>
> Ping - anyone is willing to pick this one up?
>

I haven't started gathering pending UI patches. Feel free to pick it up

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

Re: [PATCH] ui/vnc: Update display update interval when VM state changes to RUNNING

[Peter Xu]

On Wed, Jun 11, 2025 at 04:41:21PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Jun 11, 2025 at 4:34 PM Peter Xu <peterx@redhat.com> wrote:
> 
> > On Wed, May 21, 2025 at 05:16:13PM +0200, Juraj Marcin wrote:
> > > From: Juraj Marcin <jmarcin@redhat.com>
> > >
> > > If a virtual machine is paused for an extended period time, for example,
> > > due to an incoming migration, there are also no changes on the screen.
> > > VNC in such case increases the display update interval by
> > > VNC_REFRESH_INTERVAL_INC (50 ms). The update interval can then grow up
> > > to VNC_REFRESH_INTERVAL_MAX (3000 ms).
> > >
> > > When the machine resumes, it can then take up to 3 seconds for the first
> > > display update. Furthermore, the update interval is then halved with
> > > each display update with changes on the screen. If there are moving
> > > elements on the screen, such as a video, this can be perceived as
> > > freezing and stuttering for few seconds before the movement is smooth
> > > again.
> > >
> > > This patch resolves this issue, by adding a listener to VM state changes
> > > and changing the update interval when the VM state changes to RUNNING.
> > > The update_displaychangelistener() function updates the internal timer,
> > > and the display is refreshed immediately if the timer is expired.
> > >
> > > Signed-off-by: Juraj Marcin <jmarcin@redhat.com>
> >
> > [add Dan]
> >
> > Ping - anyone is willing to pick this one up?
> >
> 
> I haven't started gathering pending UI patches. Feel free to pick it up

I'm queuing it for migration if no one beats me to it. Thanks!

-- 
Peter Xu