diff mbox

[09/11] Add migration accounting for normal and duplicate pages

Message ID 1343227834-5400-11-git-send-email-owasserm@redhat.com
State New
Headers show

Commit Message

Orit Wasserman July 25, 2012, 2:50 p.m. UTC
Signed-off-by: Benoit Hudzia <benoit.hudzia@sap.com>
Signed-off-by: Petter Svard <petters@cs.umu.se>
Signed-off-by: Aidan Shribman <aidan.shribman@sap.com>
Signed-off-by: Orit Wasserman <owasserm@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 arch_init.c      |   38 ++++++++++++++++++++++++++++++++++++++
 migration.c      |    4 ++++
 migration.h      |    5 +++++
 qapi-schema.json |    8 ++++++--
 qmp-commands.hx  |    2 ++
 5 files changed, 55 insertions(+), 2 deletions(-)

Comments

Eric Blake July 26, 2012, 10:41 p.m. UTC | #1
On 07/25/2012 08:50 AM, Orit Wasserman wrote:
> Signed-off-by: Benoit Hudzia <benoit.hudzia@sap.com>
> Signed-off-by: Petter Svard <petters@cs.umu.se>
> Signed-off-by: Aidan Shribman <aidan.shribman@sap.com>
> Signed-off-by: Orit Wasserman <owasserm@redhat.com>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> +++ b/qapi-schema.json
> @@ -264,11 +264,15 @@
>  #        migration has ended, it returns the total migration
>  #        time. (since 1.2)
>  #
> -# Since: 0.14.0.
> +# @duplicate: #optional, number of duplicate pages (since 1.2)

I think I was the one that originally asked whether #optional was
appropriate for back-compat reasons when adding to a struct, but Luis
has since corrected me - #optional only makes sense for a return member
that will not appear in all uses of the struct in the current version.
But the number of duplicates is always available (even if it is 0), so
it should not be optional.  That is, this line should be:

# @duplicate: number of duplicate pages (since 1.2)

> +#
> +# @normal : #optional, number of normal pages (since 1.2)

Likewise.

> +#
> +# Since: 0.14.0
>  ##
>  { 'type': 'MigrationStats',
>    'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
> -           'total_time': 'int' } }
> +           'total_time': 'int', '*duplicate': 'int', '*normal': 'int' } }

and this should be 'duplicate' and 'normal'.

> +++ b/qmp-commands.hx
> @@ -2099,6 +2099,8 @@ The main json-object contains the following:
>           - "transferred": amount transferred (json-int)
>           - "remaining": amount remaining (json-int)
>           - "total": total (json-int)
> +	 - "duplicate": number of duplicated pages (json-int)
> +	 - "normal" : number of normal pages transferred (json-int)
>  - "disk": only present if "status" is "active" and it is a block migration,
>    it is a json-object with the following disk information (in bytes):
>           - "transferred": amount transferred (json-int)
> 

Incomplete if we decide that 'duplicate' and 'normal' are not optional;
we should be updating example 4 and 5 to list the new fields.
Orit Wasserman July 29, 2012, 6:57 a.m. UTC | #2
On 07/27/2012 01:41 AM, Eric Blake wrote:
> On 07/25/2012 08:50 AM, Orit Wasserman wrote:
>> Signed-off-by: Benoit Hudzia <benoit.hudzia@sap.com>
>> Signed-off-by: Petter Svard <petters@cs.umu.se>
>> Signed-off-by: Aidan Shribman <aidan.shribman@sap.com>
>> Signed-off-by: Orit Wasserman <owasserm@redhat.com>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>> +++ b/qapi-schema.json
>> @@ -264,11 +264,15 @@
>>  #        migration has ended, it returns the total migration
>>  #        time. (since 1.2)
>>  #
>> -# Since: 0.14.0.
>> +# @duplicate: #optional, number of duplicate pages (since 1.2)
> 
> I think I was the one that originally asked whether #optional was
> appropriate for back-compat reasons when adding to a struct, but Luis
> has since corrected me - #optional only makes sense for a return member
> that will not appear in all uses of the struct in the current version.
> But the number of duplicates is always available (even if it is 0), so
> it should not be optional.  That is, this line should be:
> 
> # @duplicate: number of duplicate pages (since 1.2)
> 
>> +#
>> +# @normal : #optional, number of normal pages (since 1.2)
> 
> Likewise.
> 
>> +#
>> +# Since: 0.14.0
>>  ##
>>  { 'type': 'MigrationStats',
>>    'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
>> -           'total_time': 'int' } }
>> +           'total_time': 'int', '*duplicate': 'int', '*normal': 'int' } }
> 
> and this should be 'duplicate' and 'normal'.
> 
>> +++ b/qmp-commands.hx
>> @@ -2099,6 +2099,8 @@ The main json-object contains the following:
>>           - "transferred": amount transferred (json-int)
>>           - "remaining": amount remaining (json-int)
>>           - "total": total (json-int)
>> +	 - "duplicate": number of duplicated pages (json-int)
>> +	 - "normal" : number of normal pages transferred (json-int)
>>  - "disk": only present if "status" is "active" and it is a block migration,
>>    it is a json-object with the following disk information (in bytes):
>>           - "transferred": amount transferred (json-int)
>>
> 
> Incomplete if we decide that 'duplicate' and 'normal' are not optional;
> we should be updating example 4 and 5 to list the new fields.
> 
I will fix it,
Orit
diff mbox

Patch

diff --git a/arch_init.c b/arch_init.c
index cdf13ef..e50cdf2 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -205,6 +205,40 @@  int64_t xbzrle_cache_resize(int64_t new_size)
     return pow2floor(new_size);
 }
 
+/* accounting for migration statistics */
+typedef struct AccountingInfo {
+    uint64_t dup_pages;
+    uint64_t norm_pages;
+    uint64_t iterations;
+} AccountingInfo;
+
+static AccountingInfo acct_info;
+
+static void acct_clear(void)
+{
+    memset(&acct_info, 0, sizeof(acct_info));
+}
+
+uint64_t dup_mig_bytes_transferred(void)
+{
+    return acct_info.dup_pages * TARGET_PAGE_SIZE;
+}
+
+uint64_t dup_mig_pages_transferred(void)
+{
+    return acct_info.dup_pages;
+}
+
+uint64_t norm_mig_bytes_transferred(void)
+{
+    return acct_info.norm_pages * TARGET_PAGE_SIZE;
+}
+
+uint64_t norm_mig_pages_transferred(void)
+{
+    return acct_info.norm_pages;
+}
+
 static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
         int cont, int flag)
 {
@@ -307,6 +341,7 @@  static int ram_save_block(QEMUFile *f)
             p = memory_region_get_ram_ptr(mr) + offset;
 
             if (is_dup_page(p)) {
+                acct_info.dup_pages++;
                 save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_COMPRESS);
                 qemu_put_byte(f, *p);
                 bytes_sent = 1;
@@ -322,6 +357,7 @@  static int ram_save_block(QEMUFile *f)
                 save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_PAGE);
                 qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
                 bytes_sent = TARGET_PAGE_SIZE;
+                acct_info.norm_pages++;
             }
 
             /* if page is unmodified, continue to the next */
@@ -443,6 +479,7 @@  static int ram_save_setup(QEMUFile *f, void *opaque)
         }
         XBZRLE.encoded_buf = g_malloc0(TARGET_PAGE_SIZE);
         XBZRLE.current_buf = g_malloc(TARGET_PAGE_SIZE);
+        acct_clear();
     }
 
     QLIST_FOREACH(block, &ram_list.blocks, next) {
@@ -490,6 +527,7 @@  static int ram_save_iterate(QEMUFile *f, void *opaque)
             break;
         }
         bytes_transferred += bytes_sent;
+        acct_info.iterations++;
         /* we want to check in the 1st loop, just in case it was the 1st time
            and we had to sync the dirty bitmap.
            qemu_get_clock_ns() is a bit expensive, so we only check each some
diff --git a/migration.c b/migration.c
index bc2231d..4dc99ba 100644
--- a/migration.c
+++ b/migration.c
@@ -156,6 +156,8 @@  MigrationInfo *qmp_query_migrate(Error **errp)
         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();
 
         if (blk_mig_active()) {
             info->has_disk = true;
@@ -175,6 +177,8 @@  MigrationInfo *qmp_query_migrate(Error **errp)
         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();
         break;
     case MIG_STATE_ERROR:
         info->has_status = true;
diff --git a/migration.h b/migration.h
index 337e225..e4a7cd7 100644
--- a/migration.h
+++ b/migration.h
@@ -87,6 +87,11 @@  uint64_t ram_bytes_total(void);
 
 extern SaveVMHandlers savevm_ram_handlers;
 
+uint64_t dup_mig_bytes_transferred(void);
+uint64_t dup_mig_pages_transferred(void);
+uint64_t norm_mig_bytes_transferred(void);
+uint64_t norm_mig_pages_transferred(void);
+
 /**
  * @migrate_add_blocker - prevent migration from proceeding
  *
diff --git a/qapi-schema.json b/qapi-schema.json
index 04adcee..88b7b56 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -264,11 +264,15 @@ 
 #        migration has ended, it returns the total migration
 #        time. (since 1.2)
 #
-# Since: 0.14.0.
+# @duplicate: #optional, number of duplicate pages (since 1.2)
+#
+# @normal : #optional, number of normal pages (since 1.2)
+#
+# Since: 0.14.0
 ##
 { 'type': 'MigrationStats',
   'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
-           'total_time': 'int' } }
+           'total_time': 'int', '*duplicate': 'int', '*normal': 'int' } }
 
 ##
 # @MigrationInfo
diff --git a/qmp-commands.hx b/qmp-commands.hx
index cfc950e..d0e8878 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2099,6 +2099,8 @@  The main json-object contains the following:
          - "transferred": amount transferred (json-int)
          - "remaining": amount remaining (json-int)
          - "total": total (json-int)
+	 - "duplicate": number of duplicated pages (json-int)
+	 - "normal" : number of normal pages transferred (json-int)
 - "disk": only present if "status" is "active" and it is a block migration,
   it is a json-object with the following disk information (in bytes):
          - "transferred": amount transferred (json-int)