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

~dengxuehua posted 1 patch 2 years, 3 months ago
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/164310239719.1016.2386682120107304758-0@git.sr.ht
Maintainers: Gerd Hoffmann <kraxel@redhat.com>
hw/display/qxl.c | 9 +++++++++
1 file changed, 9 insertions(+)
[PATCH qemu] qxl: COLO secondary node not need to release resources
Posted by ~dengxuehua 2 years, 3 months ago
From: Dengxuehua <dengxh2@chinatelecom.cn>

---
From: Dengxuehua<dengxh2@chinatelecom.cn>

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<dengxh2@chinatelecom.cn>

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

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);
-- 
2.32.0

Re: [PATCH qemu] qxl: COLO secondary node not need to release resources
Posted by Gerd Hoffmann 2 years, 3 months ago
> 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