[Qemu-devel] [PATCH 00/18] Migration thread lite (20121029)

[Qemu-devel] [PATCH 00/18] Migration thread lite (20121029)

[Juan Quintela]

Hi

After discussing with Anthony and Paolo, this is the minimal migration
thread support that we can do for 1.3.

- fixed all the comments (thanks eric, paolo and orit).
- buffered_file.c remains until 1.4.
- testing for vinod showed very nice results, that is why the bitmap
  handling optimizations remain.

Note: Writes has become blocking, and I have to change the "remove"
the feature now in qemu-sockets.c.  Checked that migration was the
only user of that feature.  If new users appear, they just need to add
the socket_set_nonblock() by hand.

Please, review.

Thanks, Juan.


The following changes since commit 50cd72148211c5e5f22ea2519d19ce024226e61f:

  hw/xtensa_sim: get rid of intermediate xtensa_sim_init (2012-10-27 15:04:00 +0000)

are available in the git repository at:

  http://repo.or.cz/r/qemu/quintela.git migration-thread-20121029

for you to fetch changes up to 2c74654f19efc7db35117a87c0d9db4776931e1b:

  ram: optimize migration bitmap walking (2012-10-29 14:14:28 +0100)

Juan Quintela (15):
  buffered_file: Move from using a timer to use a thread
  migration: make qemu_fopen_ops_buffered() return void
  migration: stop all cpus correctly
  migration: make writes blocking
  migration: remove unfreeze logic
  migration: take finer locking
  buffered_file: Unfold the trick to restart generating migration data
  buffered_file: don't flush on put buffer
  buffered_file: unfold buffered_append in buffered_put_buffer
  savevm: New save live migration method: pending
  migration: include qemu-file.h
  migration-fd: remove duplicate include
  memory: introduce memory_region_test_and_clear_dirty
  ram: Use memory_region_test_and_clear_dirty
  ram: optimize migration bitmap walking

Paolo Bonzini (1):
  split MRU ram list

Umesh Deshpande (2):
  add a version number to ram_list
  protect the ramlist with a separate mutex

 arch_init.c       | 115 ++++++++++++++++++++++++-----------------------
 block-migration.c |  49 +++++---------------
 buffered_file.c   | 130 +++++++++++++++++++++++++-----------------------------
 buffered_file.h   |   2 +-
 cpu-all.h         |  15 ++++++-
 exec.c            |  44 +++++++++++++++---
 memory.c          |  16 +++++++
 memory.h          |  18 ++++++++
 migration-exec.c  |   3 +-
 migration-fd.c    |   4 +-
 migration-tcp.c   |   2 +-
 migration-unix.c  |   2 +-
 migration.c       | 100 ++++++++++++++++-------------------------
 migration.h       |   4 +-
 qemu-file.h       |   5 ---
 qemu-sockets.c    |   4 --
 savevm.c          |  24 +++++++---
 sysemu.h          |   1 +
 vmstate.h         |   1 +
 19 files changed, 280 insertions(+), 259 deletions(-)

-- 
1.7.11.7

[Qemu-devel] [PATCH 01/18] split MRU ram list

[Juan Quintela]

From: Paolo Bonzini <pbonzini@redhat.com>

Outside the execution threads the normal, non-MRU-ized order of
the RAM blocks should always be enough.  So manage two separate
lists, which will have separate locking rules.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Orit Wasserman <owasserm@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 arch_init.c |  1 +
 cpu-all.h   |  6 +++++-
 exec.c      | 17 ++++++++++++-----
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index e6effe8..4293557 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -48,6 +48,7 @@
 #include "qemu/page_cache.h"
 #include "qmp-commands.h"
 #include "trace.h"
+#include "cpu-all.h"

 #ifdef DEBUG_ARCH_INIT
 #define DPRINTF(fmt, ...) \
diff --git a/cpu-all.h b/cpu-all.h
index 6606432..cc9f164 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -490,8 +490,9 @@ typedef struct RAMBlock {
     ram_addr_t offset;
     ram_addr_t length;
     uint32_t flags;
-    char idstr[256];
+    QLIST_ENTRY(RAMBlock) next_mru;
     QLIST_ENTRY(RAMBlock) next;
+    char idstr[256];
 #if defined(__linux__) && !defined(TARGET_S390X)
     int fd;
 #endif
@@ -499,6 +500,9 @@ typedef struct RAMBlock {

 typedef struct RAMList {
     uint8_t *phys_dirty;
+    /* memory blocks are sorted in most recently used order */
+    QLIST_HEAD(, RAMBlock) blocks_mru;
+    /* memory blocks sorted by address */
     QLIST_HEAD(, RAMBlock) blocks;
 } RAMList;
 extern RAMList ram_list;
diff --git a/exec.c b/exec.c
index b0ed593..85399da 100644
--- a/exec.c
+++ b/exec.c
@@ -56,6 +56,7 @@
 #include "xen-mapcache.h"
 #include "trace.h"
 #endif
+#include "cpu-all.h"

 #include "cputlb.h"

@@ -96,7 +97,10 @@ static uint8_t *code_gen_ptr;
 int phys_ram_fd;
 static int in_migration;

-RAMList ram_list = { .blocks = QLIST_HEAD_INITIALIZER(ram_list.blocks) };
+RAMList ram_list = {
+    .blocks = QLIST_HEAD_INITIALIZER(ram_list.blocks),
+    .blocks_mru = QLIST_HEAD_INITIALIZER(ram_list.blocks_mru)
+};

 static MemoryRegion *system_memory;
 static MemoryRegion *system_io;
@@ -2563,6 +2567,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
     new_block->length = size;

     QLIST_INSERT_HEAD(&ram_list.blocks, new_block, next);
+    QLIST_INSERT_HEAD(&ram_list.blocks_mru, new_block, next_mru);

     ram_list.phys_dirty = g_realloc(ram_list.phys_dirty,
                                        last_ram_offset() >> TARGET_PAGE_BITS);
@@ -2591,6 +2596,7 @@ void qemu_ram_free_from_ptr(ram_addr_t addr)
     QLIST_FOREACH(block, &ram_list.blocks, next) {
         if (addr == block->offset) {
             QLIST_REMOVE(block, next);
+            QLIST_REMOVE(block, next_mru);
             g_free(block);
             return;
         }
@@ -2604,6 +2610,7 @@ void qemu_ram_free(ram_addr_t addr)
     QLIST_FOREACH(block, &ram_list.blocks, next) {
         if (addr == block->offset) {
             QLIST_REMOVE(block, next);
+            QLIST_REMOVE(block, next_mru);
             if (block->flags & RAM_PREALLOC_MASK) {
                 ;
             } else if (mem_path) {
@@ -2709,12 +2716,12 @@ void *qemu_get_ram_ptr(ram_addr_t addr)
 {
     RAMBlock *block;

-    QLIST_FOREACH(block, &ram_list.blocks, next) {
+    QLIST_FOREACH(block, &ram_list.blocks_mru, next_mru) {
         if (addr - block->offset < block->length) {
             /* Move this entry to to start of the list.  */
             if (block != QLIST_FIRST(&ram_list.blocks)) {
-                QLIST_REMOVE(block, next);
-                QLIST_INSERT_HEAD(&ram_list.blocks, block, next);
+                QLIST_REMOVE(block, next_mru);
+                QLIST_INSERT_HEAD(&ram_list.blocks_mru, block, next_mru);
             }
             if (xen_enabled()) {
                 /* We need to check if the requested address is in the RAM
@@ -2809,7 +2816,7 @@ int qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr)
         return 0;
     }

-    QLIST_FOREACH(block, &ram_list.blocks, next) {
+    QLIST_FOREACH(block, &ram_list.blocks_mru, next_mru) {
         /* This case append when the block is not mapped. */
         if (block->host == NULL) {
             continue;
-- 
1.7.11.7

[Qemu-devel] [PATCH 02/18] add a version number to ram_list

[Juan Quintela]

From: Umesh Deshpande <udeshpan@redhat.com>

This will be used to detect if last_block might have become invalid
across different calls to ram_save_live.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Umesh Deshpande <udeshpan@redhat.com>

Reviewed-by: Orit Wasserman <owasserm@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 arch_init.c | 7 ++++++-
 cpu-all.h   | 1 +
 exec.c      | 4 ++++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch_init.c b/arch_init.c
index 4293557..b47313d 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -336,6 +336,7 @@ static RAMBlock *last_block;
 static ram_addr_t last_offset;
 static unsigned long *migration_bitmap;
 static uint64_t migration_dirty_pages;
+static uint32_t last_version;

 static inline bool migration_bitmap_test_and_reset_dirty(MemoryRegion *mr,
                                                          ram_addr_t offset)
@@ -406,7 +407,6 @@ static void migration_bitmap_sync(void)
     }
 }

-
 /*
  * ram_save_block: Writes a page of memory to the stream f
  *
@@ -558,6 +558,7 @@ static void reset_ram_globals(void)
 {
     last_block = NULL;
     last_offset = 0;
+    last_version = ram_list.version;
     sort_ram_list();
 }

@@ -613,6 +614,10 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
     uint64_t expected_downtime;
     MigrationState *s = migrate_get_current();

+    if (ram_list.version != last_version) {
+        reset_ram_globals();
+    }
+
     bytes_transferred_last = bytes_transferred;
     bwidth = qemu_get_clock_ns(rt_clock);

diff --git a/cpu-all.h b/cpu-all.h
index cc9f164..afa7f8e 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -500,6 +500,7 @@ typedef struct RAMBlock {

 typedef struct RAMList {
     uint8_t *phys_dirty;
+    uint32_t version;
     /* memory blocks are sorted in most recently used order */
     QLIST_HEAD(, RAMBlock) blocks_mru;
     /* memory blocks sorted by address */
diff --git a/exec.c b/exec.c
index 85399da..f5a8aca 100644
--- a/exec.c
+++ b/exec.c
@@ -2569,6 +2569,8 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
     QLIST_INSERT_HEAD(&ram_list.blocks, new_block, next);
     QLIST_INSERT_HEAD(&ram_list.blocks_mru, new_block, next_mru);

+    ram_list.version++;
+
     ram_list.phys_dirty = g_realloc(ram_list.phys_dirty,
                                        last_ram_offset() >> TARGET_PAGE_BITS);
     memset(ram_list.phys_dirty + (new_block->offset >> TARGET_PAGE_BITS),
@@ -2597,6 +2599,7 @@ void qemu_ram_free_from_ptr(ram_addr_t addr)
         if (addr == block->offset) {
             QLIST_REMOVE(block, next);
             QLIST_REMOVE(block, next_mru);
+            ram_list.version++;
             g_free(block);
             return;
         }
@@ -2611,6 +2614,7 @@ void qemu_ram_free(ram_addr_t addr)
         if (addr == block->offset) {
             QLIST_REMOVE(block, next);
             QLIST_REMOVE(block, next_mru);
+            ram_list.version++;
             if (block->flags & RAM_PREALLOC_MASK) {
                 ;
             } else if (mem_path) {
-- 
1.7.11.7

[Qemu-devel] [PATCH 03/18] protect the ramlist with a separate mutex

[Juan Quintela]

From: Umesh Deshpande <udeshpan@redhat.com>

Add the new mutex that protects shared state between ram_save_live
and the iothread.  If the iothread mutex has to be taken together
with the ramlist mutex, the iothread shall always be _outside_.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Umesh Deshpande <udeshpan@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>

Reviewed-by: Orit Wasserman <owasserm@redhat.com>
---
 arch_init.c |  9 ++++++++-
 cpu-all.h   | 10 +++++++++-
 exec.c      | 23 +++++++++++++++++++++--
 3 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index b47313d..2d29828 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -553,7 +553,6 @@ static void ram_migration_cancel(void *opaque)
     migration_end();
 }

-
 static void reset_ram_globals(void)
 {
     last_block = NULL;
@@ -573,6 +572,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
     bitmap_set(migration_bitmap, 1, ram_pages);
     migration_dirty_pages = ram_pages;

+    qemu_mutex_lock_ramlist();
     bytes_transferred = 0;
     reset_ram_globals();

@@ -600,6 +600,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
         qemu_put_be64(f, block->length);
     }

+    qemu_mutex_unlock_ramlist();
     qemu_put_be64(f, RAM_SAVE_FLAG_EOS);

     return 0;
@@ -614,6 +615,8 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
     uint64_t expected_downtime;
     MigrationState *s = migrate_get_current();

+    qemu_mutex_lock_ramlist();
+
     if (ram_list.version != last_version) {
         reset_ram_globals();
     }
@@ -662,6 +665,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
         bwidth = 0.000001;
     }

+    qemu_mutex_unlock_ramlist();
     qemu_put_be64(f, RAM_SAVE_FLAG_EOS);

     expected_downtime = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth;
@@ -682,6 +686,8 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
 {
     migration_bitmap_sync();

+    qemu_mutex_lock_ramlist();
+
     /* try transferring iterative blocks of memory */

     /* flush all remaining blocks regardless of rate limiting */
@@ -697,6 +703,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
     }
     memory_global_dirty_log_stop();

+    qemu_mutex_unlock_ramlist();
     qemu_put_be64(f, RAM_SAVE_FLAG_EOS);

     g_free(migration_bitmap);
diff --git a/cpu-all.h b/cpu-all.h
index afa7f8e..e7823d0 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -22,6 +22,7 @@
 #include "qemu-common.h"
 #include "qemu-tls.h"
 #include "cpu-common.h"
+#include "qemu-thread.h"

 /* some important defines:
  *
@@ -490,7 +491,9 @@ typedef struct RAMBlock {
     ram_addr_t offset;
     ram_addr_t length;
     uint32_t flags;
+    /* Protected by the iothread lock.  */
     QLIST_ENTRY(RAMBlock) next_mru;
+    /* Protected by the ramlist lock.  */
     QLIST_ENTRY(RAMBlock) next;
     char idstr[256];
 #if defined(__linux__) && !defined(TARGET_S390X)
@@ -499,11 +502,14 @@ typedef struct RAMBlock {
 } RAMBlock;

 typedef struct RAMList {
+    QemuMutex mutex;
+    /* Protected by the iothread lock.  */
     uint8_t *phys_dirty;
     uint32_t version;
     /* memory blocks are sorted in most recently used order */
     QLIST_HEAD(, RAMBlock) blocks_mru;
-    /* memory blocks sorted by address */
+    /* memory blocks sorted by address.
+       Protected by the ramlist lock.  */
     QLIST_HEAD(, RAMBlock) blocks;
 } RAMList;
 extern RAMList ram_list;
@@ -523,6 +529,8 @@ extern int mem_prealloc;

 void dump_exec_info(FILE *f, fprintf_function cpu_fprintf);
 ram_addr_t last_ram_offset(void);
+void qemu_mutex_lock_ramlist(void);
+void qemu_mutex_unlock_ramlist(void);
 #endif /* !CONFIG_USER_ONLY */

 int cpu_memory_rw_debug(CPUArchState *env, target_ulong addr,
diff --git a/exec.c b/exec.c
index f5a8aca..1414654 100644
--- a/exec.c
+++ b/exec.c
@@ -645,6 +645,7 @@ bool tcg_enabled(void)
 void cpu_exec_init_all(void)
 {
 #if !defined(CONFIG_USER_ONLY)
+    qemu_mutex_init(&ram_list.mutex);
     memory_map_init();
     io_mem_init();
 #endif
@@ -2324,6 +2325,16 @@ void qemu_flush_coalesced_mmio_buffer(void)
         kvm_flush_coalesced_mmio_buffer();
 }

+void qemu_mutex_lock_ramlist(void)
+{
+    qemu_mutex_lock(&ram_list.mutex);
+}
+
+void qemu_mutex_unlock_ramlist(void)
+{
+    qemu_mutex_unlock(&ram_list.mutex);
+}
+
 #if defined(__linux__) && !defined(TARGET_S390X)

 #include <sys/vfs.h>
@@ -2505,6 +2516,7 @@ void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev)
     }
     pstrcat(new_block->idstr, sizeof(new_block->idstr), name);

+    qemu_mutex_lock_ramlist();
     QLIST_FOREACH(block, &ram_list.blocks, next) {
         if (block != new_block && !strcmp(block->idstr, new_block->idstr)) {
             fprintf(stderr, "RAMBlock \"%s\" already registered, abort!\n",
@@ -2512,6 +2524,7 @@ void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev)
             abort();
         }
     }
+    qemu_mutex_unlock_ramlist();
 }

 static int memory_try_enable_merging(void *addr, size_t len)
@@ -2535,6 +2548,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
     size = TARGET_PAGE_ALIGN(size);
     new_block = g_malloc0(sizeof(*new_block));

+    qemu_mutex_lock_ramlist();
     new_block->mr = mr;
     new_block->offset = find_ram_offset(size);
     if (host) {
@@ -2570,6 +2584,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
     QLIST_INSERT_HEAD(&ram_list.blocks_mru, new_block, next_mru);

     ram_list.version++;
+    qemu_mutex_unlock_ramlist();

     ram_list.phys_dirty = g_realloc(ram_list.phys_dirty,
                                        last_ram_offset() >> TARGET_PAGE_BITS);
@@ -2595,21 +2610,24 @@ void qemu_ram_free_from_ptr(ram_addr_t addr)
 {
     RAMBlock *block;

+    qemu_mutex_lock_ramlist();
     QLIST_FOREACH(block, &ram_list.blocks, next) {
         if (addr == block->offset) {
             QLIST_REMOVE(block, next);
             QLIST_REMOVE(block, next_mru);
             ram_list.version++;
             g_free(block);
-            return;
+            break;
         }
     }
+    qemu_mutex_unlock_ramlist();
 }

 void qemu_ram_free(ram_addr_t addr)
 {
     RAMBlock *block;

+    qemu_mutex_lock_ramlist();
     QLIST_FOREACH(block, &ram_list.blocks, next) {
         if (addr == block->offset) {
             QLIST_REMOVE(block, next);
@@ -2640,9 +2658,10 @@ void qemu_ram_free(ram_addr_t addr)
 #endif
             }
             g_free(block);
-            return;
+            break;
         }
     }
+    qemu_mutex_unlock_ramlist();

 }

-- 
1.7.11.7

[Qemu-devel] [PATCH 04/18] buffered_file: Move from using a timer to use a thread

[Juan Quintela]

We still protect everything except the wait with the iothread lock.
But we moved from a timer to a thread.  Steps one by one.

We also need to detect when we have finished with a variable "complete".

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 buffered_file.c | 58 +++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 36 insertions(+), 22 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index ed92df1..ec7abc6 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -18,6 +18,7 @@
 #include "qemu-timer.h"
 #include "qemu-char.h"
 #include "buffered_file.h"
+#include "qemu-thread.h"

 //#define DEBUG_BUFFERED_FILE

@@ -31,7 +32,8 @@ typedef struct QEMUFileBuffered
     uint8_t *buffer;
     size_t buffer_size;
     size_t buffer_capacity;
-    QEMUTimer *timer;
+    QemuThread thread;
+    bool complete;
 } QEMUFileBuffered;

 #ifdef DEBUG_BUFFERED_FILE
@@ -160,11 +162,8 @@ static int buffered_close(void *opaque)
     if (ret >= 0) {
         ret = ret2;
     }
-    qemu_del_timer(s->timer);
-    qemu_free_timer(s->timer);
-    g_free(s->buffer);
-    g_free(s);
-
+    ret = migrate_fd_close(s->migration_state);
+    s->complete = true;
     return ret;
 }

@@ -215,23 +214,38 @@ static int64_t buffered_get_rate_limit(void *opaque)
     return s->xfer_limit;
 }

-static void buffered_rate_tick(void *opaque)
+/* 100ms  xfer_limit is the limit that we should write each 100ms */
+#define BUFFER_DELAY 100
+
+static void *buffered_file_thread(void *opaque)
 {
     QEMUFileBuffered *s = opaque;
+    int64_t expire_time = qemu_get_clock_ms(rt_clock) + BUFFER_DELAY;

-    if (qemu_file_get_error(s->file)) {
-        buffered_close(s);
-        return;
-    }
-
-    qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 100);
-
-    if (s->freeze_output)
-        return;
-
-    s->bytes_xfer = 0;
+    while (true) {
+        int64_t current_time = qemu_get_clock_ms(rt_clock);

-    buffered_put_buffer(s, NULL, 0, 0);
+        if (s->complete) {
+            break;
+        }
+        if (s->freeze_output) {
+            continue;
+        }
+        if (current_time >= expire_time) {
+            s->bytes_xfer = 0;
+            expire_time = current_time + BUFFER_DELAY;
+        }
+        if (s->bytes_xfer >= s->xfer_limit) {
+            /* usleep expects microseconds */
+            g_usleep((expire_time - current_time)*1000);
+        }
+        qemu_mutex_lock_iothread();
+        buffered_put_buffer(s, NULL, 0, 0);
+        qemu_mutex_unlock_iothread();
+    }
+    g_free(s->buffer);
+    g_free(s);
+    return NULL;
 }

 QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state)
@@ -242,15 +256,15 @@ QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state)

     s->migration_state = migration_state;
     s->xfer_limit = migration_state->bandwidth_limit / 10;
+    s->complete = false;

     s->file = qemu_fopen_ops(s, buffered_put_buffer, NULL,
                              buffered_close, buffered_rate_limit,
                              buffered_set_rate_limit,
 			     buffered_get_rate_limit);

-    s->timer = qemu_new_timer_ms(rt_clock, buffered_rate_tick, s);
-
-    qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 100);
+    qemu_thread_create(&s->thread, buffered_file_thread, s,
+                       QEMU_THREAD_DETACHED);

     return s->file;
 }
-- 
1.7.11.7

[Qemu-devel] [PATCH 05/18] migration: make qemu_fopen_ops_buffered() return void

[Juan Quintela]

We want the file assignment to happen before the thread is created to
avoid locking, so we just do it before creating the thread.

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

Reviewed-by: Orit Wasserman <owasserm@redhat.com>
---
 buffered_file.c | 13 ++++++-------
 buffered_file.h |  2 +-
 migration.c     |  2 +-
 migration.h     |  1 +
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index ec7abc6..7bbee13 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -33,7 +33,6 @@ typedef struct QEMUFileBuffered
     size_t buffer_size;
     size_t buffer_capacity;
     QemuThread thread;
-    bool complete;
 } QEMUFileBuffered;

 #ifdef DEBUG_BUFFERED_FILE
@@ -163,7 +162,7 @@ static int buffered_close(void *opaque)
         ret = ret2;
     }
     ret = migrate_fd_close(s->migration_state);
-    s->complete = true;
+    s->migration_state->complete = true;
     return ret;
 }

@@ -225,7 +224,7 @@ static void *buffered_file_thread(void *opaque)
     while (true) {
         int64_t current_time = qemu_get_clock_ms(rt_clock);

-        if (s->complete) {
+        if (s->migration_state->complete) {
             break;
         }
         if (s->freeze_output) {
@@ -248,7 +247,7 @@ static void *buffered_file_thread(void *opaque)
     return NULL;
 }

-QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state)
+void qemu_fopen_ops_buffered(MigrationState *migration_state)
 {
     QEMUFileBuffered *s;

@@ -256,15 +255,15 @@ QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state)

     s->migration_state = migration_state;
     s->xfer_limit = migration_state->bandwidth_limit / 10;
-    s->complete = false;
+    s->migration_state->complete = false;

     s->file = qemu_fopen_ops(s, buffered_put_buffer, NULL,
                              buffered_close, buffered_rate_limit,
                              buffered_set_rate_limit,
 			     buffered_get_rate_limit);

+    migration_state->file = s->file;
+
     qemu_thread_create(&s->thread, buffered_file_thread, s,
                        QEMU_THREAD_DETACHED);
-
-    return s->file;
 }
diff --git a/buffered_file.h b/buffered_file.h
index ef010fe..8a246fd 100644
--- a/buffered_file.h
+++ b/buffered_file.h
@@ -17,6 +17,6 @@
 #include "hw/hw.h"
 #include "migration.h"

-QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state);
+void qemu_fopen_ops_buffered(MigrationState *migration_state);

 #endif
diff --git a/migration.c b/migration.c
index bd55a15..c39fa7e 100644
--- a/migration.c
+++ b/migration.c
@@ -428,7 +428,7 @@ void migrate_fd_connect(MigrationState *s)
     int ret;

     s->state = MIG_STATE_ACTIVE;
-    s->file = qemu_fopen_ops_buffered(s);
+    qemu_fopen_ops_buffered(s);

     DPRINTF("beginning savevm\n");
     ret = qemu_savevm_state_begin(s->file, &s->params);
diff --git a/migration.h b/migration.h
index c3a23cc..b66fd60 100644
--- a/migration.h
+++ b/migration.h
@@ -45,6 +45,7 @@ struct MigrationState
     int64_t dirty_pages_rate;
     bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
     int64_t xbzrle_cache_size;
+    bool complete;
 };

 void process_incoming_migration(QEMUFile *f);
-- 
1.7.11.7

[Qemu-devel] [PATCH 06/18] migration: stop all cpus correctly

[Juan Quintela]

You can only stop all cpus from the iothread or an vcpu.  As we want
to do it from the migration_thread, we need to do this dance with the
botton handlers.

This patch is a request for ideas.  I can move this function to cpus.c, but
wondered if there is an easy way of doing this?

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/migration.c b/migration.c
index c39fa7e..d5c178d 100644
--- a/migration.c
+++ b/migration.c
@@ -20,6 +20,7 @@
 #include "sysemu.h"
 #include "block.h"
 #include "qemu_socket.h"
+#include "qemu-thread.h"
 #include "block-migration.h"
 #include "qmp-commands.h"

@@ -319,11 +320,22 @@ ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
 void migrate_fd_put_ready(MigrationState *s)
 {
     int ret;
+    static bool first_time = true;

     if (s->state != MIG_STATE_ACTIVE) {
         DPRINTF("put_ready returning because of non-active state\n");
         return;
     }
+    if (first_time) {
+        first_time = false;
+        DPRINTF("beginning savevm\n");
+        ret = qemu_savevm_state_begin(s->file, &s->params);
+        if (ret < 0) {
+            DPRINTF("failed, %d\n", ret);
+            migrate_fd_error(s);
+            return;
+        }
+    }

     DPRINTF("iterate\n");
     ret = qemu_savevm_state_iterate(s->file);
@@ -336,7 +348,11 @@ void migrate_fd_put_ready(MigrationState *s)
         DPRINTF("done iterating\n");
         start_time = qemu_get_clock_ms(rt_clock);
         qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
-        vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
+        if (old_vm_running) {
+            vm_stop(RUN_STATE_FINISH_MIGRATE);
+        } else {
+            vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
+        }

         if (qemu_savevm_state_complete(s->file) < 0) {
             migrate_fd_error(s);
@@ -425,19 +441,8 @@ bool migration_has_failed(MigrationState *s)

 void migrate_fd_connect(MigrationState *s)
 {
-    int ret;
-
     s->state = MIG_STATE_ACTIVE;
     qemu_fopen_ops_buffered(s);
-
-    DPRINTF("beginning savevm\n");
-    ret = qemu_savevm_state_begin(s->file, &s->params);
-    if (ret < 0) {
-        DPRINTF("failed, %d\n", ret);
-        migrate_fd_error(s);
-        return;
-    }
-    migrate_fd_put_ready(s);
 }

 static MigrationState *migrate_init(const MigrationParams *params)
-- 
1.7.11.7

[Qemu-devel] [PATCH 07/18] migration: make writes blocking

[Juan Quintela]

Move all the writes to the migration_thread, and make writings
blocking.  Notice that are still using the iothread for everything
that we do.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration-exec.c |  1 -
 migration-fd.c   |  1 -
 migration.c      | 21 ---------------------
 qemu-file.h      |  5 -----
 qemu-sockets.c   |  4 ----
 savevm.c         |  5 -----
 6 files changed, 37 deletions(-)

diff --git a/migration-exec.c b/migration-exec.c
index 519af57..ecc0f00 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -72,7 +72,6 @@ void exec_start_outgoing_migration(MigrationState *s, const char *command, Error

     s->fd = fileno(f);
     assert(s->fd != -1);
-    socket_set_nonblock(s->fd);

     s->opaque = qemu_popen(f, "w");

diff --git a/migration-fd.c b/migration-fd.c
index ce6932d..9398b91 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -80,7 +80,6 @@ void fd_start_outgoing_migration(MigrationState *s, const char *fdname, Error **
         return;
     }

-    fcntl(s->fd, F_SETFL, O_NONBLOCK);
     s->get_error = fd_errno;
     s->write = fd_write;
     s->close = fd_close;
diff --git a/migration.c b/migration.c
index d5c178d..23a7974 100644
--- a/migration.c
+++ b/migration.c
@@ -244,10 +244,6 @@ static int migrate_fd_cleanup(MigrationState *s)
 {
     int ret = 0;

-    if (s->fd != -1) {
-        qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
-    }
-
     if (s->file) {
         DPRINTF("closing file\n");
         ret = qemu_fclose(s->file);
@@ -282,18 +278,6 @@ static void migrate_fd_completed(MigrationState *s)
     notifier_list_notify(&migration_state_notifiers, s);
 }

-static void migrate_fd_put_notify(void *opaque)
-{
-    MigrationState *s = opaque;
-    int ret;
-
-    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
-    ret = qemu_file_put_notify(s->file);
-    if (ret) {
-        migrate_fd_error(s);
-    }
-}
-
 ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
                               size_t size)
 {
@@ -310,10 +294,6 @@ ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
     if (ret == -1)
         ret = -(s->get_error(s));

-    if (ret == -EAGAIN) {
-        qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s);
-    }
-
     return ret;
 }

@@ -409,7 +389,6 @@ int migrate_fd_wait_for_unfreeze(MigrationState *s)

 int migrate_fd_close(MigrationState *s)
 {
-    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
     return s->close(s);
 }

diff --git a/qemu-file.h b/qemu-file.h
index 9c8985b..e88892c 100644
--- a/qemu-file.h
+++ b/qemu-file.h
@@ -104,11 +104,6 @@ int64_t qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
 int64_t qemu_file_get_rate_limit(QEMUFile *f);
 int qemu_file_get_error(QEMUFile *f);

-/* Try to send any outstanding data.  This function is useful when output is
- * halted due to rate limiting or EAGAIN errors occur as it can be used to
- * resume output. */
-int qemu_file_put_notify(QEMUFile *f);
-
 static inline void qemu_put_be64s(QEMUFile *f, const uint64_t *pv)
 {
     qemu_put_be64(f, *pv);
diff --git a/qemu-sockets.c b/qemu-sockets.c
index cfed9c5..61b6e95 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -276,9 +276,6 @@ static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
         return -1;
     }
     qemu_setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
-    if (connect_state != NULL) {
-        socket_set_nonblock(sock);
-    }
     /* connect to peer */
     do {
         rc = 0;
@@ -732,7 +729,6 @@ int unix_connect_opts(QemuOpts *opts, Error **errp,
         connect_state = g_malloc0(sizeof(*connect_state));
         connect_state->callback = callback;
         connect_state->opaque = opaque;
-        socket_set_nonblock(sock);
     }

     memset(&un, 0, sizeof(un));
diff --git a/savevm.c b/savevm.c
index b080d37..69f1768 100644
--- a/savevm.c
+++ b/savevm.c
@@ -523,11 +523,6 @@ int qemu_fclose(QEMUFile *f)
     return ret;
 }

-int qemu_file_put_notify(QEMUFile *f)
-{
-    return f->put_buffer(f->opaque, NULL, 0, 0);
-}
-
 void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
 {
     int l;
-- 
1.7.11.7

Re: [Qemu-devel] [PATCH 07/18] migration: make writes blocking

[Markus Armbruster]

Juan Quintela <quintela@redhat.com> writes:

> Move all the writes to the migration_thread, and make writings
> blocking.  Notice that are still using the iothread for everything
> that we do.
[...]
> diff --git a/qemu-sockets.c b/qemu-sockets.c
> index cfed9c5..61b6e95 100644
> --- a/qemu-sockets.c
> +++ b/qemu-sockets.c
> @@ -276,9 +276,6 @@ static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
>          return -1;
>      }
>      qemu_setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
> -    if (connect_state != NULL) {
> -        socket_set_nonblock(sock);
> -    }
>      /* connect to peer */
>      do {
>          rc = 0;
> @@ -732,7 +729,6 @@ int unix_connect_opts(QemuOpts *opts, Error **errp,
>          connect_state = g_malloc0(sizeof(*connect_state));
>          connect_state->callback = callback;
>          connect_state->opaque = opaque;
> -        socket_set_nonblock(sock);
>      }
>
>      memset(&un, 0, sizeof(un));

Doesn't this break inet_nonblocking_connect() and
unix_nonblocking_connect()?

In your cover letter, you wrote:

    Note: Writes has become blocking, and I have to change the "remove"
    the feature now in qemu-sockets.c.  Checked that migration was the
    only user of that feature.  If new users appear, they just need to add
    the socket_set_nonblock() by hand.

Yes, migration-{tcp,unix} are their only users, but if they want a
blocking socket now, why not use inet_connect() and unix_connect()?

New users can't "just add socket_set_nonblock()".  They'd have to add it
right where you deleted it: between qemu_socket() and connect().  Else
the connect() is blocking.

[...]

Re: [Qemu-devel] [PATCH 07/18] migration: make writes blocking

[Juan Quintela]

Markus Armbruster <armbru@redhat.com> wrote:
> Juan Quintela <quintela@redhat.com> writes:
>
>> Move all the writes to the migration_thread, and make writings
>> blocking.  Notice that are still using the iothread for everything
>> that we do.
> [...]
>> diff --git a/qemu-sockets.c b/qemu-sockets.c
>> index cfed9c5..61b6e95 100644
>> --- a/qemu-sockets.c
>> +++ b/qemu-sockets.c
>> @@ -276,9 +276,6 @@ static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
>>          return -1;
>>      }
>>      qemu_setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
>> -    if (connect_state != NULL) {
>> -        socket_set_nonblock(sock);
>> -    }
>>      /* connect to peer */
>>      do {
>>          rc = 0;
>> @@ -732,7 +729,6 @@ int unix_connect_opts(QemuOpts *opts, Error **errp,
>>          connect_state = g_malloc0(sizeof(*connect_state));
>>          connect_state->callback = callback;
>>          connect_state->opaque = opaque;
>> -        socket_set_nonblock(sock);
>>      }
>>
>>      memset(&un, 0, sizeof(un));
>
> Doesn't this break inet_nonblocking_connect() and
> unix_nonblocking_connect()?
>
> In your cover letter, you wrote:
>
>     Note: Writes has become blocking, and I have to change the "remove"
>     the feature now in qemu-sockets.c.  Checked that migration was the
>     only user of that feature.  If new users appear, they just need to add
>     the socket_set_nonblock() by hand.
>
> Yes, migration-{tcp,unix} are their only users, but if they want a
> blocking socket now, why not use inet_connect() and unix_connect()?
>
> New users can't "just add socket_set_nonblock()".  They'd have to add it
> right where you deleted it: between qemu_socket() and connect().  Else
> the connect() is blocking.

Grrr.

So, is there any way to make a connection that is non-blocking, but then
writes are blocking?

Later, Juan.

Re: [Qemu-devel] [PATCH 07/18] migration: make writes blocking

[Paolo Bonzini]

Il 29/10/2012 18:32, Juan Quintela ha scritto:
>> > New users can't "just add socket_set_nonblock()".  They'd have to add it
>> > right where you deleted it: between qemu_socket() and connect().  Else
>> > the connect() is blocking.
> Grrr.
> 
> So, is there any way to make a connection that is non-blocking, but then
> writes are blocking?

socket_set_block.

Paolo

Re: [Qemu-devel] [PATCH 07/18] migration: make writes blocking

[Markus Armbruster]

Juan Quintela <quintela@redhat.com> writes:

> Markus Armbruster <armbru@redhat.com> wrote:
>> Juan Quintela <quintela@redhat.com> writes:
>>
>>> Move all the writes to the migration_thread, and make writings
>>> blocking.  Notice that are still using the iothread for everything
>>> that we do.
>> [...]
>>> diff --git a/qemu-sockets.c b/qemu-sockets.c
>>> index cfed9c5..61b6e95 100644
>>> --- a/qemu-sockets.c
>>> +++ b/qemu-sockets.c
>>> @@ -276,9 +276,6 @@ static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
>>>          return -1;
>>>      }
>>>      qemu_setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
>>> -    if (connect_state != NULL) {
>>> -        socket_set_nonblock(sock);
>>> -    }
>>>      /* connect to peer */
>>>      do {
>>>          rc = 0;
>>> @@ -732,7 +729,6 @@ int unix_connect_opts(QemuOpts *opts, Error **errp,
>>>          connect_state = g_malloc0(sizeof(*connect_state));
>>>          connect_state->callback = callback;
>>>          connect_state->opaque = opaque;
>>> -        socket_set_nonblock(sock);
>>>      }
>>>
>>>      memset(&un, 0, sizeof(un));
>>
>> Doesn't this break inet_nonblocking_connect() and
>> unix_nonblocking_connect()?
>>
>> In your cover letter, you wrote:
>>
>>     Note: Writes has become blocking, and I have to change the "remove"
>>     the feature now in qemu-sockets.c.  Checked that migration was the
>>     only user of that feature.  If new users appear, they just need to add
>>     the socket_set_nonblock() by hand.
>>
>> Yes, migration-{tcp,unix} are their only users, but if they want a
>> blocking socket now, why not use inet_connect() and unix_connect()?
>>
>> New users can't "just add socket_set_nonblock()".  They'd have to add it
>> right where you deleted it: between qemu_socket() and connect().  Else
>> the connect() is blocking.
>
> Grrr.
>
> So, is there any way to make a connection that is non-blocking, but then
> writes are blocking?

Which operations on the migration socket do you need to block, and which
ones do you need not to block?

If connect() should block, use inet_connect() / unix_connect().  The
returned socket will be blocking.  You can then switch to non-blocking
mode (and possibly back) at appropriate times.

If connect() should not block, use inet_nonblocking_connect() and so
forth.

Re: [Qemu-devel] [PATCH 07/18] migration: make writes blocking

[Juan Quintela]

Markus Armbruster <armbru@redhat.com> wrote:
> Juan Quintela <quintela@redhat.com> writes:
>
>> Markus Armbruster <armbru@redhat.com> wrote:
>>> Juan Quintela <quintela@redhat.com> writes:
>>>
>>>> Move all the writes to the migration_thread, and make writings
>>>> blocking.  Notice that are still using the iothread for everything
>>>> that we do.
>>> [...]
>>>> diff --git a/qemu-sockets.c b/qemu-sockets.c
>>>> index cfed9c5..61b6e95 100644
>>>> --- a/qemu-sockets.c
>>>> +++ b/qemu-sockets.c
>>>> @@ -276,9 +276,6 @@ static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
>>>>          return -1;
>>>>      }
>>>>      qemu_setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
>>>> -    if (connect_state != NULL) {
>>>> -        socket_set_nonblock(sock);
>>>> -    }
>>>>      /* connect to peer */
>>>>      do {
>>>>          rc = 0;
>>>> @@ -732,7 +729,6 @@ int unix_connect_opts(QemuOpts *opts, Error **errp,
>>>>          connect_state = g_malloc0(sizeof(*connect_state));
>>>>          connect_state->callback = callback;
>>>>          connect_state->opaque = opaque;
>>>> -        socket_set_nonblock(sock);
>>>>      }
>>>>
>>>>      memset(&un, 0, sizeof(un));
>>>
>>> Doesn't this break inet_nonblocking_connect() and
>>> unix_nonblocking_connect()?
>>>
>>> In your cover letter, you wrote:
>>>
>>>     Note: Writes has become blocking, and I have to change the "remove"
>>>     the feature now in qemu-sockets.c.  Checked that migration was the
>>>     only user of that feature.  If new users appear, they just need to add
>>>     the socket_set_nonblock() by hand.
>>>
>>> Yes, migration-{tcp,unix} are their only users, but if they want a
>>> blocking socket now, why not use inet_connect() and unix_connect()?
>>>
>>> New users can't "just add socket_set_nonblock()".  They'd have to add it
>>> right where you deleted it: between qemu_socket() and connect().  Else
>>> the connect() is blocking.
>>
>> Grrr.
>>
>> So, is there any way to make a connection that is non-blocking, but then
>> writes are blocking?
>
> Which operations on the migration socket do you need to block, and which
> ones do you need not to block?

connect: not blocking (done on the iothread)
writes: blocking, done in the migration thread.

I think thet socket_set_block() that paolo says is the right solution.

>
> If connect() should block, use inet_connect() / unix_connect().  The
> returned socket will be blocking.  You can then switch to non-blocking
> mode (and possibly back) at appropriate times.
>
> If connect() should not block, use inet_nonblocking_connect() and so
> forth.

Thanks, Juan.

Re: [Qemu-devel] [PATCH 07/18] migration: make writes blocking

[Markus Armbruster]

Juan Quintela <quintela@redhat.com> writes:

> Markus Armbruster <armbru@redhat.com> wrote:
>> Juan Quintela <quintela@redhat.com> writes:
>>
>>> Markus Armbruster <armbru@redhat.com> wrote:
>>>> Juan Quintela <quintela@redhat.com> writes:
>>>>
>>>>> Move all the writes to the migration_thread, and make writings
>>>>> blocking.  Notice that are still using the iothread for everything
>>>>> that we do.
>>>> [...]
>>>>> diff --git a/qemu-sockets.c b/qemu-sockets.c
>>>>> index cfed9c5..61b6e95 100644
>>>>> --- a/qemu-sockets.c
>>>>> +++ b/qemu-sockets.c
>>>>> @@ -276,9 +276,6 @@ static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
>>>>>          return -1;
>>>>>      }
>>>>>      qemu_setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
>>>>> -    if (connect_state != NULL) {
>>>>> -        socket_set_nonblock(sock);
>>>>> -    }
>>>>>      /* connect to peer */
>>>>>      do {
>>>>>          rc = 0;
>>>>> @@ -732,7 +729,6 @@ int unix_connect_opts(QemuOpts *opts, Error **errp,
>>>>>          connect_state = g_malloc0(sizeof(*connect_state));
>>>>>          connect_state->callback = callback;
>>>>>          connect_state->opaque = opaque;
>>>>> -        socket_set_nonblock(sock);
>>>>>      }
>>>>>
>>>>>      memset(&un, 0, sizeof(un));
>>>>
>>>> Doesn't this break inet_nonblocking_connect() and
>>>> unix_nonblocking_connect()?
>>>>
>>>> In your cover letter, you wrote:
>>>>
>>>>     Note: Writes has become blocking, and I have to change the "remove"
>>>>     the feature now in qemu-sockets.c.  Checked that migration was the
>>>>     only user of that feature.  If new users appear, they just need to add
>>>>     the socket_set_nonblock() by hand.
>>>>
>>>> Yes, migration-{tcp,unix} are their only users, but if they want a
>>>> blocking socket now, why not use inet_connect() and unix_connect()?
>>>>
>>>> New users can't "just add socket_set_nonblock()".  They'd have to add it
>>>> right where you deleted it: between qemu_socket() and connect().  Else
>>>> the connect() is blocking.
>>>
>>> Grrr.
>>>
>>> So, is there any way to make a connection that is non-blocking, but then
>>> writes are blocking?
>>
>> Which operations on the migration socket do you need to block, and which
>> ones do you need not to block?
>
> connect: not blocking (done on the iothread)
> writes: blocking, done in the migration thread.
>
> I think thet socket_set_block() that paolo says is the right solution.

Sounds good.

By the way, I probably would've missed this had you not pointed to it in
the cover letter.  Smart move on your part.

Re: [Qemu-devel] [PATCH 07/18] migration: make writes blocking

[Juan Quintela]

Markus Armbruster <armbru@redhat.com> wrote:
> Juan Quintela <quintela@redhat.com> writes:
>>>>>
>>>>> Doesn't this break inet_nonblocking_connect() and
>>>>> unix_nonblocking_connect()?
>>>>>
>>>>> In your cover letter, you wrote:
>>>>>
>>>>>     Note: Writes has become blocking, and I have to change the "remove"
>>>>>     the feature now in qemu-sockets.c.  Checked that migration was the
>>>>>     only user of that feature.  If new users appear, they just need to add
>>>>>     the socket_set_nonblock() by hand.
>>>>>
>>>>> Yes, migration-{tcp,unix} are their only users, but if they want a
>>>>> blocking socket now, why not use inet_connect() and unix_connect()?
>>>>>
>>>>> New users can't "just add socket_set_nonblock()".  They'd have to add it
>>>>> right where you deleted it: between qemu_socket() and connect().  Else
>>>>> the connect() is blocking.
>>>>
>>>> Grrr.
>>>>
>>>> So, is there any way to make a connection that is non-blocking, but then
>>>> writes are blocking?
>>>
>>> Which operations on the migration socket do you need to block, and which
>>> ones do you need not to block?
>>
>> connect: not blocking (done on the iothread)
>> writes: blocking, done in the migration thread.
>>
>> I think thet socket_set_block() that paolo says is the right solution.
>
> Sounds good.
>
> By the way, I probably would've missed this had you not pointed to it in
> the cover letter.  Smart move on your part.

I knew somebody would have noticed, so it was supposed to be a "priori"
apologize ....

thanks, Juan.

[Qemu-devel] [PATCH 08/18] migration: remove unfreeze logic

[Juan Quintela]

Now that we have a thread, and blocking writes, we don't need it.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 buffered_file.c | 24 +-----------------------
 migration.c     | 23 -----------------------
 migration.h     |  1 -
 3 files changed, 1 insertion(+), 47 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index 7bbee13..d3822d6 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -26,7 +26,6 @@ typedef struct QEMUFileBuffered
 {
     MigrationState *migration_state;
     QEMUFile *file;
-    int freeze_output;
     size_t bytes_xfer;
     size_t xfer_limit;
     uint8_t *buffer;
@@ -70,13 +69,6 @@ static ssize_t buffered_flush(QEMUFileBuffered *s)

         ret = migrate_fd_put_buffer(s->migration_state, s->buffer + offset,
                                     s->buffer_size - offset);
-        if (ret == -EAGAIN) {
-            DPRINTF("backend not ready, freezing\n");
-            ret = 0;
-            s->freeze_output = 1;
-            break;
-        }
-
         if (ret <= 0) {
             DPRINTF("error flushing data, %zd\n", ret);
             break;
@@ -110,9 +102,6 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
         return error;
     }

-    DPRINTF("unfreezing output\n");
-    s->freeze_output = 0;
-
     if (size > 0) {
         DPRINTF("buffering %d bytes\n", size - offset);
         buffered_append(s, buf, size);
@@ -126,7 +115,7 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in

     if (pos == 0 && size == 0) {
         DPRINTF("file is ready\n");
-        if (!s->freeze_output && s->bytes_xfer < s->xfer_limit) {
+        if (s->bytes_xfer < s->xfer_limit) {
             DPRINTF("notifying client\n");
             migrate_fd_put_ready(s->migration_state);
         }
@@ -149,12 +138,6 @@ static int buffered_close(void *opaque)
         if (ret < 0) {
             break;
         }
-        if (s->freeze_output) {
-            ret = migrate_fd_wait_for_unfreeze(s->migration_state);
-            if (ret < 0) {
-                break;
-            }
-        }
     }

     ret2 = migrate_fd_close(s->migration_state);
@@ -181,8 +164,6 @@ static int buffered_rate_limit(void *opaque)
     if (ret) {
         return ret;
     }
-    if (s->freeze_output)
-        return 1;

     if (s->bytes_xfer > s->xfer_limit)
         return 1;
@@ -227,9 +208,6 @@ static void *buffered_file_thread(void *opaque)
         if (s->migration_state->complete) {
             break;
         }
-        if (s->freeze_output) {
-            continue;
-        }
         if (current_time >= expire_time) {
             s->bytes_xfer = 0;
             expire_time = current_time + BUFFER_DELAY;
diff --git a/migration.c b/migration.c
index 23a7974..bc9850b 100644
--- a/migration.c
+++ b/migration.c
@@ -364,29 +364,6 @@ static void migrate_fd_cancel(MigrationState *s)
     migrate_fd_cleanup(s);
 }

-int migrate_fd_wait_for_unfreeze(MigrationState *s)
-{
-    int ret;
-
-    DPRINTF("wait for unfreeze\n");
-    if (s->state != MIG_STATE_ACTIVE)
-        return -EINVAL;
-
-    do {
-        fd_set wfds;
-
-        FD_ZERO(&wfds);
-        FD_SET(s->fd, &wfds);
-
-        ret = select(s->fd + 1, NULL, &wfds, NULL, NULL);
-    } while (ret == -1 && (s->get_error(s)) == EINTR);
-
-    if (ret == -1) {
-        return -s->get_error(s);
-    }
-    return 0;
-}
-
 int migrate_fd_close(MigrationState *s)
 {
     return s->close(s);
diff --git a/migration.h b/migration.h
index b66fd60..5ff68eb 100644
--- a/migration.h
+++ b/migration.h
@@ -81,7 +81,6 @@ void migrate_fd_connect(MigrationState *s);
 ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
                               size_t size);
 void migrate_fd_put_ready(MigrationState *s);
-int migrate_fd_wait_for_unfreeze(MigrationState *s);
 int migrate_fd_close(MigrationState *s);

 void add_migration_state_change_notifier(Notifier *notify);
-- 
1.7.11.7

[Qemu-devel] [PATCH 09/18] migration: take finer locking

[Juan Quintela]

Instead of locking the whole migration_thread inside loop, just lock
migration_fd_put_notify, that is what interacts with the rest of the
world.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 buffered_file.c | 2 --
 migration.c     | 5 +++++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index d3822d6..283854f 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -216,9 +216,7 @@ static void *buffered_file_thread(void *opaque)
             /* usleep expects microseconds */
             g_usleep((expire_time - current_time)*1000);
         }
-        qemu_mutex_lock_iothread();
         buffered_put_buffer(s, NULL, 0, 0);
-        qemu_mutex_unlock_iothread();
     }
     g_free(s->buffer);
     g_free(s);
diff --git a/migration.c b/migration.c
index bc9850b..9c409d7 100644
--- a/migration.c
+++ b/migration.c
@@ -302,8 +302,10 @@ void migrate_fd_put_ready(MigrationState *s)
     int ret;
     static bool first_time = true;

+    qemu_mutex_lock_iothread();
     if (s->state != MIG_STATE_ACTIVE) {
         DPRINTF("put_ready returning because of non-active state\n");
+        qemu_mutex_unlock_iothread();
         return;
     }
     if (first_time) {
@@ -313,6 +315,7 @@ void migrate_fd_put_ready(MigrationState *s)
         if (ret < 0) {
             DPRINTF("failed, %d\n", ret);
             migrate_fd_error(s);
+            qemu_mutex_unlock_iothread();
             return;
         }
     }
@@ -348,6 +351,8 @@ void migrate_fd_put_ready(MigrationState *s)
             }
         }
     }
+    qemu_mutex_unlock_iothread();
+
 }

 static void migrate_fd_cancel(MigrationState *s)
-- 
1.7.11.7

[Qemu-devel] [PATCH 10/18] buffered_file: Unfold the trick to restart generating migration data

[Juan Quintela]

This was needed before due to the way that the callbacks worked.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 buffered_file.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index 283854f..018cab7 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -113,14 +113,6 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
         return error;
     }

-    if (pos == 0 && size == 0) {
-        DPRINTF("file is ready\n");
-        if (s->bytes_xfer < s->xfer_limit) {
-            DPRINTF("notifying client\n");
-            migrate_fd_put_ready(s->migration_state);
-        }
-    }
-
     return size;
 }

@@ -216,8 +208,15 @@ static void *buffered_file_thread(void *opaque)
             /* usleep expects microseconds */
             g_usleep((expire_time - current_time)*1000);
         }
-        buffered_put_buffer(s, NULL, 0, 0);
+        buffered_flush(s);
+
+        DPRINTF("file is ready\n");
+        if (s->bytes_xfer < s->xfer_limit) {
+            DPRINTF("notifying client\n");
+            migrate_fd_put_ready(s->migration_state);
+        }
     }
+
     g_free(s->buffer);
     g_free(s);
     return NULL;
-- 
1.7.11.7

[Qemu-devel] [PATCH 11/18] buffered_file: don't flush on put buffer

[Juan Quintela]

We call buffered_put_buffer with iothread held, and buffered_flush() does
synchronous writes.  We only want to do the synchronous writes outside.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 buffered_file.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index 018cab7..2bdda27 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -107,12 +107,6 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
         buffered_append(s, buf, size);
     }

-    error = buffered_flush(s);
-    if (error < 0) {
-        DPRINTF("buffered flush error. bailing: %s\n", strerror(-error));
-        return error;
-    }
-
     return size;
 }

-- 
1.7.11.7

[Qemu-devel] [PATCH 12/18] buffered_file: unfold buffered_append in buffered_put_buffer

[Juan Quintela]

It was the only user, and now buffered_put_buffer just do the append

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 buffered_file.c | 33 ++++++++++++++-------------------
 1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index 2bdda27..017745d 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -42,22 +42,6 @@ typedef struct QEMUFileBuffered
     do { } while (0)
 #endif

-static void buffered_append(QEMUFileBuffered *s,
-                            const uint8_t *buf, size_t size)
-{
-    if (size > (s->buffer_capacity - s->buffer_size)) {
-        DPRINTF("increasing buffer capacity from %zu by %zu\n",
-                s->buffer_capacity, size + 1024);
-
-        s->buffer_capacity += size + 1024;
-
-        s->buffer = g_realloc(s->buffer, s->buffer_capacity);
-    }
-
-    memcpy(s->buffer + s->buffer_size, buf, size);
-    s->buffer_size += size;
-}
-
 static ssize_t buffered_flush(QEMUFileBuffered *s)
 {
     size_t offset = 0;
@@ -102,11 +86,22 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
         return error;
     }

-    if (size > 0) {
-        DPRINTF("buffering %d bytes\n", size - offset);
-        buffered_append(s, buf, size);
+    if (size <= 0) {
+        return size;
     }

+    if (size > (s->buffer_capacity - s->buffer_size)) {
+        DPRINTF("increasing buffer capacity from %zu by %zu\n",
+                s->buffer_capacity, size + 1024);
+
+        s->buffer_capacity += size + 1024;
+
+        s->buffer = g_realloc(s->buffer, s->buffer_capacity);
+    }
+
+    memcpy(s->buffer + s->buffer_size, buf, size);
+    s->buffer_size += size;
+
     return size;
 }

-- 
1.7.11.7

[Qemu-devel] [PATCH 13/18] savevm: New save live migration method: pending

[Juan Quintela]

Code just now does (simplified for clarity)

    if (qemu_savevm_state_iterate(s->file) == 1) {
       vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
       qemu_savevm_state_complete(s->file);
    }

Problem here is that qemu_savevm_state_iterate() returns 1 when it
knows that remaining memory to sent takes less than max downtime.

But this means that we could end spending 2x max_downtime, one
downtime in qemu_savevm_iterate, and the other in
qemu_savevm_state_complete.

Changed code to:

    pending_size = qemu_savevm_state_pending(s->file, max_size);
    DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
    if (pending_size >= max_size) {
        ret = qemu_savevm_state_iterate(s->file);
     } else {
        vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
        qemu_savevm_state_complete(s->file);
     }

So what we do is: at current network speed, we calculate the maximum
number of bytes we can sent: max_size.

Then we ask every save_live section how much they have pending.  If
they are less than max_size, we move to complete phase, otherwise we
do an iterate one.

This makes things much simpler, because now individual sections don't
have to caluclate the bandwidth (it was implossible to do right from
there).

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 arch_init.c       | 48 ++++++++++++++++++------------------------------
 block-migration.c | 49 ++++++++++---------------------------------------
 buffered_file.c   | 23 +++++++++++++++++------
 migration.c       | 22 +++++++++++++++-------
 migration.h       |  2 +-
 savevm.c          | 19 +++++++++++++++++++
 sysemu.h          |  1 +
 vmstate.h         |  1 +
 8 files changed, 82 insertions(+), 83 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 2d29828..1eefef8 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -608,12 +608,9 @@ static int ram_save_setup(QEMUFile *f, void *opaque)

 static int ram_save_iterate(QEMUFile *f, void *opaque)
 {
-    uint64_t bytes_transferred_last;
-    double bwidth = 0;
     int ret;
     int i;
-    uint64_t expected_downtime;
-    MigrationState *s = migrate_get_current();
+    int64_t t0;

     qemu_mutex_lock_ramlist();

@@ -621,9 +618,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
         reset_ram_globals();
     }

-    bytes_transferred_last = bytes_transferred;
-    bwidth = qemu_get_clock_ns(rt_clock);
-
+    t0 = qemu_get_clock_ns(rt_clock);
     i = 0;
     while ((ret = qemu_file_rate_limit(f)) == 0) {
         int bytes_sent;
@@ -641,7 +636,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
            iterations
         */
         if ((i & 63) == 0) {
-            uint64_t t1 = (qemu_get_clock_ns(rt_clock) - bwidth) / 1000000;
+            uint64_t t1 = (qemu_get_clock_ns(rt_clock) - t0) / 1000000;
             if (t1 > MAX_WAIT) {
                 DPRINTF("big wait: %" PRIu64 " milliseconds, %d iterations\n",
                         t1, i);
@@ -655,31 +650,10 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
         return ret;
     }

-    bwidth = qemu_get_clock_ns(rt_clock) - bwidth;
-    bwidth = (bytes_transferred - bytes_transferred_last) / bwidth;
-
-    /* if we haven't transferred anything this round, force
-     * expected_downtime to a very high value, but without
-     * crashing */
-    if (bwidth == 0) {
-        bwidth = 0.000001;
-    }
-
     qemu_mutex_unlock_ramlist();
     qemu_put_be64(f, RAM_SAVE_FLAG_EOS);

-    expected_downtime = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth;
-    DPRINTF("ram_save_live: expected(%" PRIu64 ") <= max(" PRIu64 ")?\n",
-            expected_downtime, migrate_max_downtime());
-
-    if (expected_downtime <= migrate_max_downtime()) {
-        migration_bitmap_sync();
-        expected_downtime = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth;
-        s->expected_downtime = expected_downtime / 1000000; /* ns -> ms */
-
-        return expected_downtime <= migrate_max_downtime();
-    }
-    return 0;
+    return i;
 }

 static int ram_save_complete(QEMUFile *f, void *opaque)
@@ -712,6 +686,19 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
     return 0;
 }

+static uint64_t ram_save_pending(QEMUFile *f, void *opaque, uint64_t max_size)
+{
+    uint64_t remaining_size;
+
+    remaining_size = ram_save_remaining() * TARGET_PAGE_SIZE;
+
+    if (remaining_size < max_size) {
+        migration_bitmap_sync();
+        remaining_size = ram_save_remaining() * TARGET_PAGE_SIZE;
+    }
+    return remaining_size;
+}
+
 static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host)
 {
     int ret, rc = 0;
@@ -897,6 +884,7 @@ SaveVMHandlers savevm_ram_handlers = {
     .save_live_setup = ram_save_setup,
     .save_live_iterate = ram_save_iterate,
     .save_live_complete = ram_save_complete,
+    .save_live_pending = ram_save_pending,
     .load_state = ram_load,
     .cancel = ram_migration_cancel,
 };
diff --git a/block-migration.c b/block-migration.c
index 71b9601..5db01fe 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -77,9 +77,7 @@ typedef struct BlkMigState {
     int64_t total_sector_sum;
     int prev_progress;
     int bulk_completed;
-    long double total_time;
     long double prev_time_offset;
-    int reads;
 } BlkMigState;

 static BlkMigState block_mig_state;
@@ -132,12 +130,6 @@ uint64_t blk_mig_bytes_total(void)
     return sum << BDRV_SECTOR_BITS;
 }

-static inline long double compute_read_bwidth(void)
-{
-    assert(block_mig_state.total_time != 0);
-    return (block_mig_state.reads / block_mig_state.total_time) * BLOCK_SIZE;
-}
-
 static int bmds_aio_inflight(BlkMigDevState *bmds, int64_t sector)
 {
     int64_t chunk = sector / (int64_t)BDRV_SECTORS_PER_DIRTY_CHUNK;
@@ -191,8 +183,6 @@ static void blk_mig_read_cb(void *opaque, int ret)

     blk->ret = ret;

-    block_mig_state.reads++;
-    block_mig_state.total_time += (curr_time - block_mig_state.prev_time_offset);
     block_mig_state.prev_time_offset = curr_time;

     QSIMPLEQ_INSERT_TAIL(&block_mig_state.blk_list, blk, entry);
@@ -310,8 +300,6 @@ static void init_blk_migration(QEMUFile *f)
     block_mig_state.total_sector_sum = 0;
     block_mig_state.prev_progress = -1;
     block_mig_state.bulk_completed = 0;
-    block_mig_state.total_time = 0;
-    block_mig_state.reads = 0;

     bdrv_iterate(init_blk_migration_it, NULL);
 }
@@ -493,32 +481,6 @@ static int64_t get_remaining_dirty(void)
     return dirty * BLOCK_SIZE;
 }

-static int is_stage2_completed(void)
-{
-    int64_t remaining_dirty;
-    long double bwidth;
-
-    if (block_mig_state.bulk_completed == 1) {
-
-        remaining_dirty = get_remaining_dirty();
-        if (remaining_dirty == 0) {
-            return 1;
-        }
-
-        bwidth = compute_read_bwidth();
-
-        if ((remaining_dirty / bwidth) <=
-            migrate_max_downtime()) {
-            /* finish stage2 because we think that we can finish remaining work
-               below max_downtime */
-
-            return 1;
-        }
-    }
-
-    return 0;
-}
-
 static void blk_mig_cleanup(void)
 {
     BlkMigDevState *bmds;
@@ -619,7 +581,7 @@ static int block_save_iterate(QEMUFile *f, void *opaque)

     qemu_put_be64(f, BLK_MIG_FLAG_EOS);

-    return is_stage2_completed();
+    return 0;
 }

 static int block_save_complete(QEMUFile *f, void *opaque)
@@ -659,6 +621,14 @@ static int block_save_complete(QEMUFile *f, void *opaque)
     return 0;
 }

+static uint64_t block_save_pending(QEMUFile *f, void *opaque, uint64_t max_size)
+{
+
+    DPRINTF("Enter save live pending  %ld\n", get_remaining_dirty());
+
+    return get_remaining_dirty();
+}
+
 static int block_load(QEMUFile *f, void *opaque, int version_id)
 {
     static int banner_printed;
@@ -755,6 +725,7 @@ SaveVMHandlers savevm_block_handlers = {
     .save_live_setup = block_save_setup,
     .save_live_iterate = block_save_iterate,
     .save_live_complete = block_save_complete,
+    .save_live_pending = block_save_pending,
     .load_state = block_load,
     .cancel = block_migration_cancel,
     .is_active = block_is_active,
diff --git a/buffered_file.c b/buffered_file.c
index 017745d..fbd11c4 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -181,7 +181,9 @@ static int64_t buffered_get_rate_limit(void *opaque)
 static void *buffered_file_thread(void *opaque)
 {
     QEMUFileBuffered *s = opaque;
-    int64_t expire_time = qemu_get_clock_ms(rt_clock) + BUFFER_DELAY;
+    int64_t initial_time = qemu_get_clock_ms(rt_clock);
+    int64_t max_size = 0;
+    bool last_round = false;

     while (true) {
         int64_t current_time = qemu_get_clock_ms(rt_clock);
@@ -189,20 +191,29 @@ static void *buffered_file_thread(void *opaque)
         if (s->migration_state->complete) {
             break;
         }
-        if (current_time >= expire_time) {
+        if (current_time >= initial_time + BUFFER_DELAY) {
+            uint64_t transferred_bytes = s->bytes_xfer;
+            uint64_t time_spent = current_time - initial_time;
+            double bandwidth = transferred_bytes / time_spent;
+            max_size = bandwidth * migrate_max_downtime() / 1000000;
+
+            DPRINTF("transferred %" PRIu64 " time_spent %" PRIu64
+                    " bandwidth %g max_size %" PRId64 "\n",
+                    transferred_bytes, time_spent, bandwidth, max_size);
+
             s->bytes_xfer = 0;
-            expire_time = current_time + BUFFER_DELAY;
+            initial_time = current_time;
         }
-        if (s->bytes_xfer >= s->xfer_limit) {
+        if (!last_round && (s->bytes_xfer >= s->xfer_limit)) {
             /* usleep expects microseconds */
-            g_usleep((expire_time - current_time)*1000);
+            g_usleep((initial_time + BUFFER_DELAY - current_time)*1000);
         }
         buffered_flush(s);

         DPRINTF("file is ready\n");
         if (s->bytes_xfer < s->xfer_limit) {
             DPRINTF("notifying client\n");
-            migrate_fd_put_ready(s->migration_state);
+            last_round = migrate_fd_put_ready(s->migration_state, max_size);
         }
     }

diff --git a/migration.c b/migration.c
index 9c409d7..769ae56 100644
--- a/migration.c
+++ b/migration.c
@@ -297,16 +297,18 @@ ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
     return ret;
 }

-void migrate_fd_put_ready(MigrationState *s)
+bool migrate_fd_put_ready(MigrationState *s, uint64_t max_size)
 {
     int ret;
     static bool first_time = true;
+    uint64_t pending_size;
+    bool last_round = false;

     qemu_mutex_lock_iothread();
     if (s->state != MIG_STATE_ACTIVE) {
         DPRINTF("put_ready returning because of non-active state\n");
         qemu_mutex_unlock_iothread();
-        return;
+        return false;
     }
     if (first_time) {
         first_time = false;
@@ -316,15 +318,19 @@ void migrate_fd_put_ready(MigrationState *s)
             DPRINTF("failed, %d\n", ret);
             migrate_fd_error(s);
             qemu_mutex_unlock_iothread();
-            return;
+            return false;
         }
     }

     DPRINTF("iterate\n");
-    ret = qemu_savevm_state_iterate(s->file);
-    if (ret < 0) {
-        migrate_fd_error(s);
-    } else if (ret == 1) {
+    pending_size = qemu_savevm_state_pending(s->file, max_size);
+    DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
+    if (pending_size >= max_size) {
+        ret = qemu_savevm_state_iterate(s->file);
+        if (ret < 0) {
+            migrate_fd_error(s);
+        }
+    } else {
         int old_vm_running = runstate_is_running();
         int64_t start_time, end_time;

@@ -350,9 +356,11 @@ void migrate_fd_put_ready(MigrationState *s)
                 vm_start();
             }
         }
+        last_round = true;
     }
     qemu_mutex_unlock_iothread();

+    return last_round;
 }

 static void migrate_fd_cancel(MigrationState *s)
diff --git a/migration.h b/migration.h
index 5ff68eb..72e638b 100644
--- a/migration.h
+++ b/migration.h
@@ -80,7 +80,7 @@ void migrate_fd_connect(MigrationState *s);

 ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
                               size_t size);
-void migrate_fd_put_ready(MigrationState *s);
+bool migrate_fd_put_ready(MigrationState *s, uint64_t max_size);
 int migrate_fd_close(MigrationState *s);

 void add_migration_state_change_notifier(Notifier *notify);
diff --git a/savevm.c b/savevm.c
index 69f1768..a68c37b 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1681,6 +1681,25 @@ int qemu_savevm_state_complete(QEMUFile *f)
     return qemu_file_get_error(f);
 }

+uint64_t qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size)
+{
+    SaveStateEntry *se;
+    uint64_t ret = 0;
+
+    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
+        if (!se->ops || !se->ops->save_live_pending) {
+            continue;
+        }
+        if (se->ops && se->ops->is_active) {
+            if (!se->ops->is_active(se->opaque)) {
+                continue;
+            }
+        }
+        ret += se->ops->save_live_pending(f, se->opaque, max_size);
+    }
+    return ret;
+}
+
 void qemu_savevm_state_cancel(QEMUFile *f)
 {
     SaveStateEntry *se;
diff --git a/sysemu.h b/sysemu.h
index 0c39a3a..0276205 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -83,6 +83,7 @@ int qemu_savevm_state_begin(QEMUFile *f,
 int qemu_savevm_state_iterate(QEMUFile *f);
 int qemu_savevm_state_complete(QEMUFile *f);
 void qemu_savevm_state_cancel(QEMUFile *f);
+uint64_t qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size);
 int qemu_loadvm_state(QEMUFile *f);

 /* SLIRP */
diff --git a/vmstate.h b/vmstate.h
index c9c320e..241d962 100644
--- a/vmstate.h
+++ b/vmstate.h
@@ -35,6 +35,7 @@ typedef struct SaveVMHandlers {
     int (*save_live_setup)(QEMUFile *f, void *opaque);
     int (*save_live_iterate)(QEMUFile *f, void *opaque);
     int (*save_live_complete)(QEMUFile *f, void *opaque);
+    uint64_t (*save_live_pending)(QEMUFile *f, void *opaque, uint64_t max_size);
     void (*cancel)(void *opaque);
     LoadStateHandler *load_state;
     bool (*is_active)(void *opaque);
-- 
1.7.11.7

[Qemu-devel] [PATCH 14/18] migration: include qemu-file.h

[Juan Quintela]

They don't use/know anything about buffered-file.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration-exec.c | 2 +-
 migration-fd.c   | 2 +-
 migration-tcp.c  | 2 +-
 migration-unix.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/migration-exec.c b/migration-exec.c
index ecc0f00..003a426 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -19,7 +19,7 @@
 #include "qemu_socket.h"
 #include "migration.h"
 #include "qemu-char.h"
-#include "buffered_file.h"
+#include "qemu-file.h"
 #include "block.h"
 #include <sys/types.h>
 #include <sys/wait.h>
diff --git a/migration-fd.c b/migration-fd.c
index 9398b91..b4c4b8c 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -18,7 +18,7 @@
 #include "migration.h"
 #include "monitor.h"
 #include "qemu-char.h"
-#include "buffered_file.h"
+#include "qemu-file.h"
 #include "block.h"
 #include "qemu_socket.h"

diff --git a/migration-tcp.c b/migration-tcp.c
index 46f6ac5..502cd11 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -17,7 +17,7 @@
 #include "qemu_socket.h"
 #include "migration.h"
 #include "qemu-char.h"
-#include "buffered_file.h"
+#include "qemu-file.h"
 #include "block.h"

 //#define DEBUG_MIGRATION_TCP
diff --git a/migration-unix.c b/migration-unix.c
index ed3db3a..c14a960 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -17,7 +17,7 @@
 #include "qemu_socket.h"
 #include "migration.h"
 #include "qemu-char.h"
-#include "buffered_file.h"
+#include "qemu-file.h"
 #include "block.h"

 //#define DEBUG_MIGRATION_UNIX
-- 
1.7.11.7

[Qemu-devel] [PATCH 15/18] migration-fd: remove duplicate include

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration-fd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/migration-fd.c b/migration-fd.c
index b4c4b8c..05753d8 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -20,7 +20,6 @@
 #include "qemu-char.h"
 #include "qemu-file.h"
 #include "block.h"
-#include "qemu_socket.h"

 //#define DEBUG_MIGRATION_FD

-- 
1.7.11.7

[Qemu-devel] [PATCH 16/18] memory: introduce memory_region_test_and_clear_dirty

[Juan Quintela]

This function avoids having to do two calls, one to test the dirty bit, and
other to reset it.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 memory.c | 16 ++++++++++++++++
 memory.h | 18 ++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/memory.c b/memory.c
index 36bb9a5..9ddaac9 100644
--- a/memory.c
+++ b/memory.c
@@ -1078,6 +1078,22 @@ void memory_region_set_dirty(MemoryRegion *mr, hwaddr addr,
     return cpu_physical_memory_set_dirty_range(mr->ram_addr + addr, size, -1);
 }

+bool memory_region_test_and_clear_dirty(MemoryRegion *mr, hwaddr addr,
+                                        hwaddr size, unsigned client)
+{
+    bool ret;
+    assert(mr->terminates);
+    ret = cpu_physical_memory_get_dirty(mr->ram_addr + addr, size,
+                                        1 << client);
+    if (ret) {
+        cpu_physical_memory_reset_dirty(mr->ram_addr + addr,
+                                        mr->ram_addr + addr + size,
+                                        1 << client);
+    }
+    return ret;
+}
+
+
 void memory_region_sync_dirty_bitmap(MemoryRegion *mr)
 {
     AddressSpace *as;
diff --git a/memory.h b/memory.h
index 9462bfd..99434d8 100644
--- a/memory.h
+++ b/memory.h
@@ -454,6 +454,24 @@ void memory_region_set_dirty(MemoryRegion *mr, hwaddr addr,
                              hwaddr size);

 /**
+ * memory_region_test_and_clear_dirty: Check whether a range of bytes is dirty
+ *                                     for a specified client. It clears them.
+ *
+ * Checks whether a range of bytes has been written to since the last
+ * call to memory_region_reset_dirty() with the same @client.  Dirty logging
+ * must be enabled.
+ *
+ * @mr: the memory region being queried.
+ * @addr: the address (relative to the start of the region) being queried.
+ * @size: the size of the range being queried.
+ * @client: the user of the logging information; %DIRTY_MEMORY_MIGRATION or
+ *          %DIRTY_MEMORY_VGA.
+ */
+bool memory_region_test_and_clear_dirty(MemoryRegion *mr,
+                                        hwaddr addr,
+                                        hwaddr size,
+                                        unsigned client);
+/**
  * memory_region_sync_dirty_bitmap: Synchronize a region's dirty bitmap with
  *                                  any external TLBs (e.g. kvm)
  *
-- 
1.7.11.7

[Qemu-devel] [PATCH 17/18] ram: Use memory_region_test_and_clear_dirty

[Juan Quintela]

This avoids having to do two walks over the dirty bitmap, one reading
the dirty bits, and another cleaning them.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 arch_init.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 1eefef8..79f466f 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -385,13 +385,12 @@ static void migration_bitmap_sync(void)

     QLIST_FOREACH(block, &ram_list.blocks, next) {
         for (addr = 0; addr < block->length; addr += TARGET_PAGE_SIZE) {
-            if (memory_region_get_dirty(block->mr, addr, TARGET_PAGE_SIZE,
-                                        DIRTY_MEMORY_MIGRATION)) {
+            if (memory_region_test_and_clear_dirty(block->mr,
+                                                   addr, TARGET_PAGE_SIZE,
+                                                   DIRTY_MEMORY_MIGRATION)) {
                 migration_bitmap_set_dirty(block->mr, addr);
             }
         }
-        memory_region_reset_dirty(block->mr, 0, block->length,
-                                  DIRTY_MEMORY_MIGRATION);
     }
     trace_migration_bitmap_sync_end(migration_dirty_pages
                                     - num_dirty_pages_init);
-- 
1.7.11.7

[Qemu-devel] [PATCH 18/18] ram: optimize migration bitmap walking

[Juan Quintela]

Instead of testing each page individually, we search what is the next
dirty page with a bitmap operation.  We have to reorganize the code to
move from a "for" loop, to a while(dirty) loop.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 arch_init.c | 45 ++++++++++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 79f466f..d760caa 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -338,18 +338,21 @@ static unsigned long *migration_bitmap;
 static uint64_t migration_dirty_pages;
 static uint32_t last_version;

-static inline bool migration_bitmap_test_and_reset_dirty(MemoryRegion *mr,
-                                                         ram_addr_t offset)
+static inline
+ram_addr_t migration_bitmap_find_and_reset_dirty(MemoryRegion *mr,
+                                                 ram_addr_t start)
 {
-    bool ret;
-    int nr = (mr->ram_addr + offset) >> TARGET_PAGE_BITS;
+    unsigned long base = mr->ram_addr >> TARGET_PAGE_BITS;
+    unsigned long nr = base + (start >> TARGET_PAGE_BITS);
+    unsigned long size = base + (int128_get64(mr->size) >> TARGET_PAGE_BITS);

-    ret = test_and_clear_bit(nr, migration_bitmap);
+    unsigned long next = find_next_bit(migration_bitmap, size, nr);

-    if (ret) {
+    if (next < size) {
+        clear_bit(next, migration_bitmap);
         migration_dirty_pages--;
     }
-    return ret;
+    return (next - base) << TARGET_PAGE_BITS;
 }

 static inline bool migration_bitmap_set_dirty(MemoryRegion *mr,
@@ -418,6 +421,7 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
 {
     RAMBlock *block = last_block;
     ram_addr_t offset = last_offset;
+    bool complete_round = false;
     int bytes_sent = -1;
     MemoryRegion *mr;
     ram_addr_t current_addr;
@@ -425,9 +429,21 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
     if (!block)
         block = QLIST_FIRST(&ram_list.blocks);

-    do {
+    while (true) {
         mr = block->mr;
-        if (migration_bitmap_test_and_reset_dirty(mr, offset)) {
+        offset = migration_bitmap_find_and_reset_dirty(mr, offset);
+        if (complete_round && block == last_block &&
+            offset >= last_offset) {
+            break;
+        }
+        if (offset >= block->length) {
+            offset = 0;
+            block = QLIST_NEXT(block, next);
+            if (!block) {
+                block = QLIST_FIRST(&ram_list.blocks);
+                complete_round = true;
+            }
+        } else {
             uint8_t *p;
             int cont = (block == last_block) ? RAM_SAVE_FLAG_CONTINUE : 0;

@@ -460,16 +476,7 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
                 break;
             }
         }
-
-        offset += TARGET_PAGE_SIZE;
-        if (offset >= block->length) {
-            offset = 0;
-            block = QLIST_NEXT(block, next);
-            if (!block)
-                block = QLIST_FIRST(&ram_list.blocks);
-        }
-    } while (block != last_block || offset != last_offset);
-
+    }
     last_block = block;
     last_offset = offset;

-- 
1.7.11.7