[qemu] qxl: COLO secondary node not need to release resources

Commit Message

~dengxuehua Jan. 25, 2022, 6:33 a.m. UTC

From: Dengxuehua <[e-mail]>

---
From: Dengxuehua<[e-mail]>

During COLO checkpoint,
the Secondary VM's qemu has loaded Primary VM's qxl states,
so it not
need to release qxl resources.

Resolves: https://gitlab.com/qemu-
project/qemu/-/issues/839
Signed-off-by:
Dengxuehua<[e-mail]>

 hw/display/qxl.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Gerd Hoffmann Jan. 26, 2022, 9:31 a.m. UTC | #1

> During COLO checkpoint, the Secondary VM's qemu has loaded Primary
> VM's qxl states, so it not need to release qxl resources.

> +#include "migration/colo.h"
>  #include "trace.h"
>  
>  #include "qxl.h"
> @@ -757,6 +758,10 @@ static void interface_release_resource(QXLInstance *sin,
>      if (!ext.info) {
>          return;
>      }
> +    /* The SVM load PVM states,so it not need to release resources */
> +    if (get_colo_mode() == COLO_MODE_SECONDARY) {
> +        return;
> +    }
>      if (ext.group_id == MEMSLOT_GROUP_HOST) {
>          /* host group -> vga mode update request */
>          QXLCommandExt *cmdext = (void *)(intptr_t)(ext.info->id);
> @@ -880,6 +885,10 @@ static int interface_flush_resources(QXLInstance *sin)
>      PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
>      int ret;
>  
> +    /* The SVM load PVM states,so it not need to release resources */
> +    if (get_colo_mode() == COLO_MODE_SECONDARY) {
> +        return 0;
> +    }
>      ret = qxl->num_free_res;
>      if (ret) {
>          qxl_push_free_res(qxl, 1);

Hmm, not sure what to do with this one.  I know next to nothing about
COLO, but I suspect this is just papering over the root cause.

Some qxl functionality is handed by a thread started by libspice-server.
The thread can be started and stopped (see qemu_spice_display_start,
qemu_spice_display_stop).  There is a handler (vm_change_state_handler)
which starts/stops depending on vm state, specifically the thread is
stopped when saving+loading vmstate to avoid problems simliar to the one
you are seeing with colo.

What I think you need is proper management of that thread when colo is
active.  Possibly it should just never be active on a secondary node,
but I don't know enough about colo to be sure.

HTH & take care,
  Gerd

Patch

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 1f9ad31943..41af36344a 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -31,6 +31,7 @@ 
 #include "hw/qdev-properties.h"
 #include "sysemu/runstate.h"
 #include "migration/vmstate.h"
+#include "migration/colo.h"
 #include "trace.h"
 
 #include "qxl.h"
@@ -757,6 +758,10 @@  static void interface_release_resource(QXLInstance *sin,
     if (!ext.info) {
         return;
     }
+    /* The SVM load PVM states,so it not need to release resources */
+    if (get_colo_mode() == COLO_MODE_SECONDARY) {
+        return;
+    }
     if (ext.group_id == MEMSLOT_GROUP_HOST) {
         /* host group -> vga mode update request */
         QXLCommandExt *cmdext = (void *)(intptr_t)(ext.info->id);
@@ -880,6 +885,10 @@  static int interface_flush_resources(QXLInstance *sin)
     PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
     int ret;
 
+    /* The SVM load PVM states,so it not need to release resources */
+    if (get_colo_mode() == COLO_MODE_SECONDARY) {
+        return 0;
+    }
     ret = qxl->num_free_res;
     if (ret) {
         qxl_push_free_res(qxl, 1);