[Qemu-devel] [PATCH 02/41] fix migration sync

[Qemu-devel] [PATCH 02/41] fix migration sync

[Juan Quintela]

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

diff --git a/arch_init.c b/arch_init.c
index f849f9b..cdd8ab7 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -489,6 +489,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
     ram_addr_t addr;
     RAMBlock *block;

+    memory_global_sync_dirty_bitmap(get_system_memory());
     bytes_transferred = 0;
     last_block = NULL;
     last_offset = 0;
-- 
1.7.11.4

Re: [Qemu-devel] [PATCH 02/41] fix migration sync

[Paolo Bonzini]

Il 21/09/2012 10:46, Juan Quintela ha scritto:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  arch_init.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch_init.c b/arch_init.c
> index f849f9b..cdd8ab7 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -489,6 +489,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>      ram_addr_t addr;
>      RAMBlock *block;
> 
> +    memory_global_sync_dirty_bitmap(get_system_memory());

Does it make sense to call this function before memory_global_dirty_log_start()?

Also, does this call subsume this loop:

    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)) {
                memory_region_set_dirty(block->mr, addr, TARGET_PAGE_SIZE);
            }
        }
    }

so that the right fix is

-    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)) {
-                memory_region_set_dirty(block->mr, addr, TARGET_PAGE_SIZE);
-            }
-        }
-    }
-
     memory_global_dirty_log_start();
+    memory_global_sync_dirty_bitmap(get_system_memory());

?

Paolo

>      bytes_transferred = 0;
>      last_block = NULL;
>      last_offset = 0;
> 

Re: [Qemu-devel] [PATCH 02/41] fix migration sync

[Paolo Bonzini]

Il 21/09/2012 14:17, Paolo Bonzini ha scritto:
> 
> -    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)) {
> -                memory_region_set_dirty(block->mr, addr, TARGET_PAGE_SIZE);
> -            }
> -        }
> -    }
> -
>      memory_global_dirty_log_start();
> +    memory_global_sync_dirty_bitmap(get_system_memory());

Hmm, perhaps for KVM but probably not for TCG.  But this looks like a bug in
KVM, maybe a fix there would be better?

diff --git a/kvm-all.c b/kvm-all.c
index 78e7a88..fdab4f6 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -744,6 +746,8 @@ static void kvm_log_global_start(struct MemoryListener *listener)
 {
     int r;
 
+    /* Clear the KVM bitmap.  */
+    kvm_physical_sync_dirty_bitmap(section);
     r = kvm_set_migration_log(1);
     assert(r >= 0);
 }

On the other hand, you're doing useless work syncing the KVM bitmap to the
TCG one---even though the TCG bitmap is all-ones!  Something like this ought
to be faster, but I'm not sure whether it is conceptually right:

diff --git a/kvm-all.c b/kvm-all.c
index 78e7a88..fdab4f6 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -399,7 +399,7 @@ static int kvm_get_dirty_pages_log_range(MemoryRegionSection *section,
  * @start_add: start of logged region.
  * @end_addr: end of logged region.
  */
-static int kvm_physical_sync_dirty_bitmap(MemoryRegionSection *section)
+static int kvm_physical_sync_dirty_bitmap(MemoryRegionSection *section, bool sync)
 {
     KVMState *s = kvm_state;
     unsigned long size, allocated_size = 0;
@@ -446,7 +446,9 @@ static int kvm_physical_sync_dirty_bitmap(MemoryRegionSection *section)
             break;
         }
 
-        kvm_get_dirty_pages_log_range(section, d.dirty_bitmap);
+        if (sync) {
+            kvm_get_dirty_pages_log_range(section, d.dirty_bitmap);
+        }
         start_addr = mem->start_addr + mem->memory_size;
     }
     g_free(d.dirty_bitmap);
@@ -600,7 +602,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
         old = *mem;
 
         if (mem->flags & KVM_MEM_LOG_DIRTY_PAGES) {
-            kvm_physical_sync_dirty_bitmap(section);
+            kvm_physical_sync_dirty_bitmap(section, true);
         }
 
         /* unregister the overlapping slot */
@@ -733,7 +735,7 @@ static void kvm_log_sync(MemoryListener *listener,
 {
     int r;
 
-    r = kvm_physical_sync_dirty_bitmap(section);
+    r = kvm_physical_sync_dirty_bitmap(section, true);
     if (r < 0) {
         abort();
     }
@@ -744,6 +746,8 @@ static void kvm_log_global_start(struct MemoryListener *listener)
 {
     int r;
 
+    /* Clear the KVM bitmap.  */
+    kvm_physical_sync_dirty_bitmap(section, false);
     r = kvm_set_migration_log(1);
     assert(r >= 0);
 }

Re: [Qemu-devel] [PATCH 02/41] fix migration sync

[Juan Quintela]

Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 21/09/2012 14:17, Paolo Bonzini ha scritto:
>> 
>> -    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)) {
>> -                memory_region_set_dirty(block->mr, addr, TARGET_PAGE_SIZE);
>> -            }
>> -        }
>> -    }
>> -
>>      memory_global_dirty_log_start();
>> +    memory_global_sync_dirty_bitmap(get_system_memory());

With the part of moving it after the memory_global_dirty_log_start() I
agree.

With the other suggestion, I will take another look at it.  The problem
is that vga code can also sync the kvm bitmap, and we want to get that
notifications also.

Later, Juan.

Re: [Qemu-devel] [PATCH 02/41] fix migration sync

[Paolo Bonzini]

Il 02/10/2012 12:43, Juan Quintela ha scritto:
> Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 21/09/2012 14:17, Paolo Bonzini ha scritto:
>>>
>>> -    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)) {
>>> -                memory_region_set_dirty(block->mr, addr, TARGET_PAGE_SIZE);
>>> -            }
>>> -        }
>>> -    }
>>> -
>>>      memory_global_dirty_log_start();
>>> +    memory_global_sync_dirty_bitmap(get_system_memory());
> 
> With the part of moving it after the memory_global_dirty_log_start() I
> agree.
> 
> With the other suggestion, I will take another look at it.  The problem
> is that vga code can also sync the kvm bitmap, and we want to get that
> notifications also.

I think I had replied to myself later... in any case, nothing that
cannot be improved after merging.  Thanks!

Paolo

[Qemu-devel] [PATCH v2 00/41] Migration cleanups, refactorings, stats, and more

[Juan Quintela]

Hi

v2: (20121001)
- fix qemu_in_vcpu_thread: marcelo
- fix "fix migration sync on setup"
- addressed all marcelo comments

Please, review.


v1: (20120921)

This is the mergable part of migration thread work that I am doing.
What it does:
- cleanups left and right
- 2nd patch is a (fix migration sync) should be applied to stable IMHO
   Othrewise, we "could" be sending zero pages 2 times.
- It introduces the stats discused previously
   * expected_downtime
   * downtime
   * dirty_pages_rate
     All Eric & Luiz comments have been addressed
- buffered file has been basically unfolded.  It was used only once,
  with one user.  So we de-abstracted it.
  Notice that with migration patches, it makes no sense anymore
- migration bitmap

  Now the bitmap is 1bit per page.  Notice that this only affects to
  migration.  For the rest (aka TCG & VGA), we still use the old bitmap.

- remove qemu_file* functions that were not used or used only once
- last_error: we have made that almost all error paths return one error
  instead of zero, so we use much, much less the error on the qemu_file.
  Some of the last remmants are:
  * still there due to callbacks (can be removed when the thread is
    integrated)
  * are on the read path (I haven't touched a lot of those yet)
- qemu_cpu_is_vcpu()

  We used to test if we were _not_ an vcpu testing if we were the
  iothread.  With the migration thread that is not true anymore.  So
  create a function that does the right thing.

Please review.

Thanks, Juan.

The following changes since commit e744c06fca438dc08271e626034e632a270c91c8:

  fpu/softfloat.c: Return correctly signed values from uint64_to_float32 (2012-10-01 22:06:39 +0200)

are available in the git repository at:

  http://repo.or.cz/r/qemu/quintela.git migration-next-20121001

for you to fetch changes up to 7708df6346690e7b93d6c162d6e97f0428559bd5:

  cpus: create qemu_in_vcpu_thread() (2012-10-02 13:13:47 +0200)


Juan Quintela (40):
  buffered_file: g_realloc() can't fail
  fix migration sync
  migration: store end_time in a local variable
  migration: print total downtime for final phase of migration
  migration: rename expected_time to expected_downtime
  migration: export migrate_get_current()
  migration: print expected downtime in info migrate
  savevm: Factorize ram globals reset in its own function
  ram: introduce migration_bitmap_set_dirty()
  ram: Introduce migration_bitmap_test_and_reset_dirty()
  ram: Export last_ram_offset()
  ram: introduce migration_bitmap_sync()
  ram: create trace event for migration sync bitmap
  Separate migration bitmap
  migration: Add dirty_pages_rate to query migrate output
  buffered_file: rename opaque to migration_state
  buffered_file: opaque is MigrationState
  buffered_file: unfold migrate_fd_put_buffer
  buffered_file: unfold migrate_fd_put_ready
  buffered_file: unfold migrate_fd_wait_for_unfreeze
  buffered_file: unfold migrate_fd_close
  buffered_file: We can access directly to bandwidth_limit
  buffered_file: callers of buffered_flush() already check for errors
  buffered_file: make buffered_flush return the error code
  migration: make migrate_fd_wait_for_unfreeze() return errors
  savevm: unexport qemu_fflush
  virtio-net: use qemu_get_buffer() in a temp buffer
  savevm: Remove qemu_fseek()
  savevm: make qemu_fflush() return an error code
  savevm: unfold qemu_fclose_internal()
  savevm: unexport qemu_ftell()
  savevm: make qemu_fill_buffer() be consistent
  savevm: Only qemu_fflush() can generate errors
  buffered_file: buffered_put_buffer() don't need to set last_error
  block-migration: make flush_blks() return errors
  block-migration:  Switch meaning of return value
  block-migration: handle errors with the return codes correctly
  savevm: un-export qemu_file_set_error()
  savevm: make qemu_file_put_notify() return errors
  cpus: create qemu_in_vcpu_thread()

Paolo Bonzini (1):
  BufferedFile: append, then flush

 arch_init.c       | 141 +++++++++++++++++++++++++++++++++++++++++-------------
 block-migration.c |  56 +++++++++++-----------
 buffered_file.c   | 129 ++++++++++++++++++-------------------------------
 buffered_file.h   |  12 +----
 cpu-all.h         |   3 +-
 cpus.c            |   9 +++-
 exec-obsolete.h   |  10 ----
 exec.c            |   2 +-
 hmp.c             |  12 +++++
 hw/virtio-net.c   |   4 +-
 migration.c       |  46 +++++++++---------
 migration.h       |  10 ++++
 qapi-schema.json  |  20 ++++++--
 qemu-file.h       |   8 +---
 qmp-commands.hx   |   9 ++++
 savevm.c          | 117 +++++++++++++++++---------------------------
 trace-events      |   4 ++
 17 files changed, 320 insertions(+), 272 deletions(-)

-- 
1.7.11.4

[Qemu-devel] [PATCH 01/41] buffered_file: g_realloc() can't fail

[Juan Quintela]

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Orit Wasserman <owasserm@redhat.com>
---
 buffered_file.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index f170aa0..4148abb 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -50,20 +50,12 @@ static void buffered_append(QEMUFileBuffered *s,
                             const uint8_t *buf, size_t size)
 {
     if (size > (s->buffer_capacity - s->buffer_size)) {
-        void *tmp;
-
         DPRINTF("increasing buffer capacity from %zu by %zu\n",
                 s->buffer_capacity, size + 1024);

         s->buffer_capacity += size + 1024;

-        tmp = g_realloc(s->buffer, s->buffer_capacity);
-        if (tmp == NULL) {
-            fprintf(stderr, "qemu file buffer expansion failed\n");
-            exit(1);
-        }
-
-        s->buffer = tmp;
+        s->buffer = g_realloc(s->buffer, s->buffer_capacity);
     }

     memcpy(s->buffer + s->buffer_size, buf, size);
-- 
1.7.11.4

[Qemu-devel] [PATCH 02/41] fix migration sync

[Juan Quintela]

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

diff --git a/arch_init.c b/arch_init.c
index 9904f95..671d354 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -517,6 +517,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
     }

     memory_global_dirty_log_start();
+    memory_global_sync_dirty_bitmap(get_system_memory());

     qemu_put_be64(f, ram_bytes_total() | RAM_SAVE_FLAG_MEM_SIZE);

-- 
1.7.11.4

[Qemu-devel] [PATCH 03/41] migration: store end_time in a local variable

[Juan Quintela]

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Orit Wasserman <owasserm@redhat.com>
---
 migration.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/migration.c b/migration.c
index 22a05c4..7a11026 100644
--- a/migration.c
+++ b/migration.c
@@ -329,6 +329,7 @@ static void migrate_fd_put_ready(void *opaque)
         migrate_fd_error(s);
     } else if (ret == 1) {
         int old_vm_running = runstate_is_running();
+        int64_t end_time;

         DPRINTF("done iterating\n");
         qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
@@ -339,7 +340,8 @@ static void migrate_fd_put_ready(void *opaque)
         } else {
             migrate_fd_completed(s);
         }
-        s->total_time = qemu_get_clock_ms(rt_clock) - s->total_time;
+        end_time = qemu_get_clock_ms(rt_clock);
+        s->total_time = end_time - s->total_time;
         if (s->state != MIG_STATE_COMPLETED) {
             if (old_vm_running) {
                 vm_start();
-- 
1.7.11.4

[Qemu-devel] [PATCH 04/41] migration: print total downtime for final phase of migration

[Juan Quintela]

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Orit Wasserman <owasserm@redhat.com>
---
 hmp.c            | 4 ++++
 migration.c      | 6 +++++-
 migration.h      | 1 +
 qapi-schema.json | 7 ++++++-
 qmp-commands.hx  | 3 +++
 5 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/hmp.c b/hmp.c
index ba6fbd3..40b0c05 100644
--- a/hmp.c
+++ b/hmp.c
@@ -152,6 +152,10 @@ void hmp_info_migrate(Monitor *mon)
         monitor_printf(mon, "Migration status: %s\n", info->status);
         monitor_printf(mon, "total time: %" PRIu64 " milliseconds\n",
                        info->total_time);
+        if (info->has_downtime) {
+            monitor_printf(mon, "downtime: %" PRIu64 " milliseconds\n",
+                           info->downtime);
+        }
     }

     if (info->has_ram) {
diff --git a/migration.c b/migration.c
index 7a11026..1375e7e 100644
--- a/migration.c
+++ b/migration.c
@@ -195,6 +195,8 @@ MigrationInfo *qmp_query_migrate(Error **errp)
         info->has_status = true;
         info->status = g_strdup("completed");
         info->total_time = s->total_time;
+        info->has_downtime = true;
+        info->downtime = s->downtime;

         info->has_ram = true;
         info->ram = g_malloc0(sizeof(*info->ram));
@@ -329,9 +331,10 @@ static void migrate_fd_put_ready(void *opaque)
         migrate_fd_error(s);
     } else if (ret == 1) {
         int old_vm_running = runstate_is_running();
-        int64_t end_time;
+        int64_t start_time, end_time;

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

@@ -342,6 +345,7 @@ static void migrate_fd_put_ready(void *opaque)
         }
         end_time = qemu_get_clock_ms(rt_clock);
         s->total_time = end_time - s->total_time;
+        s->downtime = end_time - start_time;
         if (s->state != MIG_STATE_COMPLETED) {
             if (old_vm_running) {
                 vm_start();
diff --git a/migration.h b/migration.h
index a9852fc..3462917 100644
--- a/migration.h
+++ b/migration.h
@@ -40,6 +40,7 @@ struct MigrationState
     void *opaque;
     MigrationParams params;
     int64_t total_time;
+    int64_t downtime;
     bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
     int64_t xbzrle_cache_size;
 };
diff --git a/qapi-schema.json b/qapi-schema.json
index 14e4419..b5a4360 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -413,13 +413,18 @@
 #        If migration has ended, it returns the total migration
 #        time. (since 1.2)
 #
+# @downtime: #optional only present when migration finishes correctly
+#        total downtime in milliseconds for the guest.
+#        (since 1.3)
+#
 # Since: 0.14.0
 ##
 { 'type': 'MigrationInfo',
   'data': {'*status': 'str', '*ram': 'MigrationStats',
            '*disk': 'MigrationStats',
            '*xbzrle-cache': 'XBZRLECacheStats',
-           '*total-time': 'int'} }
+           '*total-time': 'int',
+           '*downtime': 'int'} }

 ##
 # @query-migrate
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 6e21ddb..37be613 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2267,6 +2267,8 @@ The main json-object contains the following:
 - "total-time": total amount of ms since migration started.  If
                 migration has ended, it returns the total migration
 		 time (json-int)
+- "downtime": only present when migration has finished correctly
+                total amount in ms for downtime that happened (json-int)
 - "ram": only present if "status" is "active", it is a json-object with the
   following RAM information (in bytes):
          - "transferred": amount transferred (json-int)
@@ -2304,6 +2306,7 @@ Examples:
           "remaining":123,
           "total":246,
           "total-time":12345,
+          "downtime":12345,
           "duplicate":123,
           "normal":123,
           "normal-bytes":123456
-- 
1.7.11.4

[Qemu-devel] [PATCH 05/41] migration: rename expected_time to expected_downtime

[Juan Quintela]

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Orit Wasserman <owasserm@redhat.com>
---
 arch_init.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 671d354..1d6dda8 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -538,7 +538,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
     double bwidth = 0;
     int ret;
     int i;
-    uint64_t expected_time;
+    uint64_t expected_downtime;

     bytes_transferred_last = bytes_transferred;
     bwidth = qemu_get_clock_ns(rt_clock);
@@ -577,24 +577,24 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
     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_time to a
-     * a very high value, but without crashing */
+    /* 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_put_be64(f, RAM_SAVE_FLAG_EOS);

-    expected_time = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth;
+    expected_downtime = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth;
+    DPRINTF("ram_save_live: expected(%" PRIu64 ") <= max(" PRIu64 ")?\n",
+            expected_downtime, migrate_max_downtime());

-    DPRINTF("ram_save_live: expected(%" PRIu64 ") <= max(%" PRIu64 ")?\n",
-            expected_time, migrate_max_downtime());
-
-    if (expected_time <= migrate_max_downtime()) {
+    if (expected_downtime <= migrate_max_downtime()) {
         memory_global_sync_dirty_bitmap(get_system_memory());
-        expected_time = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth;
+        expected_downtime = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth;

-        return expected_time <= migrate_max_downtime();
+        return expected_downtime <= migrate_max_downtime();
     }
     return 0;
 }
-- 
1.7.11.4

[Qemu-devel] [PATCH 06/41] migration: export migrate_get_current()

[Juan Quintela]

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 migration.c | 2 +-
 migration.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/migration.c b/migration.c
index 1375e7e..fc615c8 100644
--- a/migration.c
+++ b/migration.c
@@ -53,7 +53,7 @@ static NotifierList migration_state_notifiers =
    migrations at once.  For now we don't need to add
    dynamic creation of migration */

-static MigrationState *migrate_get_current(void)
+MigrationState *migrate_get_current(void)
 {
     static MigrationState current_migration = {
         .state = MIG_STATE_SETUP,
diff --git a/migration.h b/migration.h
index 3462917..dabc333 100644
--- a/migration.h
+++ b/migration.h
@@ -81,6 +81,7 @@ void remove_migration_state_change_notifier(Notifier *notify);
 bool migration_is_active(MigrationState *);
 bool migration_has_finished(MigrationState *);
 bool migration_has_failed(MigrationState *);
+MigrationState *migrate_get_current(void);

 uint64_t ram_bytes_remaining(void);
 uint64_t ram_bytes_transferred(void);
-- 
1.7.11.4

[Qemu-devel] [PATCH 07/41] migration: print expected downtime in info migrate

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 arch_init.c      | 2 ++
 hmp.c            | 4 ++++
 migration.c      | 2 ++
 migration.h      | 1 +
 qapi-schema.json | 5 +++++
 qmp-commands.hx  | 6 ++++++
 6 files changed, 20 insertions(+)

diff --git a/arch_init.c b/arch_init.c
index 1d6dda8..3fddb38 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -539,6 +539,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
     int ret;
     int i;
     uint64_t expected_downtime;
+    MigrationState *s = migrate_get_current();

     bytes_transferred_last = bytes_transferred;
     bwidth = qemu_get_clock_ns(rt_clock);
@@ -593,6 +594,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
     if (expected_downtime <= migrate_max_downtime()) {
         memory_global_sync_dirty_bitmap(get_system_memory());
         expected_downtime = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth;
+        s->expected_downtime = expected_downtime / 1000000; /* ns -> ms */

         return expected_downtime <= migrate_max_downtime();
     }
diff --git a/hmp.c b/hmp.c
index 40b0c05..71c9292 100644
--- a/hmp.c
+++ b/hmp.c
@@ -152,6 +152,10 @@ void hmp_info_migrate(Monitor *mon)
         monitor_printf(mon, "Migration status: %s\n", info->status);
         monitor_printf(mon, "total time: %" PRIu64 " milliseconds\n",
                        info->total_time);
+        if (info->has_expected_downtime) {
+            monitor_printf(mon, "expected downtime: %" PRIu64 " milliseconds\n",
+                           info->expected_downtime);
+        }
         if (info->has_downtime) {
             monitor_printf(mon, "downtime: %" PRIu64 " milliseconds\n",
                            info->downtime);
diff --git a/migration.c b/migration.c
index fc615c8..8d3e018 100644
--- a/migration.c
+++ b/migration.c
@@ -169,6 +169,8 @@ MigrationInfo *qmp_query_migrate(Error **errp)
         info->has_total_time = true;
         info->total_time = qemu_get_clock_ms(rt_clock)
             - s->total_time;
+        info->has_expected_downtime = true;
+        info->expected_downtime = s->expected_downtime;

         info->has_ram = true;
         info->ram = g_malloc0(sizeof(*info->ram));
diff --git a/migration.h b/migration.h
index dabc333..552200c 100644
--- a/migration.h
+++ b/migration.h
@@ -41,6 +41,7 @@ struct MigrationState
     MigrationParams params;
     int64_t total_time;
     int64_t downtime;
+    int64_t expected_downtime;
     bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
     int64_t xbzrle_cache_size;
 };
diff --git a/qapi-schema.json b/qapi-schema.json
index b5a4360..b8a1244 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -417,6 +417,10 @@
 #        total downtime in milliseconds for the guest.
 #        (since 1.3)
 #
+# @expected-downtime: #optional only present while migration is active
+#        expected downtime in milliseconds for the guest in last walk
+#        of the dirty bitmap. (since 1.3)
+#
 # Since: 0.14.0
 ##
 { 'type': 'MigrationInfo',
@@ -424,6 +428,7 @@
            '*disk': 'MigrationStats',
            '*xbzrle-cache': 'XBZRLECacheStats',
            '*total-time': 'int',
+           '*expected-downtime': 'int',
            '*downtime': 'int'} }

 ##
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 37be613..68b6580 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2269,6 +2269,9 @@ The main json-object contains the following:
 		 time (json-int)
 - "downtime": only present when migration has finished correctly
                 total amount in ms for downtime that happened (json-int)
+- "expected-downtime": only present while migration is active
+                total amount in ms for downtime that was calculated on
+		the last bitmap round (json-int)
 - "ram": only present if "status" is "active", it is a json-object with the
   following RAM information (in bytes):
          - "transferred": amount transferred (json-int)
@@ -2330,6 +2333,7 @@ Examples:
             "remaining":123,
             "total":246,
             "total-time":12345,
+            "expected-downtime":12345,
             "duplicate":123,
             "normal":123,
             "normal-bytes":123456
@@ -2348,6 +2352,7 @@ Examples:
             "remaining":1053304,
             "transferred":3720,
             "total-time":12345,
+            "expected-downtime":12345,
             "duplicate":123,
             "normal":123,
             "normal-bytes":123456
@@ -2372,6 +2377,7 @@ Examples:
             "remaining":1053304,
             "transferred":3720,
             "total-time":12345,
+            "expected-downtime":12345,
             "duplicate":10,
             "normal":3333,
             "normal-bytes":3412992
-- 
1.7.11.4

[Qemu-devel] [PATCH 08/41] savevm: Factorize ram globals reset in its own function

[Juan Quintela]

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Orit Wasserman <owasserm@redhat.com>
---
 arch_init.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 3fddb38..6b9f949 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -481,6 +481,14 @@ static void ram_migration_cancel(void *opaque)
     migration_end();
 }

+
+static void reset_ram_globals(void)
+{
+    last_block = NULL;
+    last_offset = 0;
+    sort_ram_list();
+}
+
 #define MAX_WAIT 50 /* ms, half buffered_file limit */

 static int ram_save_setup(QEMUFile *f, void *opaque)
@@ -489,9 +497,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
     RAMBlock *block;

     bytes_transferred = 0;
-    last_block = NULL;
-    last_offset = 0;
-    sort_ram_list();
+    reset_ram_globals();

     if (migrate_use_xbzrle()) {
         XBZRLE.cache = cache_init(migrate_xbzrle_cache_size() /
-- 
1.7.11.4

[Qemu-devel] [PATCH 09/41] ram: introduce migration_bitmap_set_dirty()

[Juan Quintela]

It just marks a region of memory as dirty.

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Orit Wasserman <owasserm@redhat.com>
---
 arch_init.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 6b9f949..fa47051 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -331,6 +331,18 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
 static RAMBlock *last_block;
 static ram_addr_t last_offset;

+static inline void migration_bitmap_set_dirty(MemoryRegion *mr, int length)
+{
+    ram_addr_t addr;
+
+    for (addr = 0; addr < length; addr += TARGET_PAGE_SIZE) {
+        if (!memory_region_get_dirty(mr, addr, TARGET_PAGE_SIZE,
+                                     DIRTY_MEMORY_MIGRATION)) {
+            memory_region_set_dirty(mr, addr, TARGET_PAGE_SIZE);
+        }
+    }
+}
+
 /*
  * ram_save_block: Writes a page of memory to the stream f
  *
@@ -493,7 +505,6 @@ static void reset_ram_globals(void)

 static int ram_save_setup(QEMUFile *f, void *opaque)
 {
-    ram_addr_t addr;
     RAMBlock *block;

     bytes_transferred = 0;
@@ -514,12 +525,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)

     /* Make sure all dirty bits are set */
     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)) {
-                memory_region_set_dirty(block->mr, addr, TARGET_PAGE_SIZE);
-            }
-        }
+        migration_bitmap_set_dirty(block->mr, block->length);
     }

     memory_global_dirty_log_start();
-- 
1.7.11.4

[Qemu-devel] [PATCH 10/41] ram: Introduce migration_bitmap_test_and_reset_dirty()

[Juan Quintela]

It just test if the dirty bit is set, and clears it.

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Orit Wasserman <owasserm@redhat.com>
---
 arch_init.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index fa47051..90a722e 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -331,6 +331,19 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
 static RAMBlock *last_block;
 static ram_addr_t last_offset;

+static inline bool migration_bitmap_test_and_reset_dirty(MemoryRegion *mr,
+                                                         ram_addr_t offset)
+{
+    bool ret = memory_region_get_dirty(mr, offset, TARGET_PAGE_SIZE,
+                                       DIRTY_MEMORY_MIGRATION);
+
+    if (ret) {
+        memory_region_reset_dirty(mr, offset, TARGET_PAGE_SIZE,
+                                  DIRTY_MEMORY_MIGRATION);
+    }
+    return ret;
+}
+
 static inline void migration_bitmap_set_dirty(MemoryRegion *mr, int length)
 {
     ram_addr_t addr;
@@ -364,14 +377,10 @@ static int ram_save_block(QEMUFile *f, bool last_stage)

     do {
         mr = block->mr;
-        if (memory_region_get_dirty(mr, offset, TARGET_PAGE_SIZE,
-                                    DIRTY_MEMORY_MIGRATION)) {
+        if (migration_bitmap_test_and_reset_dirty(mr, offset)) {
             uint8_t *p;
             int cont = (block == last_block) ? RAM_SAVE_FLAG_CONTINUE : 0;

-            memory_region_reset_dirty(mr, offset, TARGET_PAGE_SIZE,
-                                      DIRTY_MEMORY_MIGRATION);
-
             p = memory_region_get_ram_ptr(mr) + offset;

             if (is_dup_page(p)) {
-- 
1.7.11.4

[Qemu-devel] [PATCH 11/41] ram: Export last_ram_offset()

[Juan Quintela]

Is the only way of knowing the RAM size.

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpu-all.h | 2 ++
 exec.c    | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/cpu-all.h b/cpu-all.h
index 74d3681..5408782 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -517,6 +517,8 @@ extern int mem_prealloc;
 void dump_exec_info(FILE *f, fprintf_function cpu_fprintf);
 #endif /* !CONFIG_USER_ONLY */

+ram_addr_t last_ram_offset(void);
+
 int cpu_memory_rw_debug(CPUArchState *env, target_ulong addr,
                         uint8_t *buf, int len, int is_write);

diff --git a/exec.c b/exec.c
index bb6aa4a..e613329 100644
--- a/exec.c
+++ b/exec.c
@@ -2462,7 +2462,7 @@ static ram_addr_t find_ram_offset(ram_addr_t size)
     return offset;
 }

-static ram_addr_t last_ram_offset(void)
+ram_addr_t last_ram_offset(void)
 {
     RAMBlock *block;
     ram_addr_t last = 0;
-- 
1.7.11.4

[Qemu-devel] [PATCH 12/41] ram: introduce migration_bitmap_sync()

[Juan Quintela]

Helper that we use each time that we need to syncronize the migration
bitmap with the other dirty bitmaps.

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch_init.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 90a722e..3a5fecc 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -356,6 +356,12 @@ static inline void migration_bitmap_set_dirty(MemoryRegion *mr, int length)
     }
 }

+static void migration_bitmap_sync(void)
+{
+    memory_global_sync_dirty_bitmap(get_system_memory());
+}
+
+
 /*
  * ram_save_block: Writes a page of memory to the stream f
  *
@@ -613,7 +619,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
             expected_downtime, migrate_max_downtime());

     if (expected_downtime <= migrate_max_downtime()) {
-        memory_global_sync_dirty_bitmap(get_system_memory());
+        migration_bitmap_sync();
         expected_downtime = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth;
         s->expected_downtime = expected_downtime / 1000000; /* ns -> ms */

@@ -624,7 +630,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)

 static int ram_save_complete(QEMUFile *f, void *opaque)
 {
-    memory_global_sync_dirty_bitmap(get_system_memory());
+    migration_bitmap_sync();

     /* try transferring iterative blocks of memory */

-- 
1.7.11.4

[Qemu-devel] [PATCH 13/41] ram: create trace event for migration sync bitmap

[Juan Quintela]

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch_init.c  | 6 ++++++
 trace-events | 4 ++++
 2 files changed, 10 insertions(+)

diff --git a/arch_init.c b/arch_init.c
index 3a5fecc..cec3ead 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -45,6 +45,7 @@
 #include "hw/pcspk.h"
 #include "qemu/page_cache.h"
 #include "qmp-commands.h"
+#include "trace.h"

 #ifdef DEBUG_ARCH_INIT
 #define DPRINTF(fmt, ...) \
@@ -358,7 +359,12 @@ static inline void migration_bitmap_set_dirty(MemoryRegion *mr, int length)

 static void migration_bitmap_sync(void)
 {
+    uint64_t num_dirty_pages_init = ram_list.dirty_pages;
+
+    trace_migration_bitmap_sync_start();
     memory_global_sync_dirty_bitmap(get_system_memory());
+    trace_migration_bitmap_sync_end(ram_list.dirty_pages
+                                    - num_dirty_pages_init);
 }


diff --git a/trace-events b/trace-events
index f5b5097..818b261 100644
--- a/trace-events
+++ b/trace-events
@@ -917,6 +917,10 @@ ppm_save(const char *filename, void *display_surface) "%s surface=%p"
 savevm_section_start(void) ""
 savevm_section_end(unsigned int section_id) "section_id %u"

+# arch_init.c
+migration_bitmap_sync_start(void) ""
+migration_bitmap_sync_end(uint64_t dirty_pages) "dirty_pages %" PRIu64""
+
 # hw/qxl.c
 disable qxl_interface_set_mm_time(int qid, uint32_t mm_time) "%d %d"
 disable qxl_io_write_vga(int qid, const char *mode, uint32_t addr, uint32_t val) "%d %s addr=%u val=%u"
-- 
1.7.11.4

[Qemu-devel] [PATCH 14/41] Separate migration bitmap

[Juan Quintela]

This patch creates a migration bitmap, which is periodically kept in
sync with the qemu bitmap. A separate copy of the dirty bitmap for the
migration limits the amount of concurrent access to the qemu bitmap
from iothread and migration thread (which requires taking the big
lock).

We use the qemu bitmap type.  We have to "undo" the dirty_pages
counting optimization on the general dirty bitmap and do the counting
optimization with the migration local bitmap.

Signed-off-by: Umesh Deshpande <udeshpan@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 arch_init.c     | 63 +++++++++++++++++++++++++++++++++++++++------------------
 cpu-all.h       |  1 -
 exec-obsolete.h | 10 ---------
 3 files changed, 43 insertions(+), 31 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index cec3ead..d918844 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -31,6 +31,8 @@
 #include "config.h"
 #include "monitor.h"
 #include "sysemu.h"
+#include "bitops.h"
+#include "bitmap.h"
 #include "arch_init.h"
 #include "audio/audio.h"
 #include "hw/pc.h"
@@ -331,39 +333,57 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,

 static RAMBlock *last_block;
 static ram_addr_t last_offset;
+static unsigned long *migration_bitmap;
+static uint64_t migration_dirty_pages;

 static inline bool migration_bitmap_test_and_reset_dirty(MemoryRegion *mr,
                                                          ram_addr_t offset)
 {
-    bool ret = memory_region_get_dirty(mr, offset, TARGET_PAGE_SIZE,
-                                       DIRTY_MEMORY_MIGRATION);
+    bool ret;
+    int nr = (mr->ram_addr + offset) >> TARGET_PAGE_BITS;
+
+    ret = test_and_clear_bit(nr, migration_bitmap);

     if (ret) {
-        memory_region_reset_dirty(mr, offset, TARGET_PAGE_SIZE,
-                                  DIRTY_MEMORY_MIGRATION);
+        migration_dirty_pages--;
     }
     return ret;
 }

-static inline void migration_bitmap_set_dirty(MemoryRegion *mr, int length)
+static inline bool migration_bitmap_set_dirty(MemoryRegion *mr,
+                                              ram_addr_t offset)
 {
-    ram_addr_t addr;
+    bool ret;
+    int nr = (mr->ram_addr + offset) >> TARGET_PAGE_BITS;

-    for (addr = 0; addr < length; addr += TARGET_PAGE_SIZE) {
-        if (!memory_region_get_dirty(mr, addr, TARGET_PAGE_SIZE,
-                                     DIRTY_MEMORY_MIGRATION)) {
-            memory_region_set_dirty(mr, addr, TARGET_PAGE_SIZE);
-        }
+    ret = test_and_set_bit(nr, migration_bitmap);
+
+    if (!ret) {
+        migration_dirty_pages++;
     }
+    return ret;
 }

 static void migration_bitmap_sync(void)
 {
-    uint64_t num_dirty_pages_init = ram_list.dirty_pages;
+    RAMBlock *block;
+    ram_addr_t addr;
+    uint64_t num_dirty_pages_init = migration_dirty_pages;

     trace_migration_bitmap_sync_start();
     memory_global_sync_dirty_bitmap(get_system_memory());
-    trace_migration_bitmap_sync_end(ram_list.dirty_pages
+
+    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)) {
+                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);
 }

@@ -442,7 +462,7 @@ static uint64_t bytes_transferred;

 static ram_addr_t ram_save_remaining(void)
 {
-    return ram_list.dirty_pages;
+    return migration_dirty_pages;
 }

 uint64_t ram_bytes_remaining(void)
@@ -527,6 +547,11 @@ static void reset_ram_globals(void)
 static int ram_save_setup(QEMUFile *f, void *opaque)
 {
     RAMBlock *block;
+    int64_t ram_pages = last_ram_offset() >> TARGET_PAGE_BITS;
+
+    migration_bitmap = bitmap_new(ram_pages);
+    bitmap_set(migration_bitmap, 1, ram_pages);
+    migration_dirty_pages = ram_pages;

     bytes_transferred = 0;
     reset_ram_globals();
@@ -544,13 +569,8 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
         acct_clear();
     }

-    /* Make sure all dirty bits are set */
-    QLIST_FOREACH(block, &ram_list.blocks, next) {
-        migration_bitmap_set_dirty(block->mr, block->length);
-    }
-
     memory_global_dirty_log_start();
-    memory_global_sync_dirty_bitmap(get_system_memory());
+    migration_bitmap_sync();

     qemu_put_be64(f, ram_bytes_total() | RAM_SAVE_FLAG_MEM_SIZE);

@@ -655,6 +675,9 @@ static int ram_save_complete(QEMUFile *f, void *opaque)

     qemu_put_be64(f, RAM_SAVE_FLAG_EOS);

+    g_free(migration_bitmap);
+    migration_bitmap = NULL;
+
     return 0;
 }

diff --git a/cpu-all.h b/cpu-all.h
index 5408782..db25f73 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -497,7 +497,6 @@ typedef struct RAMBlock {
 typedef struct RAMList {
     uint8_t *phys_dirty;
     QLIST_HEAD(, RAMBlock) blocks;
-    uint64_t dirty_pages;
 } RAMList;
 extern RAMList ram_list;

diff --git a/exec-obsolete.h b/exec-obsolete.h
index c099256..f8ffce6 100644
--- a/exec-obsolete.h
+++ b/exec-obsolete.h
@@ -74,11 +74,6 @@ static inline int cpu_physical_memory_get_dirty(ram_addr_t start,
 static inline int cpu_physical_memory_set_dirty_flags(ram_addr_t addr,
                                                       int dirty_flags)
 {
-    if ((dirty_flags & MIGRATION_DIRTY_FLAG) &&
-        !cpu_physical_memory_get_dirty(addr, TARGET_PAGE_SIZE,
-                                       MIGRATION_DIRTY_FLAG)) {
-        ram_list.dirty_pages++;
-    }
     return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] |= dirty_flags;
 }

@@ -92,11 +87,6 @@ static inline int cpu_physical_memory_clear_dirty_flags(ram_addr_t addr,
 {
     int mask = ~dirty_flags;

-    if ((dirty_flags & MIGRATION_DIRTY_FLAG) &&
-        cpu_physical_memory_get_dirty(addr, TARGET_PAGE_SIZE,
-                                      MIGRATION_DIRTY_FLAG)) {
-        ram_list.dirty_pages--;
-    }
     return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] &= mask;
 }

-- 
1.7.11.4

[Qemu-devel] [PATCH 15/41] migration: Add dirty_pages_rate to query migrate output

[Juan Quintela]

It indicates how many pages were dirtied during the last second.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 arch_init.c      | 18 ++++++++++++++++++
 hmp.c            |  4 ++++
 migration.c      |  2 ++
 migration.h      |  1 +
 qapi-schema.json |  8 ++++++--
 5 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index d918844..e6effe8 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -369,6 +369,14 @@ static void migration_bitmap_sync(void)
     RAMBlock *block;
     ram_addr_t addr;
     uint64_t num_dirty_pages_init = migration_dirty_pages;
+    MigrationState *s = migrate_get_current();
+    static int64_t start_time;
+    static int64_t num_dirty_pages_period;
+    int64_t end_time;
+
+    if (!start_time) {
+        start_time = qemu_get_clock_ms(rt_clock);
+    }

     trace_migration_bitmap_sync_start();
     memory_global_sync_dirty_bitmap(get_system_memory());
@@ -385,6 +393,16 @@ static void migration_bitmap_sync(void)
     }
     trace_migration_bitmap_sync_end(migration_dirty_pages
                                     - num_dirty_pages_init);
+    num_dirty_pages_period += migration_dirty_pages - num_dirty_pages_init;
+    end_time = qemu_get_clock_ms(rt_clock);
+
+    /* more than 1 second = 1000 millisecons */
+    if (end_time > start_time + 1000) {
+        s->dirty_pages_rate = num_dirty_pages_period * 1000
+            / (end_time - start_time);
+        start_time = end_time;
+        num_dirty_pages_period = 0;
+    }
 }


diff --git a/hmp.c b/hmp.c
index 71c9292..67a529a 100644
--- a/hmp.c
+++ b/hmp.c
@@ -175,6 +175,10 @@ void hmp_info_migrate(Monitor *mon)
                        info->ram->normal);
         monitor_printf(mon, "normal bytes: %" PRIu64 " kbytes\n",
                        info->ram->normal_bytes >> 10);
+        if (info->ram->dirty_pages_rate) {
+            monitor_printf(mon, "dirty pages rate: %" PRIu64 " pages\n",
+                           info->ram->dirty_pages_rate);
+        }
     }

     if (info->has_disk) {
diff --git a/migration.c b/migration.c
index 8d3e018..330c16a 100644
--- a/migration.c
+++ b/migration.c
@@ -180,6 +180,8 @@ MigrationInfo *qmp_query_migrate(Error **errp)
         info->ram->duplicate = dup_mig_pages_transferred();
         info->ram->normal = norm_mig_pages_transferred();
         info->ram->normal_bytes = norm_mig_bytes_transferred();
+        info->ram->dirty_pages_rate = s->dirty_pages_rate;
+

         if (blk_mig_active()) {
             info->has_disk = true;
diff --git a/migration.h b/migration.h
index 552200c..66d7f68 100644
--- a/migration.h
+++ b/migration.h
@@ -42,6 +42,7 @@ struct MigrationState
     int64_t total_time;
     int64_t downtime;
     int64_t expected_downtime;
+    int64_t dirty_pages_rate;
     bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
     int64_t xbzrle_cache_size;
 };
diff --git a/qapi-schema.json b/qapi-schema.json
index b8a1244..4a9ae52 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -358,13 +358,17 @@
 #
 # @normal : number of normal pages (since 1.2)
 #
-# @normal-bytes : number of normal bytes sent (since 1.2)
+# @normal-bytes: number of normal bytes sent (since 1.2)
+#
+# @dirty-pages-rate: number of pages dirtied by second by the
+#        guest (since 1.3)
 #
 # Since: 0.14.0
 ##
 { 'type': 'MigrationStats',
   'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
-           'duplicate': 'int', 'normal': 'int', 'normal-bytes': 'int' } }
+           'duplicate': 'int', 'normal': 'int', 'normal-bytes': 'int',
+           'dirty-pages-rate' : 'int' } }

 ##
 # @XBZRLECacheStats
-- 
1.7.11.4

[Qemu-devel] [PATCH 16/41] BufferedFile: append, then flush

[Juan Quintela]

From: Paolo Bonzini <pbonzini@redhat.com>

Simplify the logic for pushing data from the buffer to the output
pipe/socket.  This also matches more closely what will be the
operation of the migration thread.

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Orit Wasserman <owasserm@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 buffered_file.c | 50 +++++++++++---------------------------------------
 1 file changed, 11 insertions(+), 39 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index 4148abb..7155800 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -75,7 +75,7 @@ static void buffered_flush(QEMUFileBuffered *s)

     DPRINTF("flushing %zu byte(s) of data\n", s->buffer_size);

-    while (offset < s->buffer_size) {
+    while (s->bytes_xfer < s->xfer_limit && offset < s->buffer_size) {
         ssize_t ret;

         ret = s->put_buffer(s->opaque, s->buffer + offset,
@@ -93,6 +93,7 @@ static void buffered_flush(QEMUFileBuffered *s)
         } else {
             DPRINTF("flushed %zd byte(s)\n", ret);
             offset += ret;
+            s->bytes_xfer += ret;
         }
     }

@@ -104,8 +105,7 @@ static void buffered_flush(QEMUFileBuffered *s)
 static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, int size)
 {
     QEMUFileBuffered *s = opaque;
-    int offset = 0, error;
-    ssize_t ret;
+    int error;

     DPRINTF("putting %d bytes at %" PRId64 "\n", size, pos);

@@ -118,48 +118,22 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
     DPRINTF("unfreezing output\n");
     s->freeze_output = 0;

-    buffered_flush(s);
-
-    while (!s->freeze_output && offset < size) {
-        if (s->bytes_xfer > s->xfer_limit) {
-            DPRINTF("transfer limit exceeded when putting\n");
-            break;
-        }
-
-        ret = s->put_buffer(s->opaque, buf + offset, size - offset);
-        if (ret == -EAGAIN) {
-            DPRINTF("backend not ready, freezing\n");
-            s->freeze_output = 1;
-            break;
-        }
-
-        if (ret <= 0) {
-            DPRINTF("error putting\n");
-            qemu_file_set_error(s->file, ret);
-            offset = -EINVAL;
-            break;
-        }
-
-        DPRINTF("put %zd byte(s)\n", ret);
-        offset += ret;
-        s->bytes_xfer += ret;
-    }
-
-    if (offset >= 0) {
+    if (size > 0) {
         DPRINTF("buffering %d bytes\n", size - offset);
-        buffered_append(s, buf + offset, size - offset);
-        offset = size;
+        buffered_append(s, buf, size);
     }

+    buffered_flush(s);
+
     if (pos == 0 && size == 0) {
         DPRINTF("file is ready\n");
-        if (s->bytes_xfer <= s->xfer_limit) {
+        if (!s->freeze_output && s->bytes_xfer < s->xfer_limit) {
             DPRINTF("notifying client\n");
             s->put_ready(s->opaque);
         }
     }

-    return offset;
+    return size;
 }

 static int buffered_close(void *opaque)
@@ -169,6 +143,7 @@ static int buffered_close(void *opaque)

     DPRINTF("closing\n");

+    s->xfer_limit = INT_MAX;
     while (!qemu_file_get_error(s->file) && s->buffer_size) {
         buffered_flush(s);
         if (s->freeze_output)
@@ -248,10 +223,7 @@ static void buffered_rate_tick(void *opaque)

     s->bytes_xfer = 0;

-    buffered_flush(s);
-
-    /* Add some checks around this */
-    s->put_ready(s->opaque);
+    buffered_put_buffer(s, NULL, 0, 0);
 }

 QEMUFile *qemu_fopen_ops_buffered(void *opaque,
-- 
1.7.11.4

[Qemu-devel] [PATCH 17/41] buffered_file: rename opaque to migration_state

[Juan Quintela]

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 buffered_file.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index 7155800..33b700b 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -27,7 +27,7 @@ typedef struct QEMUFileBuffered
     BufferedPutReadyFunc *put_ready;
     BufferedWaitForUnfreezeFunc *wait_for_unfreeze;
     BufferedCloseFunc *close;
-    void *opaque;
+    void *migration_state;
     QEMUFile *file;
     int freeze_output;
     size_t bytes_xfer;
@@ -78,7 +78,7 @@ static void buffered_flush(QEMUFileBuffered *s)
     while (s->bytes_xfer < s->xfer_limit && offset < s->buffer_size) {
         ssize_t ret;

-        ret = s->put_buffer(s->opaque, s->buffer + offset,
+        ret = s->put_buffer(s->migration_state, s->buffer + offset,
                             s->buffer_size - offset);
         if (ret == -EAGAIN) {
             DPRINTF("backend not ready, freezing\n");
@@ -129,7 +129,7 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
         DPRINTF("file is ready\n");
         if (!s->freeze_output && s->bytes_xfer < s->xfer_limit) {
             DPRINTF("notifying client\n");
-            s->put_ready(s->opaque);
+            s->put_ready(s->migration_state);
         }
     }

@@ -147,10 +147,10 @@ static int buffered_close(void *opaque)
     while (!qemu_file_get_error(s->file) && s->buffer_size) {
         buffered_flush(s);
         if (s->freeze_output)
-            s->wait_for_unfreeze(s->opaque);
+            s->wait_for_unfreeze(s->migration_state);
     }

-    ret = s->close(s->opaque);
+    ret = s->close(s->migration_state);

     qemu_del_timer(s->timer);
     qemu_free_timer(s->timer);
@@ -237,7 +237,7 @@ QEMUFile *qemu_fopen_ops_buffered(void *opaque,

     s = g_malloc0(sizeof(*s));

-    s->opaque = opaque;
+    s->migration_state = opaque;
     s->xfer_limit = bytes_per_sec / 10;
     s->put_buffer = put_buffer;
     s->put_ready = put_ready;
-- 
1.7.11.4

[Qemu-devel] [PATCH 18/41] buffered_file: opaque is MigrationState

[Juan Quintela]

It always have that type, just change it.
We will remove buffered file later on the migration thread series.

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 buffered_file.c | 6 +++---
 buffered_file.h | 4 +++-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index 33b700b..59d952d 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -27,7 +27,7 @@ typedef struct QEMUFileBuffered
     BufferedPutReadyFunc *put_ready;
     BufferedWaitForUnfreezeFunc *wait_for_unfreeze;
     BufferedCloseFunc *close;
-    void *migration_state;
+    MigrationState *migration_state;
     QEMUFile *file;
     int freeze_output;
     size_t bytes_xfer;
@@ -226,7 +226,7 @@ static void buffered_rate_tick(void *opaque)
     buffered_put_buffer(s, NULL, 0, 0);
 }

-QEMUFile *qemu_fopen_ops_buffered(void *opaque,
+QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state,
                                   size_t bytes_per_sec,
                                   BufferedPutFunc *put_buffer,
                                   BufferedPutReadyFunc *put_ready,
@@ -237,7 +237,7 @@ QEMUFile *qemu_fopen_ops_buffered(void *opaque,

     s = g_malloc0(sizeof(*s));

-    s->migration_state = opaque;
+    s->migration_state = migration_state;
     s->xfer_limit = bytes_per_sec / 10;
     s->put_buffer = put_buffer;
     s->put_ready = put_ready;
diff --git a/buffered_file.h b/buffered_file.h
index 98d358b..39f7fa0 100644
--- a/buffered_file.h
+++ b/buffered_file.h
@@ -15,13 +15,15 @@
 #define QEMU_BUFFERED_FILE_H

 #include "hw/hw.h"
+#include "migration.h"

 typedef ssize_t (BufferedPutFunc)(void *opaque, const void *data, size_t size);
 typedef void (BufferedPutReadyFunc)(void *opaque);
 typedef void (BufferedWaitForUnfreezeFunc)(void *opaque);
 typedef int (BufferedCloseFunc)(void *opaque);

-QEMUFile *qemu_fopen_ops_buffered(void *opaque, size_t xfer_limit,
+QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state,
+                                  size_t xfer_limit,
                                   BufferedPutFunc *put_buffer,
                                   BufferedPutReadyFunc *put_ready,
                                   BufferedWaitForUnfreezeFunc *wait_for_unfreeze,
-- 
1.7.11.4

[Qemu-devel] [PATCH 19/41] buffered_file: unfold migrate_fd_put_buffer

[Juan Quintela]

We only used it once, just remove the callback indirection

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 buffered_file.c | 7 ++-----
 buffered_file.h | 2 --
 migration.c     | 6 ++----
 migration.h     | 3 +++
 4 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index 59d952d..702a726 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -23,7 +23,6 @@

 typedef struct QEMUFileBuffered
 {
-    BufferedPutFunc *put_buffer;
     BufferedPutReadyFunc *put_ready;
     BufferedWaitForUnfreezeFunc *wait_for_unfreeze;
     BufferedCloseFunc *close;
@@ -78,8 +77,8 @@ static void buffered_flush(QEMUFileBuffered *s)
     while (s->bytes_xfer < s->xfer_limit && offset < s->buffer_size) {
         ssize_t ret;

-        ret = s->put_buffer(s->migration_state, s->buffer + offset,
-                            s->buffer_size - offset);
+        ret = migrate_fd_put_buffer(s->migration_state, s->buffer + offset,
+                                    s->buffer_size - offset);
         if (ret == -EAGAIN) {
             DPRINTF("backend not ready, freezing\n");
             s->freeze_output = 1;
@@ -228,7 +227,6 @@ static void buffered_rate_tick(void *opaque)

 QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state,
                                   size_t bytes_per_sec,
-                                  BufferedPutFunc *put_buffer,
                                   BufferedPutReadyFunc *put_ready,
                                   BufferedWaitForUnfreezeFunc *wait_for_unfreeze,
                                   BufferedCloseFunc *close)
@@ -239,7 +237,6 @@ QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state,

     s->migration_state = migration_state;
     s->xfer_limit = bytes_per_sec / 10;
-    s->put_buffer = put_buffer;
     s->put_ready = put_ready;
     s->wait_for_unfreeze = wait_for_unfreeze;
     s->close = close;
diff --git a/buffered_file.h b/buffered_file.h
index 39f7fa0..ca7e62d 100644
--- a/buffered_file.h
+++ b/buffered_file.h
@@ -17,14 +17,12 @@
 #include "hw/hw.h"
 #include "migration.h"

-typedef ssize_t (BufferedPutFunc)(void *opaque, const void *data, size_t size);
 typedef void (BufferedPutReadyFunc)(void *opaque);
 typedef void (BufferedWaitForUnfreezeFunc)(void *opaque);
 typedef int (BufferedCloseFunc)(void *opaque);

 QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state,
                                   size_t xfer_limit,
-                                  BufferedPutFunc *put_buffer,
                                   BufferedPutReadyFunc *put_ready,
                                   BufferedWaitForUnfreezeFunc *wait_for_unfreeze,
                                   BufferedCloseFunc *close);
diff --git a/migration.c b/migration.c
index 330c16a..f6ee7cd 100644
--- a/migration.c
+++ b/migration.c
@@ -295,10 +295,9 @@ static void migrate_fd_put_notify(void *opaque)
     }
 }

-static ssize_t migrate_fd_put_buffer(void *opaque, const void *data,
-                                     size_t size)
+ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
+                              size_t size)
 {
-    MigrationState *s = opaque;
     ssize_t ret;

     if (s->state != MIG_STATE_ACTIVE) {
@@ -436,7 +435,6 @@ void migrate_fd_connect(MigrationState *s)
     s->state = MIG_STATE_ACTIVE;
     s->file = qemu_fopen_ops_buffered(s,
                                       s->bandwidth_limit,
-                                      migrate_fd_put_buffer,
                                       migrate_fd_put_ready,
                                       migrate_fd_wait_for_unfreeze,
                                       migrate_fd_close);
diff --git a/migration.h b/migration.h
index 66d7f68..02d0219 100644
--- a/migration.h
+++ b/migration.h
@@ -78,6 +78,9 @@ void migrate_fd_error(MigrationState *s);

 void migrate_fd_connect(MigrationState *s);

+ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
+                              size_t size);
+
 void add_migration_state_change_notifier(Notifier *notify);
 void remove_migration_state_change_notifier(Notifier *notify);
 bool migration_is_active(MigrationState *);
-- 
1.7.11.4

[Qemu-devel] [PATCH 20/41] buffered_file: unfold migrate_fd_put_ready

[Juan Quintela]

We only use it once, just remove the callback indirection.

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 buffered_file.c | 5 +----
 buffered_file.h | 2 --
 migration.c     | 4 +---
 migration.h     | 1 +
 4 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index 702a726..4c6a797 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -23,7 +23,6 @@

 typedef struct QEMUFileBuffered
 {
-    BufferedPutReadyFunc *put_ready;
     BufferedWaitForUnfreezeFunc *wait_for_unfreeze;
     BufferedCloseFunc *close;
     MigrationState *migration_state;
@@ -128,7 +127,7 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
         DPRINTF("file is ready\n");
         if (!s->freeze_output && s->bytes_xfer < s->xfer_limit) {
             DPRINTF("notifying client\n");
-            s->put_ready(s->migration_state);
+            migrate_fd_put_ready(s->migration_state);
         }
     }

@@ -227,7 +226,6 @@ static void buffered_rate_tick(void *opaque)

 QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state,
                                   size_t bytes_per_sec,
-                                  BufferedPutReadyFunc *put_ready,
                                   BufferedWaitForUnfreezeFunc *wait_for_unfreeze,
                                   BufferedCloseFunc *close)
 {
@@ -237,7 +235,6 @@ QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state,

     s->migration_state = migration_state;
     s->xfer_limit = bytes_per_sec / 10;
-    s->put_ready = put_ready;
     s->wait_for_unfreeze = wait_for_unfreeze;
     s->close = close;

diff --git a/buffered_file.h b/buffered_file.h
index ca7e62d..dd239b3 100644
--- a/buffered_file.h
+++ b/buffered_file.h
@@ -17,13 +17,11 @@
 #include "hw/hw.h"
 #include "migration.h"

-typedef void (BufferedPutReadyFunc)(void *opaque);
 typedef void (BufferedWaitForUnfreezeFunc)(void *opaque);
 typedef int (BufferedCloseFunc)(void *opaque);

 QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state,
                                   size_t xfer_limit,
-                                  BufferedPutReadyFunc *put_ready,
                                   BufferedWaitForUnfreezeFunc *wait_for_unfreeze,
                                   BufferedCloseFunc *close);

diff --git a/migration.c b/migration.c
index f6ee7cd..051b1b5 100644
--- a/migration.c
+++ b/migration.c
@@ -318,9 +318,8 @@ ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
     return ret;
 }

-static void migrate_fd_put_ready(void *opaque)
+void migrate_fd_put_ready(MigrationState *s)
 {
-    MigrationState *s = opaque;
     int ret;

     if (s->state != MIG_STATE_ACTIVE) {
@@ -435,7 +434,6 @@ void migrate_fd_connect(MigrationState *s)
     s->state = MIG_STATE_ACTIVE;
     s->file = qemu_fopen_ops_buffered(s,
                                       s->bandwidth_limit,
-                                      migrate_fd_put_ready,
                                       migrate_fd_wait_for_unfreeze,
                                       migrate_fd_close);

diff --git a/migration.h b/migration.h
index 02d0219..031c2ab 100644
--- a/migration.h
+++ b/migration.h
@@ -80,6 +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);

 void add_migration_state_change_notifier(Notifier *notify);
 void remove_migration_state_change_notifier(Notifier *notify);
-- 
1.7.11.4

[Qemu-devel] [PATCH 21/41] buffered_file: unfold migrate_fd_wait_for_unfreeze

[Juan Quintela]

We only used it once, just remove the callback indirection.

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

diff --git a/buffered_file.c b/buffered_file.c
index 4c6a797..d257496 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -23,7 +23,6 @@

 typedef struct QEMUFileBuffered
 {
-    BufferedWaitForUnfreezeFunc *wait_for_unfreeze;
     BufferedCloseFunc *close;
     MigrationState *migration_state;
     QEMUFile *file;
@@ -145,7 +144,7 @@ static int buffered_close(void *opaque)
     while (!qemu_file_get_error(s->file) && s->buffer_size) {
         buffered_flush(s);
         if (s->freeze_output)
-            s->wait_for_unfreeze(s->migration_state);
+            migrate_fd_wait_for_unfreeze(s->migration_state);
     }

     ret = s->close(s->migration_state);
@@ -226,7 +225,6 @@ static void buffered_rate_tick(void *opaque)

 QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state,
                                   size_t bytes_per_sec,
-                                  BufferedWaitForUnfreezeFunc *wait_for_unfreeze,
                                   BufferedCloseFunc *close)
 {
     QEMUFileBuffered *s;
@@ -235,7 +233,6 @@ QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state,

     s->migration_state = migration_state;
     s->xfer_limit = bytes_per_sec / 10;
-    s->wait_for_unfreeze = wait_for_unfreeze;
     s->close = close;

     s->file = qemu_fopen_ops(s, buffered_put_buffer, NULL,
diff --git a/buffered_file.h b/buffered_file.h
index dd239b3..926e5c6 100644
--- a/buffered_file.h
+++ b/buffered_file.h
@@ -17,12 +17,10 @@
 #include "hw/hw.h"
 #include "migration.h"

-typedef void (BufferedWaitForUnfreezeFunc)(void *opaque);
 typedef int (BufferedCloseFunc)(void *opaque);

 QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state,
                                   size_t xfer_limit,
-                                  BufferedWaitForUnfreezeFunc *wait_for_unfreeze,
                                   BufferedCloseFunc *close);

 #endif
diff --git a/migration.c b/migration.c
index 051b1b5..30def68 100644
--- a/migration.c
+++ b/migration.c
@@ -370,9 +370,8 @@ static void migrate_fd_cancel(MigrationState *s)
     migrate_fd_cleanup(s);
 }

-static void migrate_fd_wait_for_unfreeze(void *opaque)
+void migrate_fd_wait_for_unfreeze(MigrationState *s)
 {
-    MigrationState *s = opaque;
     int ret;

     DPRINTF("wait for unfreeze\n");
@@ -434,7 +433,6 @@ void migrate_fd_connect(MigrationState *s)
     s->state = MIG_STATE_ACTIVE;
     s->file = qemu_fopen_ops_buffered(s,
                                       s->bandwidth_limit,
-                                      migrate_fd_wait_for_unfreeze,
                                       migrate_fd_close);

     DPRINTF("beginning savevm\n");
diff --git a/migration.h b/migration.h
index 031c2ab..d6341d6 100644
--- a/migration.h
+++ b/migration.h
@@ -81,6 +81,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);
+void migrate_fd_wait_for_unfreeze(MigrationState *s);

 void add_migration_state_change_notifier(Notifier *notify);
 void remove_migration_state_change_notifier(Notifier *notify);
-- 
1.7.11.4

[Qemu-devel] [PATCH 22/41] buffered_file: unfold migrate_fd_close

[Juan Quintela]

We only used it once, just remove the callback indirection.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 buffered_file.c | 7 ++-----
 buffered_file.h | 5 +----
 migration.c     | 8 ++------
 migration.h     | 1 +
 4 files changed, 6 insertions(+), 15 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index d257496..4fca774 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -23,7 +23,6 @@

 typedef struct QEMUFileBuffered
 {
-    BufferedCloseFunc *close;
     MigrationState *migration_state;
     QEMUFile *file;
     int freeze_output;
@@ -147,7 +146,7 @@ static int buffered_close(void *opaque)
             migrate_fd_wait_for_unfreeze(s->migration_state);
     }

-    ret = s->close(s->migration_state);
+    ret = migrate_fd_close(s->migration_state);

     qemu_del_timer(s->timer);
     qemu_free_timer(s->timer);
@@ -224,8 +223,7 @@ static void buffered_rate_tick(void *opaque)
 }

 QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state,
-                                  size_t bytes_per_sec,
-                                  BufferedCloseFunc *close)
+                                  size_t bytes_per_sec)
 {
     QEMUFileBuffered *s;

@@ -233,7 +231,6 @@ QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state,

     s->migration_state = migration_state;
     s->xfer_limit = bytes_per_sec / 10;
-    s->close = close;

     s->file = qemu_fopen_ops(s, buffered_put_buffer, NULL,
                              buffered_close, buffered_rate_limit,
diff --git a/buffered_file.h b/buffered_file.h
index 926e5c6..8a38754 100644
--- a/buffered_file.h
+++ b/buffered_file.h
@@ -17,10 +17,7 @@
 #include "hw/hw.h"
 #include "migration.h"

-typedef int (BufferedCloseFunc)(void *opaque);
-
 QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state,
-                                  size_t xfer_limit,
-                                  BufferedCloseFunc *close);
+                                  size_t xfer_limit);

 #endif
diff --git a/migration.c b/migration.c
index 30def68..f647a3f 100644
--- a/migration.c
+++ b/migration.c
@@ -392,10 +392,8 @@ void migrate_fd_wait_for_unfreeze(MigrationState *s)
     }
 }

-static int migrate_fd_close(void *opaque)
+int migrate_fd_close(MigrationState *s)
 {
-    MigrationState *s = opaque;
-
     qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
     return s->close(s);
 }
@@ -431,9 +429,7 @@ void migrate_fd_connect(MigrationState *s)
     int ret;

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

     DPRINTF("beginning savevm\n");
     ret = qemu_savevm_state_begin(s->file, &s->params);
diff --git a/migration.h b/migration.h
index d6341d6..ec022d6 100644
--- a/migration.h
+++ b/migration.h
@@ -82,6 +82,7 @@ ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
                               size_t size);
 void migrate_fd_put_ready(MigrationState *s);
 void migrate_fd_wait_for_unfreeze(MigrationState *s);
+int migrate_fd_close(MigrationState *s);

 void add_migration_state_change_notifier(Notifier *notify);
 void remove_migration_state_change_notifier(Notifier *notify);
-- 
1.7.11.4

[Qemu-devel] [PATCH 23/41] buffered_file: We can access directly to bandwidth_limit

[Juan Quintela]

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 buffered_file.c | 5 ++---
 buffered_file.h | 3 +--
 migration.c     | 2 +-
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index 4fca774..43e68b6 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -222,15 +222,14 @@ static void buffered_rate_tick(void *opaque)
     buffered_put_buffer(s, NULL, 0, 0);
 }

-QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state,
-                                  size_t bytes_per_sec)
+QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state)
 {
     QEMUFileBuffered *s;

     s = g_malloc0(sizeof(*s));

     s->migration_state = migration_state;
-    s->xfer_limit = bytes_per_sec / 10;
+    s->xfer_limit = migration_state->bandwidth_limit / 10;

     s->file = qemu_fopen_ops(s, buffered_put_buffer, NULL,
                              buffered_close, buffered_rate_limit,
diff --git a/buffered_file.h b/buffered_file.h
index 8a38754..ef010fe 100644
--- a/buffered_file.h
+++ b/buffered_file.h
@@ -17,7 +17,6 @@
 #include "hw/hw.h"
 #include "migration.h"

-QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state,
-                                  size_t xfer_limit);
+QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state);

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

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

     DPRINTF("beginning savevm\n");
     ret = qemu_savevm_state_begin(s->file, &s->params);
-- 
1.7.11.4

[Qemu-devel] [PATCH 24/41] buffered_file: callers of buffered_flush() already check for errors

[Juan Quintela]

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 buffered_file.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index 43e68b6..747d672 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -61,13 +61,6 @@ static void buffered_append(QEMUFileBuffered *s,
 static void buffered_flush(QEMUFileBuffered *s)
 {
     size_t offset = 0;
-    int error;
-
-    error = qemu_file_get_error(s->file);
-    if (error != 0) {
-        DPRINTF("flush when error, bailing: %s\n", strerror(-error));
-        return;
-    }

     DPRINTF("flushing %zu byte(s) of data\n", s->buffer_size);

-- 
1.7.11.4

[Qemu-devel] [PATCH 25/41] buffered_file: make buffered_flush return the error code

[Juan Quintela]

Or the amount of data written if there is no error.  Adjust all callers.

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

diff --git a/buffered_file.c b/buffered_file.c
index 747d672..d3bc160 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -58,26 +58,26 @@ static void buffered_append(QEMUFileBuffered *s,
     s->buffer_size += size;
 }

-static void buffered_flush(QEMUFileBuffered *s)
+static ssize_t buffered_flush(QEMUFileBuffered *s)
 {
     size_t offset = 0;
+    ssize_t ret = 0;

     DPRINTF("flushing %zu byte(s) of data\n", s->buffer_size);

     while (s->bytes_xfer < s->xfer_limit && offset < s->buffer_size) {
-        ssize_t ret;

         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);
-            qemu_file_set_error(s->file, ret);
             break;
         } else {
             DPRINTF("flushed %zd byte(s)\n", ret);
@@ -89,12 +89,17 @@ static void buffered_flush(QEMUFileBuffered *s)
     DPRINTF("flushed %zu of %zu byte(s)\n", offset, s->buffer_size);
     memmove(s->buffer, s->buffer + offset, s->buffer_size - offset);
     s->buffer_size -= offset;
+
+    if (ret < 0) {
+        return ret;
+    }
+    return offset;
 }

 static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, int size)
 {
     QEMUFileBuffered *s = opaque;
-    int error;
+    ssize_t error;

     DPRINTF("putting %d bytes at %" PRId64 "\n", size, pos);

@@ -112,7 +117,13 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
         buffered_append(s, buf, size);
     }

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

     if (pos == 0 && size == 0) {
         DPRINTF("file is ready\n");
@@ -128,19 +139,25 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
 static int buffered_close(void *opaque)
 {
     QEMUFileBuffered *s = opaque;
-    int ret;
+    ssize_t ret = 0;
+    int ret2;

     DPRINTF("closing\n");

     s->xfer_limit = INT_MAX;
     while (!qemu_file_get_error(s->file) && s->buffer_size) {
-        buffered_flush(s);
+        ret = buffered_flush(s);
+        if (ret < 0) {
+            break;
+        }
         if (s->freeze_output)
             migrate_fd_wait_for_unfreeze(s->migration_state);
     }

-    ret = migrate_fd_close(s->migration_state);
-
+    ret2 = migrate_fd_close(s->migration_state);
+    if (ret >= 0) {
+        ret = ret2;
+    }
     qemu_del_timer(s->timer);
     qemu_free_timer(s->timer);
     g_free(s->buffer);
-- 
1.7.11.4

[Qemu-devel] [PATCH 26/41] migration: make migrate_fd_wait_for_unfreeze() return errors

[Juan Quintela]

Adjust all callers

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 buffered_file.c | 8 ++++++--
 migration.c     | 7 ++++---
 migration.h     | 2 +-
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index d3bc160..77928f0 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -150,8 +150,12 @@ static int buffered_close(void *opaque)
         if (ret < 0) {
             break;
         }
-        if (s->freeze_output)
-            migrate_fd_wait_for_unfreeze(s->migration_state);
+        if (s->freeze_output) {
+            ret = migrate_fd_wait_for_unfreeze(s->migration_state);
+            if (ret < 0) {
+                break;
+            }
+        }
     }

     ret2 = migrate_fd_close(s->migration_state);
diff --git a/migration.c b/migration.c
index ea21dc4..2c93951 100644
--- a/migration.c
+++ b/migration.c
@@ -370,13 +370,13 @@ static void migrate_fd_cancel(MigrationState *s)
     migrate_fd_cleanup(s);
 }

-void migrate_fd_wait_for_unfreeze(MigrationState *s)
+int migrate_fd_wait_for_unfreeze(MigrationState *s)
 {
     int ret;

     DPRINTF("wait for unfreeze\n");
     if (s->state != MIG_STATE_ACTIVE)
-        return;
+        return -EINVAL;

     do {
         fd_set wfds;
@@ -388,8 +388,9 @@ void migrate_fd_wait_for_unfreeze(MigrationState *s)
     } while (ret == -1 && (s->get_error(s)) == EINTR);

     if (ret == -1) {
-        qemu_file_set_error(s->file, -s->get_error(s));
+        return -s->get_error(s);
     }
+    return 0;
 }

 int migrate_fd_close(MigrationState *s)
diff --git a/migration.h b/migration.h
index ec022d6..1c3e9b7 100644
--- a/migration.h
+++ b/migration.h
@@ -81,7 +81,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);
-void migrate_fd_wait_for_unfreeze(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.4

[Qemu-devel] [PATCH 27/41] savevm: unexport qemu_fflush

[Juan Quintela]

It is not used outside of savevm.c

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-file.h | 1 -
 savevm.c    | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/qemu-file.h b/qemu-file.h
index 31b83f6..d8487cd 100644
--- a/qemu-file.h
+++ b/qemu-file.h
@@ -71,7 +71,6 @@ QEMUFile *qemu_fopen_socket(int fd);
 QEMUFile *qemu_popen(FILE *popen_file, const char *mode);
 QEMUFile *qemu_popen_cmd(const char *command, const char *mode);
 int qemu_stdio_fd(QEMUFile *f);
-void qemu_fflush(QEMUFile *f);
 int qemu_fclose(QEMUFile *f);
 void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size);
 void qemu_put_byte(QEMUFile *f, int v);
diff --git a/savevm.c b/savevm.c
index c7fe283..1ec6ff1 100644
--- a/savevm.c
+++ b/savevm.c
@@ -461,7 +461,7 @@ static void qemu_file_set_if_error(QEMUFile *f, int ret)
  *
  * In case of error, last_error is set.
  */
-void qemu_fflush(QEMUFile *f)
+static void qemu_fflush(QEMUFile *f)
 {
     if (!f->put_buffer)
         return;
-- 
1.7.11.4

[Qemu-devel] [PATCH 28/41] virtio-net: use qemu_get_buffer() in a temp buffer

[Juan Quintela]

qemu_fseek() is known to be wrong.  Would be removed on the next
commit.  This code should never been used (value has been
MAC_TABLE_ENTRIES since 2009).

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio-net.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 6490743..e8c43af 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -921,7 +921,9 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
             qemu_get_buffer(f, n->mac_table.macs,
                             n->mac_table.in_use * ETH_ALEN);
         } else if (n->mac_table.in_use) {
-            qemu_fseek(f, n->mac_table.in_use * ETH_ALEN, SEEK_CUR);
+            uint8_t *buf = g_malloc0(n->mac_table.in_use);
+            qemu_get_buffer(f, buf, n->mac_table.in_use * ETH_ALEN);
+            g_free(buf);
             n->mac_table.multi_overflow = n->mac_table.uni_overflow = 1;
             n->mac_table.in_use = 0;
         }
-- 
1.7.11.4

[Qemu-devel] [PATCH 29/41] savevm: Remove qemu_fseek()

[Juan Quintela]

It has no users, and is only half implemented.

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-file.h |  1 -
 savevm.c    | 21 ---------------------
 2 files changed, 22 deletions(-)

diff --git a/qemu-file.h b/qemu-file.h
index d8487cd..7fe7274 100644
--- a/qemu-file.h
+++ b/qemu-file.h
@@ -232,6 +232,5 @@ static inline void qemu_get_sbe64s(QEMUFile *f, int64_t *pv)
 }

 int64_t qemu_ftell(QEMUFile *f);
-int64_t qemu_fseek(QEMUFile *f, int64_t pos, int whence);

 #endif
diff --git a/savevm.c b/savevm.c
index 1ec6ff1..6865862 100644
--- a/savevm.c
+++ b/savevm.c
@@ -676,27 +676,6 @@ int64_t qemu_ftell(QEMUFile *f)
     return f->buf_offset - f->buf_size + f->buf_index;
 }

-int64_t qemu_fseek(QEMUFile *f, int64_t pos, int whence)
-{
-    if (whence == SEEK_SET) {
-        /* nothing to do */
-    } else if (whence == SEEK_CUR) {
-        pos += qemu_ftell(f);
-    } else {
-        /* SEEK_END not supported */
-        return -1;
-    }
-    if (f->put_buffer) {
-        qemu_fflush(f);
-        f->buf_offset = pos;
-    } else {
-        f->buf_offset = pos;
-        f->buf_index = 0;
-        f->buf_size = 0;
-    }
-    return pos;
-}
-
 int qemu_file_rate_limit(QEMUFile *f)
 {
     if (f->rate_limit)
-- 
1.7.11.4

[Qemu-devel] [PATCH 30/41] savevm: make qemu_fflush() return an error code

[Juan Quintela]

Adjust all the callers.  We moved the set of last_error from inside
qemu_fflush() to all the callers.

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 savevm.c | 39 +++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/savevm.c b/savevm.c
index 6865862..0953695 100644
--- a/savevm.c
+++ b/savevm.c
@@ -459,23 +459,22 @@ static void qemu_file_set_if_error(QEMUFile *f, int ret)

 /** Flushes QEMUFile buffer
  *
- * In case of error, last_error is set.
  */
-static void qemu_fflush(QEMUFile *f)
+static int qemu_fflush(QEMUFile *f)
 {
+    int ret = 0;
+
     if (!f->put_buffer)
-        return;
+        return 0;

     if (f->is_write && f->buf_index > 0) {
-        int len;
-
-        len = f->put_buffer(f->opaque, f->buf, f->buf_offset, f->buf_index);
-        if (len > 0)
+        ret = f->put_buffer(f->opaque, f->buf, f->buf_offset, f->buf_index);
+        if (ret >= 0) {
             f->buf_offset += f->buf_index;
-        else
-            qemu_file_set_error(f, -EINVAL);
+        }
         f->buf_index = 0;
     }
+    return ret;
 }

 static void qemu_fill_buffer(QEMUFile *f)
@@ -533,9 +532,13 @@ static int qemu_fclose_internal(QEMUFile *f)
  */
 int qemu_fclose(QEMUFile *f)
 {
-    int ret;
-    qemu_fflush(f);
-    ret = qemu_fclose_internal(f);
+    int ret, ret2;
+    ret = qemu_fflush(f);
+    ret2 = qemu_fclose_internal(f);
+
+    if (ret >= 0) {
+        ret = ret2;
+    }
     /* If any error was spotted before closing, we should report it
      * instead of the close() return value.
      */
@@ -570,8 +573,10 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
         f->buf_index += l;
         buf += l;
         size -= l;
-        if (f->buf_index >= IO_BUF_SIZE)
-            qemu_fflush(f);
+        if (f->buf_index >= IO_BUF_SIZE) {
+            int ret = qemu_fflush(f);
+            qemu_file_set_if_error(f, ret);
+        }
     }
 }

@@ -585,8 +590,10 @@ void qemu_put_byte(QEMUFile *f, int v)

     f->buf[f->buf_index++] = v;
     f->is_write = 1;
-    if (f->buf_index >= IO_BUF_SIZE)
-        qemu_fflush(f);
+    if (f->buf_index >= IO_BUF_SIZE) {
+        int ret = qemu_fflush(f);
+        qemu_file_set_if_error(f, ret);
+    }
 }

 static void qemu_file_skip(QEMUFile *f, int size)
-- 
1.7.11.4

[Qemu-devel] [PATCH 31/41] savevm: unfold qemu_fclose_internal()

[Juan Quintela]

It was used only one, and was only one if.  It makes error handling
saner.

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 savevm.c | 26 ++++++--------------------
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/savevm.c b/savevm.c
index 0953695..8efa7cc 100644
--- a/savevm.c
+++ b/savevm.c
@@ -506,22 +506,6 @@ static void qemu_fill_buffer(QEMUFile *f)
         qemu_file_set_error(f, len);
 }

-/** Calls close function and set last_error if needed
- *
- * Internal function. qemu_fflush() must be called before this.
- *
- * Returns f->close() return value, or 0 if close function is not set.
- */
-static int qemu_fclose_internal(QEMUFile *f)
-{
-    int ret = 0;
-    if (f->close) {
-        ret = f->close(f->opaque);
-        qemu_file_set_if_error(f, ret);
-    }
-    return ret;
-}
-
 /** Closes the file
  *
  * Returns negative error value if any error happened on previous operations or
@@ -532,12 +516,14 @@ static int qemu_fclose_internal(QEMUFile *f)
  */
 int qemu_fclose(QEMUFile *f)
 {
-    int ret, ret2;
+    int ret;
     ret = qemu_fflush(f);
-    ret2 = qemu_fclose_internal(f);

-    if (ret >= 0) {
-        ret = ret2;
+    if (f->close) {
+        int ret2 = f->close(f->opaque);
+        if (ret >= 0) {
+            ret = ret2;
+        }
     }
     /* If any error was spotted before closing, we should report it
      * instead of the close() return value.
-- 
1.7.11.4

[Qemu-devel] [PATCH 32/41] savevm: unexport qemu_ftell()

[Juan Quintela]

It was unused out of savevm.c.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 qemu-file.h | 3 ---
 savevm.c    | 2 +-
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/qemu-file.h b/qemu-file.h
index 7fe7274..289849a 100644
--- a/qemu-file.h
+++ b/qemu-file.h
@@ -230,7 +230,4 @@ static inline void qemu_get_sbe64s(QEMUFile *f, int64_t *pv)
 {
     qemu_get_be64s(f, (uint64_t *)pv);
 }
-
-int64_t qemu_ftell(QEMUFile *f);
-
 #endif
diff --git a/savevm.c b/savevm.c
index 8efa7cc..8ddb9d5 100644
--- a/savevm.c
+++ b/savevm.c
@@ -664,7 +664,7 @@ int qemu_get_byte(QEMUFile *f)
     return result;
 }

-int64_t qemu_ftell(QEMUFile *f)
+static int64_t qemu_ftell(QEMUFile *f)
 {
     return f->buf_offset - f->buf_size + f->buf_index;
 }
-- 
1.7.11.4

[Qemu-devel] [PATCH 33/41] savevm: make qemu_fill_buffer() be consistent

[Juan Quintela]

It was setting last_error directly once, and with the helper the other time.

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 savevm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/savevm.c b/savevm.c
index 8ddb9d5..4e4aa3c 100644
--- a/savevm.c
+++ b/savevm.c
@@ -501,7 +501,7 @@ static void qemu_fill_buffer(QEMUFile *f)
         f->buf_size += len;
         f->buf_offset += len;
     } else if (len == 0) {
-        f->last_error = -EIO;
+        qemu_file_set_error(f, -EIO);
     } else if (len != -EAGAIN)
         qemu_file_set_error(f, len);
 }
-- 
1.7.11.4

[Qemu-devel] [PATCH 34/41] savevm: Only qemu_fflush() can generate errors

[Juan Quintela]

Move the error check to the beggining of the callers.  Once this is fixed
qemu_file_set_if_error() is not used anymore, so remove it.

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 savevm.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/savevm.c b/savevm.c
index 4e4aa3c..59ec8bf 100644
--- a/savevm.c
+++ b/savevm.c
@@ -445,18 +445,6 @@ void qemu_file_set_error(QEMUFile *f, int ret)
     f->last_error = ret;
 }

-/** Sets last_error conditionally
- *
- * Sets last_error only if ret is negative _and_ no error
- * was set before.
- */
-static void qemu_file_set_if_error(QEMUFile *f, int ret)
-{
-    if (ret < 0 && !f->last_error) {
-        qemu_file_set_error(f, ret);
-    }
-}
-
 /** Flushes QEMUFile buffer
  *
  */
@@ -544,13 +532,17 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
 {
     int l;

-    if (!f->last_error && f->is_write == 0 && f->buf_index > 0) {
+    if (f->last_error) {
+        return;
+    }
+
+    if (f->is_write == 0 && f->buf_index > 0) {
         fprintf(stderr,
                 "Attempted to write to buffer while read buffer is not empty\n");
         abort();
     }

-    while (!f->last_error && size > 0) {
+    while (size > 0) {
         l = IO_BUF_SIZE - f->buf_index;
         if (l > size)
             l = size;
@@ -561,14 +553,21 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
         size -= l;
         if (f->buf_index >= IO_BUF_SIZE) {
             int ret = qemu_fflush(f);
-            qemu_file_set_if_error(f, ret);
+            if (ret < 0) {
+                qemu_file_set_error(f, ret);
+                break;
+            }
         }
     }
 }

 void qemu_put_byte(QEMUFile *f, int v)
 {
-    if (!f->last_error && f->is_write == 0 && f->buf_index > 0) {
+    if (f->last_error) {
+        return;
+    }
+
+    if (f->is_write == 0 && f->buf_index > 0) {
         fprintf(stderr,
                 "Attempted to write to buffer while read buffer is not empty\n");
         abort();
@@ -578,7 +577,9 @@ void qemu_put_byte(QEMUFile *f, int v)
     f->is_write = 1;
     if (f->buf_index >= IO_BUF_SIZE) {
         int ret = qemu_fflush(f);
-        qemu_file_set_if_error(f, ret);
+        if (ret < 0) {
+            qemu_file_set_error(f, ret);
+        }
     }
 }

-- 
1.7.11.4

[Qemu-devel] [PATCH 35/41] buffered_file: buffered_put_buffer() don't need to set last_error

[Juan Quintela]

Callers on savevm.c:qemu_fflush() will set it.

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 buffered_file.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index 77928f0..ed92df1 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -120,8 +120,6 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
     error = buffered_flush(s);
     if (error < 0) {
         DPRINTF("buffered flush error. bailing: %s\n", strerror(-error));
-        qemu_file_set_error(s->file, error);
-
         return error;
     }

-- 
1.7.11.4

[Qemu-devel] [PATCH 36/41] block-migration: make flush_blks() return errors

[Juan Quintela]

This means we don't need to pass through qemu_file to get the errors.
Adjust all callers.

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block-migration.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/block-migration.c b/block-migration.c
index 7def8ab..a822bb2 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -444,9 +444,10 @@ static int blk_mig_save_dirty_block(QEMUFile *f, int is_async)
     return ret;
 }

-static void flush_blks(QEMUFile* f)
+static int flush_blks(QEMUFile *f)
 {
     BlkMigBlock *blk;
+    int ret = 0;

     DPRINTF("%s Enter submitted %d read_done %d transferred %d\n",
             __FUNCTION__, block_mig_state.submitted, block_mig_state.read_done,
@@ -457,7 +458,7 @@ static void flush_blks(QEMUFile* f)
             break;
         }
         if (blk->ret < 0) {
-            qemu_file_set_error(f, blk->ret);
+            ret = blk->ret;
             break;
         }
         blk_send(f, blk);
@@ -474,6 +475,7 @@ static void flush_blks(QEMUFile* f)
     DPRINTF("%s Exit submitted %d read_done %d transferred %d\n", __FUNCTION__,
             block_mig_state.submitted, block_mig_state.read_done,
             block_mig_state.transferred);
+    return ret;
 }

 static int64_t get_remaining_dirty(void)
@@ -553,9 +555,7 @@ static int block_save_setup(QEMUFile *f, void *opaque)
     /* start track dirty blocks */
     set_dirty_tracking(1);

-    flush_blks(f);
-
-    ret = qemu_file_get_error(f);
+    ret = flush_blks(f);
     if (ret) {
         blk_mig_cleanup();
         return ret;
@@ -575,9 +575,7 @@ static int block_save_iterate(QEMUFile *f, void *opaque)
     DPRINTF("Enter save live iterate submitted %d transferred %d\n",
             block_mig_state.submitted, block_mig_state.transferred);

-    flush_blks(f);
-
-    ret = qemu_file_get_error(f);
+    ret = flush_blks(f);
     if (ret) {
         blk_mig_cleanup();
         return ret;
@@ -603,9 +601,7 @@ static int block_save_iterate(QEMUFile *f, void *opaque)
         }
     }

-    flush_blks(f);
-
-    ret = qemu_file_get_error(f);
+    ret = flush_blks(f);
     if (ret) {
         blk_mig_cleanup();
         return ret;
@@ -623,9 +619,7 @@ static int block_save_complete(QEMUFile *f, void *opaque)
     DPRINTF("Enter save live complete submitted %d transferred %d\n",
             block_mig_state.submitted, block_mig_state.transferred);

-    flush_blks(f);
-
-    ret = qemu_file_get_error(f);
+    ret = flush_blks(f);
     if (ret) {
         blk_mig_cleanup();
         return ret;
-- 
1.7.11.4

[Qemu-devel] [PATCH 37/41] block-migration: Switch meaning of return value

[Juan Quintela]

Make consistent the result of blk_mig_save_dirty_block() and
mig_save_device_dirty()

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block-migration.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/block-migration.c b/block-migration.c
index a822bb2..565628f 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -429,14 +429,18 @@ error:
     return 0;
 }

+/* return value:
+ * 0: too much data for max_downtime
+ * 1: few enough data for max_downtime
+*/
 static int blk_mig_save_dirty_block(QEMUFile *f, int is_async)
 {
     BlkMigDevState *bmds;
-    int ret = 0;
+    int ret = 1;

     QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) {
-        if (mig_save_device_dirty(f, bmds, is_async) == 0) {
-            ret = 1;
+        ret = mig_save_device_dirty(f, bmds, is_async);
+        if (ret == 0) {
             break;
         }
     }
@@ -594,7 +598,7 @@ static int block_save_iterate(QEMUFile *f, void *opaque)
                 block_mig_state.bulk_completed = 1;
             }
         } else {
-            if (blk_mig_save_dirty_block(f, 1) == 0) {
+            if (blk_mig_save_dirty_block(f, 1) != 0) {
                 /* no more dirty blocks */
                 break;
             }
@@ -631,7 +635,7 @@ static int block_save_complete(QEMUFile *f, void *opaque)
        all async read completed */
     assert(block_mig_state.submitted == 0);

-    while (blk_mig_save_dirty_block(f, 0) != 0) {
+    while (blk_mig_save_dirty_block(f, 0) == 0) {
         /* Do nothing */
     }
     blk_mig_cleanup();
-- 
1.7.11.4

[Qemu-devel] [PATCH 38/41] block-migration: handle errors with the return codes correctly

[Juan Quintela]

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block-migration.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/block-migration.c b/block-migration.c
index 565628f..8b585d3 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -423,10 +423,9 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds,

 error:
     DPRINTF("Error reading sector %" PRId64 "\n", sector);
-    qemu_file_set_error(f, ret);
     g_free(blk->buf);
     g_free(blk);
-    return 0;
+    return ret;
 }

 /* return value:
@@ -440,7 +439,7 @@ static int blk_mig_save_dirty_block(QEMUFile *f, int is_async)

     QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) {
         ret = mig_save_device_dirty(f, bmds, is_async);
-        if (ret == 0) {
+        if (ret <= 0) {
             break;
         }
     }
@@ -598,12 +597,17 @@ static int block_save_iterate(QEMUFile *f, void *opaque)
                 block_mig_state.bulk_completed = 1;
             }
         } else {
-            if (blk_mig_save_dirty_block(f, 1) != 0) {
+            ret = blk_mig_save_dirty_block(f, 1);
+            if (ret != 0) {
                 /* no more dirty blocks */
                 break;
             }
         }
     }
+    if (ret) {
+        blk_mig_cleanup();
+        return ret;
+    }

     ret = flush_blks(f);
     if (ret) {
@@ -635,18 +639,16 @@ static int block_save_complete(QEMUFile *f, void *opaque)
        all async read completed */
     assert(block_mig_state.submitted == 0);

-    while (blk_mig_save_dirty_block(f, 0) == 0) {
-        /* Do nothing */
-    }
-    blk_mig_cleanup();
-
-    /* report completion */
-    qemu_put_be64(f, (100 << BDRV_SECTOR_BITS) | BLK_MIG_FLAG_PROGRESS);
+    do {
+        ret = blk_mig_save_dirty_block(f, 0);
+    } while (ret == 0);

-    ret = qemu_file_get_error(f);
+    blk_mig_cleanup();
     if (ret) {
         return ret;
     }
+    /* report completion */
+    qemu_put_be64(f, (100 << BDRV_SECTOR_BITS) | BLK_MIG_FLAG_PROGRESS);

     DPRINTF("Block migration completed\n");

-- 
1.7.11.4

[Qemu-devel] [PATCH 39/41] savevm: un-export qemu_file_set_error()

[Juan Quintela]

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-file.h | 1 -
 savevm.c    | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/qemu-file.h b/qemu-file.h
index 289849a..8dd9207 100644
--- a/qemu-file.h
+++ b/qemu-file.h
@@ -103,7 +103,6 @@ int qemu_file_rate_limit(QEMUFile *f);
 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);
-void qemu_file_set_error(QEMUFile *f, int error);

 /* 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
diff --git a/savevm.c b/savevm.c
index 59ec8bf..68c0464 100644
--- a/savevm.c
+++ b/savevm.c
@@ -440,7 +440,7 @@ int qemu_file_get_error(QEMUFile *f)
     return f->last_error;
 }

-void qemu_file_set_error(QEMUFile *f, int ret)
+static void qemu_file_set_error(QEMUFile *f, int ret)
 {
     f->last_error = ret;
 }
-- 
1.7.11.4

[Qemu-devel] [PATCH 40/41] savevm: make qemu_file_put_notify() return errors

[Juan Quintela]

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 migration.c | 5 +++--
 qemu-file.h | 2 +-
 savevm.c    | 4 ++--
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/migration.c b/migration.c
index 2c93951..62e0304 100644
--- a/migration.c
+++ b/migration.c
@@ -287,10 +287,11 @@ static void migrate_fd_completed(MigrationState *s)
 static void migrate_fd_put_notify(void *opaque)
 {
     MigrationState *s = opaque;
+    int ret;

     qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
-    qemu_file_put_notify(s->file);
-    if (s->file && qemu_file_get_error(s->file)) {
+    ret = qemu_file_put_notify(s->file);
+    if (ret) {
         migrate_fd_error(s);
     }
 }
diff --git a/qemu-file.h b/qemu-file.h
index 8dd9207..9c8985b 100644
--- a/qemu-file.h
+++ b/qemu-file.h
@@ -107,7 +107,7 @@ 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. */
-void qemu_file_put_notify(QEMUFile *f);
+int qemu_file_put_notify(QEMUFile *f);

 static inline void qemu_put_be64s(QEMUFile *f, const uint64_t *pv)
 {
diff --git a/savevm.c b/savevm.c
index 68c0464..2ea1fa6 100644
--- a/savevm.c
+++ b/savevm.c
@@ -523,9 +523,9 @@ int qemu_fclose(QEMUFile *f)
     return ret;
 }

-void qemu_file_put_notify(QEMUFile *f)
+int qemu_file_put_notify(QEMUFile *f)
 {
-    f->put_buffer(f->opaque, NULL, 0, 0);
+    return f->put_buffer(f->opaque, NULL, 0, 0);
 }

 void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
-- 
1.7.11.4

[Qemu-devel] [PATCH 41/41] cpus: create qemu_in_vcpu_thread()

[Juan Quintela]

Old code used !io_thread to know if a thread was an vcpu or not.  That
fails when we introduce the iothread.

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

diff --git a/cpus.c b/cpus.c
index 4b726ef..76c4255 100644
--- a/cpus.c
+++ b/cpus.c
@@ -902,6 +902,11 @@ int qemu_cpu_is_self(void *_env)
     return qemu_thread_is_self(cpu->thread);
 }

+static bool qemu_in_vcpu_thread(void)
+{
+    return cpu_single_env && qemu_cpu_is_self(cpu_single_env);
+}
+
 void qemu_mutex_lock_iothread(void)
 {
     if (!tcg_enabled()) {
@@ -947,7 +952,7 @@ void pause_all_vcpus(void)
         penv = penv->next_cpu;
     }

-    if (!qemu_thread_is_self(&io_thread)) {
+    if (qemu_in_vcpu_thread()) {
         cpu_stop_current();
         if (!kvm_enabled()) {
             while (penv) {
@@ -1064,7 +1069,7 @@ void cpu_stop_current(void)

 void vm_stop(RunState state)
 {
-    if (!qemu_thread_is_self(&io_thread)) {
+    if (qemu_in_vcpu_thread()) {
         qemu_system_vmstop_request(state);
         /*
          * FIXME: should not return to device code in case
-- 
1.7.11.4

Re: [Qemu-devel] [PATCH 02/41] fix migration sync

[Paolo Bonzini]

Il 02/10/2012 13:32, Juan Quintela ha scritto:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  arch_init.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 9904f95..671d354 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -517,6 +517,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>      }
> 
>      memory_global_dirty_log_start();
> +    memory_global_sync_dirty_bitmap(get_system_memory());
> 
>      qemu_put_be64(f, ram_bytes_total() | RAM_SAVE_FLAG_MEM_SIZE);
> 

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

Re: [Qemu-devel] [PATCH 15/41] migration: Add dirty_pages_rate to query migrate output

[Eric Blake]

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

On 10/02/2012 05:33 AM, Juan Quintela wrote:
> It indicates how many pages were dirtied during the last second.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---

> +# @dirty-pages-rate: number of pages dirtied by second by the

Reads slightly better as:

pages dirtied per second by the guest

> +#        guest (since 1.3)
>  #
>  # Since: 0.14.0
>  ##
>  { 'type': 'MigrationStats',
>    'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
> -           'duplicate': 'int', 'normal': 'int', 'normal-bytes': 'int' } }
> +           'duplicate': 'int', 'normal': 'int', 'normal-bytes': 'int',
> +           'dirty-pages-rate' : 'int' } }

Consistent formatting:
s/ :/:/

-- 
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: 617 bytes --]

Re: [Qemu-devel] [PATCH 04/41] migration: print total downtime for final phase of migration

[Eric Blake]

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

On 10/02/2012 05:32 AM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Orit Wasserman <owasserm@redhat.com>
> ---

> +++ b/qmp-commands.hx
> @@ -2267,6 +2267,8 @@ The main json-object contains the following:
>  - "total-time": total amount of ms since migration started.  If
>                  migration has ended, it returns the total migration
>  		 time (json-int)
> +- "downtime": only present when migration has finished correctly
> +                total amount in ms for downtime that happened (json-int)

I think the convention is to indent the second line to the same column
as the first character after the colon.  But unless it affects the
appearance of any actual generated documentation, I don't think it's
worth a respin for two fewer spaces.

-- 
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: 617 bytes --]

Re: [Qemu-devel] [PATCH 11/41] ram: Export last_ram_offset()

[Anthony Liguori]

Juan Quintela <quintela@redhat.com> writes:

> Is the only way of knowing the RAM size.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  cpu-all.h | 2 ++
>  exec.c    | 2 +-
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/cpu-all.h b/cpu-all.h
> index 74d3681..5408782 100644
> --- a/cpu-all.h
> +++ b/cpu-all.h
> @@ -517,6 +517,8 @@ extern int mem_prealloc;
>  void dump_exec_info(FILE *f, fprintf_function cpu_fprintf);
>  #endif /* !CONFIG_USER_ONLY */
>
> +ram_addr_t last_ram_offset(void);
> +
>  int cpu_memory_rw_debug(CPUArchState *env, target_ulong addr,
>                          uint8_t *buf, int len, int is_write);

cpu-all.h is included by linux-user and ram_addr_t isn't available when
!CONFIG_USER_ONLY.

This breaks the build which also implies this series has only been
tested with a restricted target list.

Please make sure you don't submit pull requests without *at least* doing
a full build and 'make check'.

Regards,

Anthony Liguori

>
> diff --git a/exec.c b/exec.c
> index bb6aa4a..e613329 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2462,7 +2462,7 @@ static ram_addr_t find_ram_offset(ram_addr_t size)
>      return offset;
>  }
>
> -static ram_addr_t last_ram_offset(void)
> +ram_addr_t last_ram_offset(void)
>  {
>      RAMBlock *block;
>      ram_addr_t last = 0;
> -- 
> 1.7.11.4