[Qemu-devel] [PATCH v2 0/6] Migration: Small fixes and improvements

[Qemu-devel] [PATCH v2 0/6] Migration: Small fixes and improvements

[Dr. David Alan Gilbert (git)]

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

This is a set of 4 small fixes and improvements I've been collecting
during the 2.5 freeze; I've added in a couple of Zhanghailiang's patches
that are easier to merge before rather than after.
(Of which one I've added a small fix of my own, see the message)

Jiri was after events from the incoming postcopy side to make it
easy for libvirt; I'd forgotten to add them - Zhanghailiang's
patches change the way the events get sent on the incoming side.

Jiri also wanted an event on the outgoing side at each pass,
which makes it easy for libvirt to trigger the transition to postcopy.

The other two patches avoid a couple of buffer copies on the incoming
side; although in both xbzrle and multithread compression it's rare that
the destination is the slow path (but one patch is trivial and the
other saves code).

Dave

v2
  1/6 update text based on Juan's comments
  4/6 Document 'pass' component of event, update qmp-events.txt


Dr. David Alan Gilbert (4):
  Postcopy: Send events/change state on incoming side
  Migration: Emit event at start of pass
  Use qemu_get_buffer_in_place for xbzrle data
  multithread decompression: Avoid one copy

zhanghailiang (2):
  migration: Export migrate_set_state()
  migration: Add state records for migration incoming

 docs/qmp-events.txt           | 14 ++++++++++++
 include/migration/migration.h |  3 +++
 migration/migration.c         | 52 ++++++++++++++++++++++++++-----------------
 migration/ram.c               | 21 ++++++++---------
 migration/savevm.c            | 22 ++++++++++++------
 qapi/event.json               | 13 +++++++++++
 6 files changed, 87 insertions(+), 38 deletions(-)

-- 
2.5.0

[Qemu-devel] [PATCH v2 1/6] migration: Export migrate_set_state()

[Dr. David Alan Gilbert (git)]

From: zhanghailiang <zhang.zhanghailiang@huawei.com>

Change the first parameter of migrate_set_state(), and export it.
We will use it in a later patch to update incoming state.

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

dgilbert: Updated comment as per Juan's review
---
 include/migration/migration.h |  2 ++
 migration/migration.c         | 36 +++++++++++++++++++++---------------
 2 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/include/migration/migration.h b/include/migration/migration.h
index fd018b7..8340432 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -169,6 +169,8 @@ struct MigrationState
     RAMBlock *last_req_rb;
 };
 
+void migrate_set_state(int *state, int old_state, int new_state);
+
 void process_incoming_migration(QEMUFile *f);
 
 void qemu_start_incoming_migration(const char *uri, Error **errp);
diff --git a/migration/migration.c b/migration/migration.c
index adc6b6f..ba6d4f6 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -787,9 +787,9 @@ void qmp_migrate_start_postcopy(Error **errp)
 
 /* shared migration helpers */
 
-static void migrate_set_state(MigrationState *s, int old_state, int new_state)
+void migrate_set_state(int *state, int old_state, int new_state)
 {
-    if (atomic_cmpxchg(&s->state, old_state, new_state) == old_state) {
+    if (atomic_cmpxchg(state, old_state, new_state) == old_state) {
         trace_migrate_set_state(new_state);
         migrate_generate_event(new_state);
     }
@@ -822,7 +822,7 @@ static void migrate_fd_cleanup(void *opaque)
            (s->state != MIGRATION_STATUS_POSTCOPY_ACTIVE));
 
     if (s->state == MIGRATION_STATUS_CANCELLING) {
-        migrate_set_state(s, MIGRATION_STATUS_CANCELLING,
+        migrate_set_state(&s->state, MIGRATION_STATUS_CANCELLING,
                           MIGRATION_STATUS_CANCELLED);
     }
 
@@ -833,7 +833,8 @@ void migrate_fd_error(MigrationState *s)
 {
     trace_migrate_fd_error();
     assert(s->file == NULL);
-    migrate_set_state(s, MIGRATION_STATUS_SETUP, MIGRATION_STATUS_FAILED);
+    migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
+                      MIGRATION_STATUS_FAILED);
     notifier_list_notify(&migration_state_notifiers, s);
 }
 
@@ -853,7 +854,7 @@ static void migrate_fd_cancel(MigrationState *s)
         if (!migration_is_setup_or_active(old_state)) {
             break;
         }
-        migrate_set_state(s, old_state, MIGRATION_STATUS_CANCELLING);
+        migrate_set_state(&s->state, old_state, MIGRATION_STATUS_CANCELLING);
     } while (s->state != MIGRATION_STATUS_CANCELLING);
 
     /*
@@ -927,7 +928,7 @@ MigrationState *migrate_init(const MigrationParams *params)
     s->migration_thread_running = false;
     s->last_req_rb = NULL;
 
-    migrate_set_state(s, MIGRATION_STATUS_NONE, MIGRATION_STATUS_SETUP);
+    migrate_set_state(&s->state, MIGRATION_STATUS_NONE, MIGRATION_STATUS_SETUP);
 
     QSIMPLEQ_INIT(&s->src_page_requests);
 
@@ -1026,7 +1027,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
     } else {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "uri",
                    "a valid migration protocol");
-        migrate_set_state(s, MIGRATION_STATUS_SETUP, MIGRATION_STATUS_FAILED);
+        migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
+                          MIGRATION_STATUS_FAILED);
         return;
     }
 
@@ -1405,7 +1407,7 @@ static int postcopy_start(MigrationState *ms, bool *old_vm_running)
     int ret;
     const QEMUSizedBuffer *qsb;
     int64_t time_at_stop = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
-    migrate_set_state(ms, MIGRATION_STATUS_ACTIVE,
+    migrate_set_state(&ms->state, MIGRATION_STATUS_ACTIVE,
                       MIGRATION_STATUS_POSTCOPY_ACTIVE);
 
     trace_postcopy_start();
@@ -1496,7 +1498,7 @@ static int postcopy_start(MigrationState *ms, bool *old_vm_running)
     ret = qemu_file_get_error(ms->file);
     if (ret) {
         error_report("postcopy_start: Migration stream errored");
-        migrate_set_state(ms, MIGRATION_STATUS_POSTCOPY_ACTIVE,
+        migrate_set_state(&ms->state, MIGRATION_STATUS_POSTCOPY_ACTIVE,
                               MIGRATION_STATUS_FAILED);
     }
 
@@ -1505,7 +1507,7 @@ static int postcopy_start(MigrationState *ms, bool *old_vm_running)
 fail_closefb:
     qemu_fclose(fb);
 fail:
-    migrate_set_state(ms, MIGRATION_STATUS_POSTCOPY_ACTIVE,
+    migrate_set_state(&ms->state, MIGRATION_STATUS_POSTCOPY_ACTIVE,
                           MIGRATION_STATUS_FAILED);
     qemu_mutex_unlock_iothread();
     return -1;
@@ -1574,11 +1576,13 @@ static void migration_completion(MigrationState *s, int current_active_state,
         goto fail;
     }
 
-    migrate_set_state(s, current_active_state, MIGRATION_STATUS_COMPLETED);
+    migrate_set_state(&s->state, current_active_state,
+                      MIGRATION_STATUS_COMPLETED);
     return;
 
 fail:
-    migrate_set_state(s, current_active_state, MIGRATION_STATUS_FAILED);
+    migrate_set_state(&s->state, current_active_state,
+                      MIGRATION_STATUS_FAILED);
 }
 
 /*
@@ -1623,7 +1627,8 @@ static void *migration_thread(void *opaque)
 
     s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start;
     current_active_state = MIGRATION_STATUS_ACTIVE;
-    migrate_set_state(s, MIGRATION_STATUS_SETUP, MIGRATION_STATUS_ACTIVE);
+    migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
+                      MIGRATION_STATUS_ACTIVE);
 
     trace_migration_thread_setup_complete();
 
@@ -1666,7 +1671,8 @@ static void *migration_thread(void *opaque)
         }
 
         if (qemu_file_get_error(s->file)) {
-            migrate_set_state(s, current_active_state, MIGRATION_STATUS_FAILED);
+            migrate_set_state(&s->state, current_active_state,
+                              MIGRATION_STATUS_FAILED);
             trace_migration_thread_file_err();
             break;
         }
@@ -1747,7 +1753,7 @@ void migrate_fd_connect(MigrationState *s)
     if (migrate_postcopy_ram()) {
         if (open_return_path_on_source(s)) {
             error_report("Unable to open return-path for postcopy");
-            migrate_set_state(s, MIGRATION_STATUS_SETUP,
+            migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
                               MIGRATION_STATUS_FAILED);
             migrate_fd_cleanup(s);
             return;
-- 
2.5.0

[Qemu-devel] [PATCH v2 2/6] migration: Add state records for migration incoming

[Dr. David Alan Gilbert (git)]

From: zhanghailiang <zhang.zhanghailiang@huawei.com>

For migration destination, we also need to know its state,
we will use it in COLO.

Here we add a new member 'state' for MigrationIncomingState,
and also use migrate_set_state() to modify its value.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>

dgilbert: Fixed early free of MigraitonIncomingState
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
---
 include/migration/migration.h |  1 +
 migration/migration.c         | 16 ++++++++++------
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/migration/migration.h b/include/migration/migration.h
index 8340432..4912e7a 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -105,6 +105,7 @@ struct MigrationIncomingState {
     QemuMutex rp_mutex;    /* We send replies from multiple threads */
     void     *postcopy_tmp_page;
 
+    int state;
     /* See savevm.c */
     LoadStateEntry_Head loadvm_handlers;
 };
diff --git a/migration/migration.c b/migration/migration.c
index ba6d4f6..50fe218 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -111,6 +111,7 @@ MigrationIncomingState *migration_incoming_state_new(QEMUFile* f)
 {
     mis_current = g_new0(MigrationIncomingState, 1);
     mis_current->from_src_file = f;
+    mis_current->state = MIGRATION_STATUS_NONE;
     QLIST_INIT(&mis_current->loadvm_handlers);
     qemu_mutex_init(&mis_current->rp_mutex);
     qemu_event_init(&mis_current->main_thread_load_event, false);
@@ -331,8 +332,8 @@ static void process_incoming_migration_co(void *opaque)
 
     mis = migration_incoming_state_new(f);
     postcopy_state_set(POSTCOPY_INCOMING_NONE);
-    migrate_generate_event(MIGRATION_STATUS_ACTIVE);
-
+    migrate_set_state(&mis->state, MIGRATION_STATUS_NONE,
+                      MIGRATION_STATUS_ACTIVE);
     ret = qemu_loadvm_state(f);
 
     ps = postcopy_state_get();
@@ -358,10 +359,10 @@ static void process_incoming_migration_co(void *opaque)
 
     qemu_fclose(f);
     free_xbzrle_decoded_buf();
-    migration_incoming_state_destroy();
 
     if (ret < 0) {
-        migrate_generate_event(MIGRATION_STATUS_FAILED);
+        migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
+                          MIGRATION_STATUS_FAILED);
         error_report("load of migration failed: %s", strerror(-ret));
         migrate_decompress_threads_join();
         exit(EXIT_FAILURE);
@@ -370,7 +371,8 @@ static void process_incoming_migration_co(void *opaque)
     /* Make sure all file formats flush their mutable metadata */
     bdrv_invalidate_cache_all(&local_err);
     if (local_err) {
-        migrate_generate_event(MIGRATION_STATUS_FAILED);
+        migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
+                          MIGRATION_STATUS_FAILED);
         error_report_err(local_err);
         migrate_decompress_threads_join();
         exit(EXIT_FAILURE);
@@ -402,7 +404,9 @@ static void process_incoming_migration_co(void *opaque)
      * observer sees this event they might start to prod at the VM assuming
      * it's ready to use.
      */
-    migrate_generate_event(MIGRATION_STATUS_COMPLETED);
+    migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
+                      MIGRATION_STATUS_COMPLETED);
+    migration_incoming_state_destroy();
 }
 
 void process_incoming_migration(QEMUFile *f)
-- 
2.5.0

[Qemu-devel] [PATCH v2 3/6] Postcopy: Send events/change state on incoming side

[Dr. David Alan Gilbert (git)]

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

I missed the calls to send migration events on the destination side
as we enter postcopy.
Take care when adding them not to do it after state has been freed.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
---
 migration/savevm.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 0ad1b93..c469bad 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1399,6 +1399,8 @@ static void *postcopy_ram_listen_thread(void *opaque)
     MigrationIncomingState *mis = migration_incoming_get_current();
     int load_res;
 
+    migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
+                                   MIGRATION_STATUS_POSTCOPY_ACTIVE);
     qemu_sem_post(&mis->listen_thread_sem);
     trace_postcopy_ram_listen_thread_start();
 
@@ -1415,6 +1417,8 @@ static void *postcopy_ram_listen_thread(void *opaque)
     if (load_res < 0) {
         error_report("%s: loadvm failed: %d", __func__, load_res);
         qemu_file_set_error(f, load_res);
+        migrate_set_state(&mis->state, MIGRATION_STATUS_POSTCOPY_ACTIVE,
+                                       MIGRATION_STATUS_FAILED);
     } else {
         /*
          * This looks good, but it's possible that the device loading in the
@@ -1424,13 +1428,6 @@ static void *postcopy_ram_listen_thread(void *opaque)
         qemu_event_wait(&mis->main_thread_load_event);
     }
     postcopy_ram_incoming_cleanup(mis);
-    /*
-     * If everything has worked fine, then the main thread has waited
-     * for us to start, and we're the last use of the mis.
-     * (If something broke then qemu will have to exit anyway since it's
-     * got a bad migration state).
-     */
-    migration_incoming_state_destroy();
 
     if (load_res < 0) {
         /*
@@ -1442,6 +1439,17 @@ static void *postcopy_ram_listen_thread(void *opaque)
         exit(EXIT_FAILURE);
     }
 
+    migrate_set_state(&mis->state, MIGRATION_STATUS_POSTCOPY_ACTIVE,
+                                   MIGRATION_STATUS_COMPLETED);
+    /*
+     * If everything has worked fine, then the main thread has waited
+     * for us to start, and we're the last use of the mis.
+     * (If something broke then qemu will have to exit anyway since it's
+     * got a bad migration state).
+     */
+    migration_incoming_state_destroy();
+
+
     return NULL;
 }
 
-- 
2.5.0

[Qemu-devel] [PATCH v2 4/6] Migration: Emit event at start of pass

[Dr. David Alan Gilbert (git)]

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

Emit an event each time we sync the dirty bitmap on the source;
this helps libvirt use postcopy by giving it a kick when it
might be a good idea to start the postcopy.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 docs/qmp-events.txt | 14 ++++++++++++++
 migration/ram.c     |  4 ++++
 qapi/event.json     | 13 +++++++++++++
 3 files changed, 31 insertions(+)

diff --git a/docs/qmp-events.txt b/docs/qmp-events.txt
index d2f1ce4..52eb7e2 100644
--- a/docs/qmp-events.txt
+++ b/docs/qmp-events.txt
@@ -496,6 +496,20 @@ Example:
 {"timestamp": {"seconds": 1432121972, "microseconds": 744001},
  "event": "MIGRATION", "data": {"status": "completed"}}
 
+MIGRATION_PASS
+--------------
+
+Emitted from the source side of a migration at the start of each pass
+(when it syncs the dirty bitmap)
+
+Data: None.
+
+  - "pass": An incrementing count (starting at 1 on the first pass)
+
+Example:
+{"timestamp": {"seconds": 1449669631, "microseconds": 239225},
+ "event": "MIGRATION_PASS", "data": {"pass": 2}}
+
 STOP
 ----
 
diff --git a/migration/ram.c b/migration/ram.c
index 0490f00..102d1f2 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -27,6 +27,7 @@
  */
 #include <stdint.h>
 #include <zlib.h>
+#include "qapi-event.h"
 #include "qemu/bitops.h"
 #include "qemu/bitmap.h"
 #include "qemu/timer.h"
@@ -682,6 +683,9 @@ static void migration_bitmap_sync(void)
         num_dirty_pages_period = 0;
     }
     s->dirty_sync_count = bitmap_sync_count;
+    if (migrate_use_events()) {
+        qapi_event_send_migration_pass(bitmap_sync_count, NULL);
+    }
 }
 
 /**
diff --git a/qapi/event.json b/qapi/event.json
index f0cef01..390fd45 100644
--- a/qapi/event.json
+++ b/qapi/event.json
@@ -255,6 +255,19 @@
   'data': {'status': 'MigrationStatus'}}
 
 ##
+# @MIGRATION_PASS
+#
+# Emitted from the source side of a migration at the start of each pass
+# (when it syncs the dirty bitmap)
+#
+# @pass: An incrementing count (starting at 1 on the first pass)
+#
+# Since: 2.6
+##
+{ 'event': 'MIGRATION_PASS',
+  'data': { 'pass': 'int' } }
+
+##
 # @ACPI_DEVICE_OST
 #
 # Emitted when guest executes ACPI _OST method.
-- 
2.5.0

[Qemu-devel] [PATCH v2 5/6] Use qemu_get_buffer_in_place for xbzrle data

[Dr. David Alan Gilbert (git)]

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

Avoid a data copy (if we're lucky) in the xbzrle code.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
---
 migration/ram.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 102d1f2..994552c 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2088,10 +2088,12 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host)
 {
     unsigned int xh_len;
     int xh_flags;
+    uint8_t *loaded_data;
 
     if (!xbzrle_decoded_buf) {
         xbzrle_decoded_buf = g_malloc(TARGET_PAGE_SIZE);
     }
+    loaded_data = xbzrle_decoded_buf;
 
     /* extract RLE header */
     xh_flags = qemu_get_byte(f);
@@ -2107,10 +2109,10 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host)
         return -1;
     }
     /* load data and decode */
-    qemu_get_buffer(f, xbzrle_decoded_buf, xh_len);
+    qemu_get_buffer_in_place(f, &loaded_data, xh_len);
 
     /* decode RLE */
-    if (xbzrle_decode_buffer(xbzrle_decoded_buf, xh_len, host,
+    if (xbzrle_decode_buffer(loaded_data, xh_len, host,
                              TARGET_PAGE_SIZE) == -1) {
         error_report("Failed to load XBZRLE page - decode error!");
         return -1;
-- 
2.5.0

[Qemu-devel] [PATCH v2 6/6] multithread decompression: Avoid one copy

[Dr. David Alan Gilbert (git)]

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

qemu_get_buffer does a copy, we can avoid the memcpy, and
we can then remove the extra buffer.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Liang Li <liang.z.li@intel.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
---
 migration/ram.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 994552c..2da3b51 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -286,7 +286,6 @@ static bool quit_comp_thread;
 static bool quit_decomp_thread;
 static DecompressParam *decomp_param;
 static QemuThread *decompress_threads;
-static uint8_t *compressed_data_buf;
 
 static int do_compress_ram_page(CompressParam *param);
 
@@ -2207,7 +2206,6 @@ void migrate_decompress_threads_create(void)
     thread_count = migrate_decompress_threads();
     decompress_threads = g_new0(QemuThread, thread_count);
     decomp_param = g_new0(DecompressParam, thread_count);
-    compressed_data_buf = g_malloc0(compressBound(TARGET_PAGE_SIZE));
     quit_decomp_thread = false;
     for (i = 0; i < thread_count; i++) {
         qemu_mutex_init(&decomp_param[i].mutex);
@@ -2238,13 +2236,11 @@ void migrate_decompress_threads_join(void)
     }
     g_free(decompress_threads);
     g_free(decomp_param);
-    g_free(compressed_data_buf);
     decompress_threads = NULL;
     decomp_param = NULL;
-    compressed_data_buf = NULL;
 }
 
-static void decompress_data_with_multi_threads(uint8_t *compbuf,
+static void decompress_data_with_multi_threads(QEMUFile *f,
                                                void *host, int len)
 {
     int idx, thread_count;
@@ -2253,7 +2249,7 @@ static void decompress_data_with_multi_threads(uint8_t *compbuf,
     while (true) {
         for (idx = 0; idx < thread_count; idx++) {
             if (!decomp_param[idx].start) {
-                memcpy(decomp_param[idx].compbuf, compbuf, len);
+                qemu_get_buffer(f, decomp_param[idx].compbuf, len);
                 decomp_param[idx].des = host;
                 decomp_param[idx].len = len;
                 start_decompression(&decomp_param[idx]);
@@ -2498,8 +2494,7 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
                 ret = -EINVAL;
                 break;
             }
-            qemu_get_buffer(f, compressed_data_buf, len);
-            decompress_data_with_multi_threads(compressed_data_buf, host, len);
+            decompress_data_with_multi_threads(f, host, len);
             break;
 
         case RAM_SAVE_FLAG_XBZRLE:
-- 
2.5.0

Re: [Qemu-devel] [PATCH v2 1/6] migration: Export migrate_set_state()

[Juan Quintela]

"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: zhanghailiang <zhang.zhanghailiang@huawei.com>
>
> Change the first parameter of migrate_set_state(), and export it.
> We will use it in a later patch to update incoming state.
>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Re: [Qemu-devel] [PATCH v2 4/6] Migration: Emit event at start of pass

[Juan Quintela]

"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Emit an event each time we sync the dirty bitmap on the source;
> this helps libvirt use postcopy by giving it a kick when it
> might be a good idea to start the postcopy.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Re: [Qemu-devel] [PATCH v2 4/6] Migration: Emit event at start of pass

[Eric Blake]

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

On 12/16/2015 04:47 AM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Emit an event each time we sync the dirty bitmap on the source;
> this helps libvirt use postcopy by giving it a kick when it
> might be a good idea to start the postcopy.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  docs/qmp-events.txt | 14 ++++++++++++++
>  migration/ram.c     |  4 ++++
>  qapi/event.json     | 13 +++++++++++++
>  3 files changed, 31 insertions(+)
> 

>  
> +MIGRATION_PASS
> +--------------
> +
> +Emitted from the source side of a migration at the start of each pass
> +(when it syncs the dirty bitmap)
> +
> +Data: None.
> +
> +  - "pass": An incrementing count (starting at 1 on the first pass)
> +
> +Example:
> +{"timestamp": {"seconds": 1449669631, "microseconds": 239225},
> + "event": "MIGRATION_PASS", "data": {"pass": 2}}

How frequently can this event be emitted?  It is not under guest
control, so it's harder to argue that it could be abused as a way for
the guest to DoS the host.  And hopefully it is slow enough (considering
that each pass also requires lots more effort for actually transmitting
state from source to dest), so I don't think we have to worry about
adding rate-limiting to avoid overwhelming the monitor client.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v2 4/6] Migration: Emit event at start of pass

[Dr. David Alan Gilbert]

* Eric Blake (eblake@redhat.com) wrote:
> On 12/16/2015 04:47 AM, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > Emit an event each time we sync the dirty bitmap on the source;
> > this helps libvirt use postcopy by giving it a kick when it
> > might be a good idea to start the postcopy.
> > 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  docs/qmp-events.txt | 14 ++++++++++++++
> >  migration/ram.c     |  4 ++++
> >  qapi/event.json     | 13 +++++++++++++
> >  3 files changed, 31 insertions(+)
> > 
> 
> >  
> > +MIGRATION_PASS
> > +--------------
> > +
> > +Emitted from the source side of a migration at the start of each pass
> > +(when it syncs the dirty bitmap)
> > +
> > +Data: None.
> > +
> > +  - "pass": An incrementing count (starting at 1 on the first pass)
> > +
> > +Example:
> > +{"timestamp": {"seconds": 1449669631, "microseconds": 239225},
> > + "event": "MIGRATION_PASS", "data": {"pass": 2}}
> 
> How frequently can this event be emitted?  It is not under guest
> control, so it's harder to argue that it could be abused as a way for
> the guest to DoS the host.  And hopefully it is slow enough (considering
> that each pass also requires lots more effort for actually transmitting
> state from source to dest), so I don't think we have to worry about
> adding rate-limiting to avoid overwhelming the monitor client.

The only way that you'd get a rapid repetition of the events
is if the pass was very short.  However a very short pass is normally
the signal to exit the iterative loop of migration and send the
last blob at the end of migration.  And that definition of 'short' is
determined by the downtime set by the management layer anyway.

> 
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks.

Dave

> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 


--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 4/6] Migration: Emit event at start of pass

[Amit Shah]

On (Wed) 16 Dec 2015 [16:52:14], Dr. David Alan Gilbert wrote:
> * Eric Blake (eblake@redhat.com) wrote:
> > On 12/16/2015 04:47 AM, Dr. David Alan Gilbert (git) wrote:
> > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > > 
> > > Emit an event each time we sync the dirty bitmap on the source;
> > > this helps libvirt use postcopy by giving it a kick when it
> > > might be a good idea to start the postcopy.
> > > 
> > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > > ---
> > >  docs/qmp-events.txt | 14 ++++++++++++++
> > >  migration/ram.c     |  4 ++++
> > >  qapi/event.json     | 13 +++++++++++++
> > >  3 files changed, 31 insertions(+)
> > > 
> > 
> > >  
> > > +MIGRATION_PASS
> > > +--------------
> > > +
> > > +Emitted from the source side of a migration at the start of each pass
> > > +(when it syncs the dirty bitmap)
> > > +
> > > +Data: None.
> > > +
> > > +  - "pass": An incrementing count (starting at 1 on the first pass)
> > > +
> > > +Example:
> > > +{"timestamp": {"seconds": 1449669631, "microseconds": 239225},
> > > + "event": "MIGRATION_PASS", "data": {"pass": 2}}
> > 
> > How frequently can this event be emitted?  It is not under guest
> > control, so it's harder to argue that it could be abused as a way for
> > the guest to DoS the host.  And hopefully it is slow enough (considering
> > that each pass also requires lots more effort for actually transmitting
> > state from source to dest), so I don't think we have to worry about
> > adding rate-limiting to avoid overwhelming the monitor client.
> 
> The only way that you'd get a rapid repetition of the events
> is if the pass was very short.  However a very short pass is normally
> the signal to exit the iterative loop of migration and send the
> last blob at the end of migration.  And that definition of 'short' is
> determined by the downtime set by the management layer anyway.

Yes, and this can also give us stats on how quickly migration is
progressing.

		Amit

Re: [Qemu-devel] [PATCH v2 0/6] Migration: Small fixes and improvements

[Amit Shah]

On (Wed) 16 Dec 2015 [11:47:32], Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> This is a set of 4 small fixes and improvements I've been collecting
> during the 2.5 freeze; I've added in a couple of Zhanghailiang's patches
> that are easier to merge before rather than after.
> (Of which one I've added a small fix of my own, see the message)
> 
> Jiri was after events from the incoming postcopy side to make it
> easy for libvirt; I'd forgotten to add them - Zhanghailiang's
> patches change the way the events get sent on the incoming side.
> 
> Jiri also wanted an event on the outgoing side at each pass,
> which makes it easy for libvirt to trigger the transition to postcopy.
> 
> The other two patches avoid a couple of buffer copies on the incoming
> side; although in both xbzrle and multithread compression it's rare that
> the destination is the slow path (but one patch is trivial and the
> other saves code).

Reviewed-by: Amit Shah <amit.shah@redhat.com>

		Amit