Re: [Qemu-devel] [RFC PATCH v2 07/12] mc: introduce additional QMP statistics for micro-checkpointing

2014-04-03 Thread Michael R. Hines 

On 03/12/2014 05:45 AM, Eric Blake wrote:

+++ b/qapi-schema.json
@@ -603,6 +603,36 @@
 'cache-miss': 'int', 'overflow': 'int' } }
  
  ##

+# @MCStats
+#
+# Detailed Micro Checkpointing (MC) statistics
+#
+# @mbps: throughput of transmitting last MC
+#
+# @xmit-time: milliseconds to transmit last MC

Trailing whitespace.

Rather than abbreviate, how about naming this 'transmit-time'.


Acknowledged.


+#
+# @checkpoints: cummulative total number of MCs generated

More trailing whitespace.  Please run your series through
scripts/checkpatch.pl.

s/cummulative total/cumulative/


Acknowledged.



+#
+# Since: 2.x
+##
+{ 'type': 'MCStats',
+  'data': {'mbps': 'number',
+   'xmit-time': 'uint64',
+   'log-dirty-time': 'uint64',
+   'migration-bitmap-time': 'uint64',
+   'ram-copy-time': 'uint64',
+   'checkpoints' : 'uint64',
+   'copy-mbps': 'number' }}

Again, it helps to document the fields in the same order as they are
declared (no, it's not a hard requirement, but being nice to readers is
always worth the effort).


Acknowledged.


+
+##
  # @MigrationInfo
  #
  # Information about current migration process.
@@ -624,6 +654,8 @@
  #migration statistics, only returned if XBZRLE feature is on 
and
  #status is 'active' or 'completed' (since 1.2)
  #
+# @mc: #options @MCStats containing details Micro-Checkpointing statistics

s/options/optional/ - I'm assuming it is optional because it only
appears when MC is in use.

'mc' is a rather short name, maybe 'micro-checkpoint' is better?


Funny. I thought 'micro-checkpoint' was too long, particularly
in the ./configure output and the QEMU Monitor 'help' command output =)


Missing a '(since 2.1)' designation (or 2.x, as you used above as a
placeholder, although obviously we'd fix the .x before actually bringing
into mainline)



Acknowledged.

- Michael

Re: [Qemu-devel] [RFC PATCH v2 07/12] mc: introduce additional QMP statistics for micro-checkpointing

2014-04-03 Thread Michael R. Hines 

On 03/12/2014 05:59 AM, Juan Quintela wrote:

mrhi...@linux.vnet.ibm.com wrote:

From: Michael R. Hines mrhi...@us.ibm.com

MC provides a lot of new information, including the same RAM statistics
that ordinary migration does, so we centralize a lot of that printing
code into a common function so that the QMP printing statements don't
get duplicated too much.

We also introduce a new MCStats structure (like MigrationStats) due
to the large number of non-migration related statistics - don't want
to confuse migration and MC too much, so let's keep them separate for now.

Signed-off-by: Michael R. Hines mrhi...@us.ibm.com

We can add the non-mc stats if you split them.  And you get a smaller
series.


Well, the MIG_STATE_COMPLETED and the MIG_STATE_ACTIVE cleanup
is removing a ton of duplicated code anyway - that needed to be cleaned
up regardless.

But, for the MC-related statistics, this really is a completely new state
in the migration state machine with so many new statistics - I don't think
they belong in the MigrationInfo structure at all..

- Michael


Later, Juan.



---
  hmp.c | 17 +
  include/migration/migration.h |  6 +++
  migration.c   | 86 ++-
  qapi-schema.json  | 33 +
  4 files changed, 109 insertions(+), 33 deletions(-)

diff --git a/hmp.c b/hmp.c
index 1af0809..edf062e 100644
--- a/hmp.c
+++ b/hmp.c
@@ -203,6 +203,23 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
 info-disk-total  10);
  }
  
+if (info-has_mc) {

+monitor_printf(mon, checkpoints: % PRIu64 \n,
+   info-mc-checkpoints);
+monitor_printf(mon, xmit_time: % PRIu64  ms\n,
+   info-mc-xmit_time);
+monitor_printf(mon, log_dirty_time: % PRIu64  ms\n,
+   info-mc-log_dirty_time);
+monitor_printf(mon, migration_bitmap_time: % PRIu64  ms\n,
+   info-mc-migration_bitmap_time);
+monitor_printf(mon, ram_copy_time: % PRIu64  ms\n,
+   info-mc-ram_copy_time);
+monitor_printf(mon, copy_mbps: %0.2f mbps\n,
+   info-mc-copy_mbps);
+monitor_printf(mon, throughput: %0.2f mbps\n,
+   info-mc-mbps);
+}
+
  if (info-has_xbzrle_cache) {
  monitor_printf(mon, cache size: % PRIu64  bytes\n,
 info-xbzrle_cache-cache_size);
diff --git a/include/migration/migration.h b/include/migration/migration.h
index e876a2c..f18ff5e 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -53,14 +53,20 @@ struct MigrationState
  int state;
  MigrationParams params;
  double mbps;
+double copy_mbps;
  int64_t total_time;
  int64_t downtime;
  int64_t expected_downtime;
+int64_t xmit_time;
+int64_t ram_copy_time;
+int64_t log_dirty_time;
+int64_t bitmap_time;
  int64_t dirty_pages_rate;
  int64_t dirty_bytes_rate;
  bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
  int64_t xbzrle_cache_size;
  int64_t setup_time;
+int64_t checkpoints;
  };
  
  void process_incoming_migration(QEMUFile *f);

diff --git a/migration.c b/migration.c
index f42dae4..0ccbeaa 100644
--- a/migration.c
+++ b/migration.c
@@ -59,7 +59,6 @@ MigrationState *migrate_get_current(void)
  .state = MIG_STATE_NONE,
  .bandwidth_limit = MAX_THROTTLE,
  .xbzrle_cache_size = DEFAULT_MIGRATE_CACHE_SIZE,
-.mbps = -1,
  };
  
  return current_migration;

@@ -173,6 +172,31 @@ static void get_xbzrle_cache_stats(MigrationInfo *info)
  }
  }
  
+static void get_ram_stats(MigrationState *s, MigrationInfo *info)

+{
+info-has_total_time = true;
+info-total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME)
+- s-total_time;
+
+info-has_ram = true;
+info-ram = g_malloc0(sizeof(*info-ram));
+info-ram-transferred = ram_bytes_transferred();
+info-ram-total = ram_bytes_total();
+info-ram-duplicate = dup_mig_pages_transferred();
+info-ram-skipped = skipped_mig_pages_transferred();
+info-ram-normal = norm_mig_pages_transferred();
+info-ram-normal_bytes = norm_mig_bytes_transferred();
+info-ram-mbps = s-mbps;
+
+if (blk_mig_active()) {
+info-has_disk = true;
+info-disk = g_malloc0(sizeof(*info-disk));
+info-disk-transferred = blk_mig_bytes_transferred();
+info-disk-remaining = blk_mig_bytes_remaining();
+info-disk-total = blk_mig_bytes_total();
+}
+}
+
  MigrationInfo *qmp_query_migrate(Error **errp)
  {
  MigrationInfo *info = g_malloc0(sizeof(*info));
@@ -199,26 +223,8 @@ MigrationInfo *qmp_query_migrate(Error **errp)
  info-has_setup_time = true;
  info-setup_time = s-setup_time;
  
-info-has_ram = true;

-info-ram = g_malloc0(sizeof(*info-ram));
-

Re: [Qemu-devel] [RFC PATCH v2 07/12] mc: introduce additional QMP statistics for micro-checkpointing

2014-04-03 Thread Eric Blake 
On 04/03/2014 09:15 PM, Michael R. Hines wrote:

   #
 +# @mc: #options @MCStats containing details Micro-Checkpointing
 statistics
 s/options/optional/ - I'm assuming it is optional because it only
 appears when MC is in use.

 'mc' is a rather short name, maybe 'micro-checkpoint' is better?
 
 Funny. I thought 'micro-checkpoint' was too long, particularly
 in the ./configure output and the QEMU Monitor 'help' command output =)

For ./configure output, short is fine.  For QMP, it's a programmatic
interface, and management programs like libvirt have no problem
outputting longer names.  Where the longer names are nice is when
reading logs generated by a management program, as we tried hard to make
the QMP wire format still seem more or less legible to a human, even
though humans aren't the primary driver of the interface.  At the end of
the day, though, the name doesn't matter, so it's not worth arguing what
color to paint the bikeshed - you as author have some sway on what name
you want :)

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RFC PATCH v2 07/12] mc: introduce additional QMP statistics for micro-checkpointing

2014-03-11 Thread Eric Blake 
On 02/18/2014 01:50 AM, mrhi...@linux.vnet.ibm.com wrote:
 From: Michael R. Hines mrhi...@us.ibm.com
 
 MC provides a lot of new information, including the same RAM statistics
 that ordinary migration does, so we centralize a lot of that printing
 code into a common function so that the QMP printing statements don't
 get duplicated too much.
 
 We also introduce a new MCStats structure (like MigrationStats) due
 to the large number of non-migration related statistics - don't want
 to confuse migration and MC too much, so let's keep them separate for now.
 
 Signed-off-by: Michael R. Hines mrhi...@us.ibm.com
 ---

 +++ b/qapi-schema.json
 @@ -603,6 +603,36 @@
 'cache-miss': 'int', 'overflow': 'int' } }
  
  ##
 +# @MCStats
 +#
 +# Detailed Micro Checkpointing (MC) statistics
 +#
 +# @mbps: throughput of transmitting last MC 
 +#
 +# @xmit-time: milliseconds to transmit last MC 

Trailing whitespace.

Rather than abbreviate, how about naming this 'transmit-time'.

 +#
 +# @checkpoints: cummulative total number of MCs generated 

More trailing whitespace.  Please run your series through
scripts/checkpatch.pl.

s/cummulative total/cumulative/

 +#
 +# Since: 2.x
 +##
 +{ 'type': 'MCStats',
 +  'data': {'mbps': 'number',
 +   'xmit-time': 'uint64',
 +   'log-dirty-time': 'uint64',
 +   'migration-bitmap-time': 'uint64', 
 +   'ram-copy-time': 'uint64',
 +   'checkpoints' : 'uint64',
 +   'copy-mbps': 'number' }}

Again, it helps to document the fields in the same order as they are
declared (no, it's not a hard requirement, but being nice to readers is
always worth the effort).

 +
 +##
  # @MigrationInfo
  #
  # Information about current migration process.
 @@ -624,6 +654,8 @@
  #migration statistics, only returned if XBZRLE feature is on 
 and
  #status is 'active' or 'completed' (since 1.2)
  #
 +# @mc: #options @MCStats containing details Micro-Checkpointing statistics

s/options/optional/ - I'm assuming it is optional because it only
appears when MC is in use.

'mc' is a rather short name, maybe 'micro-checkpoint' is better?

Missing a '(since 2.1)' designation (or 2.x, as you used above as a
placeholder, although obviously we'd fix the .x before actually bringing
into mainline)

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RFC PATCH v2 07/12] mc: introduce additional QMP statistics for micro-checkpointing

2014-03-11 Thread Juan Quintela 
mrhi...@linux.vnet.ibm.com wrote:
 From: Michael R. Hines mrhi...@us.ibm.com

 MC provides a lot of new information, including the same RAM statistics
 that ordinary migration does, so we centralize a lot of that printing
 code into a common function so that the QMP printing statements don't
 get duplicated too much.

 We also introduce a new MCStats structure (like MigrationStats) due
 to the large number of non-migration related statistics - don't want
 to confuse migration and MC too much, so let's keep them separate for now.

 Signed-off-by: Michael R. Hines mrhi...@us.ibm.com

We can add the non-mc stats if you split them.  And you get a smaller
series.

Later, Juan.


 ---
  hmp.c | 17 +
  include/migration/migration.h |  6 +++
  migration.c   | 86 
 ++-
  qapi-schema.json  | 33 +
  4 files changed, 109 insertions(+), 33 deletions(-)

 diff --git a/hmp.c b/hmp.c
 index 1af0809..edf062e 100644
 --- a/hmp.c
 +++ b/hmp.c
 @@ -203,6 +203,23 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
 info-disk-total  10);
  }
  
 +if (info-has_mc) {
 +monitor_printf(mon, checkpoints: % PRIu64 \n,
 +   info-mc-checkpoints);
 +monitor_printf(mon, xmit_time: % PRIu64  ms\n,
 +   info-mc-xmit_time);
 +monitor_printf(mon, log_dirty_time: % PRIu64  ms\n,
 +   info-mc-log_dirty_time);
 +monitor_printf(mon, migration_bitmap_time: % PRIu64  ms\n,
 +   info-mc-migration_bitmap_time);
 +monitor_printf(mon, ram_copy_time: % PRIu64  ms\n,
 +   info-mc-ram_copy_time);
 +monitor_printf(mon, copy_mbps: %0.2f mbps\n,
 +   info-mc-copy_mbps);
 +monitor_printf(mon, throughput: %0.2f mbps\n,
 +   info-mc-mbps);
 +}
 +
  if (info-has_xbzrle_cache) {
  monitor_printf(mon, cache size: % PRIu64  bytes\n,
 info-xbzrle_cache-cache_size);
 diff --git a/include/migration/migration.h b/include/migration/migration.h
 index e876a2c..f18ff5e 100644
 --- a/include/migration/migration.h
 +++ b/include/migration/migration.h
 @@ -53,14 +53,20 @@ struct MigrationState
  int state;
  MigrationParams params;
  double mbps;
 +double copy_mbps;
  int64_t total_time;
  int64_t downtime;
  int64_t expected_downtime;
 +int64_t xmit_time;
 +int64_t ram_copy_time;
 +int64_t log_dirty_time;
 +int64_t bitmap_time;
  int64_t dirty_pages_rate;
  int64_t dirty_bytes_rate;
  bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
  int64_t xbzrle_cache_size;
  int64_t setup_time;
 +int64_t checkpoints;
  };
  
  void process_incoming_migration(QEMUFile *f);
 diff --git a/migration.c b/migration.c
 index f42dae4..0ccbeaa 100644
 --- a/migration.c
 +++ b/migration.c
 @@ -59,7 +59,6 @@ MigrationState *migrate_get_current(void)
  .state = MIG_STATE_NONE,
  .bandwidth_limit = MAX_THROTTLE,
  .xbzrle_cache_size = DEFAULT_MIGRATE_CACHE_SIZE,
 -.mbps = -1,
  };
  
  return current_migration;
 @@ -173,6 +172,31 @@ static void get_xbzrle_cache_stats(MigrationInfo *info)
  }
  }
  
 +static void get_ram_stats(MigrationState *s, MigrationInfo *info)
 +{
 +info-has_total_time = true;
 +info-total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME)
 +- s-total_time;
 +
 +info-has_ram = true;
 +info-ram = g_malloc0(sizeof(*info-ram));
 +info-ram-transferred = ram_bytes_transferred();
 +info-ram-total = ram_bytes_total();
 +info-ram-duplicate = dup_mig_pages_transferred();
 +info-ram-skipped = skipped_mig_pages_transferred();
 +info-ram-normal = norm_mig_pages_transferred();
 +info-ram-normal_bytes = norm_mig_bytes_transferred();
 +info-ram-mbps = s-mbps;
 +
 +if (blk_mig_active()) {
 +info-has_disk = true;
 +info-disk = g_malloc0(sizeof(*info-disk));
 +info-disk-transferred = blk_mig_bytes_transferred();
 +info-disk-remaining = blk_mig_bytes_remaining();
 +info-disk-total = blk_mig_bytes_total();
 +}
 +}
 +
  MigrationInfo *qmp_query_migrate(Error **errp)
  {
  MigrationInfo *info = g_malloc0(sizeof(*info));
 @@ -199,26 +223,8 @@ MigrationInfo *qmp_query_migrate(Error **errp)
  info-has_setup_time = true;
  info-setup_time = s-setup_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-duplicate = dup_mig_pages_transferred();
 -info-ram-skipped = skipped_mig_pages_transferred();
 -info-ram-normal = norm_mig_pages_transferred();
 -

[Qemu-devel] [RFC PATCH v2 07/12] mc: introduce additional QMP statistics for micro-checkpointing

2014-02-18 Thread mrhines 
From: Michael R. Hines mrhi...@us.ibm.com

MC provides a lot of new information, including the same RAM statistics
that ordinary migration does, so we centralize a lot of that printing
code into a common function so that the QMP printing statements don't
get duplicated too much.

We also introduce a new MCStats structure (like MigrationStats) due
to the large number of non-migration related statistics - don't want
to confuse migration and MC too much, so let's keep them separate for now.

Signed-off-by: Michael R. Hines mrhi...@us.ibm.com
---
 hmp.c | 17 +
 include/migration/migration.h |  6 +++
 migration.c   | 86 ++-
 qapi-schema.json  | 33 +
 4 files changed, 109 insertions(+), 33 deletions(-)

diff --git a/hmp.c b/hmp.c
index 1af0809..edf062e 100644
--- a/hmp.c
+++ b/hmp.c
@@ -203,6 +203,23 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
info-disk-total  10);
 }
 
+if (info-has_mc) {
+monitor_printf(mon, checkpoints: % PRIu64 \n,
+   info-mc-checkpoints);
+monitor_printf(mon, xmit_time: % PRIu64  ms\n,
+   info-mc-xmit_time);
+monitor_printf(mon, log_dirty_time: % PRIu64  ms\n,
+   info-mc-log_dirty_time);
+monitor_printf(mon, migration_bitmap_time: % PRIu64  ms\n,
+   info-mc-migration_bitmap_time);
+monitor_printf(mon, ram_copy_time: % PRIu64  ms\n,
+   info-mc-ram_copy_time);
+monitor_printf(mon, copy_mbps: %0.2f mbps\n,
+   info-mc-copy_mbps);
+monitor_printf(mon, throughput: %0.2f mbps\n,
+   info-mc-mbps);
+}
+
 if (info-has_xbzrle_cache) {
 monitor_printf(mon, cache size: % PRIu64  bytes\n,
info-xbzrle_cache-cache_size);
diff --git a/include/migration/migration.h b/include/migration/migration.h
index e876a2c..f18ff5e 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -53,14 +53,20 @@ struct MigrationState
 int state;
 MigrationParams params;
 double mbps;
+double copy_mbps;
 int64_t total_time;
 int64_t downtime;
 int64_t expected_downtime;
+int64_t xmit_time;
+int64_t ram_copy_time;
+int64_t log_dirty_time;
+int64_t bitmap_time;
 int64_t dirty_pages_rate;
 int64_t dirty_bytes_rate;
 bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
 int64_t xbzrle_cache_size;
 int64_t setup_time;
+int64_t checkpoints;
 };
 
 void process_incoming_migration(QEMUFile *f);
diff --git a/migration.c b/migration.c
index f42dae4..0ccbeaa 100644
--- a/migration.c
+++ b/migration.c
@@ -59,7 +59,6 @@ MigrationState *migrate_get_current(void)
 .state = MIG_STATE_NONE,
 .bandwidth_limit = MAX_THROTTLE,
 .xbzrle_cache_size = DEFAULT_MIGRATE_CACHE_SIZE,
-.mbps = -1,
 };
 
 return current_migration;
@@ -173,6 +172,31 @@ static void get_xbzrle_cache_stats(MigrationInfo *info)
 }
 }
 
+static void get_ram_stats(MigrationState *s, MigrationInfo *info)
+{
+info-has_total_time = true;
+info-total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME)
+- s-total_time;
+
+info-has_ram = true;
+info-ram = g_malloc0(sizeof(*info-ram));
+info-ram-transferred = ram_bytes_transferred();
+info-ram-total = ram_bytes_total();
+info-ram-duplicate = dup_mig_pages_transferred();
+info-ram-skipped = skipped_mig_pages_transferred();
+info-ram-normal = norm_mig_pages_transferred();
+info-ram-normal_bytes = norm_mig_bytes_transferred();
+info-ram-mbps = s-mbps;
+
+if (blk_mig_active()) {
+info-has_disk = true;
+info-disk = g_malloc0(sizeof(*info-disk));
+info-disk-transferred = blk_mig_bytes_transferred();
+info-disk-remaining = blk_mig_bytes_remaining();
+info-disk-total = blk_mig_bytes_total();
+}
+}
+
 MigrationInfo *qmp_query_migrate(Error **errp)
 {
 MigrationInfo *info = g_malloc0(sizeof(*info));
@@ -199,26 +223,8 @@ MigrationInfo *qmp_query_migrate(Error **errp)
 info-has_setup_time = true;
 info-setup_time = s-setup_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-duplicate = dup_mig_pages_transferred();
-info-ram-skipped = skipped_mig_pages_transferred();
-info-ram-normal = norm_mig_pages_transferred();
-info-ram-normal_bytes = norm_mig_bytes_transferred();
+get_ram_stats(s, info);
 info-ram-dirty_pages_rate = s-dirty_pages_rate;
-info-ram-mbps = s-mbps;
-
-if (blk_mig_active()) {
-info-has_disk = true;
-