diff mbox

[10/11] Add XBZRLE statistics

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

Commit Message

Orit Wasserman July 29, 2012, 9:43 a.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      |   28 ++++++++++++++++++++++++++++
 hmp.c            |   21 +++++++++++++++++++++
 migration.c      |   28 ++++++++++++++++++++++++++++
 migration.h      |    4 ++++
 qapi-schema.json |   32 +++++++++++++++++++++++++++++++-
 qmp-commands.hx  |   35 +++++++++++++++++++++++++++++++++++
 6 files changed, 147 insertions(+), 1 deletions(-)

Comments

Luiz Capitulino July 30, 2012, 7:37 p.m. UTC | #1
On Sun, 29 Jul 2012 12:43:02 +0300
Orit Wasserman <owasserm@redhat.com> 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>
> ---
>  arch_init.c      |   28 ++++++++++++++++++++++++++++
>  hmp.c            |   21 +++++++++++++++++++++
>  migration.c      |   28 ++++++++++++++++++++++++++++
>  migration.h      |    4 ++++
>  qapi-schema.json |   32 +++++++++++++++++++++++++++++++-
>  qmp-commands.hx  |   35 +++++++++++++++++++++++++++++++++++
>  6 files changed, 147 insertions(+), 1 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 7f12317..9833d54 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -203,7 +203,11 @@ int64_t xbzrle_cache_resize(int64_t new_size)
>  typedef struct AccountingInfo {
>      uint64_t dup_pages;
>      uint64_t norm_pages;
> +    uint64_t xbzrle_bytes;
> +    uint64_t xbzrle_pages;
> +    uint64_t xbzrle_cache_miss;
>      uint64_t iterations;
> +    uint64_t xbzrle_overflows;
>  } AccountingInfo;
>  
>  static AccountingInfo acct_info;
> @@ -233,6 +237,26 @@ uint64_t norm_mig_pages_transferred(void)
>      return acct_info.norm_pages;
>  }
>  
> +uint64_t xbzrle_mig_bytes_transferred(void)
> +{
> +    return acct_info.xbzrle_bytes;
> +}
> +
> +uint64_t xbzrle_mig_pages_transferred(void)
> +{
> +    return acct_info.xbzrle_pages;
> +}
> +
> +uint64_t xbzrle_mig_pages_cache_miss(void)
> +{
> +    return acct_info.xbzrle_cache_miss;
> +}
> +
> +uint64_t xbzrle_mig_pages_overflow(void)
> +{
> +    return acct_info.xbzrle_overflows;
> +}
> +
>  static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
>          int cont, int flag)
>  {
> @@ -257,6 +281,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
>      if (!cache_is_cached(XBZRLE.cache, current_addr)) {
>          cache_insert(XBZRLE.cache, current_addr,
>                       g_memdup(current_data, TARGET_PAGE_SIZE));
> +        acct_info.xbzrle_cache_miss++;
>          return -1;
>      }
>  
> @@ -274,6 +299,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
>          return 0;
>      } else if (encoded_len == -1) {
>          DPRINTF("Overflow\n");
> +        acct_info.xbzrle_overflows++;
>          /* update data in the cache */
>          memcpy(prev_cached_page, current_data, TARGET_PAGE_SIZE);
>          return -1;
> @@ -288,6 +314,8 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
>      qemu_put_be16(f, encoded_len);
>      qemu_put_buffer(f, XBZRLE.encoded_buf, encoded_len);
>      bytes_sent = encoded_len + 1 + 2;
> +    acct_info.xbzrle_pages++;
> +    acct_info.xbzrle_bytes += bytes_sent;
>  
>      return bytes_sent;
>  }
> diff --git a/hmp.c b/hmp.c
> index 9770d7b..383d5b1 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -172,6 +172,27 @@ void hmp_info_migrate(Monitor *mon)
>                         info->disk->total >> 10);
>      }
>  
> +    if (info->has_xbzrle_cache) {
> +        monitor_printf(mon, "cache size: %" PRIu64 " bytes\n",
> +                       info->xbzrle_cache->cache_size);
> +        if (info->xbzrle_cache->has_xbzrle_bytes) {
> +            monitor_printf(mon, "xbzrle transferred: %" PRIu64 " kbytes\n",
> +                           info->xbzrle_cache->xbzrle_bytes >> 10);
> +        }
> +        if (info->xbzrle_cache->has_xbzrle_pages) {
> +            monitor_printf(mon, "xbzrle pages: %" PRIu64 " pages\n",
> +                           info->xbzrle_cache->xbzrle_pages);
> +        }
> +        if (info->xbzrle_cache->has_xbzrle_cache_miss) {
> +            monitor_printf(mon, "xbzrle cache miss: %" PRIu64 "\n",
> +                           info->xbzrle_cache->xbzrle_cache_miss);
> +        }
> +        if (info->xbzrle_cache->has_xbzrle_overflow) {
> +            monitor_printf(mon, "xbzrle overflow : %" PRIu64 "\n",
> +                           info->xbzrle_cache->xbzrle_overflow);
> +        }
> +    }
> +
>      qapi_free_MigrationInfo(info);
>      qapi_free_MigrationParameters(params);
>  }
> diff --git a/migration.c b/migration.c
> index 4dc99ba..fb802bc 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -136,6 +136,23 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
>      return params;
>  }
>  
> +static void get_xbzrle_cache_stats(MigrationInfo *info)
> +{
> +    if (migrate_use_xbzrle()) {
> +        info->has_xbzrle_cache = true;
> +        info->xbzrle_cache = g_malloc0(sizeof(*info->xbzrle_cache));
> +        info->xbzrle_cache->cache_size = migrate_xbzrle_cache_size();
> +        info->xbzrle_cache->has_xbzrle_bytes = true;
> +        info->xbzrle_cache->xbzrle_bytes = xbzrle_mig_bytes_transferred();
> +        info->xbzrle_cache->has_xbzrle_pages = true;
> +        info->xbzrle_cache->xbzrle_pages = xbzrle_mig_pages_transferred();
> +        info->xbzrle_cache->has_xbzrle_cache_miss = true;
> +        info->xbzrle_cache->xbzrle_cache_miss = xbzrle_mig_pages_cache_miss();
> +        info->xbzrle_cache->has_xbzrle_overflow = true;
> +        info->xbzrle_cache->xbzrle_overflow = xbzrle_mig_pages_overflow();
> +    }
> +}
> +
>  MigrationInfo *qmp_query_migrate(Error **errp)
>  {
>      MigrationInfo *info = g_malloc0(sizeof(*info));
> @@ -144,6 +161,13 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>      switch (s->state) {
>      case MIG_STATE_SETUP:
>          /* no migration has happened ever */
> +
> +        /* display xbzrle cache size */
> +        if (migrate_use_xbzrle()) {
> +            info->has_xbzrle_cache = true;
> +            info->xbzrle_cache = g_malloc0(sizeof(*info->xbzrle_cache));
> +            info->xbzrle_cache->cache_size = migrate_xbzrle_cache_size();
> +        }

Is it really useful to return this in MIG_SETUP? Can't the cache size
be queried by some query- command?

>          break;
>      case MIG_STATE_ACTIVE:
>          info->has_status = true;
> @@ -166,8 +190,12 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>              info->disk->remaining = blk_mig_bytes_remaining();
>              info->disk->total = blk_mig_bytes_total();
>          }
> +
> +        get_xbzrle_cache_stats(info);
>          break;
>      case MIG_STATE_COMPLETED:
> +        get_xbzrle_cache_stats(info);
> +
>          info->has_status = true;
>          info->status = g_strdup("completed");
>  
> diff --git a/migration.h b/migration.h
> index e4a7cd7..a9852fc 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -91,6 +91,10 @@ 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);
> +uint64_t xbzrle_mig_bytes_transferred(void);
> +uint64_t xbzrle_mig_pages_transferred(void);
> +uint64_t xbzrle_mig_pages_overflow(void);
> +uint64_t xbzrle_mig_pages_cache_miss(void);
>  
>  /**
>   * @migrate_add_blocker - prevent migration from proceeding
> diff --git a/qapi-schema.json b/qapi-schema.json
> index a936714..91dee72 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -275,6 +275,31 @@
>             'total_time': 'int', 'duplicate': 'int', 'normal': 'int' } }
>  
>  ##
> +# @XBZRLECacheStats
> +#
> +# Detailed XBZRLE migration cache statistics
> +#
> +# @cache-size: XBZRLE cache size
> +#
> +# @xbzrle-bytes: @optional, amount of bytes already transferred to the target VM
> +#                only returned when migration is active or completed

s/xbzrle-bytes/transferred-bytes

> +#
> +# @xbzrle-pages: @optional, amount of pages transferred to the target VM
> +#                only returned when migration is active or completed

s/xbzrle-pages/transferred-pages

> +#
> +# @xbzrle-cache-miss: @optional, number of cache miss
> +#                     only returned when migration is active or completed

s/xbzrle/xbzrle//

> +#
> +# @xbzrle-overflow: @optional, number of overflows
> +#                   only returned when migration is active or completed

s/xbzrle///

Besides, all these parameters shouldn't be optional as they are unconditionally
included (BZRLECacheStats itself is optional as returned by query-migrate).

> +#
> +# Since: 1.2
> +##
> +{ 'type': 'XBZRLECacheStats',
> +  'data': {'cache-size': 'int', '*xbzrle-bytes': 'int', '*xbzrle-pages': 'int',
> +           '*xbzrle-cache-miss': 'int', '*xbzrle-overflow': 'int' } }
> +
> +##
>  # @MigrationInfo
>  #
>  # Information about current migration process.
> @@ -292,11 +317,16 @@
>  #        status, only returned if status is 'active' and it is a block
>  #        migration
>  #
> +# @xbzrle-cache: #optional @XBZRLECacheStats containing detailed XBZRLE
> +#                migration statistics, only returned if XBZRLE feature is on
> +#                (since 1.2)
> +#
>  # Since: 0.14.0
>  ##
>  { 'type': 'MigrationInfo',
>    'data': {'*status': 'str', '*ram': 'MigrationStats',
> -           '*disk': 'MigrationStats'} }
> +           '*disk': 'MigrationStats',
> +           '*xbzrle-cache': 'XBZRLECacheStats'} }
>  
>  ##
>  # @query-migrate
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index a5a67eb..0546f42 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2106,6 +2106,17 @@ The main json-object contains the following:
>           - "transferred": amount transferred (json-int)
>           - "remaining": amount remaining (json-int)
>           - "total": total (json-int)
> +- "xbzrle-cache": only present if XBZRLE is active.
> +  It is a json-object with the following XBZRLE information:
> +         - "cache-size": XBZRLE cache size
> +         - "xbzrle-bytes": total XBZRLE bytes transferred, only present if
> +                           status is "active" or "completed"
> +         - "xbzrle-pages": number of XBZRLE compressed pages, only present if
> +                           status is "active" or "completed"
> +         - "cache-miss": number of cache misses, only present if
> +                           status is "active" or "completed"
> +         - "overflow": number of XBZRLE overflows, only present if
> +                           status is "active" or "completed"
>  Examples:
>  
>  1. Before the first migration
> @@ -2170,6 +2181,30 @@ Examples:
>        }
>     }
>  
> +6. Migration is being performed and XBZRLE is active:
> +
> +-> { "execute": "query-migrate" }
> +<- {
> +      "return":{
> +         "status":"active",
> +         "capabilities" : [ { "capability": "xbzrle", "state" : true } ],
> +         "ram":{
> +            "total":1057024,
> +            "remaining":1053304,
> +            "transferred":3720,
> +            "duplicate": 10,
> +            "normal" : 3333
> +         },
> +         "cache":{
> +            "cache-size": 1024
> +            "xbzrle-transferred":20971520,
> +            "xbzrle-pages":2444343,
> +            "xbzrle-cache-miss":2244,
> +            "xbzrle-overflow":34434
> +         }
> +      }
> +   }
> +
>  EQMP
>  
>      {
Orit Wasserman July 31, 2012, 8:31 a.m. UTC | #2
On 07/30/2012 10:37 PM, Luiz Capitulino wrote:
> On Sun, 29 Jul 2012 12:43:02 +0300
> Orit Wasserman <owasserm@redhat.com> 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>
>> ---
>>  arch_init.c      |   28 ++++++++++++++++++++++++++++
>>  hmp.c            |   21 +++++++++++++++++++++
>>  migration.c      |   28 ++++++++++++++++++++++++++++
>>  migration.h      |    4 ++++
>>  qapi-schema.json |   32 +++++++++++++++++++++++++++++++-
>>  qmp-commands.hx  |   35 +++++++++++++++++++++++++++++++++++
>>  6 files changed, 147 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch_init.c b/arch_init.c
>> index 7f12317..9833d54 100644
>> --- a/arch_init.c
>> +++ b/arch_init.c
>> @@ -203,7 +203,11 @@ int64_t xbzrle_cache_resize(int64_t new_size)
>>  typedef struct AccountingInfo {
>>      uint64_t dup_pages;
>>      uint64_t norm_pages;
>> +    uint64_t xbzrle_bytes;
>> +    uint64_t xbzrle_pages;
>> +    uint64_t xbzrle_cache_miss;
>>      uint64_t iterations;
>> +    uint64_t xbzrle_overflows;
>>  } AccountingInfo;
>>  
>>  static AccountingInfo acct_info;
>> @@ -233,6 +237,26 @@ uint64_t norm_mig_pages_transferred(void)
>>      return acct_info.norm_pages;
>>  }
>>  
>> +uint64_t xbzrle_mig_bytes_transferred(void)
>> +{
>> +    return acct_info.xbzrle_bytes;
>> +}
>> +
>> +uint64_t xbzrle_mig_pages_transferred(void)
>> +{
>> +    return acct_info.xbzrle_pages;
>> +}
>> +
>> +uint64_t xbzrle_mig_pages_cache_miss(void)
>> +{
>> +    return acct_info.xbzrle_cache_miss;
>> +}
>> +
>> +uint64_t xbzrle_mig_pages_overflow(void)
>> +{
>> +    return acct_info.xbzrle_overflows;
>> +}
>> +
>>  static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
>>          int cont, int flag)
>>  {
>> @@ -257,6 +281,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
>>      if (!cache_is_cached(XBZRLE.cache, current_addr)) {
>>          cache_insert(XBZRLE.cache, current_addr,
>>                       g_memdup(current_data, TARGET_PAGE_SIZE));
>> +        acct_info.xbzrle_cache_miss++;
>>          return -1;
>>      }
>>  
>> @@ -274,6 +299,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
>>          return 0;
>>      } else if (encoded_len == -1) {
>>          DPRINTF("Overflow\n");
>> +        acct_info.xbzrle_overflows++;
>>          /* update data in the cache */
>>          memcpy(prev_cached_page, current_data, TARGET_PAGE_SIZE);
>>          return -1;
>> @@ -288,6 +314,8 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
>>      qemu_put_be16(f, encoded_len);
>>      qemu_put_buffer(f, XBZRLE.encoded_buf, encoded_len);
>>      bytes_sent = encoded_len + 1 + 2;
>> +    acct_info.xbzrle_pages++;
>> +    acct_info.xbzrle_bytes += bytes_sent;
>>  
>>      return bytes_sent;
>>  }
>> diff --git a/hmp.c b/hmp.c
>> index 9770d7b..383d5b1 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -172,6 +172,27 @@ void hmp_info_migrate(Monitor *mon)
>>                         info->disk->total >> 10);
>>      }
>>  
>> +    if (info->has_xbzrle_cache) {
>> +        monitor_printf(mon, "cache size: %" PRIu64 " bytes\n",
>> +                       info->xbzrle_cache->cache_size);
>> +        if (info->xbzrle_cache->has_xbzrle_bytes) {
>> +            monitor_printf(mon, "xbzrle transferred: %" PRIu64 " kbytes\n",
>> +                           info->xbzrle_cache->xbzrle_bytes >> 10);
>> +        }
>> +        if (info->xbzrle_cache->has_xbzrle_pages) {
>> +            monitor_printf(mon, "xbzrle pages: %" PRIu64 " pages\n",
>> +                           info->xbzrle_cache->xbzrle_pages);
>> +        }
>> +        if (info->xbzrle_cache->has_xbzrle_cache_miss) {
>> +            monitor_printf(mon, "xbzrle cache miss: %" PRIu64 "\n",
>> +                           info->xbzrle_cache->xbzrle_cache_miss);
>> +        }
>> +        if (info->xbzrle_cache->has_xbzrle_overflow) {
>> +            monitor_printf(mon, "xbzrle overflow : %" PRIu64 "\n",
>> +                           info->xbzrle_cache->xbzrle_overflow);
>> +        }
>> +    }
>> +
>>      qapi_free_MigrationInfo(info);
>>      qapi_free_MigrationParameters(params);
>>  }
>> diff --git a/migration.c b/migration.c
>> index 4dc99ba..fb802bc 100644
>> --- a/migration.c
>> +++ b/migration.c
>> @@ -136,6 +136,23 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
>>      return params;
>>  }
>>  
>> +static void get_xbzrle_cache_stats(MigrationInfo *info)
>> +{
>> +    if (migrate_use_xbzrle()) {
>> +        info->has_xbzrle_cache = true;
>> +        info->xbzrle_cache = g_malloc0(sizeof(*info->xbzrle_cache));
>> +        info->xbzrle_cache->cache_size = migrate_xbzrle_cache_size();
>> +        info->xbzrle_cache->has_xbzrle_bytes = true;
>> +        info->xbzrle_cache->xbzrle_bytes = xbzrle_mig_bytes_transferred();
>> +        info->xbzrle_cache->has_xbzrle_pages = true;
>> +        info->xbzrle_cache->xbzrle_pages = xbzrle_mig_pages_transferred();
>> +        info->xbzrle_cache->has_xbzrle_cache_miss = true;
>> +        info->xbzrle_cache->xbzrle_cache_miss = xbzrle_mig_pages_cache_miss();
>> +        info->xbzrle_cache->has_xbzrle_overflow = true;
>> +        info->xbzrle_cache->xbzrle_overflow = xbzrle_mig_pages_overflow();
>> +    }
>> +}
>> +
>>  MigrationInfo *qmp_query_migrate(Error **errp)
>>  {
>>      MigrationInfo *info = g_malloc0(sizeof(*info));
>> @@ -144,6 +161,13 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>>      switch (s->state) {
>>      case MIG_STATE_SETUP:
>>          /* no migration has happened ever */
>> +
>> +        /* display xbzrle cache size */
>> +        if (migrate_use_xbzrle()) {
>> +            info->has_xbzrle_cache = true;
>> +            info->xbzrle_cache = g_malloc0(sizeof(*info->xbzrle_cache));
>> +            info->xbzrle_cache->cache_size = migrate_xbzrle_cache_size();
>> +        }
> 
> Is it really useful to return this in MIG_SETUP? Can't the cache size
> be queried by some query- command?
I like it this way and it is very simple to use, you need to use query-migrate command if you do migration 
anyway.
Otherwise we will need to add a query command for each configurable parameters,
so one for speed and another for downtime and probably more in the future.
This way we use one command to query the migration setup.

Orit
> 
>>          break;
>>      case MIG_STATE_ACTIVE:
>>          info->has_status = true;
>> @@ -166,8 +190,12 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>>              info->disk->remaining = blk_mig_bytes_remaining();
>>              info->disk->total = blk_mig_bytes_total();
>>          }
>> +
>> +        get_xbzrle_cache_stats(info);
>>          break;
>>      case MIG_STATE_COMPLETED:
>> +        get_xbzrle_cache_stats(info);
>> +
>>          info->has_status = true;
>>          info->status = g_strdup("completed");
>>  
>> diff --git a/migration.h b/migration.h
>> index e4a7cd7..a9852fc 100644
>> --- a/migration.h
>> +++ b/migration.h
>> @@ -91,6 +91,10 @@ 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);
>> +uint64_t xbzrle_mig_bytes_transferred(void);
>> +uint64_t xbzrle_mig_pages_transferred(void);
>> +uint64_t xbzrle_mig_pages_overflow(void);
>> +uint64_t xbzrle_mig_pages_cache_miss(void);
>>  
>>  /**
>>   * @migrate_add_blocker - prevent migration from proceeding
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index a936714..91dee72 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -275,6 +275,31 @@
>>             'total_time': 'int', 'duplicate': 'int', 'normal': 'int' } }
>>  
>>  ##
>> +# @XBZRLECacheStats
>> +#
>> +# Detailed XBZRLE migration cache statistics
>> +#
>> +# @cache-size: XBZRLE cache size
>> +#
>> +# @xbzrle-bytes: @optional, amount of bytes already transferred to the target VM
>> +#                only returned when migration is active or completed
> 
> s/xbzrle-bytes/transferred-bytes
> 
>> +#
>> +# @xbzrle-pages: @optional, amount of pages transferred to the target VM
>> +#                only returned when migration is active or completed
> 
> s/xbzrle-pages/transferred-pages
> 
>> +#
>> +# @xbzrle-cache-miss: @optional, number of cache miss
>> +#                     only returned when migration is active or completed
> 
> s/xbzrle/xbzrle//
> 
>> +#
>> +# @xbzrle-overflow: @optional, number of overflows
>> +#                   only returned when migration is active or completed
> 
> s/xbzrle///
> 
> Besides, all these parameters shouldn't be optional as they are unconditionally
> included (BZRLECacheStats itself is optional as returned by query-migrate).
> 
>> +#
>> +# Since: 1.2
>> +##
>> +{ 'type': 'XBZRLECacheStats',
>> +  'data': {'cache-size': 'int', '*xbzrle-bytes': 'int', '*xbzrle-pages': 'int',
>> +           '*xbzrle-cache-miss': 'int', '*xbzrle-overflow': 'int' } }
>> +
>> +##
>>  # @MigrationInfo
>>  #
>>  # Information about current migration process.
>> @@ -292,11 +317,16 @@
>>  #        status, only returned if status is 'active' and it is a block
>>  #        migration
>>  #
>> +# @xbzrle-cache: #optional @XBZRLECacheStats containing detailed XBZRLE
>> +#                migration statistics, only returned if XBZRLE feature is on
>> +#                (since 1.2)
>> +#
>>  # Since: 0.14.0
>>  ##
>>  { 'type': 'MigrationInfo',
>>    'data': {'*status': 'str', '*ram': 'MigrationStats',
>> -           '*disk': 'MigrationStats'} }
>> +           '*disk': 'MigrationStats',
>> +           '*xbzrle-cache': 'XBZRLECacheStats'} }
>>  
>>  ##
>>  # @query-migrate
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index a5a67eb..0546f42 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -2106,6 +2106,17 @@ The main json-object contains the following:
>>           - "transferred": amount transferred (json-int)
>>           - "remaining": amount remaining (json-int)
>>           - "total": total (json-int)
>> +- "xbzrle-cache": only present if XBZRLE is active.
>> +  It is a json-object with the following XBZRLE information:
>> +         - "cache-size": XBZRLE cache size
>> +         - "xbzrle-bytes": total XBZRLE bytes transferred, only present if
>> +                           status is "active" or "completed"
>> +         - "xbzrle-pages": number of XBZRLE compressed pages, only present if
>> +                           status is "active" or "completed"
>> +         - "cache-miss": number of cache misses, only present if
>> +                           status is "active" or "completed"
>> +         - "overflow": number of XBZRLE overflows, only present if
>> +                           status is "active" or "completed"
>>  Examples:
>>  
>>  1. Before the first migration
>> @@ -2170,6 +2181,30 @@ Examples:
>>        }
>>     }
>>  
>> +6. Migration is being performed and XBZRLE is active:
>> +
>> +-> { "execute": "query-migrate" }
>> +<- {
>> +      "return":{
>> +         "status":"active",
>> +         "capabilities" : [ { "capability": "xbzrle", "state" : true } ],
>> +         "ram":{
>> +            "total":1057024,
>> +            "remaining":1053304,
>> +            "transferred":3720,
>> +            "duplicate": 10,
>> +            "normal" : 3333
>> +         },
>> +         "cache":{
>> +            "cache-size": 1024
>> +            "xbzrle-transferred":20971520,
>> +            "xbzrle-pages":2444343,
>> +            "xbzrle-cache-miss":2244,
>> +            "xbzrle-overflow":34434
>> +         }
>> +      }
>> +   }
>> +
>>  EQMP
>>  
>>      {
>
Luiz Capitulino July 31, 2012, 1:16 p.m. UTC | #3
On Tue, 31 Jul 2012 11:31:09 +0300
Orit Wasserman <owasserm@redhat.com> wrote:

> On 07/30/2012 10:37 PM, Luiz Capitulino wrote:
> > On Sun, 29 Jul 2012 12:43:02 +0300
> > Orit Wasserman <owasserm@redhat.com> 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>
> >> ---
> >>  arch_init.c      |   28 ++++++++++++++++++++++++++++
> >>  hmp.c            |   21 +++++++++++++++++++++
> >>  migration.c      |   28 ++++++++++++++++++++++++++++
> >>  migration.h      |    4 ++++
> >>  qapi-schema.json |   32 +++++++++++++++++++++++++++++++-
> >>  qmp-commands.hx  |   35 +++++++++++++++++++++++++++++++++++
> >>  6 files changed, 147 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/arch_init.c b/arch_init.c
> >> index 7f12317..9833d54 100644
> >> --- a/arch_init.c
> >> +++ b/arch_init.c
> >> @@ -203,7 +203,11 @@ int64_t xbzrle_cache_resize(int64_t new_size)
> >>  typedef struct AccountingInfo {
> >>      uint64_t dup_pages;
> >>      uint64_t norm_pages;
> >> +    uint64_t xbzrle_bytes;
> >> +    uint64_t xbzrle_pages;
> >> +    uint64_t xbzrle_cache_miss;
> >>      uint64_t iterations;
> >> +    uint64_t xbzrle_overflows;
> >>  } AccountingInfo;
> >>  
> >>  static AccountingInfo acct_info;
> >> @@ -233,6 +237,26 @@ uint64_t norm_mig_pages_transferred(void)
> >>      return acct_info.norm_pages;
> >>  }
> >>  
> >> +uint64_t xbzrle_mig_bytes_transferred(void)
> >> +{
> >> +    return acct_info.xbzrle_bytes;
> >> +}
> >> +
> >> +uint64_t xbzrle_mig_pages_transferred(void)
> >> +{
> >> +    return acct_info.xbzrle_pages;
> >> +}
> >> +
> >> +uint64_t xbzrle_mig_pages_cache_miss(void)
> >> +{
> >> +    return acct_info.xbzrle_cache_miss;
> >> +}
> >> +
> >> +uint64_t xbzrle_mig_pages_overflow(void)
> >> +{
> >> +    return acct_info.xbzrle_overflows;
> >> +}
> >> +
> >>  static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
> >>          int cont, int flag)
> >>  {
> >> @@ -257,6 +281,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
> >>      if (!cache_is_cached(XBZRLE.cache, current_addr)) {
> >>          cache_insert(XBZRLE.cache, current_addr,
> >>                       g_memdup(current_data, TARGET_PAGE_SIZE));
> >> +        acct_info.xbzrle_cache_miss++;
> >>          return -1;
> >>      }
> >>  
> >> @@ -274,6 +299,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
> >>          return 0;
> >>      } else if (encoded_len == -1) {
> >>          DPRINTF("Overflow\n");
> >> +        acct_info.xbzrle_overflows++;
> >>          /* update data in the cache */
> >>          memcpy(prev_cached_page, current_data, TARGET_PAGE_SIZE);
> >>          return -1;
> >> @@ -288,6 +314,8 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
> >>      qemu_put_be16(f, encoded_len);
> >>      qemu_put_buffer(f, XBZRLE.encoded_buf, encoded_len);
> >>      bytes_sent = encoded_len + 1 + 2;
> >> +    acct_info.xbzrle_pages++;
> >> +    acct_info.xbzrle_bytes += bytes_sent;
> >>  
> >>      return bytes_sent;
> >>  }
> >> diff --git a/hmp.c b/hmp.c
> >> index 9770d7b..383d5b1 100644
> >> --- a/hmp.c
> >> +++ b/hmp.c
> >> @@ -172,6 +172,27 @@ void hmp_info_migrate(Monitor *mon)
> >>                         info->disk->total >> 10);
> >>      }
> >>  
> >> +    if (info->has_xbzrle_cache) {
> >> +        monitor_printf(mon, "cache size: %" PRIu64 " bytes\n",
> >> +                       info->xbzrle_cache->cache_size);
> >> +        if (info->xbzrle_cache->has_xbzrle_bytes) {
> >> +            monitor_printf(mon, "xbzrle transferred: %" PRIu64 " kbytes\n",
> >> +                           info->xbzrle_cache->xbzrle_bytes >> 10);
> >> +        }
> >> +        if (info->xbzrle_cache->has_xbzrle_pages) {
> >> +            monitor_printf(mon, "xbzrle pages: %" PRIu64 " pages\n",
> >> +                           info->xbzrle_cache->xbzrle_pages);
> >> +        }
> >> +        if (info->xbzrle_cache->has_xbzrle_cache_miss) {
> >> +            monitor_printf(mon, "xbzrle cache miss: %" PRIu64 "\n",
> >> +                           info->xbzrle_cache->xbzrle_cache_miss);
> >> +        }
> >> +        if (info->xbzrle_cache->has_xbzrle_overflow) {
> >> +            monitor_printf(mon, "xbzrle overflow : %" PRIu64 "\n",
> >> +                           info->xbzrle_cache->xbzrle_overflow);
> >> +        }
> >> +    }
> >> +
> >>      qapi_free_MigrationInfo(info);
> >>      qapi_free_MigrationParameters(params);
> >>  }
> >> diff --git a/migration.c b/migration.c
> >> index 4dc99ba..fb802bc 100644
> >> --- a/migration.c
> >> +++ b/migration.c
> >> @@ -136,6 +136,23 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
> >>      return params;
> >>  }
> >>  
> >> +static void get_xbzrle_cache_stats(MigrationInfo *info)
> >> +{
> >> +    if (migrate_use_xbzrle()) {
> >> +        info->has_xbzrle_cache = true;
> >> +        info->xbzrle_cache = g_malloc0(sizeof(*info->xbzrle_cache));
> >> +        info->xbzrle_cache->cache_size = migrate_xbzrle_cache_size();
> >> +        info->xbzrle_cache->has_xbzrle_bytes = true;
> >> +        info->xbzrle_cache->xbzrle_bytes = xbzrle_mig_bytes_transferred();
> >> +        info->xbzrle_cache->has_xbzrle_pages = true;
> >> +        info->xbzrle_cache->xbzrle_pages = xbzrle_mig_pages_transferred();
> >> +        info->xbzrle_cache->has_xbzrle_cache_miss = true;
> >> +        info->xbzrle_cache->xbzrle_cache_miss = xbzrle_mig_pages_cache_miss();
> >> +        info->xbzrle_cache->has_xbzrle_overflow = true;
> >> +        info->xbzrle_cache->xbzrle_overflow = xbzrle_mig_pages_overflow();
> >> +    }
> >> +}
> >> +
> >>  MigrationInfo *qmp_query_migrate(Error **errp)
> >>  {
> >>      MigrationInfo *info = g_malloc0(sizeof(*info));
> >> @@ -144,6 +161,13 @@ MigrationInfo *qmp_query_migrate(Error **errp)
> >>      switch (s->state) {
> >>      case MIG_STATE_SETUP:
> >>          /* no migration has happened ever */
> >> +
> >> +        /* display xbzrle cache size */
> >> +        if (migrate_use_xbzrle()) {
> >> +            info->has_xbzrle_cache = true;
> >> +            info->xbzrle_cache = g_malloc0(sizeof(*info->xbzrle_cache));
> >> +            info->xbzrle_cache->cache_size = migrate_xbzrle_cache_size();
> >> +        }
> > 
> > Is it really useful to return this in MIG_SETUP? Can't the cache size
> > be queried by some query- command?
> I like it this way and it is very simple to use, you need to use query-migrate command if you do migration 
> anyway.

But this will only be returned before a migration takes place. Then it will
never be returned again. Doesn't seem to be the right place for it.

> Otherwise we will need to add a query command for each configurable parameters,
> so one for speed and another for downtime and probably more in the future.

I don't think this is a problem. We're trying to design QMP commands as we
would design a C API, and I'd have different functions to query different things.

Besides, let's not mix what's best to a human user and what's best for a
machine protocol. If we add different query commands for different parameters,
then it's possible to call them from 'info migrate' if you wish.

> This way we use one command to query the migration setup.

Again, that information would only be returned before a first migration
occurs.

> 
> Orit
> > 
> >>          break;
> >>      case MIG_STATE_ACTIVE:
> >>          info->has_status = true;
> >> @@ -166,8 +190,12 @@ MigrationInfo *qmp_query_migrate(Error **errp)
> >>              info->disk->remaining = blk_mig_bytes_remaining();
> >>              info->disk->total = blk_mig_bytes_total();
> >>          }
> >> +
> >> +        get_xbzrle_cache_stats(info);
> >>          break;
> >>      case MIG_STATE_COMPLETED:
> >> +        get_xbzrle_cache_stats(info);
> >> +
> >>          info->has_status = true;
> >>          info->status = g_strdup("completed");
> >>  
> >> diff --git a/migration.h b/migration.h
> >> index e4a7cd7..a9852fc 100644
> >> --- a/migration.h
> >> +++ b/migration.h
> >> @@ -91,6 +91,10 @@ 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);
> >> +uint64_t xbzrle_mig_bytes_transferred(void);
> >> +uint64_t xbzrle_mig_pages_transferred(void);
> >> +uint64_t xbzrle_mig_pages_overflow(void);
> >> +uint64_t xbzrle_mig_pages_cache_miss(void);
> >>  
> >>  /**
> >>   * @migrate_add_blocker - prevent migration from proceeding
> >> diff --git a/qapi-schema.json b/qapi-schema.json
> >> index a936714..91dee72 100644
> >> --- a/qapi-schema.json
> >> +++ b/qapi-schema.json
> >> @@ -275,6 +275,31 @@
> >>             'total_time': 'int', 'duplicate': 'int', 'normal': 'int' } }
> >>  
> >>  ##
> >> +# @XBZRLECacheStats
> >> +#
> >> +# Detailed XBZRLE migration cache statistics
> >> +#
> >> +# @cache-size: XBZRLE cache size
> >> +#
> >> +# @xbzrle-bytes: @optional, amount of bytes already transferred to the target VM
> >> +#                only returned when migration is active or completed
> > 
> > s/xbzrle-bytes/transferred-bytes
> > 
> >> +#
> >> +# @xbzrle-pages: @optional, amount of pages transferred to the target VM
> >> +#                only returned when migration is active or completed
> > 
> > s/xbzrle-pages/transferred-pages
> > 
> >> +#
> >> +# @xbzrle-cache-miss: @optional, number of cache miss
> >> +#                     only returned when migration is active or completed
> > 
> > s/xbzrle/xbzrle//
> > 
> >> +#
> >> +# @xbzrle-overflow: @optional, number of overflows
> >> +#                   only returned when migration is active or completed
> > 
> > s/xbzrle///
> > 
> > Besides, all these parameters shouldn't be optional as they are unconditionally
> > included (BZRLECacheStats itself is optional as returned by query-migrate).
> > 
> >> +#
> >> +# Since: 1.2
> >> +##
> >> +{ 'type': 'XBZRLECacheStats',
> >> +  'data': {'cache-size': 'int', '*xbzrle-bytes': 'int', '*xbzrle-pages': 'int',
> >> +           '*xbzrle-cache-miss': 'int', '*xbzrle-overflow': 'int' } }
> >> +
> >> +##
> >>  # @MigrationInfo
> >>  #
> >>  # Information about current migration process.
> >> @@ -292,11 +317,16 @@
> >>  #        status, only returned if status is 'active' and it is a block
> >>  #        migration
> >>  #
> >> +# @xbzrle-cache: #optional @XBZRLECacheStats containing detailed XBZRLE
> >> +#                migration statistics, only returned if XBZRLE feature is on
> >> +#                (since 1.2)
> >> +#
> >>  # Since: 0.14.0
> >>  ##
> >>  { 'type': 'MigrationInfo',
> >>    'data': {'*status': 'str', '*ram': 'MigrationStats',
> >> -           '*disk': 'MigrationStats'} }
> >> +           '*disk': 'MigrationStats',
> >> +           '*xbzrle-cache': 'XBZRLECacheStats'} }
> >>  
> >>  ##
> >>  # @query-migrate
> >> diff --git a/qmp-commands.hx b/qmp-commands.hx
> >> index a5a67eb..0546f42 100644
> >> --- a/qmp-commands.hx
> >> +++ b/qmp-commands.hx
> >> @@ -2106,6 +2106,17 @@ The main json-object contains the following:
> >>           - "transferred": amount transferred (json-int)
> >>           - "remaining": amount remaining (json-int)
> >>           - "total": total (json-int)
> >> +- "xbzrle-cache": only present if XBZRLE is active.
> >> +  It is a json-object with the following XBZRLE information:
> >> +         - "cache-size": XBZRLE cache size
> >> +         - "xbzrle-bytes": total XBZRLE bytes transferred, only present if
> >> +                           status is "active" or "completed"
> >> +         - "xbzrle-pages": number of XBZRLE compressed pages, only present if
> >> +                           status is "active" or "completed"
> >> +         - "cache-miss": number of cache misses, only present if
> >> +                           status is "active" or "completed"
> >> +         - "overflow": number of XBZRLE overflows, only present if
> >> +                           status is "active" or "completed"
> >>  Examples:
> >>  
> >>  1. Before the first migration
> >> @@ -2170,6 +2181,30 @@ Examples:
> >>        }
> >>     }
> >>  
> >> +6. Migration is being performed and XBZRLE is active:
> >> +
> >> +-> { "execute": "query-migrate" }
> >> +<- {
> >> +      "return":{
> >> +         "status":"active",
> >> +         "capabilities" : [ { "capability": "xbzrle", "state" : true } ],
> >> +         "ram":{
> >> +            "total":1057024,
> >> +            "remaining":1053304,
> >> +            "transferred":3720,
> >> +            "duplicate": 10,
> >> +            "normal" : 3333
> >> +         },
> >> +         "cache":{
> >> +            "cache-size": 1024
> >> +            "xbzrle-transferred":20971520,
> >> +            "xbzrle-pages":2444343,
> >> +            "xbzrle-cache-miss":2244,
> >> +            "xbzrle-overflow":34434
> >> +         }
> >> +      }
> >> +   }
> >> +
> >>  EQMP
> >>  
> >>      {
> > 
>
Orit Wasserman July 31, 2012, 2:13 p.m. UTC | #4
On 07/31/2012 04:16 PM, Luiz Capitulino wrote:
> On Tue, 31 Jul 2012 11:31:09 +0300
> Orit Wasserman <owasserm@redhat.com> wrote:
> 
>> On 07/30/2012 10:37 PM, Luiz Capitulino wrote:
>>> On Sun, 29 Jul 2012 12:43:02 +0300
>>> Orit Wasserman <owasserm@redhat.com> 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>
>>>> ---
>>>>  arch_init.c      |   28 ++++++++++++++++++++++++++++
>>>>  hmp.c            |   21 +++++++++++++++++++++
>>>>  migration.c      |   28 ++++++++++++++++++++++++++++
>>>>  migration.h      |    4 ++++
>>>>  qapi-schema.json |   32 +++++++++++++++++++++++++++++++-
>>>>  qmp-commands.hx  |   35 +++++++++++++++++++++++++++++++++++
>>>>  6 files changed, 147 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/arch_init.c b/arch_init.c
>>>> index 7f12317..9833d54 100644
>>>> --- a/arch_init.c
>>>> +++ b/arch_init.c
>>>> @@ -203,7 +203,11 @@ int64_t xbzrle_cache_resize(int64_t new_size)
>>>>  typedef struct AccountingInfo {
>>>>      uint64_t dup_pages;
>>>>      uint64_t norm_pages;
>>>> +    uint64_t xbzrle_bytes;
>>>> +    uint64_t xbzrle_pages;
>>>> +    uint64_t xbzrle_cache_miss;
>>>>      uint64_t iterations;
>>>> +    uint64_t xbzrle_overflows;
>>>>  } AccountingInfo;
>>>>  
>>>>  static AccountingInfo acct_info;
>>>> @@ -233,6 +237,26 @@ uint64_t norm_mig_pages_transferred(void)
>>>>      return acct_info.norm_pages;
>>>>  }
>>>>  
>>>> +uint64_t xbzrle_mig_bytes_transferred(void)
>>>> +{
>>>> +    return acct_info.xbzrle_bytes;
>>>> +}
>>>> +
>>>> +uint64_t xbzrle_mig_pages_transferred(void)
>>>> +{
>>>> +    return acct_info.xbzrle_pages;
>>>> +}
>>>> +
>>>> +uint64_t xbzrle_mig_pages_cache_miss(void)
>>>> +{
>>>> +    return acct_info.xbzrle_cache_miss;
>>>> +}
>>>> +
>>>> +uint64_t xbzrle_mig_pages_overflow(void)
>>>> +{
>>>> +    return acct_info.xbzrle_overflows;
>>>> +}
>>>> +
>>>>  static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
>>>>          int cont, int flag)
>>>>  {
>>>> @@ -257,6 +281,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
>>>>      if (!cache_is_cached(XBZRLE.cache, current_addr)) {
>>>>          cache_insert(XBZRLE.cache, current_addr,
>>>>                       g_memdup(current_data, TARGET_PAGE_SIZE));
>>>> +        acct_info.xbzrle_cache_miss++;
>>>>          return -1;
>>>>      }
>>>>  
>>>> @@ -274,6 +299,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
>>>>          return 0;
>>>>      } else if (encoded_len == -1) {
>>>>          DPRINTF("Overflow\n");
>>>> +        acct_info.xbzrle_overflows++;
>>>>          /* update data in the cache */
>>>>          memcpy(prev_cached_page, current_data, TARGET_PAGE_SIZE);
>>>>          return -1;
>>>> @@ -288,6 +314,8 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
>>>>      qemu_put_be16(f, encoded_len);
>>>>      qemu_put_buffer(f, XBZRLE.encoded_buf, encoded_len);
>>>>      bytes_sent = encoded_len + 1 + 2;
>>>> +    acct_info.xbzrle_pages++;
>>>> +    acct_info.xbzrle_bytes += bytes_sent;
>>>>  
>>>>      return bytes_sent;
>>>>  }
>>>> diff --git a/hmp.c b/hmp.c
>>>> index 9770d7b..383d5b1 100644
>>>> --- a/hmp.c
>>>> +++ b/hmp.c
>>>> @@ -172,6 +172,27 @@ void hmp_info_migrate(Monitor *mon)
>>>>                         info->disk->total >> 10);
>>>>      }
>>>>  
>>>> +    if (info->has_xbzrle_cache) {
>>>> +        monitor_printf(mon, "cache size: %" PRIu64 " bytes\n",
>>>> +                       info->xbzrle_cache->cache_size);
>>>> +        if (info->xbzrle_cache->has_xbzrle_bytes) {
>>>> +            monitor_printf(mon, "xbzrle transferred: %" PRIu64 " kbytes\n",
>>>> +                           info->xbzrle_cache->xbzrle_bytes >> 10);
>>>> +        }
>>>> +        if (info->xbzrle_cache->has_xbzrle_pages) {
>>>> +            monitor_printf(mon, "xbzrle pages: %" PRIu64 " pages\n",
>>>> +                           info->xbzrle_cache->xbzrle_pages);
>>>> +        }
>>>> +        if (info->xbzrle_cache->has_xbzrle_cache_miss) {
>>>> +            monitor_printf(mon, "xbzrle cache miss: %" PRIu64 "\n",
>>>> +                           info->xbzrle_cache->xbzrle_cache_miss);
>>>> +        }
>>>> +        if (info->xbzrle_cache->has_xbzrle_overflow) {
>>>> +            monitor_printf(mon, "xbzrle overflow : %" PRIu64 "\n",
>>>> +                           info->xbzrle_cache->xbzrle_overflow);
>>>> +        }
>>>> +    }
>>>> +
>>>>      qapi_free_MigrationInfo(info);
>>>>      qapi_free_MigrationParameters(params);
>>>>  }
>>>> diff --git a/migration.c b/migration.c
>>>> index 4dc99ba..fb802bc 100644
>>>> --- a/migration.c
>>>> +++ b/migration.c
>>>> @@ -136,6 +136,23 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
>>>>      return params;
>>>>  }
>>>>  
>>>> +static void get_xbzrle_cache_stats(MigrationInfo *info)
>>>> +{
>>>> +    if (migrate_use_xbzrle()) {
>>>> +        info->has_xbzrle_cache = true;
>>>> +        info->xbzrle_cache = g_malloc0(sizeof(*info->xbzrle_cache));
>>>> +        info->xbzrle_cache->cache_size = migrate_xbzrle_cache_size();
>>>> +        info->xbzrle_cache->has_xbzrle_bytes = true;
>>>> +        info->xbzrle_cache->xbzrle_bytes = xbzrle_mig_bytes_transferred();
>>>> +        info->xbzrle_cache->has_xbzrle_pages = true;
>>>> +        info->xbzrle_cache->xbzrle_pages = xbzrle_mig_pages_transferred();
>>>> +        info->xbzrle_cache->has_xbzrle_cache_miss = true;
>>>> +        info->xbzrle_cache->xbzrle_cache_miss = xbzrle_mig_pages_cache_miss();
>>>> +        info->xbzrle_cache->has_xbzrle_overflow = true;
>>>> +        info->xbzrle_cache->xbzrle_overflow = xbzrle_mig_pages_overflow();
>>>> +    }
>>>> +}
>>>> +
>>>>  MigrationInfo *qmp_query_migrate(Error **errp)
>>>>  {
>>>>      MigrationInfo *info = g_malloc0(sizeof(*info));
>>>> @@ -144,6 +161,13 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>>>>      switch (s->state) {
>>>>      case MIG_STATE_SETUP:
>>>>          /* no migration has happened ever */
>>>> +
>>>> +        /* display xbzrle cache size */
>>>> +        if (migrate_use_xbzrle()) {
>>>> +            info->has_xbzrle_cache = true;
>>>> +            info->xbzrle_cache = g_malloc0(sizeof(*info->xbzrle_cache));
>>>> +            info->xbzrle_cache->cache_size = migrate_xbzrle_cache_size();
>>>> +        }
>>>
>>> Is it really useful to return this in MIG_SETUP? Can't the cache size
>>> be queried by some query- command?
>> I like it this way and it is very simple to use, you need to use query-migrate command if you do migration 
>> anyway.
> 
> But this will only be returned before a migration takes place. Then it will
> never be returned again. Doesn't seem to be the right place for it.
It is returned also in MIG_ACTIVE and MIG_COMPLETE (when XBZRLE are active).
> 
>> Otherwise we will need to add a query command for each configurable parameters,
>> so one for speed and another for downtime and probably more in the future.
> 
> I don't think this is a problem. We're trying to design QMP commands as we
> would design a C API, and I'd have different functions to query different things.
> 
> Besides, let's not mix what's best to a human user and what's best for a
> machine protocol. If we add different query commands for different parameters,
> then it's possible to call them from 'info migrate' if you wish.
sounds reasonable, I will change it.

Orit 
> 
>> This way we use one command to query the migration setup.
> 
> Again, that information would only be returned before a first migration
> occurs.
> 
>>
>> Orit
>>>
>>>>          break;
>>>>      case MIG_STATE_ACTIVE:
>>>>          info->has_status = true;
>>>> @@ -166,8 +190,12 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>>>>              info->disk->remaining = blk_mig_bytes_remaining();
>>>>              info->disk->total = blk_mig_bytes_total();
>>>>          }
>>>> +
>>>> +        get_xbzrle_cache_stats(info);
>>>>          break;
>>>>      case MIG_STATE_COMPLETED:
>>>> +        get_xbzrle_cache_stats(info);
>>>> +
>>>>          info->has_status = true;
>>>>          info->status = g_strdup("completed");
>>>>  
>>>> diff --git a/migration.h b/migration.h
>>>> index e4a7cd7..a9852fc 100644
>>>> --- a/migration.h
>>>> +++ b/migration.h
>>>> @@ -91,6 +91,10 @@ 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);
>>>> +uint64_t xbzrle_mig_bytes_transferred(void);
>>>> +uint64_t xbzrle_mig_pages_transferred(void);
>>>> +uint64_t xbzrle_mig_pages_overflow(void);
>>>> +uint64_t xbzrle_mig_pages_cache_miss(void);
>>>>  
>>>>  /**
>>>>   * @migrate_add_blocker - prevent migration from proceeding
>>>> diff --git a/qapi-schema.json b/qapi-schema.json
>>>> index a936714..91dee72 100644
>>>> --- a/qapi-schema.json
>>>> +++ b/qapi-schema.json
>>>> @@ -275,6 +275,31 @@
>>>>             'total_time': 'int', 'duplicate': 'int', 'normal': 'int' } }
>>>>  
>>>>  ##
>>>> +# @XBZRLECacheStats
>>>> +#
>>>> +# Detailed XBZRLE migration cache statistics
>>>> +#
>>>> +# @cache-size: XBZRLE cache size
>>>> +#
>>>> +# @xbzrle-bytes: @optional, amount of bytes already transferred to the target VM
>>>> +#                only returned when migration is active or completed
>>>
>>> s/xbzrle-bytes/transferred-bytes
>>>
>>>> +#
>>>> +# @xbzrle-pages: @optional, amount of pages transferred to the target VM
>>>> +#                only returned when migration is active or completed
>>>
>>> s/xbzrle-pages/transferred-pages
>>>
>>>> +#
>>>> +# @xbzrle-cache-miss: @optional, number of cache miss
>>>> +#                     only returned when migration is active or completed
>>>
>>> s/xbzrle/xbzrle//
>>>
>>>> +#
>>>> +# @xbzrle-overflow: @optional, number of overflows
>>>> +#                   only returned when migration is active or completed
>>>
>>> s/xbzrle///
>>>
>>> Besides, all these parameters shouldn't be optional as they are unconditionally
>>> included (BZRLECacheStats itself is optional as returned by query-migrate).
>>>
>>>> +#
>>>> +# Since: 1.2
>>>> +##
>>>> +{ 'type': 'XBZRLECacheStats',
>>>> +  'data': {'cache-size': 'int', '*xbzrle-bytes': 'int', '*xbzrle-pages': 'int',
>>>> +           '*xbzrle-cache-miss': 'int', '*xbzrle-overflow': 'int' } }
>>>> +
>>>> +##
>>>>  # @MigrationInfo
>>>>  #
>>>>  # Information about current migration process.
>>>> @@ -292,11 +317,16 @@
>>>>  #        status, only returned if status is 'active' and it is a block
>>>>  #        migration
>>>>  #
>>>> +# @xbzrle-cache: #optional @XBZRLECacheStats containing detailed XBZRLE
>>>> +#                migration statistics, only returned if XBZRLE feature is on
>>>> +#                (since 1.2)
>>>> +#
>>>>  # Since: 0.14.0
>>>>  ##
>>>>  { 'type': 'MigrationInfo',
>>>>    'data': {'*status': 'str', '*ram': 'MigrationStats',
>>>> -           '*disk': 'MigrationStats'} }
>>>> +           '*disk': 'MigrationStats',
>>>> +           '*xbzrle-cache': 'XBZRLECacheStats'} }
>>>>  
>>>>  ##
>>>>  # @query-migrate
>>>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>>>> index a5a67eb..0546f42 100644
>>>> --- a/qmp-commands.hx
>>>> +++ b/qmp-commands.hx
>>>> @@ -2106,6 +2106,17 @@ The main json-object contains the following:
>>>>           - "transferred": amount transferred (json-int)
>>>>           - "remaining": amount remaining (json-int)
>>>>           - "total": total (json-int)
>>>> +- "xbzrle-cache": only present if XBZRLE is active.
>>>> +  It is a json-object with the following XBZRLE information:
>>>> +         - "cache-size": XBZRLE cache size
>>>> +         - "xbzrle-bytes": total XBZRLE bytes transferred, only present if
>>>> +                           status is "active" or "completed"
>>>> +         - "xbzrle-pages": number of XBZRLE compressed pages, only present if
>>>> +                           status is "active" or "completed"
>>>> +         - "cache-miss": number of cache misses, only present if
>>>> +                           status is "active" or "completed"
>>>> +         - "overflow": number of XBZRLE overflows, only present if
>>>> +                           status is "active" or "completed"
>>>>  Examples:
>>>>  
>>>>  1. Before the first migration
>>>> @@ -2170,6 +2181,30 @@ Examples:
>>>>        }
>>>>     }
>>>>  
>>>> +6. Migration is being performed and XBZRLE is active:
>>>> +
>>>> +-> { "execute": "query-migrate" }
>>>> +<- {
>>>> +      "return":{
>>>> +         "status":"active",
>>>> +         "capabilities" : [ { "capability": "xbzrle", "state" : true } ],
>>>> +         "ram":{
>>>> +            "total":1057024,
>>>> +            "remaining":1053304,
>>>> +            "transferred":3720,
>>>> +            "duplicate": 10,
>>>> +            "normal" : 3333
>>>> +         },
>>>> +         "cache":{
>>>> +            "cache-size": 1024
>>>> +            "xbzrle-transferred":20971520,
>>>> +            "xbzrle-pages":2444343,
>>>> +            "xbzrle-cache-miss":2244,
>>>> +            "xbzrle-overflow":34434
>>>> +         }
>>>> +      }
>>>> +   }
>>>> +
>>>>  EQMP
>>>>  
>>>>      {
>>>
>>
>
Luiz Capitulino July 31, 2012, 3:54 p.m. UTC | #5
On Tue, 31 Jul 2012 17:13:46 +0300
Orit Wasserman <owasserm@redhat.com> wrote:

> On 07/31/2012 04:16 PM, Luiz Capitulino wrote:
> > On Tue, 31 Jul 2012 11:31:09 +0300
> > Orit Wasserman <owasserm@redhat.com> wrote:
> > 
> >> On 07/30/2012 10:37 PM, Luiz Capitulino wrote:
> >>> On Sun, 29 Jul 2012 12:43:02 +0300
> >>> Orit Wasserman <owasserm@redhat.com> 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>
> >>>> ---
> >>>>  arch_init.c      |   28 ++++++++++++++++++++++++++++
> >>>>  hmp.c            |   21 +++++++++++++++++++++
> >>>>  migration.c      |   28 ++++++++++++++++++++++++++++
> >>>>  migration.h      |    4 ++++
> >>>>  qapi-schema.json |   32 +++++++++++++++++++++++++++++++-
> >>>>  qmp-commands.hx  |   35 +++++++++++++++++++++++++++++++++++
> >>>>  6 files changed, 147 insertions(+), 1 deletions(-)
> >>>>
> >>>> diff --git a/arch_init.c b/arch_init.c
> >>>> index 7f12317..9833d54 100644
> >>>> --- a/arch_init.c
> >>>> +++ b/arch_init.c
> >>>> @@ -203,7 +203,11 @@ int64_t xbzrle_cache_resize(int64_t new_size)
> >>>>  typedef struct AccountingInfo {
> >>>>      uint64_t dup_pages;
> >>>>      uint64_t norm_pages;
> >>>> +    uint64_t xbzrle_bytes;
> >>>> +    uint64_t xbzrle_pages;
> >>>> +    uint64_t xbzrle_cache_miss;
> >>>>      uint64_t iterations;
> >>>> +    uint64_t xbzrle_overflows;
> >>>>  } AccountingInfo;
> >>>>  
> >>>>  static AccountingInfo acct_info;
> >>>> @@ -233,6 +237,26 @@ uint64_t norm_mig_pages_transferred(void)
> >>>>      return acct_info.norm_pages;
> >>>>  }
> >>>>  
> >>>> +uint64_t xbzrle_mig_bytes_transferred(void)
> >>>> +{
> >>>> +    return acct_info.xbzrle_bytes;
> >>>> +}
> >>>> +
> >>>> +uint64_t xbzrle_mig_pages_transferred(void)
> >>>> +{
> >>>> +    return acct_info.xbzrle_pages;
> >>>> +}
> >>>> +
> >>>> +uint64_t xbzrle_mig_pages_cache_miss(void)
> >>>> +{
> >>>> +    return acct_info.xbzrle_cache_miss;
> >>>> +}
> >>>> +
> >>>> +uint64_t xbzrle_mig_pages_overflow(void)
> >>>> +{
> >>>> +    return acct_info.xbzrle_overflows;
> >>>> +}
> >>>> +
> >>>>  static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
> >>>>          int cont, int flag)
> >>>>  {
> >>>> @@ -257,6 +281,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
> >>>>      if (!cache_is_cached(XBZRLE.cache, current_addr)) {
> >>>>          cache_insert(XBZRLE.cache, current_addr,
> >>>>                       g_memdup(current_data, TARGET_PAGE_SIZE));
> >>>> +        acct_info.xbzrle_cache_miss++;
> >>>>          return -1;
> >>>>      }
> >>>>  
> >>>> @@ -274,6 +299,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
> >>>>          return 0;
> >>>>      } else if (encoded_len == -1) {
> >>>>          DPRINTF("Overflow\n");
> >>>> +        acct_info.xbzrle_overflows++;
> >>>>          /* update data in the cache */
> >>>>          memcpy(prev_cached_page, current_data, TARGET_PAGE_SIZE);
> >>>>          return -1;
> >>>> @@ -288,6 +314,8 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
> >>>>      qemu_put_be16(f, encoded_len);
> >>>>      qemu_put_buffer(f, XBZRLE.encoded_buf, encoded_len);
> >>>>      bytes_sent = encoded_len + 1 + 2;
> >>>> +    acct_info.xbzrle_pages++;
> >>>> +    acct_info.xbzrle_bytes += bytes_sent;
> >>>>  
> >>>>      return bytes_sent;
> >>>>  }
> >>>> diff --git a/hmp.c b/hmp.c
> >>>> index 9770d7b..383d5b1 100644
> >>>> --- a/hmp.c
> >>>> +++ b/hmp.c
> >>>> @@ -172,6 +172,27 @@ void hmp_info_migrate(Monitor *mon)
> >>>>                         info->disk->total >> 10);
> >>>>      }
> >>>>  
> >>>> +    if (info->has_xbzrle_cache) {
> >>>> +        monitor_printf(mon, "cache size: %" PRIu64 " bytes\n",
> >>>> +                       info->xbzrle_cache->cache_size);
> >>>> +        if (info->xbzrle_cache->has_xbzrle_bytes) {
> >>>> +            monitor_printf(mon, "xbzrle transferred: %" PRIu64 " kbytes\n",
> >>>> +                           info->xbzrle_cache->xbzrle_bytes >> 10);
> >>>> +        }
> >>>> +        if (info->xbzrle_cache->has_xbzrle_pages) {
> >>>> +            monitor_printf(mon, "xbzrle pages: %" PRIu64 " pages\n",
> >>>> +                           info->xbzrle_cache->xbzrle_pages);
> >>>> +        }
> >>>> +        if (info->xbzrle_cache->has_xbzrle_cache_miss) {
> >>>> +            monitor_printf(mon, "xbzrle cache miss: %" PRIu64 "\n",
> >>>> +                           info->xbzrle_cache->xbzrle_cache_miss);
> >>>> +        }
> >>>> +        if (info->xbzrle_cache->has_xbzrle_overflow) {
> >>>> +            monitor_printf(mon, "xbzrle overflow : %" PRIu64 "\n",
> >>>> +                           info->xbzrle_cache->xbzrle_overflow);
> >>>> +        }
> >>>> +    }
> >>>> +
> >>>>      qapi_free_MigrationInfo(info);
> >>>>      qapi_free_MigrationParameters(params);
> >>>>  }
> >>>> diff --git a/migration.c b/migration.c
> >>>> index 4dc99ba..fb802bc 100644
> >>>> --- a/migration.c
> >>>> +++ b/migration.c
> >>>> @@ -136,6 +136,23 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
> >>>>      return params;
> >>>>  }
> >>>>  
> >>>> +static void get_xbzrle_cache_stats(MigrationInfo *info)
> >>>> +{
> >>>> +    if (migrate_use_xbzrle()) {
> >>>> +        info->has_xbzrle_cache = true;
> >>>> +        info->xbzrle_cache = g_malloc0(sizeof(*info->xbzrle_cache));
> >>>> +        info->xbzrle_cache->cache_size = migrate_xbzrle_cache_size();
> >>>> +        info->xbzrle_cache->has_xbzrle_bytes = true;
> >>>> +        info->xbzrle_cache->xbzrle_bytes = xbzrle_mig_bytes_transferred();
> >>>> +        info->xbzrle_cache->has_xbzrle_pages = true;
> >>>> +        info->xbzrle_cache->xbzrle_pages = xbzrle_mig_pages_transferred();
> >>>> +        info->xbzrle_cache->has_xbzrle_cache_miss = true;
> >>>> +        info->xbzrle_cache->xbzrle_cache_miss = xbzrle_mig_pages_cache_miss();
> >>>> +        info->xbzrle_cache->has_xbzrle_overflow = true;
> >>>> +        info->xbzrle_cache->xbzrle_overflow = xbzrle_mig_pages_overflow();
> >>>> +    }
> >>>> +}
> >>>> +
> >>>>  MigrationInfo *qmp_query_migrate(Error **errp)
> >>>>  {
> >>>>      MigrationInfo *info = g_malloc0(sizeof(*info));
> >>>> @@ -144,6 +161,13 @@ MigrationInfo *qmp_query_migrate(Error **errp)
> >>>>      switch (s->state) {
> >>>>      case MIG_STATE_SETUP:
> >>>>          /* no migration has happened ever */
> >>>> +
> >>>> +        /* display xbzrle cache size */
> >>>> +        if (migrate_use_xbzrle()) {
> >>>> +            info->has_xbzrle_cache = true;
> >>>> +            info->xbzrle_cache = g_malloc0(sizeof(*info->xbzrle_cache));
> >>>> +            info->xbzrle_cache->cache_size = migrate_xbzrle_cache_size();
> >>>> +        }
> >>>
> >>> Is it really useful to return this in MIG_SETUP? Can't the cache size
> >>> be queried by some query- command?
> >> I like it this way and it is very simple to use, you need to use query-migrate command if you do migration 
> >> anyway.
> > 
> > But this will only be returned before a migration takes place. Then it will
> > never be returned again. Doesn't seem to be the right place for it.
> It is returned also in MIG_ACTIVE and MIG_COMPLETE (when XBZRLE are active).

That's fine.

> > 
> >> Otherwise we will need to add a query command for each configurable parameters,
> >> so one for speed and another for downtime and probably more in the future.
> > 
> > I don't think this is a problem. We're trying to design QMP commands as we
> > would design a C API, and I'd have different functions to query different things.
> > 
> > Besides, let's not mix what's best to a human user and what's best for a
> > machine protocol. If we add different query commands for different parameters,
> > then it's possible to call them from 'info migrate' if you wish.
> sounds reasonable, I will change it.
> 
> Orit 
> > 
> >> This way we use one command to query the migration setup.
> > 
> > Again, that information would only be returned before a first migration
> > occurs.
> > 
> >>
> >> Orit
> >>>
> >>>>          break;
> >>>>      case MIG_STATE_ACTIVE:
> >>>>          info->has_status = true;
> >>>> @@ -166,8 +190,12 @@ MigrationInfo *qmp_query_migrate(Error **errp)
> >>>>              info->disk->remaining = blk_mig_bytes_remaining();
> >>>>              info->disk->total = blk_mig_bytes_total();
> >>>>          }
> >>>> +
> >>>> +        get_xbzrle_cache_stats(info);
> >>>>          break;
> >>>>      case MIG_STATE_COMPLETED:
> >>>> +        get_xbzrle_cache_stats(info);
> >>>> +
> >>>>          info->has_status = true;
> >>>>          info->status = g_strdup("completed");
> >>>>  
> >>>> diff --git a/migration.h b/migration.h
> >>>> index e4a7cd7..a9852fc 100644
> >>>> --- a/migration.h
> >>>> +++ b/migration.h
> >>>> @@ -91,6 +91,10 @@ 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);
> >>>> +uint64_t xbzrle_mig_bytes_transferred(void);
> >>>> +uint64_t xbzrle_mig_pages_transferred(void);
> >>>> +uint64_t xbzrle_mig_pages_overflow(void);
> >>>> +uint64_t xbzrle_mig_pages_cache_miss(void);
> >>>>  
> >>>>  /**
> >>>>   * @migrate_add_blocker - prevent migration from proceeding
> >>>> diff --git a/qapi-schema.json b/qapi-schema.json
> >>>> index a936714..91dee72 100644
> >>>> --- a/qapi-schema.json
> >>>> +++ b/qapi-schema.json
> >>>> @@ -275,6 +275,31 @@
> >>>>             'total_time': 'int', 'duplicate': 'int', 'normal': 'int' } }
> >>>>  
> >>>>  ##
> >>>> +# @XBZRLECacheStats
> >>>> +#
> >>>> +# Detailed XBZRLE migration cache statistics
> >>>> +#
> >>>> +# @cache-size: XBZRLE cache size
> >>>> +#
> >>>> +# @xbzrle-bytes: @optional, amount of bytes already transferred to the target VM
> >>>> +#                only returned when migration is active or completed
> >>>
> >>> s/xbzrle-bytes/transferred-bytes
> >>>
> >>>> +#
> >>>> +# @xbzrle-pages: @optional, amount of pages transferred to the target VM
> >>>> +#                only returned when migration is active or completed
> >>>
> >>> s/xbzrle-pages/transferred-pages
> >>>
> >>>> +#
> >>>> +# @xbzrle-cache-miss: @optional, number of cache miss
> >>>> +#                     only returned when migration is active or completed
> >>>
> >>> s/xbzrle/xbzrle//
> >>>
> >>>> +#
> >>>> +# @xbzrle-overflow: @optional, number of overflows
> >>>> +#                   only returned when migration is active or completed
> >>>
> >>> s/xbzrle///
> >>>
> >>> Besides, all these parameters shouldn't be optional as they are unconditionally
> >>> included (BZRLECacheStats itself is optional as returned by query-migrate).
> >>>
> >>>> +#
> >>>> +# Since: 1.2
> >>>> +##
> >>>> +{ 'type': 'XBZRLECacheStats',
> >>>> +  'data': {'cache-size': 'int', '*xbzrle-bytes': 'int', '*xbzrle-pages': 'int',
> >>>> +           '*xbzrle-cache-miss': 'int', '*xbzrle-overflow': 'int' } }
> >>>> +
> >>>> +##
> >>>>  # @MigrationInfo
> >>>>  #
> >>>>  # Information about current migration process.
> >>>> @@ -292,11 +317,16 @@
> >>>>  #        status, only returned if status is 'active' and it is a block
> >>>>  #        migration
> >>>>  #
> >>>> +# @xbzrle-cache: #optional @XBZRLECacheStats containing detailed XBZRLE
> >>>> +#                migration statistics, only returned if XBZRLE feature is on
> >>>> +#                (since 1.2)
> >>>> +#
> >>>>  # Since: 0.14.0
> >>>>  ##
> >>>>  { 'type': 'MigrationInfo',
> >>>>    'data': {'*status': 'str', '*ram': 'MigrationStats',
> >>>> -           '*disk': 'MigrationStats'} }
> >>>> +           '*disk': 'MigrationStats',
> >>>> +           '*xbzrle-cache': 'XBZRLECacheStats'} }
> >>>>  
> >>>>  ##
> >>>>  # @query-migrate
> >>>> diff --git a/qmp-commands.hx b/qmp-commands.hx
> >>>> index a5a67eb..0546f42 100644
> >>>> --- a/qmp-commands.hx
> >>>> +++ b/qmp-commands.hx
> >>>> @@ -2106,6 +2106,17 @@ The main json-object contains the following:
> >>>>           - "transferred": amount transferred (json-int)
> >>>>           - "remaining": amount remaining (json-int)
> >>>>           - "total": total (json-int)
> >>>> +- "xbzrle-cache": only present if XBZRLE is active.
> >>>> +  It is a json-object with the following XBZRLE information:
> >>>> +         - "cache-size": XBZRLE cache size
> >>>> +         - "xbzrle-bytes": total XBZRLE bytes transferred, only present if
> >>>> +                           status is "active" or "completed"
> >>>> +         - "xbzrle-pages": number of XBZRLE compressed pages, only present if
> >>>> +                           status is "active" or "completed"
> >>>> +         - "cache-miss": number of cache misses, only present if
> >>>> +                           status is "active" or "completed"
> >>>> +         - "overflow": number of XBZRLE overflows, only present if
> >>>> +                           status is "active" or "completed"
> >>>>  Examples:
> >>>>  
> >>>>  1. Before the first migration
> >>>> @@ -2170,6 +2181,30 @@ Examples:
> >>>>        }
> >>>>     }
> >>>>  
> >>>> +6. Migration is being performed and XBZRLE is active:
> >>>> +
> >>>> +-> { "execute": "query-migrate" }
> >>>> +<- {
> >>>> +      "return":{
> >>>> +         "status":"active",
> >>>> +         "capabilities" : [ { "capability": "xbzrle", "state" : true } ],
> >>>> +         "ram":{
> >>>> +            "total":1057024,
> >>>> +            "remaining":1053304,
> >>>> +            "transferred":3720,
> >>>> +            "duplicate": 10,
> >>>> +            "normal" : 3333
> >>>> +         },
> >>>> +         "cache":{
> >>>> +            "cache-size": 1024
> >>>> +            "xbzrle-transferred":20971520,
> >>>> +            "xbzrle-pages":2444343,
> >>>> +            "xbzrle-cache-miss":2244,
> >>>> +            "xbzrle-overflow":34434
> >>>> +         }
> >>>> +      }
> >>>> +   }
> >>>> +
> >>>>  EQMP
> >>>>  
> >>>>      {
> >>>
> >>
> > 
>
diff mbox

Patch

diff --git a/arch_init.c b/arch_init.c
index 7f12317..9833d54 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -203,7 +203,11 @@  int64_t xbzrle_cache_resize(int64_t new_size)
 typedef struct AccountingInfo {
     uint64_t dup_pages;
     uint64_t norm_pages;
+    uint64_t xbzrle_bytes;
+    uint64_t xbzrle_pages;
+    uint64_t xbzrle_cache_miss;
     uint64_t iterations;
+    uint64_t xbzrle_overflows;
 } AccountingInfo;
 
 static AccountingInfo acct_info;
@@ -233,6 +237,26 @@  uint64_t norm_mig_pages_transferred(void)
     return acct_info.norm_pages;
 }
 
+uint64_t xbzrle_mig_bytes_transferred(void)
+{
+    return acct_info.xbzrle_bytes;
+}
+
+uint64_t xbzrle_mig_pages_transferred(void)
+{
+    return acct_info.xbzrle_pages;
+}
+
+uint64_t xbzrle_mig_pages_cache_miss(void)
+{
+    return acct_info.xbzrle_cache_miss;
+}
+
+uint64_t xbzrle_mig_pages_overflow(void)
+{
+    return acct_info.xbzrle_overflows;
+}
+
 static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
         int cont, int flag)
 {
@@ -257,6 +281,7 @@  static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
     if (!cache_is_cached(XBZRLE.cache, current_addr)) {
         cache_insert(XBZRLE.cache, current_addr,
                      g_memdup(current_data, TARGET_PAGE_SIZE));
+        acct_info.xbzrle_cache_miss++;
         return -1;
     }
 
@@ -274,6 +299,7 @@  static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
         return 0;
     } else if (encoded_len == -1) {
         DPRINTF("Overflow\n");
+        acct_info.xbzrle_overflows++;
         /* update data in the cache */
         memcpy(prev_cached_page, current_data, TARGET_PAGE_SIZE);
         return -1;
@@ -288,6 +314,8 @@  static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
     qemu_put_be16(f, encoded_len);
     qemu_put_buffer(f, XBZRLE.encoded_buf, encoded_len);
     bytes_sent = encoded_len + 1 + 2;
+    acct_info.xbzrle_pages++;
+    acct_info.xbzrle_bytes += bytes_sent;
 
     return bytes_sent;
 }
diff --git a/hmp.c b/hmp.c
index 9770d7b..383d5b1 100644
--- a/hmp.c
+++ b/hmp.c
@@ -172,6 +172,27 @@  void hmp_info_migrate(Monitor *mon)
                        info->disk->total >> 10);
     }
 
+    if (info->has_xbzrle_cache) {
+        monitor_printf(mon, "cache size: %" PRIu64 " bytes\n",
+                       info->xbzrle_cache->cache_size);
+        if (info->xbzrle_cache->has_xbzrle_bytes) {
+            monitor_printf(mon, "xbzrle transferred: %" PRIu64 " kbytes\n",
+                           info->xbzrle_cache->xbzrle_bytes >> 10);
+        }
+        if (info->xbzrle_cache->has_xbzrle_pages) {
+            monitor_printf(mon, "xbzrle pages: %" PRIu64 " pages\n",
+                           info->xbzrle_cache->xbzrle_pages);
+        }
+        if (info->xbzrle_cache->has_xbzrle_cache_miss) {
+            monitor_printf(mon, "xbzrle cache miss: %" PRIu64 "\n",
+                           info->xbzrle_cache->xbzrle_cache_miss);
+        }
+        if (info->xbzrle_cache->has_xbzrle_overflow) {
+            monitor_printf(mon, "xbzrle overflow : %" PRIu64 "\n",
+                           info->xbzrle_cache->xbzrle_overflow);
+        }
+    }
+
     qapi_free_MigrationInfo(info);
     qapi_free_MigrationParameters(params);
 }
diff --git a/migration.c b/migration.c
index 4dc99ba..fb802bc 100644
--- a/migration.c
+++ b/migration.c
@@ -136,6 +136,23 @@  MigrationParameters *qmp_query_migrate_parameters(Error **errp)
     return params;
 }
 
+static void get_xbzrle_cache_stats(MigrationInfo *info)
+{
+    if (migrate_use_xbzrle()) {
+        info->has_xbzrle_cache = true;
+        info->xbzrle_cache = g_malloc0(sizeof(*info->xbzrle_cache));
+        info->xbzrle_cache->cache_size = migrate_xbzrle_cache_size();
+        info->xbzrle_cache->has_xbzrle_bytes = true;
+        info->xbzrle_cache->xbzrle_bytes = xbzrle_mig_bytes_transferred();
+        info->xbzrle_cache->has_xbzrle_pages = true;
+        info->xbzrle_cache->xbzrle_pages = xbzrle_mig_pages_transferred();
+        info->xbzrle_cache->has_xbzrle_cache_miss = true;
+        info->xbzrle_cache->xbzrle_cache_miss = xbzrle_mig_pages_cache_miss();
+        info->xbzrle_cache->has_xbzrle_overflow = true;
+        info->xbzrle_cache->xbzrle_overflow = xbzrle_mig_pages_overflow();
+    }
+}
+
 MigrationInfo *qmp_query_migrate(Error **errp)
 {
     MigrationInfo *info = g_malloc0(sizeof(*info));
@@ -144,6 +161,13 @@  MigrationInfo *qmp_query_migrate(Error **errp)
     switch (s->state) {
     case MIG_STATE_SETUP:
         /* no migration has happened ever */
+
+        /* display xbzrle cache size */
+        if (migrate_use_xbzrle()) {
+            info->has_xbzrle_cache = true;
+            info->xbzrle_cache = g_malloc0(sizeof(*info->xbzrle_cache));
+            info->xbzrle_cache->cache_size = migrate_xbzrle_cache_size();
+        }
         break;
     case MIG_STATE_ACTIVE:
         info->has_status = true;
@@ -166,8 +190,12 @@  MigrationInfo *qmp_query_migrate(Error **errp)
             info->disk->remaining = blk_mig_bytes_remaining();
             info->disk->total = blk_mig_bytes_total();
         }
+
+        get_xbzrle_cache_stats(info);
         break;
     case MIG_STATE_COMPLETED:
+        get_xbzrle_cache_stats(info);
+
         info->has_status = true;
         info->status = g_strdup("completed");
 
diff --git a/migration.h b/migration.h
index e4a7cd7..a9852fc 100644
--- a/migration.h
+++ b/migration.h
@@ -91,6 +91,10 @@  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);
+uint64_t xbzrle_mig_bytes_transferred(void);
+uint64_t xbzrle_mig_pages_transferred(void);
+uint64_t xbzrle_mig_pages_overflow(void);
+uint64_t xbzrle_mig_pages_cache_miss(void);
 
 /**
  * @migrate_add_blocker - prevent migration from proceeding
diff --git a/qapi-schema.json b/qapi-schema.json
index a936714..91dee72 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -275,6 +275,31 @@ 
            'total_time': 'int', 'duplicate': 'int', 'normal': 'int' } }
 
 ##
+# @XBZRLECacheStats
+#
+# Detailed XBZRLE migration cache statistics
+#
+# @cache-size: XBZRLE cache size
+#
+# @xbzrle-bytes: @optional, amount of bytes already transferred to the target VM
+#                only returned when migration is active or completed
+#
+# @xbzrle-pages: @optional, amount of pages transferred to the target VM
+#                only returned when migration is active or completed
+#
+# @xbzrle-cache-miss: @optional, number of cache miss
+#                     only returned when migration is active or completed
+#
+# @xbzrle-overflow: @optional, number of overflows
+#                   only returned when migration is active or completed
+#
+# Since: 1.2
+##
+{ 'type': 'XBZRLECacheStats',
+  'data': {'cache-size': 'int', '*xbzrle-bytes': 'int', '*xbzrle-pages': 'int',
+           '*xbzrle-cache-miss': 'int', '*xbzrle-overflow': 'int' } }
+
+##
 # @MigrationInfo
 #
 # Information about current migration process.
@@ -292,11 +317,16 @@ 
 #        status, only returned if status is 'active' and it is a block
 #        migration
 #
+# @xbzrle-cache: #optional @XBZRLECacheStats containing detailed XBZRLE
+#                migration statistics, only returned if XBZRLE feature is on
+#                (since 1.2)
+#
 # Since: 0.14.0
 ##
 { 'type': 'MigrationInfo',
   'data': {'*status': 'str', '*ram': 'MigrationStats',
-           '*disk': 'MigrationStats'} }
+           '*disk': 'MigrationStats',
+           '*xbzrle-cache': 'XBZRLECacheStats'} }
 
 ##
 # @query-migrate
diff --git a/qmp-commands.hx b/qmp-commands.hx
index a5a67eb..0546f42 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2106,6 +2106,17 @@  The main json-object contains the following:
          - "transferred": amount transferred (json-int)
          - "remaining": amount remaining (json-int)
          - "total": total (json-int)
+- "xbzrle-cache": only present if XBZRLE is active.
+  It is a json-object with the following XBZRLE information:
+         - "cache-size": XBZRLE cache size
+         - "xbzrle-bytes": total XBZRLE bytes transferred, only present if
+                           status is "active" or "completed"
+         - "xbzrle-pages": number of XBZRLE compressed pages, only present if
+                           status is "active" or "completed"
+         - "cache-miss": number of cache misses, only present if
+                           status is "active" or "completed"
+         - "overflow": number of XBZRLE overflows, only present if
+                           status is "active" or "completed"
 Examples:
 
 1. Before the first migration
@@ -2170,6 +2181,30 @@  Examples:
       }
    }
 
+6. Migration is being performed and XBZRLE is active:
+
+-> { "execute": "query-migrate" }
+<- {
+      "return":{
+         "status":"active",
+         "capabilities" : [ { "capability": "xbzrle", "state" : true } ],
+         "ram":{
+            "total":1057024,
+            "remaining":1053304,
+            "transferred":3720,
+            "duplicate": 10,
+            "normal" : 3333
+         },
+         "cache":{
+            "cache-size": 1024
+            "xbzrle-transferred":20971520,
+            "xbzrle-pages":2444343,
+            "xbzrle-cache-miss":2244,
+            "xbzrle-overflow":34434
+         }
+      }
+   }
+
 EQMP
 
     {