Re: [Qemu-devel] [PATCH v5 9/9] Add XBRLE statistics information

[Michael Roth <mdroth@linux.vnet.ibm.com>]

On 01/03/2012 09:34 AM, Orit Wasserman wrote:
> Signed-off-by: Orit Wasserman<owasserm@redhat.com>
> ---
>   arch_init.c |   67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   migration.c |   11 +++++++++
>   migration.h |    9 ++++++++
>   3 files changed, 87 insertions(+), 0 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 6b839a1..037d8ba 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -149,6 +149,65 @@ void arch_set_params(int blk_enable, int shared_base, int use_xbrle,
>   }
>
>   /***********************************************************/
> +/* accounting */
> +typedef struct AccountingInfo {
> +    uint64_t dup_pages;
> +    uint64_t norm_pages;
> +    uint64_t xbrle_bytes;
> +    uint64_t xbrle_pages;
> +    uint64_t xbrle_overflow;
> +    uint64_t xbrle_cache_miss;
> +    uint64_t iterations;
> +} AccountingInfo;
> +
> +static AccountingInfo acct_info;
> +
> +static void acct_clear(void)
> +{
> +    bzero(&acct_info, 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;
> +}
> +
> +uint64_t xbrle_mig_bytes_transferred(void)
> +{
> +    return acct_info.xbrle_bytes;
> +}
> +
> +uint64_t xbrle_mig_pages_transferred(void)
> +{
> +    return acct_info.xbrle_pages;
> +}
> +
> +uint64_t xbrle_mig_pages_overflow(void)
> +{
> +    return acct_info.xbrle_overflow;
> +}
> +
> +uint64_t xbrle_mig_pages_cache_miss(void)
> +{
> +    return acct_info.xbrle_cache_miss;
> +}
> +
> +/***********************************************************/
>   /* XBRLE (Xor Based Run-Length Encoding) */
>   typedef struct XBRLEHeader {
>       uint8_t xh_flags;
> @@ -376,6 +435,7 @@ static int save_xbrle_page(QEMUFile *f, uint8_t *current_data,
>       /* get location */
>       slot = cache_is_cached(current_addr);
>       if (slot == -1) {
> +        acct_info.xbrle_cache_miss++;
>           goto done;
>       }
>       cache_location = cache_get_cache_pos(current_addr);
> @@ -394,6 +454,7 @@ static int save_xbrle_page(QEMUFile *f, uint8_t *current_data,
>
>       if (encoded_len<  0) {
>           DPRINTF("XBRLE encoding oeverflow - sending uncompressed\n");

*overflow

> +        acct_info.xbrle_overflow++;
>           goto done;
>       }
>
> @@ -404,7 +465,9 @@ static int save_xbrle_page(QEMUFile *f, uint8_t *current_data,
>       save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_XBRLE);
>       qemu_put_buffer(f, (uint8_t *)&hdr, sizeof(hdr));
>       qemu_put_buffer(f, xbrle_buf, encoded_len);
> +    acct_info.xbrle_pages++;
>       bytes_sent = encoded_len + sizeof(hdr);
> +    acct_info.xbrle_bytes += bytes_sent;
>
>   done:
>       g_free(xor_buf);
> @@ -457,6 +520,7 @@ static int ram_save_block(QEMUFile *f, int stage)
>                   save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_COMPRESS);
>                   qemu_put_byte(f, *p);
>                   bytes_sent = 1;
> +                acct_info.dup_pages++;
>               } else if (stage == 2&&  arch_mig_state.use_xbrle) {
>                   bytes_sent = save_xbrle_page(f, p, current_addr, block,
>                       offset, cont);
> @@ -465,6 +529,7 @@ static int ram_save_block(QEMUFile *f, int stage)
>                   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 (arch_mig_state.use_xbrle) {
>                   cache_insert(current_addr, p);
> @@ -596,6 +661,7 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
>
>           if (arch_mig_state.use_xbrle) {
>               cache_init(arch_mig_state.xbrle_cache_size);
> +            acct_clear();
>           }
>
>           /* Make sure all dirty bits are set */
> @@ -629,6 +695,7 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
>
>           bytes_sent = ram_save_block(f, stage);
>           bytes_transferred += bytes_sent;
> +        acct_info.iterations++;
>           if (bytes_sent == 0) { /* no more blocks */
>               break;
>           }
> diff --git a/migration.c b/migration.c
> index 3d88cdd..383ceef 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -141,6 +141,17 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>               info->disk->remaining = blk_mig_bytes_remaining();
>               info->disk->total = blk_mig_bytes_total();
>           }
> +
> +       if (s->use_xbrle) {
> +            info->has_xbrle = true;
> +            info->cache = g_malloc0(sizeof(*info->cache));
> +            info->cache->dup_pages = dup_mig_pages_transferred();
> +            info->cache->norm_pages = norm_mig_pages_transferred();
> +            info->cache->xbrle_bytes  = xbrle_mig_bytes_transferred();
> +            info->cache->xbrle_pages  = xbrle_mig_pages_transferred();
> +            info->cache->xbrle_overflow = xbrle_mig_pages_overflow();
> +            info->cache->xbrle_cache_miss = xbrle_mig_pages_cache_miss();
> +        }

Hmm, I'm not seeing where you added the has_xbrle/cache fields to 
MigrationInfo? Are you missing your qapi-schema.json diff?

Also, I think the dup/norm stats are generally useful and don't appear 
to be dependent on xbrle having been used, so maybe just the xbrle_* 
fields should be filtered based on s->use_xbrle? And hang 
dup_pages/norm_pages off of info directly?

>           break;
>       case MIG_STATE_COMPLETED:
>           info->has_status = true;
> diff --git a/migration.h b/migration.h
> index 6de09c8..dd06ef6 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -81,6 +81,15 @@ uint64_t ram_bytes_remaining(void);
>   uint64_t ram_bytes_transferred(void);
>   uint64_t ram_bytes_total(void);
>
> +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 xbrle_mig_bytes_transferred(void);
> +uint64_t xbrle_mig_pages_transferred(void);
> +uint64_t xbrle_mig_pages_overflow(void);
> +uint64_t xbrle_mig_pages_cache_miss(void);
> +
>   int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque);
>   int ram_load(QEMUFile *f, void *opaque, int version_id);
>