[PULL 00/16] migration queue

[PULL 00/16] migration queue

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

The following changes since commit a46e72710566eea0f90f9c673a0f02da0064acce:

  Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20201026' into staging (2020-10-26 14:50:03 +0000)

are available in the Git repository at:

  git://github.com/dagrh/qemu.git tags/pull-migration-20201026a

for you to fetch changes up to a47295014de56e108f359ec859d5499b851f62b8:

  migration-test: Only hide error if !QTEST_LOG (2020-10-26 16:15:04 +0000)

----------------------------------------------------------------
migration pull: 2020-10-26

Another go at Peter's postcopy fixes

Cleanups from Bihong Yu and Peter Maydell.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

----------------------------------------------------------------
Bihong Yu (9):
      migration: Do not use C99 // comments
      migration: Don't use '#' flag of printf format
      migration: Add spaces around operator
      migration: Open brace '{' following struct go on the same line
      migration: Add braces {} for if statement
      migration: Do not initialise statics and globals to 0 or NULL
      migration: Open brace '{' following function declarations go on the next line
      migration: Delete redundant spaces
      migration: using trace_ to replace DPRINTF

Peter Maydell (1):
      migration: Drop unused VMSTATE_FLOAT64 support

Peter Xu (6):
      migration: Pass incoming state into qemu_ufd_copy_ioctl()
      migration: Introduce migrate_send_rp_message_req_pages()
      migration: Maintain postcopy faulted addresses
      migration: Sync requested pages after postcopy recovery
      migration/postcopy: Release fd before going into 'postcopy-pause'
      migration-test: Only hide error if !QTEST_LOG

 include/migration/vmstate.h  | 13 ----------
 migration/block.c            | 40 ++++++++++++++---------------
 migration/migration.c        | 59 +++++++++++++++++++++++++++++++++++++-----
 migration/migration.h        | 24 ++++++++++++++---
 migration/page_cache.c       | 13 +++-------
 migration/postcopy-ram.c     | 27 +++++++++++++++-----
 migration/ram.c              | 14 +++++-----
 migration/rdma.c             |  7 ++---
 migration/savevm.c           | 61 ++++++++++++++++++++++++++++++++++++++++++--
 migration/trace-events       | 16 ++++++++++++
 migration/vmstate-types.c    | 26 -------------------
 migration/vmstate.c          | 10 ++++----
 tests/qtest/migration-test.c |  6 ++++-
 13 files changed, 213 insertions(+), 103 deletions(-)

[PULL 01/16] migration: Drop unused VMSTATE_FLOAT64 support

[Dr. David Alan Gilbert (git)]

From: Peter Maydell <peter.maydell@linaro.org>

Commit ef96e3ae9698d6 in January 2019 removed the last user of the
VMSTATE_FLOAT64* macros. These were used by targets which defined
their floating point register file as an array of 'float64'.

We used to try to maintain a stricter distinction between
'float64' (a type for holding an integer representing an IEEE float)
and 'uint64_t', including having a debug option for 'float64' being
a struct and supposedly mandatory macros for converting between
float64 and uint64_t. We no longer think that's a usefully
strong distinction to draw and we allow ourselves to freely
assume that float64 really is just a 64-bit integer type, so
for new targets we would simply recommend use of the uint64_t type
for a floating point register file. The float64 type remains
as a useful way of documenting in the type signature of helper
functions and the like that they expect to receive an IEEE float
from the TCG generated code rather than an arbitrary integer.

Since the VMSTATE_FLOAT64* macros have no remaining users and
we don't recommend new code uses them, delete them.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20201022120830.5938-1-peter.maydell@linaro.org>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/migration/vmstate.h | 13 -------------
 migration/vmstate-types.c   | 26 --------------------------
 2 files changed, 39 deletions(-)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index f68ed7db13..4d71dc8fba 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -219,7 +219,6 @@ extern const VMStateInfo vmstate_info_uint64;
 #define VMS_NULLPTR_MARKER (0x30U) /* '0' */
 extern const VMStateInfo vmstate_info_nullptr;
 
-extern const VMStateInfo vmstate_info_float64;
 extern const VMStateInfo vmstate_info_cpudouble;
 
 extern const VMStateInfo vmstate_info_timer;
@@ -997,12 +996,6 @@ extern const VMStateInfo vmstate_info_qlist;
     VMSTATE_SINGLE_TEST(_f, _s, _t, 0, vmstate_info_uint64, uint64_t)
 
 
-#define VMSTATE_FLOAT64_V(_f, _s, _v)                                 \
-    VMSTATE_SINGLE(_f, _s, _v, vmstate_info_float64, float64)
-
-#define VMSTATE_FLOAT64(_f, _s)                                       \
-    VMSTATE_FLOAT64_V(_f, _s, 0)
-
 #define VMSTATE_TIMER_PTR_TEST(_f, _s, _test)                             \
     VMSTATE_POINTER_TEST(_f, _s, _test, vmstate_info_timer, QEMUTimer *)
 
@@ -1114,12 +1107,6 @@ extern const VMStateInfo vmstate_info_qlist;
 #define VMSTATE_INT64_ARRAY(_f, _s, _n)                               \
     VMSTATE_INT64_ARRAY_V(_f, _s, _n, 0)
 
-#define VMSTATE_FLOAT64_ARRAY_V(_f, _s, _n, _v)                       \
-    VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_float64, float64)
-
-#define VMSTATE_FLOAT64_ARRAY(_f, _s, _n)                             \
-    VMSTATE_FLOAT64_ARRAY_V(_f, _s, _n, 0)
-
 #define VMSTATE_CPUDOUBLE_ARRAY_V(_f, _s, _n, _v)                     \
     VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_cpudouble, CPU_DoubleU)
 
diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
index 35e784c9d9..e22d41d73d 100644
--- a/migration/vmstate-types.c
+++ b/migration/vmstate-types.c
@@ -420,32 +420,6 @@ const VMStateInfo vmstate_info_uint16_equal = {
     .put  = put_uint16,
 };
 
-/* floating point */
-
-static int get_float64(QEMUFile *f, void *pv, size_t size,
-                       const VMStateField *field)
-{
-    float64 *v = pv;
-
-    *v = make_float64(qemu_get_be64(f));
-    return 0;
-}
-
-static int put_float64(QEMUFile *f, void *pv, size_t size,
-                       const VMStateField *field, QJSON *vmdesc)
-{
-    uint64_t *v = pv;
-
-    qemu_put_be64(f, float64_val(*v));
-    return 0;
-}
-
-const VMStateInfo vmstate_info_float64 = {
-    .name = "float64",
-    .get  = get_float64,
-    .put  = put_float64,
-};
-
 /* CPU_DoubleU type */
 
 static int get_cpudouble(QEMUFile *f, void *pv, size_t size,
-- 
2.28.0

[PULL 02/16] migration: Do not use C99 // comments

[Dr. David Alan Gilbert (git)]

From: Bihong Yu <yubihong@huawei.com>

Signed-off-by: Bihong Yu <yubihong@huawei.com>
Reviewed-by: Chuan Zheng <zhengchuan@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-Id: <1603163448-27122-2-git-send-email-yubihong@huawei.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/block.c | 2 +-
 migration/rdma.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/migration/block.c b/migration/block.c
index 737b6499f9..4b8576b8d6 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -40,7 +40,7 @@
 #define MAX_IO_BUFFERS 512
 #define MAX_PARALLEL_IO 16
 
-//#define DEBUG_BLK_MIGRATION
+/* #define DEBUG_BLK_MIGRATION */
 
 #ifdef DEBUG_BLK_MIGRATION
 #define DPRINTF(fmt, ...) \
diff --git a/migration/rdma.c b/migration/rdma.c
index 0340841fad..0eb42b74f0 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -1288,7 +1288,7 @@ const char *print_wrid(int wrid)
  * workload information or LRU information is available, do not attempt to use
  * this feature except for basic testing.
  */
-//#define RDMA_UNREGISTRATION_EXAMPLE
+/* #define RDMA_UNREGISTRATION_EXAMPLE */
 
 /*
  * Perform a non-optimized memory unregistration after every transfer
-- 
2.28.0

[PULL 03/16] migration: Don't use '#' flag of printf format

[Dr. David Alan Gilbert (git)]

From: Bihong Yu <yubihong@huawei.com>

Signed-off-by: Bihong Yu <yubihong@huawei.com>
Reviewed-by: Chuan Zheng <zhengchuan@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-Id: <1603163448-27122-3-git-send-email-yubihong@huawei.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/block.c | 2 +-
 migration/ram.c   | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/migration/block.c b/migration/block.c
index 4b8576b8d6..273392be78 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -998,7 +998,7 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
                    (addr == 100) ? '\n' : '\r');
             fflush(stdout);
         } else if (!(flags & BLK_MIG_FLAG_EOS)) {
-            fprintf(stderr, "Unknown block migration flags: %#x\n", flags);
+            fprintf(stderr, "Unknown block migration flags: 0x%x\n", flags);
             return -EINVAL;
         }
         ret = qemu_file_get_error(f);
diff --git a/migration/ram.c b/migration/ram.c
index 433489d633..6ed4f9e507 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3298,7 +3298,7 @@ static int ram_load_postcopy(QEMUFile *f)
             multifd_recv_sync_main();
             break;
         default:
-            error_report("Unknown combination of migration flags: %#x"
+            error_report("Unknown combination of migration flags: 0x%x"
                          " (postcopy mode)", flags);
             ret = -EINVAL;
             break;
@@ -3576,7 +3576,7 @@ static int ram_load_precopy(QEMUFile *f)
             if (flags & RAM_SAVE_FLAG_HOOK) {
                 ram_control_load_hook(f, RAM_CONTROL_HOOK, NULL);
             } else {
-                error_report("Unknown combination of migration flags: %#x",
+                error_report("Unknown combination of migration flags: 0x%x",
                              flags);
                 ret = -EINVAL;
             }
-- 
2.28.0

[PULL 04/16] migration: Add spaces around operator

[Dr. David Alan Gilbert (git)]

From: Bihong Yu <yubihong@huawei.com>

Signed-off-by: Bihong Yu <yubihong@huawei.com>
Reviewed-by: Chuan Zheng <zhengchuan@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <1603163448-27122-4-git-send-email-yubihong@huawei.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/migration.c    |  4 ++--
 migration/postcopy-ram.c |  2 +-
 migration/ram.c          |  2 +-
 migration/savevm.c       |  2 +-
 migration/vmstate.c      | 10 +++++-----
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 0575ecb379..e050f5726b 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2478,8 +2478,8 @@ static void migrate_handle_rp_req_pages(MigrationState *ms, const char* rbname,
      * Since we currently insist on matching page sizes, just sanity check
      * we're being asked for whole host pages.
      */
-    if (start & (our_host_ps-1) ||
-       (len & (our_host_ps-1))) {
+    if (start & (our_host_ps - 1) ||
+       (len & (our_host_ps - 1))) {
         error_report("%s: Misaligned page request, start: " RAM_ADDR_FMT
                      " len: %zd", __func__, start, len);
         mark_source_rp_bad(ms);
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 0a2f88a87d..eea92bbd36 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -403,7 +403,7 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
                      strerror(errno));
         goto out;
     }
-    g_assert(((size_t)testarea & (pagesize-1)) == 0);
+    g_assert(((size_t)testarea & (pagesize - 1)) == 0);
 
     reg_struct.range.start = (uintptr_t)testarea;
     reg_struct.range.len = pagesize;
diff --git a/migration/ram.c b/migration/ram.c
index 6ed4f9e507..0aea78f8a1 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1563,7 +1563,7 @@ int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len)
         rs->last_req_rb = ramblock;
     }
     trace_ram_save_queue_pages(ramblock->idstr, start, len);
-    if (start+len > ramblock->used_length) {
+    if (start + len > ramblock->used_length) {
         error_report("%s request overrun start=" RAM_ADDR_FMT " len="
                      RAM_ADDR_FMT " blocklen=" RAM_ADDR_FMT,
                      __func__, start, len, ramblock->used_length);
diff --git a/migration/savevm.c b/migration/savevm.c
index ff33e210eb..e9fd95b1ba 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -520,7 +520,7 @@ static const VMStateDescription vmstate_configuration = {
         VMSTATE_VBUFFER_ALLOC_UINT32(name, SaveState, 0, NULL, len),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (const VMStateDescription*[]) {
+    .subsections = (const VMStateDescription *[]) {
         &vmstate_target_page_bits,
         &vmstate_capabilites,
         &vmstate_uuid,
diff --git a/migration/vmstate.c b/migration/vmstate.c
index bafa890384..e9d2aef66b 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -32,13 +32,13 @@ static int vmstate_n_elems(void *opaque, const VMStateField *field)
     if (field->flags & VMS_ARRAY) {
         n_elems = field->num;
     } else if (field->flags & VMS_VARRAY_INT32) {
-        n_elems = *(int32_t *)(opaque+field->num_offset);
+        n_elems = *(int32_t *)(opaque + field->num_offset);
     } else if (field->flags & VMS_VARRAY_UINT32) {
-        n_elems = *(uint32_t *)(opaque+field->num_offset);
+        n_elems = *(uint32_t *)(opaque + field->num_offset);
     } else if (field->flags & VMS_VARRAY_UINT16) {
-        n_elems = *(uint16_t *)(opaque+field->num_offset);
+        n_elems = *(uint16_t *)(opaque + field->num_offset);
     } else if (field->flags & VMS_VARRAY_UINT8) {
-        n_elems = *(uint8_t *)(opaque+field->num_offset);
+        n_elems = *(uint8_t *)(opaque + field->num_offset);
     }
 
     if (field->flags & VMS_MULTIPLY_ELEMENTS) {
@@ -54,7 +54,7 @@ static int vmstate_size(void *opaque, const VMStateField *field)
     int size = field->size;
 
     if (field->flags & VMS_VBUFFER) {
-        size = *(int32_t *)(opaque+field->size_offset);
+        size = *(int32_t *)(opaque + field->size_offset);
         if (field->flags & VMS_MULTIPLY) {
             size *= field->size;
         }
-- 
2.28.0

[PULL 05/16] migration: Open brace '{' following struct go on the same line

[Dr. David Alan Gilbert (git)]

From: Bihong Yu <yubihong@huawei.com>

Signed-off-by: Bihong Yu <yubihong@huawei.com>
Reviewed-by: Chuan Zheng <zhengchuan@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <1603163448-27122-5-git-send-email-yubihong@huawei.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/migration.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/migration/migration.h b/migration/migration.h
index deb411aaad..99784b44bd 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -124,8 +124,7 @@ struct MigrationClass {
     DeviceClass parent_class;
 };
 
-struct MigrationState
-{
+struct MigrationState {
     /*< private >*/
     DeviceState parent_obj;
 
-- 
2.28.0

[PULL 06/16] migration: Add braces {} for if statement

[Dr. David Alan Gilbert (git)]

From: Bihong Yu <yubihong@huawei.com>

Signed-off-by: Bihong Yu <yubihong@huawei.com>
Reviewed-by: Chuan Zheng <zhengchuan@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <1603163448-27122-6-git-send-email-yubihong@huawei.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/ram.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 0aea78f8a1..09178cc3a3 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -101,14 +101,16 @@ static struct {
 
 static void XBZRLE_cache_lock(void)
 {
-    if (migrate_use_xbzrle())
+    if (migrate_use_xbzrle()) {
         qemu_mutex_lock(&XBZRLE.lock);
+    }
 }
 
 static void XBZRLE_cache_unlock(void)
 {
-    if (migrate_use_xbzrle())
+    if (migrate_use_xbzrle()) {
         qemu_mutex_unlock(&XBZRLE.lock);
+    }
 }
 
 /**
-- 
2.28.0

[PULL 07/16] migration: Do not initialise statics and globals to 0 or NULL

[Dr. David Alan Gilbert (git)]

From: Bihong Yu <yubihong@huawei.com>

Signed-off-by: Bihong Yu <yubihong@huawei.com>
Reviewed-by: Chuan Zheng <zhengchuan@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <1603163448-27122-7-git-send-email-yubihong@huawei.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/ram.c    | 2 +-
 migration/savevm.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 09178cc3a3..2da2b622ab 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2743,7 +2743,7 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host)
  */
 static inline RAMBlock *ram_block_from_stream(QEMUFile *f, int flags)
 {
-    static RAMBlock *block = NULL;
+    static RAMBlock *block;
     char id[256];
     uint8_t len;
 
diff --git a/migration/savevm.c b/migration/savevm.c
index e9fd95b1ba..6ebd7e66ad 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -63,7 +63,7 @@
 #include "qemu/bitmap.h"
 #include "net/announce.h"
 
-const unsigned int postcopy_ram_discard_version = 0;
+const unsigned int postcopy_ram_discard_version;
 
 /* Subcommands for QEMU_VM_COMMAND */
 enum qemu_vm_cmd {
-- 
2.28.0

[PULL 08/16] migration: Open brace '{' following function declarations go on the next line

[Dr. David Alan Gilbert (git)]

From: Bihong Yu <yubihong@huawei.com>

Signed-off-by: Bihong Yu <yubihong@huawei.com>
Reviewed-by: Chuan Zheng <zhengchuan@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <1603163448-27122-8-git-send-email-yubihong@huawei.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/rdma.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 0eb42b74f0..ca4d315597 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -273,7 +273,8 @@ static uint64_t htonll(uint64_t v)
     return u.llv;
 }
 
-static uint64_t ntohll(uint64_t v) {
+static uint64_t ntohll(uint64_t v)
+{
     union { uint32_t lv[2]; uint64_t llv; } u;
     u.llv = v;
     return ((uint64_t)ntohl(u.lv[0]) << 32) | (uint64_t) ntohl(u.lv[1]);
-- 
2.28.0

[PULL 09/16] migration: Delete redundant spaces

[Dr. David Alan Gilbert (git)]

From: Bihong Yu <yubihong@huawei.com>

Signed-off-by: Bihong Yu <yubihong@huawei.com>
Reviewed-by: Chuan Zheng <zhengchuan@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <1603163448-27122-9-git-send-email-yubihong@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/rdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index ca4d315597..00eac34232 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -855,7 +855,7 @@ static int qemu_rdma_broken_ipv6_kernel(struct ibv_context *verbs, Error **errp)
      */
     if (!verbs) {
         int num_devices, x;
-        struct ibv_device ** dev_list = ibv_get_device_list(&num_devices);
+        struct ibv_device **dev_list = ibv_get_device_list(&num_devices);
         bool roce_found = false;
         bool ib_found = false;
 
-- 
2.28.0

[PULL 10/16] migration: using trace_ to replace DPRINTF

[Dr. David Alan Gilbert (git)]

From: Bihong Yu <yubihong@huawei.com>

Signed-off-by: Bihong Yu <yubihong@huawei.com>
Message-Id: <1603179176-5360-1-git-send-email-yubihong@huawei.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/block.c      | 36 ++++++++++++++++++------------------
 migration/page_cache.c | 13 +++----------
 migration/trace-events | 13 +++++++++++++
 3 files changed, 34 insertions(+), 28 deletions(-)

diff --git a/migration/block.c b/migration/block.c
index 273392be78..a950977855 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -26,6 +26,7 @@
 #include "qemu-file.h"
 #include "migration/vmstate.h"
 #include "sysemu/block-backend.h"
+#include "trace.h"
 
 #define BLK_MIG_BLOCK_SIZE           (1 << 20)
 #define BDRV_SECTORS_PER_DIRTY_CHUNK (BLK_MIG_BLOCK_SIZE >> BDRV_SECTOR_BITS)
@@ -434,10 +435,9 @@ static int init_blk_migration(QEMUFile *f)
         block_mig_state.total_sector_sum += sectors;
 
         if (bmds->shared_base) {
-            DPRINTF("Start migration for %s with shared base image\n",
-                    bdrv_get_device_name(bs));
+            trace_migration_block_init_shared(bdrv_get_device_name(bs));
         } else {
-            DPRINTF("Start full migration for %s\n", bdrv_get_device_name(bs));
+            trace_migration_block_init_full(bdrv_get_device_name(bs));
         }
 
         QSIMPLEQ_INSERT_TAIL(&block_mig_state.bmds_list, bmds, entry);
@@ -592,7 +592,7 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds,
     return (bmds->cur_dirty >= bmds->total_sectors);
 
 error:
-    DPRINTF("Error reading sector %" PRId64 "\n", sector);
+    trace_migration_block_save_device_dirty(sector);
     g_free(blk->buf);
     g_free(blk);
     return ret;
@@ -628,9 +628,9 @@ static int flush_blks(QEMUFile *f)
     BlkMigBlock *blk;
     int ret = 0;
 
-    DPRINTF("%s Enter submitted %d read_done %d transferred %d\n",
-            __func__, block_mig_state.submitted, block_mig_state.read_done,
-            block_mig_state.transferred);
+    trace_migration_block_flush_blks("Enter", block_mig_state.submitted,
+                                     block_mig_state.read_done,
+                                     block_mig_state.transferred);
 
     blk_mig_lock();
     while ((blk = QSIMPLEQ_FIRST(&block_mig_state.blk_list)) != NULL) {
@@ -656,9 +656,9 @@ static int flush_blks(QEMUFile *f)
     }
     blk_mig_unlock();
 
-    DPRINTF("%s Exit submitted %d read_done %d transferred %d\n", __func__,
-            block_mig_state.submitted, block_mig_state.read_done,
-            block_mig_state.transferred);
+    trace_migration_block_flush_blks("Exit", block_mig_state.submitted,
+                                     block_mig_state.read_done,
+                                     block_mig_state.transferred);
     return ret;
 }
 
@@ -727,8 +727,8 @@ static int block_save_setup(QEMUFile *f, void *opaque)
 {
     int ret;
 
-    DPRINTF("Enter save live setup submitted %d transferred %d\n",
-            block_mig_state.submitted, block_mig_state.transferred);
+    trace_migration_block_save("setup", block_mig_state.submitted,
+                               block_mig_state.transferred);
 
     qemu_mutex_lock_iothread();
     ret = init_blk_migration(f);
@@ -759,8 +759,8 @@ static int block_save_iterate(QEMUFile *f, void *opaque)
     int64_t last_ftell = qemu_ftell(f);
     int64_t delta_ftell;
 
-    DPRINTF("Enter save live iterate submitted %d transferred %d\n",
-            block_mig_state.submitted, block_mig_state.transferred);
+    trace_migration_block_save("iterate", block_mig_state.submitted,
+                               block_mig_state.transferred);
 
     ret = flush_blks(f);
     if (ret) {
@@ -825,8 +825,8 @@ static int block_save_complete(QEMUFile *f, void *opaque)
 {
     int ret;
 
-    DPRINTF("Enter save live complete submitted %d transferred %d\n",
-            block_mig_state.submitted, block_mig_state.transferred);
+    trace_migration_block_save("complete", block_mig_state.submitted,
+                               block_mig_state.transferred);
 
     ret = flush_blks(f);
     if (ret) {
@@ -851,7 +851,7 @@ static int block_save_complete(QEMUFile *f, void *opaque)
     /* report completion */
     qemu_put_be64(f, (100 << BDRV_SECTOR_BITS) | BLK_MIG_FLAG_PROGRESS);
 
-    DPRINTF("Block migration completed\n");
+    trace_migration_block_save_complete();
 
     qemu_put_be64(f, BLK_MIG_FLAG_EOS);
 
@@ -884,7 +884,7 @@ static void block_save_pending(QEMUFile *f, void *opaque, uint64_t max_size,
         pending = max_size + BLK_MIG_BLOCK_SIZE;
     }
 
-    DPRINTF("Enter save live pending  %" PRIu64 "\n", pending);
+    trace_migration_block_save_pending(pending);
     /* We don't do postcopy */
     *res_precopy_only += pending;
 }
diff --git a/migration/page_cache.c b/migration/page_cache.c
index 775582f453..098b436223 100644
--- a/migration/page_cache.c
+++ b/migration/page_cache.c
@@ -18,14 +18,7 @@
 #include "qapi/error.h"
 #include "qemu/host-utils.h"
 #include "page_cache.h"
-
-#ifdef DEBUG_CACHE
-#define DPRINTF(fmt, ...) \
-    do { fprintf(stdout, "cache: " fmt, ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-    do { } while (0)
-#endif
+#include "trace.h"
 
 /* the page in cache will not be replaced in two cycles */
 #define CACHED_PAGE_LIFETIME 2
@@ -75,7 +68,7 @@ PageCache *cache_init(int64_t new_size, size_t page_size, Error **errp)
     cache->num_items = 0;
     cache->max_num_items = num_pages;
 
-    DPRINTF("Setting cache buckets to %" PRId64 "\n", cache->max_num_items);
+    trace_migration_pagecache_init(cache->max_num_items);
 
     /* We prefer not to abort if there is no memory */
     cache->page_cache = g_try_malloc((cache->max_num_items) *
@@ -169,7 +162,7 @@ int cache_insert(PageCache *cache, uint64_t addr, const uint8_t *pdata,
     if (!it->it_data) {
         it->it_data = g_try_malloc(cache->page_size);
         if (!it->it_data) {
-            DPRINTF("Error allocating page\n");
+            trace_migration_pagecache_insert();
             return -1;
         }
         cache->num_items++;
diff --git a/migration/trace-events b/migration/trace-events
index 338f38b3dd..c7755116b8 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -325,3 +325,16 @@ get_ramblock_vfn_hash(const char *idstr, uint64_t vfn, uint32_t crc) "ramblock n
 calc_page_dirty_rate(const char *idstr, uint32_t new_crc, uint32_t old_crc) "ramblock name: %s, new crc: %" PRIu32 ", old crc: %" PRIu32
 skip_sample_ramblock(const char *idstr, uint64_t ramblock_size) "ramblock name: %s, ramblock size: %" PRIu64
 find_page_matched(const char *idstr) "ramblock %s addr or size changed"
+
+# block.c
+migration_block_init_shared(const char *blk_device_name) "Start migration for %s with shared base image"
+migration_block_init_full(const char *blk_device_name) "Start full migration for %s"
+migration_block_save_device_dirty(int64_t sector) "Error reading sector %" PRId64
+migration_block_flush_blks(const char *action, int submitted, int read_done, int transferred) "%s submitted %d read_done %d transferred %d"
+migration_block_save(const char *mig_stage, int submitted, int transferred) "Enter save live %s submitted %d transferred %d"
+migration_block_save_complete(void) "Block migration completed"
+migration_block_save_pending(uint64_t pending) "Enter save live pending  %" PRIu64
+
+# page_cache.c
+migration_pagecache_init(int64_t max_num_items) "Setting cache buckets to %" PRId64
+migration_pagecache_insert(void) "Error allocating page"
-- 
2.28.0

[PULL 11/16] migration: Pass incoming state into qemu_ufd_copy_ioctl()

[Dr. David Alan Gilbert (git)]

From: Peter Xu <peterx@redhat.com>

It'll be used in follow up patches to access more fields out of it.  Meanwhile
fetch the userfaultfd inside the function.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20201021212721.440373-2-peterx@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/postcopy-ram.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index eea92bbd36..aed4cdaa38 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -1128,10 +1128,12 @@ int postcopy_ram_incoming_setup(MigrationIncomingState *mis)
     return 0;
 }
 
-static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,
+static int qemu_ufd_copy_ioctl(MigrationIncomingState *mis, void *host_addr,
                                void *from_addr, uint64_t pagesize, RAMBlock *rb)
 {
+    int userfault_fd = mis->userfault_fd;
     int ret;
+
     if (from_addr) {
         struct uffdio_copy copy_struct;
         copy_struct.dst = (uint64_t)(uintptr_t)host_addr;
@@ -1185,7 +1187,7 @@ int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
      * which would be slightly cheaper, but we'd have to be careful
      * of the order of updating our page state.
      */
-    if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize, rb)) {
+    if (qemu_ufd_copy_ioctl(mis, host, from, pagesize, rb)) {
         int e = errno;
         error_report("%s: %s copy host: %p from: %p (size: %zd)",
                      __func__, strerror(e), host, from, pagesize);
@@ -1212,7 +1214,7 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
      * but it's not available for everything (e.g. hugetlbpages)
      */
     if (qemu_ram_is_uf_zeroable(rb)) {
-        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, NULL, pagesize, rb)) {
+        if (qemu_ufd_copy_ioctl(mis, host, NULL, pagesize, rb)) {
             int e = errno;
             error_report("%s: %s zero host: %p",
                          __func__, strerror(e), host);
-- 
2.28.0

[PULL 12/16] migration: Introduce migrate_send_rp_message_req_pages()

[Dr. David Alan Gilbert (git)]

From: Peter Xu <peterx@redhat.com>

This is another layer wrapper for sending a page request to the source VM.  The
new migrate_send_rp_message_req_pages() will be used elsewhere in coming
patches.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20201021212721.440373-3-peterx@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/migration.c | 10 ++++++++--
 migration/migration.h |  2 ++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index e050f5726b..ed122ff8d9 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -316,8 +316,8 @@ error:
  *   Start: Address offset within the RB
  *   Len: Length in bytes required - must be a multiple of pagesize
  */
-int migrate_send_rp_req_pages(MigrationIncomingState *mis, RAMBlock *rb,
-                              ram_addr_t start)
+int migrate_send_rp_message_req_pages(MigrationIncomingState *mis,
+                                      RAMBlock *rb, ram_addr_t start)
 {
     uint8_t bufc[12 + 1 + 255]; /* start (8), len (4), rbname up to 256 */
     size_t msglen = 12; /* start + len */
@@ -353,6 +353,12 @@ int migrate_send_rp_req_pages(MigrationIncomingState *mis, RAMBlock *rb,
     return migrate_send_rp_message(mis, msg_type, msglen, bufc);
 }
 
+int migrate_send_rp_req_pages(MigrationIncomingState *mis,
+                              RAMBlock *rb, ram_addr_t start)
+{
+    return migrate_send_rp_message_req_pages(mis, rb, start);
+}
+
 static bool migration_colo_enabled;
 bool migration_incoming_colo_enabled(void)
 {
diff --git a/migration/migration.h b/migration/migration.h
index 99784b44bd..9c1cba98d6 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -332,6 +332,8 @@ void migrate_send_rp_pong(MigrationIncomingState *mis,
                           uint32_t value);
 int migrate_send_rp_req_pages(MigrationIncomingState *mis, RAMBlock *rb,
                               ram_addr_t start);
+int migrate_send_rp_message_req_pages(MigrationIncomingState *mis,
+                                      RAMBlock *rb, ram_addr_t start);
 void migrate_send_rp_recv_bitmap(MigrationIncomingState *mis,
                                  char *block_name);
 void migrate_send_rp_resume_ack(MigrationIncomingState *mis, uint32_t value);
-- 
2.28.0

[PULL 13/16] migration: Maintain postcopy faulted addresses

[Dr. David Alan Gilbert (git)]

From: Peter Xu <peterx@redhat.com>

Maintain a list of faulted addresses on the destination host for which we're
waiting on.  This is implemented using a GTree rather than a real list to make
sure even there're plenty of vCPUs/threads that are faulting, the lookup will
still be fast with O(log(N)) (because we'll do that after placing each page).
It should bring a slight overhead, but ideally that shouldn't be a big problem
simply because in most cases the requested page list will be short.

Actually we did similar things for postcopy blocktime measurements.  This patch
didn't use that simply because:

  (1) blocktime measurement is towards vcpu threads only, but here we need to
      record all faulted addresses, including main thread and external
      thread (like, DPDK via vhost-user).

  (2) blocktime measurement will require UFFD_FEATURE_THREAD_ID, but here we
      don't want to add that extra dependency on the kernel version since not
      necessary.  E.g., we don't need to know which thread faulted on which
      page, we also don't care about multiple threads faulting on the same
      page.  But we only care about what addresses are faulted so waiting for a
      page copying from src.

  (3) blocktime measurement is not enabled by default.  However we need this by
      default especially for postcopy recover.

Another thing to mention is that this patch introduced a new mutex to serialize
the receivedmap and the page_requested tree, however that serialization does
not cover other procedures like UFFDIO_COPY.

Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20201021212721.440373-4-peterx@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/migration.c    | 41 +++++++++++++++++++++++++++++++++++++++-
 migration/migration.h    | 19 ++++++++++++++++++-
 migration/postcopy-ram.c | 17 ++++++++++++++---
 migration/trace-events   |  2 ++
 4 files changed, 74 insertions(+), 5 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index ed122ff8d9..5421042d4a 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -143,6 +143,13 @@ static int migration_maybe_pause(MigrationState *s,
                                  int new_state);
 static void migrate_fd_cancel(MigrationState *s);
 
+static gint page_request_addr_cmp(gconstpointer ap, gconstpointer bp)
+{
+    uintptr_t a = (uintptr_t) ap, b = (uintptr_t) bp;
+
+    return (a > b) - (a < b);
+}
+
 void migration_object_init(void)
 {
     MachineState *ms = MACHINE(qdev_get_machine());
@@ -165,6 +172,8 @@ void migration_object_init(void)
     qemu_event_init(&current_incoming->main_thread_load_event, false);
     qemu_sem_init(&current_incoming->postcopy_pause_sem_dst, 0);
     qemu_sem_init(&current_incoming->postcopy_pause_sem_fault, 0);
+    qemu_mutex_init(&current_incoming->page_request_mutex);
+    current_incoming->page_requested = g_tree_new(page_request_addr_cmp);
 
     if (!migration_object_check(current_migration, &err)) {
         error_report_err(err);
@@ -240,6 +249,11 @@ void migration_incoming_state_destroy(void)
 
     qemu_event_reset(&mis->main_thread_load_event);
 
+    if (mis->page_requested) {
+        g_tree_destroy(mis->page_requested);
+        mis->page_requested = NULL;
+    }
+
     if (mis->socket_address_list) {
         qapi_free_SocketAddressList(mis->socket_address_list);
         mis->socket_address_list = NULL;
@@ -354,8 +368,33 @@ int migrate_send_rp_message_req_pages(MigrationIncomingState *mis,
 }
 
 int migrate_send_rp_req_pages(MigrationIncomingState *mis,
-                              RAMBlock *rb, ram_addr_t start)
+                              RAMBlock *rb, ram_addr_t start, uint64_t haddr)
 {
+    void *aligned = (void *)(uintptr_t)(haddr & (-qemu_ram_pagesize(rb)));
+    bool received;
+
+    WITH_QEMU_LOCK_GUARD(&mis->page_request_mutex) {
+        received = ramblock_recv_bitmap_test_byte_offset(rb, start);
+        if (!received && !g_tree_lookup(mis->page_requested, aligned)) {
+            /*
+             * The page has not been received, and it's not yet in the page
+             * request list.  Queue it.  Set the value of element to 1, so that
+             * things like g_tree_lookup() will return TRUE (1) when found.
+             */
+            g_tree_insert(mis->page_requested, aligned, (gpointer)1);
+            mis->page_requested_count++;
+            trace_postcopy_page_req_add(aligned, mis->page_requested_count);
+        }
+    }
+
+    /*
+     * If the page is there, skip sending the message.  We don't even need the
+     * lock because as long as the page arrived, it'll be there forever.
+     */
+    if (received) {
+        return 0;
+    }
+
     return migrate_send_rp_message_req_pages(mis, rb, start);
 }
 
diff --git a/migration/migration.h b/migration/migration.h
index 9c1cba98d6..d096b77f74 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -104,6 +104,23 @@ struct MigrationIncomingState {
 
     /* List of listening socket addresses  */
     SocketAddressList *socket_address_list;
+
+    /* A tree of pages that we requested to the source VM */
+    GTree *page_requested;
+    /* For debugging purpose only, but would be nice to keep */
+    int page_requested_count;
+    /*
+     * The mutex helps to maintain the requested pages that we sent to the
+     * source, IOW, to guarantee coherent between the page_requests tree and
+     * the per-ramblock receivedmap.  Note! This does not guarantee consistency
+     * of the real page copy procedures (using UFFDIO_[ZERO]COPY).  E.g., even
+     * if one bit in receivedmap is cleared, UFFDIO_COPY could have happened
+     * for that page already.  This is intended so that the mutex won't
+     * serialize and blocked by slow operations like UFFDIO_* ioctls.  However
+     * this should be enough to make sure the page_requested tree always
+     * contains valid information.
+     */
+    QemuMutex page_request_mutex;
 };
 
 MigrationIncomingState *migration_incoming_get_current(void);
@@ -331,7 +348,7 @@ void migrate_send_rp_shut(MigrationIncomingState *mis,
 void migrate_send_rp_pong(MigrationIncomingState *mis,
                           uint32_t value);
 int migrate_send_rp_req_pages(MigrationIncomingState *mis, RAMBlock *rb,
-                              ram_addr_t start);
+                              ram_addr_t start, uint64_t haddr);
 int migrate_send_rp_message_req_pages(MigrationIncomingState *mis,
                                       RAMBlock *rb, ram_addr_t start);
 void migrate_send_rp_recv_bitmap(MigrationIncomingState *mis,
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index aed4cdaa38..d3bb3a744b 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -684,7 +684,7 @@ int postcopy_request_shared_page(struct PostCopyFD *pcfd, RAMBlock *rb,
                                         qemu_ram_get_idstr(rb), rb_offset);
         return postcopy_wake_shared(pcfd, client_addr, rb);
     }
-    migrate_send_rp_req_pages(mis, rb, aligned_rbo);
+    migrate_send_rp_req_pages(mis, rb, aligned_rbo, client_addr);
     return 0;
 }
 
@@ -979,7 +979,8 @@ retry:
              * Send the request to the source - we want to request one
              * of our host page sizes (which is >= TPS)
              */
-            ret = migrate_send_rp_req_pages(mis, rb, rb_offset);
+            ret = migrate_send_rp_req_pages(mis, rb, rb_offset,
+                                            msg.arg.pagefault.address);
             if (ret) {
                 /* May be network failure, try to wait for recovery */
                 if (ret == -EIO && postcopy_pause_fault_thread(mis)) {
@@ -1149,10 +1150,20 @@ static int qemu_ufd_copy_ioctl(MigrationIncomingState *mis, void *host_addr,
         ret = ioctl(userfault_fd, UFFDIO_ZEROPAGE, &zero_struct);
     }
     if (!ret) {
+        qemu_mutex_lock(&mis->page_request_mutex);
         ramblock_recv_bitmap_set_range(rb, host_addr,
                                        pagesize / qemu_target_page_size());
+        /*
+         * If this page resolves a page fault for a previous recorded faulted
+         * address, take a special note to maintain the requested page list.
+         */
+        if (g_tree_lookup(mis->page_requested, host_addr)) {
+            g_tree_remove(mis->page_requested, host_addr);
+            mis->page_requested_count--;
+            trace_postcopy_page_req_del(host_addr, mis->page_requested_count);
+        }
+        qemu_mutex_unlock(&mis->page_request_mutex);
         mark_postcopy_blocktime_end((uintptr_t)host_addr);
-
     }
     return ret;
 }
diff --git a/migration/trace-events b/migration/trace-events
index c7755116b8..5f0001f808 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -162,6 +162,7 @@ postcopy_pause_return_path(void) ""
 postcopy_pause_return_path_continued(void) ""
 postcopy_pause_continued(void) ""
 postcopy_start_set_run(void) ""
+postcopy_page_req_add(void *addr, int count) "new page req %p total %d"
 source_return_path_thread_bad_end(void) ""
 source_return_path_thread_end(void) ""
 source_return_path_thread_entry(void) ""
@@ -272,6 +273,7 @@ postcopy_ram_incoming_cleanup_blocktime(uint64_t total) "total blocktime %" PRIu
 postcopy_request_shared_page(const char *sharer, const char *rb, uint64_t rb_offset) "for %s in %s offset 0x%"PRIx64
 postcopy_request_shared_page_present(const char *sharer, const char *rb, uint64_t rb_offset) "%s already %s offset 0x%"PRIx64
 postcopy_wake_shared(uint64_t client_addr, const char *rb) "at 0x%"PRIx64" in %s"
+postcopy_page_req_del(void *addr, int count) "resolved page req %p total %d"
 
 get_mem_fault_cpu_index(int cpu, uint32_t pid) "cpu: %d, pid: %u"
 
-- 
2.28.0

[PULL 14/16] migration: Sync requested pages after postcopy recovery

[Dr. David Alan Gilbert (git)]

From: Peter Xu <peterx@redhat.com>

We synchronize the requested pages right after a postcopy recovery happens.
This helps to synchronize the prioritized pages on source so that the faulted
threads can be served faster.

Reported-by: Xiaohui Li <xiaohli@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20201021212721.440373-5-peterx@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/savevm.c     | 57 ++++++++++++++++++++++++++++++++++++++++++
 migration/trace-events |  1 +
 2 files changed, 58 insertions(+)

diff --git a/migration/savevm.c b/migration/savevm.c
index 6ebd7e66ad..21ccba9fb3 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2010,6 +2010,49 @@ static int loadvm_postcopy_handle_run(MigrationIncomingState *mis)
     return LOADVM_QUIT;
 }
 
+/* We must be with page_request_mutex held */
+static gboolean postcopy_sync_page_req(gpointer key, gpointer value,
+                                       gpointer data)
+{
+    MigrationIncomingState *mis = data;
+    void *host_addr = (void *) key;
+    ram_addr_t rb_offset;
+    RAMBlock *rb;
+    int ret;
+
+    rb = qemu_ram_block_from_host(host_addr, true, &rb_offset);
+    if (!rb) {
+        /*
+         * This should _never_ happen.  However be nice for a migrating VM to
+         * not crash/assert.  Post an error (note: intended to not use *_once
+         * because we do want to see all the illegal addresses; and this can
+         * never be triggered by the guest so we're safe) and move on next.
+         */
+        error_report("%s: illegal host addr %p", __func__, host_addr);
+        /* Try the next entry */
+        return FALSE;
+    }
+
+    ret = migrate_send_rp_message_req_pages(mis, rb, rb_offset);
+    if (ret) {
+        /* Please refer to above comment. */
+        error_report("%s: send rp message failed for addr %p",
+                     __func__, host_addr);
+        return FALSE;
+    }
+
+    trace_postcopy_page_req_sync(host_addr);
+
+    return FALSE;
+}
+
+static void migrate_send_rp_req_pages_pending(MigrationIncomingState *mis)
+{
+    WITH_QEMU_LOCK_GUARD(&mis->page_request_mutex) {
+        g_tree_foreach(mis->page_requested, postcopy_sync_page_req, mis);
+    }
+}
+
 static int loadvm_postcopy_handle_resume(MigrationIncomingState *mis)
 {
     if (mis->state != MIGRATION_STATUS_POSTCOPY_RECOVER) {
@@ -2032,6 +2075,20 @@ static int loadvm_postcopy_handle_resume(MigrationIncomingState *mis)
     /* Tell source that "we are ready" */
     migrate_send_rp_resume_ack(mis, MIGRATION_RESUME_ACK_VALUE);
 
+    /*
+     * After a postcopy recovery, the source should have lost the postcopy
+     * queue, or potentially the requested pages could have been lost during
+     * the network down phase.  Let's re-sync with the source VM by re-sending
+     * all the pending pages that we eagerly need, so these threads won't get
+     * blocked too long due to the recovery.
+     *
+     * Without this procedure, the faulted destination VM threads (waiting for
+     * page requests right before the postcopy is interrupted) can keep hanging
+     * until the pages are sent by the source during the background copying of
+     * pages, or another thread faulted on the same address accidentally.
+     */
+    migrate_send_rp_req_pages_pending(mis);
+
     return 0;
 }
 
diff --git a/migration/trace-events b/migration/trace-events
index 5f0001f808..75de5004ac 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -49,6 +49,7 @@ vmstate_save(const char *idstr, const char *vmsd_name) "%s, %s"
 vmstate_load(const char *idstr, const char *vmsd_name) "%s, %s"
 postcopy_pause_incoming(void) ""
 postcopy_pause_incoming_continued(void) ""
+postcopy_page_req_sync(void *host_addr) "sync page req %p"
 
 # vmstate.c
 vmstate_load_field_error(const char *field, int ret) "field \"%s\" load failed, ret = %d"
-- 
2.28.0

[PULL 15/16] migration/postcopy: Release fd before going into 'postcopy-pause'

[Dr. David Alan Gilbert (git)]

From: Peter Xu <peterx@redhat.com>

Logically below race could trigger with the old code:

          test program                        migration thread
          ------------                        ----------------
       wait_until('postcopy-pause')
                                          postcopy_pause()
                                            set_state('postcopy-pause')
       do_postcopy_recover()
         arm s->to_dst_file with new fd
                                            release s->to_dst_file [1]

Here [1] could have released the just-installed recoverying channel.  Then the
migration could hang without really resuming.

Instead, it should be very safe to release the fd before setting the state into
'postcopy-pause', because there's no reason for any other thread to touch it
during 'postcopy-active'.

Dave reported a very rare postcopy recovery hang that the migration-test
program waited for the migration to complete in migrate_postcopy_complete().
We do suspect it's the same thing that we're gonna fix here.  Hard to tell.
However since we've noticed this, fix this irrelevant of the hang report.

Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20201021212721.440373-6-peterx@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/migration.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 5421042d4a..627f194b8b 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3178,9 +3178,6 @@ static MigThrError postcopy_pause(MigrationState *s)
     while (true) {
         QEMUFile *file;
 
-        migrate_set_state(&s->state, s->state,
-                          MIGRATION_STATUS_POSTCOPY_PAUSED);
-
         /* Current channel is possibly broken. Release it. */
         assert(s->to_dst_file);
         qemu_mutex_lock(&s->qemu_file_lock);
@@ -3191,6 +3188,9 @@ static MigThrError postcopy_pause(MigrationState *s)
         qemu_file_shutdown(file);
         qemu_fclose(file);
 
+        migrate_set_state(&s->state, s->state,
+                          MIGRATION_STATUS_POSTCOPY_PAUSED);
+
         error_report("Detected IO failure for postcopy. "
                      "Migration paused.");
 
-- 
2.28.0

[PULL 16/16] migration-test: Only hide error if !QTEST_LOG

[Dr. David Alan Gilbert (git)]

From: Peter Xu <peterx@redhat.com>

The errors are very useful when debugging qtest failures, especially when
QTEST_LOG=1 is set.  Let's allow override MigrateStart.hide_stderr when
QTEST_LOG=1 is specified, because that means the user wants to be verbose.

Not very nice to introduce the first QTEST_LOG env access in migration-test.c,
however it should be handy.  Without this patch, I was hacking error_report()
when debugging such errors.  Let's make things easier.

Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20201021212721.440373-7-peterx@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 tests/qtest/migration-test.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index f410ec5996..f2142fbd3c 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -464,6 +464,10 @@ static void migrate_postcopy_start(QTestState *from, QTestState *to)
 }
 
 typedef struct {
+    /*
+     * QTEST_LOG=1 may override this.  When QTEST_LOG=1, we always dump errors
+     * unconditionally, because it means the user would like to be verbose.
+     */
     bool hide_stderr;
     bool use_shmem;
     /* only launch the target process */
@@ -557,7 +561,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
 
     g_free(bootpath);
 
-    if (args->hide_stderr) {
+    if (!getenv("QTEST_LOG") && args->hide_stderr) {
         ignore_stderr = "2>/dev/null";
     } else {
         ignore_stderr = "";
-- 
2.28.0

Re: [PULL 00/16] migration queue

[no-reply]

Patchew URL: https://patchew.org/QEMU/20201026161952.149188-1-dgilbert@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20201026161952.149188-1-dgilbert@redhat.com
Subject: [PULL 00/16] migration queue

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/1603704987-20977-1-git-send-email-kwankhede@nvidia.com -> patchew/1603704987-20977-1-git-send-email-kwankhede@nvidia.com
 * [new tag]         patchew/20201026161952.149188-1-dgilbert@redhat.com -> patchew/20201026161952.149188-1-dgilbert@redhat.com
Switched to a new branch 'test'
eec7173 migration-test: Only hide error if !QTEST_LOG
3cf5619 migration/postcopy: Release fd before going into 'postcopy-pause'
05826d6 migration: Sync requested pages after postcopy recovery
e3ab9bd migration: Maintain postcopy faulted addresses
94c47fd migration: Introduce migrate_send_rp_message_req_pages()
37f5d13 migration: Pass incoming state into qemu_ufd_copy_ioctl()
d212778 migration: using trace_ to replace DPRINTF
57fda59 migration: Delete redundant spaces
5b093f1 migration: Open brace '{' following function declarations go on the next line
0b60dca migration: Do not initialise statics and globals to 0 or NULL
2f219b6 migration: Add braces {} for if statement
3fdcabc migration: Open brace '{' following struct go on the same line
63bb26e migration: Add spaces around operator
fadaa39 migration: Don't use '#' flag of printf format
e568828 migration: Do not use C99 // comments
e477d01 migration: Drop unused VMSTATE_FLOAT64 support

=== OUTPUT BEGIN ===
1/16 Checking commit e477d010bf93 (migration: Drop unused VMSTATE_FLOAT64 support)
2/16 Checking commit e568828c0e63 (migration: Do not use C99 // comments)
3/16 Checking commit fadaa39cfb42 (migration: Don't use '#' flag of printf format)
4/16 Checking commit 63bb26e930c4 (migration: Add spaces around operator)
ERROR: spaces required around that '*' (ctx:WxV)
#65: FILE: migration/savevm.c:523:
+    .subsections = (const VMStateDescription *[]) {
                                              ^

total: 1 errors, 0 warnings, 59 lines checked

Patch 4/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/16 Checking commit 3fdcabce2015 (migration: Open brace '{' following struct go on the same line)
6/16 Checking commit 2f219b67cf64 (migration: Add braces {} for if statement)
7/16 Checking commit 0b60dca56f19 (migration: Do not initialise statics and globals to 0 or NULL)
8/16 Checking commit 5b093f15f482 (migration: Open brace '{' following function declarations go on the next line)
9/16 Checking commit 57fda592d44b (migration: Delete redundant spaces)
10/16 Checking commit d21277810b4a (migration: using trace_ to replace DPRINTF)
11/16 Checking commit 37f5d13fa1bb (migration: Pass incoming state into qemu_ufd_copy_ioctl())
12/16 Checking commit 94c47fdc32f4 (migration: Introduce migrate_send_rp_message_req_pages())
13/16 Checking commit e3ab9bded382 (migration: Maintain postcopy faulted addresses)
14/16 Checking commit 05826d6ec62a (migration: Sync requested pages after postcopy recovery)
15/16 Checking commit 3cf5619e375b (migration/postcopy: Release fd before going into 'postcopy-pause')
16/16 Checking commit eec7173d989d (migration-test: Only hide error if !QTEST_LOG)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20201026161952.149188-1-dgilbert@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PULL 00/16] migration queue

[Dr. David Alan Gilbert]

* no-reply@patchew.org (no-reply@patchew.org) wrote:
> Patchew URL: https://patchew.org/QEMU/20201026161952.149188-1-dgilbert@redhat.com/
> 
> 
> 
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Type: series
> Message-id: 20201026161952.149188-1-dgilbert@redhat.com
> Subject: [PULL 00/16] migration queue
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> git rev-parse base > /dev/null || exit 0
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
> ./scripts/checkpatch.pl --mailback base..
> === TEST SCRIPT END ===
> 
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> From https://github.com/patchew-project/qemu
>  - [tag update]      patchew/1603704987-20977-1-git-send-email-kwankhede@nvidia.com -> patchew/1603704987-20977-1-git-send-email-kwankhede@nvidia.com
>  * [new tag]         patchew/20201026161952.149188-1-dgilbert@redhat.com -> patchew/20201026161952.149188-1-dgilbert@redhat.com
> Switched to a new branch 'test'
> eec7173 migration-test: Only hide error if !QTEST_LOG
> 3cf5619 migration/postcopy: Release fd before going into 'postcopy-pause'
> 05826d6 migration: Sync requested pages after postcopy recovery
> e3ab9bd migration: Maintain postcopy faulted addresses
> 94c47fd migration: Introduce migrate_send_rp_message_req_pages()
> 37f5d13 migration: Pass incoming state into qemu_ufd_copy_ioctl()
> d212778 migration: using trace_ to replace DPRINTF
> 57fda59 migration: Delete redundant spaces
> 5b093f1 migration: Open brace '{' following function declarations go on the next line
> 0b60dca migration: Do not initialise statics and globals to 0 or NULL
> 2f219b6 migration: Add braces {} for if statement
> 3fdcabc migration: Open brace '{' following struct go on the same line
> 63bb26e migration: Add spaces around operator
> fadaa39 migration: Don't use '#' flag of printf format
> e568828 migration: Do not use C99 // comments
> e477d01 migration: Drop unused VMSTATE_FLOAT64 support
> 
> === OUTPUT BEGIN ===
> 1/16 Checking commit e477d010bf93 (migration: Drop unused VMSTATE_FLOAT64 support)
> 2/16 Checking commit e568828c0e63 (migration: Do not use C99 // comments)
> 3/16 Checking commit fadaa39cfb42 (migration: Don't use '#' flag of printf format)
> 4/16 Checking commit 63bb26e930c4 (migration: Add spaces around operator)
> ERROR: spaces required around that '*' (ctx:WxV)
> #65: FILE: migration/savevm.c:523:
> +    .subsections = (const VMStateDescription *[]) {
>                                               ^
> 
> total: 1 errors, 0 warnings, 59 lines checked

We decided that was preferable

> 
> Patch 4/16 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> 5/16 Checking commit 3fdcabce2015 (migration: Open brace '{' following struct go on the same line)
> 6/16 Checking commit 2f219b67cf64 (migration: Add braces {} for if statement)
> 7/16 Checking commit 0b60dca56f19 (migration: Do not initialise statics and globals to 0 or NULL)
> 8/16 Checking commit 5b093f15f482 (migration: Open brace '{' following function declarations go on the next line)
> 9/16 Checking commit 57fda592d44b (migration: Delete redundant spaces)
> 10/16 Checking commit d21277810b4a (migration: using trace_ to replace DPRINTF)
> 11/16 Checking commit 37f5d13fa1bb (migration: Pass incoming state into qemu_ufd_copy_ioctl())
> 12/16 Checking commit 94c47fdc32f4 (migration: Introduce migrate_send_rp_message_req_pages())
> 13/16 Checking commit e3ab9bded382 (migration: Maintain postcopy faulted addresses)
> 14/16 Checking commit 05826d6ec62a (migration: Sync requested pages after postcopy recovery)
> 15/16 Checking commit 3cf5619e375b (migration/postcopy: Release fd before going into 'postcopy-pause')
> 16/16 Checking commit eec7173d989d (migration-test: Only hide error if !QTEST_LOG)
> === OUTPUT END ===
> 
> Test command exited with code: 1
> 
> 
> The full log is available at
> http://patchew.org/logs/20201026161952.149188-1-dgilbert@redhat.com/testing.checkpatch/?type=message.
> ---
> Email generated automatically by Patchew [https://patchew.org/].
> Please send your feedback to patchew-devel@redhat.com
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PULL 00/16] migration queue

[Peter Maydell]

On Mon, 26 Oct 2020 at 16:20, Dr. David Alan Gilbert (git)
<dgilbert@redhat.com> wrote:
>
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> The following changes since commit a46e72710566eea0f90f9c673a0f02da0064acce:
>
>   Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20201026' into staging (2020-10-26 14:50:03 +0000)
>
> are available in the Git repository at:
>
>   git://github.com/dagrh/qemu.git tags/pull-migration-20201026a
>
> for you to fetch changes up to a47295014de56e108f359ec859d5499b851f62b8:
>
>   migration-test: Only hide error if !QTEST_LOG (2020-10-26 16:15:04 +0000)
>
> ----------------------------------------------------------------
> migration pull: 2020-10-26
>
> Another go at Peter's postcopy fixes
>
> Cleanups from Bihong Yu and Peter Maydell.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.

-- PMM

Re: [PULL 00/16] migration queue

[Christian Schoenebeck]

On Montag, 26. Oktober 2020 17:19:36 CET Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> The following changes since commit a46e72710566eea0f90f9c673a0f02da0064acce:
> 
>   Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20201026' into
> staging (2020-10-26 14:50:03 +0000)
> 
> are available in the Git repository at:
> 
>   git://github.com/dagrh/qemu.git tags/pull-migration-20201026a
> 
> for you to fetch changes up to a47295014de56e108f359ec859d5499b851f62b8:
> 
>   migration-test: Only hide error if !QTEST_LOG (2020-10-26 16:15:04 +0000)
> 
> ----------------------------------------------------------------
> migration pull: 2020-10-26
> 
> Another go at Peter's postcopy fixes
> 
> Cleanups from Bihong Yu and Peter Maydell.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 

May it be possible that this PR introduced a lockup of the qtests that I am 
encountering in this week's upstream revisions?

PASS 25 qtest-x86_64/bios-tables-test /x86_64/acpi/microvm/pcie

Looking for expected file 'tests/data/acpi/microvm/FACP.rtc'
Looking for expected file 'tests/data/acpi/microvm/FACP'
Using expected file 'tests/data/acpi/microvm/FACP'
Looking for expected file 'tests/data/acpi/microvm/APIC.rtc'
Looking for expected file 'tests/data/acpi/microvm/APIC'
Using expected file 'tests/data/acpi/microvm/APIC'
Looking for expected file 'tests/data/acpi/microvm/DSDT.rtc'
Using expected file 'tests/data/acpi/microvm/DSDT.rtc'
PASS 24 qtest-i386/bios-tables-test /i386/acpi/microvm/rtc
PASS 15 qtest-i386/migration-test /i386/migration/multifd/tcp/cancel
PASS 16 qtest-i386/migration-test /i386/migration/multifd/tcp/zlib
^Cmake: *** [Makefile.mtest:1169: run-test-144] Interrupt

I tried to bisect the causing commit and came up to this PR merge. But I'm not 
absolutely sure it really is, because it sometimes takes 2 hours or more to 
trigger the lockup, so the relevant commit may as well have slipped during 
bisection.

Last week's revisions run here for >24h without issues, right now I get 
lockups of test runs after ~3min .. ~2h when running the qtests in an endless 
loop:

#/bin/sh
i=0
while true; do
  i=$[$i+1]
  echo '**************************************************'
  echo "* RUN $i                                         *"
  echo '**************************************************'
  make check-qtest -j32 V=1
  if [[ "$?" -ne 0 ]]; then
    echo "Aborted after $i runs due to failure"
    break
  fi
done

> ----------------------------------------------------------------
> Bihong Yu (9):
>       migration: Do not use C99 // comments
>       migration: Don't use '#' flag of printf format
>       migration: Add spaces around operator
>       migration: Open brace '{' following struct go on the same line
>       migration: Add braces {} for if statement
>       migration: Do not initialise statics and globals to 0 or NULL
>       migration: Open brace '{' following function declarations go on the
> next line migration: Delete redundant spaces
>       migration: using trace_ to replace DPRINTF
> 
> Peter Maydell (1):
>       migration: Drop unused VMSTATE_FLOAT64 support
> 
> Peter Xu (6):
>       migration: Pass incoming state into qemu_ufd_copy_ioctl()
>       migration: Introduce migrate_send_rp_message_req_pages()
>       migration: Maintain postcopy faulted addresses
>       migration: Sync requested pages after postcopy recovery
>       migration/postcopy: Release fd before going into 'postcopy-pause'
>       migration-test: Only hide error if !QTEST_LOG
> 
>  include/migration/vmstate.h  | 13 ----------
>  migration/block.c            | 40 ++++++++++++++---------------
>  migration/migration.c        | 59
> +++++++++++++++++++++++++++++++++++++----- migration/migration.h        |
> 24 ++++++++++++++---
>  migration/page_cache.c       | 13 +++-------
>  migration/postcopy-ram.c     | 27 +++++++++++++++-----
>  migration/ram.c              | 14 +++++-----
>  migration/rdma.c             |  7 ++---
>  migration/savevm.c           | 61
> ++++++++++++++++++++++++++++++++++++++++++-- migration/trace-events       |
> 16 ++++++++++++
>  migration/vmstate-types.c    | 26 -------------------
>  migration/vmstate.c          | 10 ++++----
>  tests/qtest/migration-test.c |  6 ++++-
>  13 files changed, 213 insertions(+), 103 deletions(-)

Re: [PULL 00/16] migration queue

[Peter Maydell]

On Sat, 31 Oct 2020 at 16:12, Christian Schoenebeck
<qemu_oss@crudebyte.com> wrote:
>
> On Montag, 26. Oktober 2020 17:19:36 CET Dr. David Alan Gilbert (git) wrote:
> > ----------------------------------------------------------------
> > migration pull: 2020-10-26
> >
> > Another go at Peter's postcopy fixes
> >
> > Cleanups from Bihong Yu and Peter Maydell.
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >
>
> May it be possible that this PR introduced a lockup of the qtests that I am
> encountering in this week's upstream revisions?

If you try the patches Peter Xu attached to this thread
does the lockup go away ?

https://lore.kernel.org/qemu-devel/20201030135350.GA588069@xz-x1/

(I'm also seeing intermittent hangs, for some reason almost always
on s390x host.)

thanks
-- PMM

Re: [PULL 00/16] migration queue

[Peter Xu]

On Sat, Oct 31, 2020 at 05:26:28PM +0000, Peter Maydell wrote:
> On Sat, 31 Oct 2020 at 16:12, Christian Schoenebeck
> <qemu_oss@crudebyte.com> wrote:
> >
> > On Montag, 26. Oktober 2020 17:19:36 CET Dr. David Alan Gilbert (git) wrote:
> > > ----------------------------------------------------------------
> > > migration pull: 2020-10-26
> > >
> > > Another go at Peter's postcopy fixes
> > >
> > > Cleanups from Bihong Yu and Peter Maydell.
> > >
> > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > >
> >
> > May it be possible that this PR introduced a lockup of the qtests that I am
> > encountering in this week's upstream revisions?
> 
> If you try the patches Peter Xu attached to this thread
> does the lockup go away ?
> 
> https://lore.kernel.org/qemu-devel/20201030135350.GA588069@xz-x1/
> 
> (I'm also seeing intermittent hangs, for some reason almost always
> on s390x host.)

It would be good to know exactly which test hanged.  If it's migration-test
then it's very possible.

The race above patch(es) tried to fix should logically be reproducable on all
archs, not s390x only.

Thanks,

-- 
Peter Xu

Re: [PULL 00/16] migration queue

[Christian Schoenebeck]

On Samstag, 31. Oktober 2020 18:46:11 CET Peter Xu wrote:
> On Sat, Oct 31, 2020 at 05:26:28PM +0000, Peter Maydell wrote:
> > On Sat, 31 Oct 2020 at 16:12, Christian Schoenebeck
> > 
> > <qemu_oss@crudebyte.com> wrote:
> > > On Montag, 26. Oktober 2020 17:19:36 CET Dr. David Alan Gilbert (git) 
wrote:
> > > > ----------------------------------------------------------------
> > > > migration pull: 2020-10-26
> > > > 
> > > > Another go at Peter's postcopy fixes
> > > > 
> > > > Cleanups from Bihong Yu and Peter Maydell.
> > > > 
> > > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > > 
> > > May it be possible that this PR introduced a lockup of the qtests that I
> > > am
> > > encountering in this week's upstream revisions?
> > 
> > If you try the patches Peter Xu attached to this thread
> > does the lockup go away ?
> > 
> > https://lore.kernel.org/qemu-devel/20201030135350.GA588069@xz-x1/
> > 
> > (I'm also seeing intermittent hangs, for some reason almost always
> > on s390x host.)
> 
> It would be good to know exactly which test hanged.  If it's migration-test
> then it's very possible.

It's run-test-144 that does not return; according to Makefile.mtest that's 
migration-test, so chances are high that it's indeed introduced by this PR.

> The race above patch(es) tried to fix should logically be reproducable on
> all archs, not s390x only.
> 
> Thanks,

Yes, it's i386 here that locks up.

I'm running the loop with your patches now, so far so good, let's see if it's 
still alive tomorrow.

Best regards,
Christian Schoenebeck

Re: [PULL 00/16] migration queue

[Christian Schoenebeck]

On Samstag, 31. Oktober 2020 20:10:49 CET Christian Schoenebeck wrote:
> On Samstag, 31. Oktober 2020 18:46:11 CET Peter Xu wrote:
> > On Sat, Oct 31, 2020 at 05:26:28PM +0000, Peter Maydell wrote:
> > > On Sat, 31 Oct 2020 at 16:12, Christian Schoenebeck
> > > 
> > > <qemu_oss@crudebyte.com> wrote:
> > > > On Montag, 26. Oktober 2020 17:19:36 CET Dr. David Alan Gilbert (git)
> 
> wrote:
> > > > > ----------------------------------------------------------------
> > > > > migration pull: 2020-10-26
> > > > > 
> > > > > Another go at Peter's postcopy fixes
> > > > > 
> > > > > Cleanups from Bihong Yu and Peter Maydell.
> > > > > 
> > > > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > > > 
> > > > May it be possible that this PR introduced a lockup of the qtests that
> > > > I
> > > > am
> > > > encountering in this week's upstream revisions?
> > > 
> > > If you try the patches Peter Xu attached to this thread
> > > does the lockup go away ?
> > > 
> > > https://lore.kernel.org/qemu-devel/20201030135350.GA588069@xz-x1/
> > > 
> > > (I'm also seeing intermittent hangs, for some reason almost always
> > > on s390x host.)
> > 
> > It would be good to know exactly which test hanged.  If it's
> > migration-test
> > then it's very possible.
> 
> It's run-test-144 that does not return; according to Makefile.mtest that's
> migration-test, so chances are high that it's indeed introduced by this PR.
> 
> > The race above patch(es) tried to fix should logically be reproducable on
> > all archs, not s390x only.
> > 
> > Thanks,
> 
> Yes, it's i386 here that locks up.
> 
> I'm running the loop with your patches now, so far so good, let's see if
> it's still alive tomorrow.
> 
> Best regards,
> Christian Schoenebeck

Looks good! 16h later and the loop is still running here; it also made the 
lockup to disappear on Travis-CI. So Peter Xu's two patches fix the lockup 
problem for me.

Best regards,
Christian Schoenebeck