Re: [PATCH 1/2] migration/calc-dirty-rate: new metrics in sampling mode

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Feb 28, 2023 at 04:16:02PM +0300, Andrei Gudkov via wrote:
> * Collect number of all-zero pages
> * Collect vector of number of dirty pages for different time periods
> * Report total number of pages, number of sampled pages and page size
> * Replaced CRC32 with xxHash for performance reasons

I'd suggest that the CRC32 -> xxHash change should be a separate
commit from the newly reported statistics, since they're independant
functional changes.

> 
> Signed-off-by: Andrei Gudkov <gudkov.andrei@huawei.com>
> ---
>  migration/dirtyrate.c | 219 +++++++++++++++++++++++++++++++++---------
>  migration/dirtyrate.h |  26 ++++-
>  qapi/migration.json   |  25 +++++
>  3 files changed, 218 insertions(+), 52 deletions(-)

> diff --git a/qapi/migration.json b/qapi/migration.json
> index c84fa10e86..1a1d7cb30a 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1830,6 +1830,25 @@
>  # @mode: mode containing method of calculate dirtyrate includes
>  #        'page-sampling' and 'dirty-ring' (Since 6.2)
>  #
> +# @page-size: page size in bytes
> +#
> +# @n-total-pages: total number of VM pages
> +#
> +# @n-sampled-pages: number of sampled pages
> +#
> +# @n-zero-pages: number of observed zero pages among all sampled pages.
> +#                Normally all pages are zero when VM starts, but
> +#                their number progressively goes down as VM fills more
> +#                and more memory with useful data.
> +#                Migration of zero pages is optimized: only their headers
> +#                are copied but not the (zero) data.
> +#
> +# @periods: array of time periods expressed in milliseconds for which
> +#           dirty-sample measurements are collected
> +#
> +# @n-dirty-pages: number of pages among all sampled pages that were observed
> +#                 as changed after respective time period
> +#

Each field addition needs a "(Since ....)" tag with QEMU version

The docs probably ought to be explicit that the size of @periods
array is the same as @n-dirty-pages array.

>  # @vcpu-dirty-rate: dirtyrate for each vcpu if dirty-ring
>  #                   mode specified (Since 6.2)
>  #
> @@ -1842,6 +1861,12 @@
>             'calc-time': 'int64',
>             'sample-pages': 'uint64',
>             'mode': 'DirtyRateMeasureMode',
> +           'page-size': 'int64',
> +           '*n-total-pages': 'int64',
> +           '*n-sampled-pages': 'int64',
> +           '*n-zero-pages': 'int64',
> +           '*periods': ['int64'],
> +           '*n-dirty-pages': ['int64'],
>             '*vcpu-dirty-rate': [ 'DirtyRateVcpu' ] } }
>  
>  ##
> -- 
> 2.30.2
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|