[Qemu-devel] [RFC 0/7] Migration stats

[Qemu-devel] [RFC 0/7] Migration stats

[Juan Quintela]

Hi

This modifies the output of info migrate/qmp_query_migrate to add the
stats that I got request for.

- It moves total time to MigrationInfo instead of ram (luiz suggestion)
- Prints the real downtime that we have had

  really, it prints the total downtime of the complete phase, but the
  downtime also includes the last ram_iterate phase.  Working on
  fixing that one.

- Prints the expected downtime of the last time that we synchronized
  the dirty bitmap with kvm.  So we have one idea of what downtime
  value we need for migration to converge.

- Prints the dirty_pages_rate, that is the number of pages that we
  have written in the last second.  This one prints always zero.  To
  fill it, I need the dirty bitmap changes on the migration_thread
  series.

Patch series apply on top of the migration-next-20120808 series sent
to anthony.

What do I want to know:

- is there any stat that you want?  Once here, adding a new one should
  be easy.

- examples are not done, waiting until people agree with what params
  are needed.

- luiz added in case he has QMP commets.

- erik added for libvirt comments.

Added before is the link to the branch on my repository.

The following changes since commit 346fe0c4c0b88f11a3d0c01c34d9a170d73429cc:

  Merge remote-tracking branch 'stefanha/trivial-patches' into staging (2012-08-11 19:49:03 -0500)

are available in the git repository at:


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

for you to fetch changes up to e0599012abfc4f9a68185c6f0a10a7b98c0a180f:

  migration: Add dirty_pages_rate to query migrate output (2012-08-13 12:33:35 +0200)

Please review, and comment.

Juan Quintela (7):
  migration: move total_time from ram stats to migration info
  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 migration_get_current()
  migration: print expected downtime in info migrate
  migration: Add dirty_pages_rate to query migrate output

 arch_init.c      | 19 +++++++++++--------
 hmp.c            | 16 ++++++++++++++--
 migration.c      | 19 ++++++++++++++-----
 migration.h      |  4 ++++
 qapi-schema.json | 26 +++++++++++++++++++-------
 5 files changed, 62 insertions(+), 22 deletions(-)

-- 
1.7.11.2

[Qemu-devel] [PATCH 1/7] migration: move total_time from ram stats to migration info

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hmp.c            |  4 ++--
 migration.c      |  6 +++---
 qapi-schema.json | 14 +++++++-------
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/hmp.c b/hmp.c
index c13386b..c0b0a10 100644
--- a/hmp.c
+++ b/hmp.c
@@ -149,6 +149,8 @@ void hmp_info_migrate(Monitor *mon)

     if (info->has_status) {
         monitor_printf(mon, "Migration status: %s\n", info->status);
+        monitor_printf(mon, "total time: %" PRIu64 " milliseconds\n",
+                       info->total_time);
     }

     if (info->has_ram) {
@@ -158,8 +160,6 @@ void hmp_info_migrate(Monitor *mon)
                        info->ram->remaining >> 10);
         monitor_printf(mon, "total ram: %" PRIu64 " kbytes\n",
                        info->ram->total >> 10);
-        monitor_printf(mon, "total time: %" PRIu64 " milliseconds\n",
-                       info->ram->total_time);
         monitor_printf(mon, "duplicate: %" PRIu64 " pages\n",
                        info->ram->duplicate);
         monitor_printf(mon, "normal: %" PRIu64 " pages\n",
diff --git a/migration.c b/migration.c
index 653a3c1..8e4c508 100644
--- a/migration.c
+++ b/migration.c
@@ -166,14 +166,14 @@ MigrationInfo *qmp_query_migrate(Error **errp)
     case MIG_STATE_ACTIVE:
         info->has_status = true;
         info->status = g_strdup("active");
+        info->total_time = qemu_get_clock_ms(rt_clock)
+            - s->total_time;

         info->has_ram = true;
         info->ram = g_malloc0(sizeof(*info->ram));
         info->ram->transferred = ram_bytes_transferred();
         info->ram->remaining = ram_bytes_remaining();
         info->ram->total = ram_bytes_total();
-        info->ram->total_time = qemu_get_clock_ms(rt_clock)
-            - s->total_time;
         info->ram->duplicate = dup_mig_pages_transferred();
         info->ram->normal = norm_mig_pages_transferred();
         info->ram->normal_bytes = norm_mig_bytes_transferred();
@@ -193,13 +193,13 @@ MigrationInfo *qmp_query_migrate(Error **errp)

         info->has_status = true;
         info->status = g_strdup("completed");
+        info->total_time = s->total_time;

         info->has_ram = true;
         info->ram = g_malloc0(sizeof(*info->ram));
         info->ram->transferred = ram_bytes_transferred();
         info->ram->remaining = 0;
         info->ram->total = ram_bytes_total();
-        info->ram->total_time = s->total_time;
         info->ram->duplicate = dup_mig_pages_transferred();
         info->ram->normal = norm_mig_pages_transferred();
         info->ram->normal_bytes = norm_mig_bytes_transferred();
diff --git a/qapi-schema.json b/qapi-schema.json
index 56d9d7b..907051a 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -260,10 +260,6 @@
 #
 # @total: total amount of bytes involved in the migration process
 #
-# @total-time: total amount of ms since migration started.  If
-#        migration has ended, it returns the total migration
-#        time. (since 1.2)
-#
 # @duplicate: number of duplicate pages (since 1.2)
 #
 # @normal : number of normal pages (since 1.2)
@@ -274,8 +270,7 @@
 ##
 { 'type': 'MigrationStats',
   'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
-           'total-time': 'int', 'duplicate': 'int', 'normal': 'int',
-           'normal-bytes': 'int' } }
+           'duplicate': 'int', 'normal': 'int', 'normal-bytes': 'int' } }

 ##
 # @XBZRLECacheStats
@@ -320,12 +315,17 @@
 #                migration statistics, only returned if XBZRLE feature is on and
 #                status is 'active' or 'completed' (since 1.2)
 #
+# @total-time: total amount of milliseconds since migration started.
+#        If migration has ended, it returns the total migration
+#        time. (since 1.2)
+#
 # Since: 0.14.0
 ##
 { 'type': 'MigrationInfo',
   'data': {'*status': 'str', '*ram': 'MigrationStats',
            '*disk': 'MigrationStats',
-           '*xbzrle-cache': 'XBZRLECacheStats'} }
+           '*xbzrle-cache': 'XBZRLECacheStats',
+           'total-time': 'int'} }

 ##
 # @query-migrate
-- 
1.7.11.2

Re: [Qemu-devel] [PATCH 1/7] migration: move total_time from ram stats to migration info

[Eric Blake]

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

On 08/13/2012 04:50 AM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  hmp.c            |  4 ++--
>  migration.c      |  6 +++---
>  qapi-schema.json | 14 +++++++-------
>  3 files changed, 12 insertions(+), 12 deletions(-)
> 

Looks reasonable to me!

Missed qmp-commands.hx for the examples (but your cover letter mentioned
that).

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

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


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

[Qemu-devel] [PATCH 2/7] migration: store end_time in a local variable

[Juan Quintela]

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

diff --git a/migration.c b/migration.c
index 8e4c508..159728d 100644
--- a/migration.c
+++ b/migration.c
@@ -326,6 +326,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);
@@ -336,7 +337,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.2

Re: [Qemu-devel] [PATCH 2/7] migration: store end_time in a local variable

[Eric Blake]

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

On 08/13/2012 04:50 AM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  migration.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

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

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


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

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

[Juan Quintela]

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

diff --git a/hmp.c b/hmp.c
index c0b0a10..10fee1b 100644
--- a/hmp.c
+++ b/hmp.c
@@ -151,6 +151,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 (strcmp(info->status, "completed") == 0) {
+            monitor_printf(mon, "downtime: %" PRIu64 " milliseconds\n",
+                           info->downtime);
+        }
     }

     if (info->has_ram) {
diff --git a/migration.c b/migration.c
index 159728d..4c2ac6c 100644
--- a/migration.c
+++ b/migration.c
@@ -194,6 +194,7 @@ MigrationInfo *qmp_query_migrate(Error **errp)
         info->has_status = true;
         info->status = g_strdup("completed");
         info->total_time = s->total_time;
+        info->downtime = s->downtime;

         info->has_ram = true;
         info->ram = g_malloc0(sizeof(*info->ram));
@@ -326,9 +327,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);

@@ -339,6 +341,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 907051a..6df1ce3 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -319,13 +319,17 @@
 #        If migration has ended, it returns the total migration
 #        time. (since 1.2)
 #
+# @downtime: total downtime in milliseconds for the guest.
+#        (since 1.2)
+#
 # 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
-- 
1.7.11.2

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

[Eric Blake]

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

On 08/13/2012 04:50 AM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  hmp.c            | 4 ++++
>  migration.c      | 5 ++++-
>  migration.h      | 1 +
>  qapi-schema.json | 6 +++++-
>  4 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index c0b0a10..10fee1b 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -151,6 +151,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 (strcmp(info->status, "completed") == 0) {
> +            monitor_printf(mon, "downtime: %" PRIu64 " milliseconds\n",
> +                           info->downtime);
> +        }

Very nice stat - useful at showing how 'live' a live migration really is.

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

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

[Eric Blake]

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

On 08/13/2012 09:02 AM, Eric Blake wrote:
> On 08/13/2012 04:50 AM, Juan Quintela wrote:
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  hmp.c            | 4 ++++
>>  migration.c      | 5 ++++-
>>  migration.h      | 1 +
>>  qapi-schema.json | 6 +++++-
>>  4 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/hmp.c b/hmp.c
>> index c0b0a10..10fee1b 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -151,6 +151,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 (strcmp(info->status, "completed") == 0) {
>> +            monitor_printf(mon, "downtime: %" PRIu64 " milliseconds\n",
>> +                           info->downtime);
>> +        }
> 
> Very nice stat - useful at showing how 'live' a live migration really is.
> 

Hit send too soon:

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

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


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

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

[Luiz Capitulino]

On Mon, 13 Aug 2012 12:50:53 +0200
Juan Quintela <quintela@redhat.com> wrote:

> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  hmp.c            | 4 ++++
>  migration.c      | 5 ++++-
>  migration.h      | 1 +
>  qapi-schema.json | 6 +++++-
>  4 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index c0b0a10..10fee1b 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -151,6 +151,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 (strcmp(info->status, "completed") == 0) {
> +            monitor_printf(mon, "downtime: %" PRIu64 " milliseconds\n",
> +                           info->downtime);
> +        }
>      }
> 
>      if (info->has_ram) {
> diff --git a/migration.c b/migration.c
> index 159728d..4c2ac6c 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -194,6 +194,7 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>          info->has_status = true;
>          info->status = g_strdup("completed");
>          info->total_time = s->total_time;
> +        info->downtime = s->downtime;
> 
>          info->has_ram = true;
>          info->ram = g_malloc0(sizeof(*info->ram));
> @@ -326,9 +327,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);
> 
> @@ -339,6 +341,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 907051a..6df1ce3 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -319,13 +319,17 @@
>  #        If migration has ended, it returns the total migration
>  #        time. (since 1.2)
>  #
> +# @downtime: total downtime in milliseconds for the guest.
> +#        (since 1.2)

Should be marked #optional and add "only available if status is completed".

Btw, the field status should be an enum, it was a mistake to make it a string :(
But I don't think this can be changed now.

> +#
>  # 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

[Qemu-devel] [PATCH 4/7] migration: rename expected_time to expected_downtime

[Juan Quintela]

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

diff --git a/arch_init.c b/arch_init.c
index 9b46bfc..5559d68 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -537,7 +537,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);
@@ -576,24 +576,25 @@ 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 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_time, migrate_max_downtime());
+            expected_downtime, 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.2

Re: [Qemu-devel] [PATCH 4/7] migration: rename expected_time to expected_downtime

[Eric Blake]

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

On 08/13/2012 04:50 AM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  arch_init.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)

Trivial rename.

> @@ -576,24 +576,25 @@ 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 a very high value, but without

s/a a/a/

Latent typo, but the reformatting exposes it more.  With that fixed,

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

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


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

[Qemu-devel] [PATCH 5/7] migration: export migration_get_current()

[Juan Quintela]

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

diff --git a/migration.c b/migration.c
index 4c2ac6c..12de941 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.2

Re: [Qemu-devel] [PATCH 5/7] migration: export migration_get_current()

[Eric Blake]

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

On 08/13/2012 04:50 AM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  migration.c | 2 +-
>  migration.h | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)

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

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


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

[Qemu-devel] [PATCH 6/7] 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 | 4 ++++
 5 files changed, 13 insertions(+)

diff --git a/arch_init.c b/arch_init.c
index 5559d68..b150852 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -538,6 +538,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 10fee1b..fc75ec3 100644
--- a/hmp.c
+++ b/hmp.c
@@ -151,6 +151,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->expected_downtime) {
+            monitor_printf(mon, "expected downtime: %" PRIu64 " milliseconds\n",
+                           info->expected_downtime);
+        }
         if (strcmp(info->status, "completed") == 0) {
             monitor_printf(mon, "downtime: %" PRIu64 " milliseconds\n",
                            info->downtime);
diff --git a/migration.c b/migration.c
index 12de941..28e23db 100644
--- a/migration.c
+++ b/migration.c
@@ -168,6 +168,7 @@ MigrationInfo *qmp_query_migrate(Error **errp)
         info->status = g_strdup("active");
         info->total_time = qemu_get_clock_ms(rt_clock)
             - s->total_time;
+        info->expected_downtime = s->expected_downtime;

         info->has_ram = true;
         info->ram = g_malloc0(sizeof(*info->ram));
@@ -195,6 +196,7 @@ MigrationInfo *qmp_query_migrate(Error **errp)
         info->status = g_strdup("completed");
         info->total_time = s->total_time;
         info->downtime = s->downtime;
+        info->expected_downtime = 0;

         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 6df1ce3..3dcc12f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -322,6 +322,9 @@
 # @downtime: total downtime in milliseconds for the guest.
 #        (since 1.2)
 #
+# @expected-downtime: expected downtime in milliseconds for the
+#        guest in last walk of the dirty bitmap. (sincte 1.2)
+#
 # Since: 0.14.0
 ##
 { 'type': 'MigrationInfo',
@@ -329,6 +332,7 @@
            '*disk': 'MigrationStats',
            '*xbzrle-cache': 'XBZRLECacheStats',
            'total-time': 'int',
+           'expected-downtime': 'int',
            'downtime': 'int'} }

 ##
-- 
1.7.11.2

Re: [Qemu-devel] [PATCH 6/7] migration: print expected downtime in info migrate

[Eric Blake]

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

On 08/13/2012 04:50 AM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  arch_init.c      | 2 ++
>  hmp.c            | 4 ++++
>  migration.c      | 2 ++
>  migration.h      | 1 +
>  qapi-schema.json | 4 ++++
>  5 files changed, 13 insertions(+)
> 

> +++ b/qapi-schema.json
> @@ -322,6 +322,9 @@
>  # @downtime: total downtime in milliseconds for the guest.
>  #        (since 1.2)
>  #
> +# @expected-downtime: expected downtime in milliseconds for the
> +#        guest in last walk of the dirty bitmap. (sincte 1.2)

s/sincte/since/

Should this field be marked optional?  That is, it makes sense to expose
the field while migration is ongoing, but when reporting the stats after
migration is complete, there is no longer any expected downtime.

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

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

[Juan Quintela]

For now this is a placeholder, real info will appear once the bitmap
changes in the migration thread series is integrated.

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

diff --git a/hmp.c b/hmp.c
index fc75ec3..dd40631 100644
--- a/hmp.c
+++ b/hmp.c
@@ -174,6 +174,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 " pagfes\n",
+                           info->ram->dirty_pages_rate);
+        }
     }

     if (info->has_disk) {
diff --git a/migration.c b/migration.c
index 28e23db..8d67e9b 100644
--- a/migration.c
+++ b/migration.c
@@ -178,6 +178,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 3dcc12f..55ef73c 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -266,11 +266,15 @@
 #
 # @normal-bytes : number of normal bytes sent (since 1.2)
 #
+# @dirty-pages-rate: number of pages dirtied by second by the
+#        guest. (since 1.2)
+#
 # 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.2

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

[Eric Blake]

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

On 08/13/2012 04:50 AM, Juan Quintela wrote:
> For now this is a placeholder, real info will appear once the bitmap
> changes in the migration thread series is integrated.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  hmp.c            | 4 ++++
>  migration.c      | 2 ++
>  migration.h      | 1 +
>  qapi-schema.json | 6 +++++-
>  4 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/hmp.c b/hmp.c
> index fc75ec3..dd40631 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -174,6 +174,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 " pagfes\n",

s/pagfes/pages/

> +++ b/qapi-schema.json
> @@ -266,11 +266,15 @@
>  #
>  # @normal-bytes : number of normal bytes sent (since 1.2)

As long as you are touching here: s/ :/:/

>  #
> +# @dirty-pages-rate: number of pages dirtied by second by the
> +#        guest. (since 1.2)

Inconsistent on whether we have a '.' prior to the (since 1.2) marking.

Since HMP only prints this stat when it is non-zero, should this field
be marked optional?  Then again, once you have dirty page tracking, I
suspect this would never be zero (or even showing an explicit zero would
help detect stalls).

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

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

[Juan Quintela]

Eric Blake <eblake@redhat.com> wrote:
> On 08/13/2012 04:50 AM, Juan Quintela wrote:
>> For now this is a placeholder, real info will appear once the bitmap
>> changes in the migration thread series is integrated.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  hmp.c            | 4 ++++
>>  migration.c      | 2 ++
>>  migration.h      | 1 +
>>  qapi-schema.json | 6 +++++-
>>  4 files changed, 12 insertions(+), 1 deletion(-)
>> 
>>  #
>> +# @dirty-pages-rate: number of pages dirtied by second by the
>> +#        guest. (since 1.2)
>
> Inconsistent on whether we have a '.' prior to the (since 1.2) marking.
>
> Since HMP only prints this stat when it is non-zero, should this field
> be marked optional?  Then again, once you have dirty page tracking, I
> suspect this would never be zero (or even showing an explicit zero would
> help detect stalls).

What is easier for libvirt to have here.

At the beggining, this value is going to be wrong/cero. I can:
- not print it until it is != 0
- print a big enough number )
- make it optional?  I guess this would alse be more difficult for
  libvirt.
- put your good idea here?

Later, Juan.

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

[Eric Blake]

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

On 08/13/2012 10:04 AM, Juan Quintela wrote:

>>> +# @dirty-pages-rate: number of pages dirtied by second by the
>>> +#        guest. (since 1.2)
>>

>> Since HMP only prints this stat when it is non-zero, should this field
>> be marked optional?  Then again, once you have dirty page tracking, I
>> suspect this would never be zero (or even showing an explicit zero would
>> help detect stalls).
> 
> What is easier for libvirt to have here.
> 
> At the beggining, this value is going to be wrong/cero. I can:
> - not print it until it is != 0
> - print a big enough number )
> - make it optional?  I guess this would alse be more difficult for
>   libvirt.
> - put your good idea here?

I see your point about querying early enough that there is not yet
enough data to have a good estimate.  In that case, either outputting 0
or omitting the field will do the trick.  Libvirt already has to support
older qemu that always omitted the field, and it is a one-liner code
addition in libvirt to say that if the field is omitted, treat it like
'0'.  I'm not even quite sure what libvirt will be able to do with this
number; libvirt made some unfortunate API decisions where migration
statistics are exposed to the user in a hard-coded struct
(virDomainJobInfo), and since we didn't allow for the struct to grow
without breaking ABI, exposing additional information would require a
new libvirt API.  So don't let libvirt hold up the inclusion of this
useful information on the qemu side of things.

Maybe another option would be to return [U]INT_MAX when there is not
enough data to yet provide a valid number, where having a non-zero value
at least lets people know that the stat will be available in a later call.

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

Re: [Qemu-devel] [RFC 0/7] Migration stats

[Eric Blake]

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

On 08/13/2012 04:50 AM, Juan Quintela wrote:
> Hi
> 
> This modifies the output of info migrate/qmp_query_migrate to add the
> stats that I got request for.
> 
> - It moves total time to MigrationInfo instead of ram (luiz suggestion)

Now's the time to do this, since the stat is new to 1.2 and we haven't
yet made a release with it.

Should we also rename 'MigrationInfo' to 'MigrationRamInfo', so that we
don't make a similar mistake in the future of putting a stat in the
wrong category?

> What do I want to know:
> 
> - is there any stat that you want?  Once here, adding a new one should
>   be easy.

Should we have some sort of stat for the number of pages that are sent
more than once, and/or for the maximum count of times that a given page
was sent?  Having hot/cold page analysis might make it easier to decide
in the future which pages to avoid sending until the very end, and
knowing how many pages are sent multiple times as well as the maximum
times any one page is sent might help.

> 
> - examples are not done, waiting until people agree with what params
>   are needed.

Fair enough for RFC purposes.

> 
> - luiz added in case he has QMP commets.
> 
> - erik added for libvirt comments.

Eric, actually.

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

Re: [Qemu-devel] [RFC 0/7] Migration stats

[Juan Quintela]

Eric Blake <eblake@redhat.com> wrote:
> On 08/13/2012 04:50 AM, Juan Quintela wrote:
>> Hi
>> 
>> This modifies the output of info migrate/qmp_query_migrate to add the
>> stats that I got request for.
>> 
>> - It moves total time to MigrationInfo instead of ram (luiz suggestion)
>
> Now's the time to do this, since the stat is new to 1.2 and we haven't
> yet made a release with it.
>
> Should we also rename 'MigrationInfo' to 'MigrationRamInfo', so that we
> don't make a similar mistake in the future of putting a stat in the
> wrong category?

Luiz?  I dont' care one way or the other O:-)

>> What do I want to know:
>> 
>> - is there any stat that you want?  Once here, adding a new one should
>>   be easy.
>
> Should we have some sort of stat for the number of pages that are sent
> more than once, and/or for the maximum count of times that a given page
> was sent?  Having hot/cold page analysis might make it easier to decide
> in the future which pages to avoid sending until the very end, and
> knowing how many pages are sent multiple times as well as the maximum
> times any one page is sent might help.

That is not trivial to do without "duplicating" the bitmap. Bitmap is
already quite big on machines with huge memory.  Adding a field to show
how many times a page have been sent sounds too much.  Furthermore, how
could this information could be used externally.

>> 
>> - examples are not done, waiting until people agree with what params
>>   are needed.
>
> Fair enough for RFC purposes.
>
>> 
>> - luiz added in case he has QMP commets.
>> 
>> - erik added for libvirt comments.
>
> Eric, actually.

Sorry :p

Later, Juan.

Re: [Qemu-devel] [RFC 0/7] Migration stats

[Eric Blake]

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

On 08/13/2012 09:08 AM, Juan Quintela wrote:
>>
>> Should we have some sort of stat for the number of pages that are sent
>> more than once, and/or for the maximum count of times that a given page
>> was sent?  Having hot/cold page analysis might make it easier to decide
>> in the future which pages to avoid sending until the very end, and
>> knowing how many pages are sent multiple times as well as the maximum
>> times any one page is sent might help.
> 
> That is not trivial to do without "duplicating" the bitmap. Bitmap is
> already quite big on machines with huge memory.  Adding a field to show
> how many times a page have been sent sounds too much.  Furthermore, how
> could this information could be used externally.

No big loss, then.  I was just wondering out loud, and don't really have
a particular need for this information.

I guess with XBZRLE, you _do_ have a count of how many cache hits you
had, which is somewhat related (a cache hit implies sending a page more
than once).

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

Re: [Qemu-devel] [RFC 0/7] Migration stats

[Luiz Capitulino]

On Mon, 13 Aug 2012 08:59:00 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 08/13/2012 04:50 AM, Juan Quintela wrote:
> > Hi
> > 
> > This modifies the output of info migrate/qmp_query_migrate to add the
> > stats that I got request for.
> > 
> > - It moves total time to MigrationInfo instead of ram (luiz suggestion)
> 
> Now's the time to do this, since the stat is new to 1.2 and we haven't
> yet made a release with it.
> 
> Should we also rename 'MigrationInfo' to 'MigrationRamInfo', so that we
> don't make a similar mistake in the future of putting a stat in the
> wrong category?

It also has disk migration info there.

Re: [Qemu-devel] [RFC 0/7] Migration stats

[Qunfang Zhang]

Hi, Juan
I have a brief test with these patches applied and it's very useful.  
It's more precise and time-saving than calculate it with some other 
method for the downtime,etc.

Thank you,
Qunfang

On 08/13/2012 06:50 PM, Juan Quintela wrote:
> Hi
>
> This modifies the output of info migrate/qmp_query_migrate to add the
> stats that I got request for.
>
> - It moves total time to MigrationInfo instead of ram (luiz suggestion)
> - Prints the real downtime that we have had
>
>    really, it prints the total downtime of the complete phase, but the
>    downtime also includes the last ram_iterate phase.  Working on
>    fixing that one.
>
> - Prints the expected downtime of the last time that we synchronized
>    the dirty bitmap with kvm.  So we have one idea of what downtime
>    value we need for migration to converge.
>
> - Prints the dirty_pages_rate, that is the number of pages that we
>    have written in the last second.  This one prints always zero.  To
>    fill it, I need the dirty bitmap changes on the migration_thread
>    series.
>
> Patch series apply on top of the migration-next-20120808 series sent
> to anthony.
>
> What do I want to know:
>
> - is there any stat that you want?  Once here, adding a new one should
>    be easy.
>
> - examples are not done, waiting until people agree with what params
>    are needed.
>
> - luiz added in case he has QMP commets.
>
> - erik added for libvirt comments.
>
> Added before is the link to the branch on my repository.
>
> The following changes since commit 346fe0c4c0b88f11a3d0c01c34d9a170d73429cc:
>
>    Merge remote-tracking branch 'stefanha/trivial-patches' into staging (2012-08-11 19:49:03 -0500)
>
> are available in the git repository at:
>
>
>    http://repo.or.cz/r/qemu/quintela.git migration-stats
>
> for you to fetch changes up to e0599012abfc4f9a68185c6f0a10a7b98c0a180f:
>
>    migration: Add dirty_pages_rate to query migrate output (2012-08-13 12:33:35 +0200)
>
> Please review, and comment.
>
> Juan Quintela (7):
>    migration: move total_time from ram stats to migration info
>    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 migration_get_current()
>    migration: print expected downtime in info migrate
>    migration: Add dirty_pages_rate to query migrate output
>
>   arch_init.c      | 19 +++++++++++--------
>   hmp.c            | 16 ++++++++++++++--
>   migration.c      | 19 ++++++++++++++-----
>   migration.h      |  4 ++++
>   qapi-schema.json | 26 +++++++++++++++++++-------
>   5 files changed, 62 insertions(+), 22 deletions(-)
>
>