Re: [Qemu-devel] [PATCH v12 15/15] rdma: account for the time spent in MIG_STATE_SETUP through QMP

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2240 bytes --]

On 06/25/2013 07:35 PM, mrhines@linux.vnet.ibm.com wrote:
> From: "Michael R. Hines" <mrhines@us.ibm.com>
> 
> Using the previous patches, we're now able to timestamp the SETUP
> state. Once we have this time, let the user know about it in the
> schema.
> 
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> Signed-off-by: Michael R. Hines <mrhines@us.ibm.com>

Usually, Reviewed-by lines are listed _after_ S-o-b lines - signature
lines are typically chronological, but the patch has to be signed before
a review can have any weight at getting the patch into a pull request :)

> ---
>  hmp.c                         |    4 ++++
>  include/migration/migration.h |    1 +
>  migration.c                   |    9 +++++++++
>  qapi-schema.json              |    9 ++++++++-
>  4 files changed, 22 insertions(+), 1 deletion(-)
> 

> +++ b/qapi-schema.json
> @@ -578,6 +578,12 @@
>  #        expected downtime in milliseconds for the guest in last walk
>  #        of the dirty bitmap. (since 1.3)
>  #
> +# @setup-time: #optional amount of setup time spent _before_ the iterations
> +#        begin but _after_ the QMP command is issued. This is designed to

In what units?  One can easily assume milliseconds, based on the docs of
other elapsed time parameters, but being explicit never hurts.

> +#        provide an accounting of any activities (such as RDMA pinning) which
> +#        may be expensive, but do not actually occur during the iterative
> +#        migration rounds themselves. (since 1.6)
> +#
>  # Since: 0.14.0
>  ##
>  { 'type': 'MigrationInfo',
> @@ -586,7 +592,8 @@
>             '*xbzrle-cache': 'XBZRLECacheStats',
>             '*total-time': 'int',
>             '*expected-downtime': 'int',
> -           '*downtime': 'int'} }
> +           '*downtime': 'int',
> +           '*setup-time' : 'int'} }

We typically don't have space before ':' (as seen in the other lines
just above).  I can live with the patch as-is, but if you respin the
series for other things, then fix those two things before adding

Reviewed-by: Eric Blake <eblake@redhat.com>

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]