Re: [PATCH QEMU v23 18/18] qapi: Add VFIO devices migration stats in Migration stats

[Markus Armbruster <armbru@redhat.com>]

Kirti Wankhede <kwankhede@nvidia.com> writes:

> Added amount of bytes transferred to the target VM by all VFIO devices
>
> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>

QAPI review only.

> diff --git a/qapi/migration.json b/qapi/migration.json
> index d5000558c6c9..acbc42a1efe8 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -146,6 +146,18 @@
>              'active', 'postcopy-active', 'postcopy-paused',
>              'postcopy-recover', 'completed', 'failed', 'colo',
>              'pre-switchover', 'device', 'wait-unplug' ] }
> +##
> +# @VfioStats:
> +#
> +# Detailed VFIO devices migration statistics
> +#
> +# @bytes: amount of bytes transferred to the target VM by VFIO devices
> +#
> +# Since: 5.1
> +#
> +##
> +{ 'struct': 'VfioStats',
> +  'data': {'bytes': 'int' } }

Pardon my ignorance...  What exactly do VFIO devices transfer to the
target VM?  How is that related to MigrationInfo member @ram?

For what it's worth, MigrationStats has

   # @transferred: amount of bytes already transferred to the target VM

Name yours @transferred as well, for consistency?

MigrationStats has much more information, and some of it is pretty
useful to track how migration is doing, in particular whether it
converges, and how fast.  Absent in VfioStats due to "not implemented",
or due to "can't be done"?

Byte counts should use QAPI type 'size'.  Many existing ones don't.
Since MigrationStats uses 'int', I'll let the migration maintainers
decide whether they want 'int' or 'size' here.

>  
>  ##
>  # @MigrationInfo:
> @@ -207,6 +219,10 @@
>  #
>  # @socket-address: Only used for tcp, to know what the real port is (Since 4.0)
>  #
> +# @vfio: @VfioStats containing detailed VFIO devices migration statistics,
> +#        only returned if VFIO device is present, migration is supported by all
> +#         VFIO devices and status is 'active' or 'completed' (since 5.1)
> +#
>  # Since: 0.14.0
>  ##
>  { 'struct': 'MigrationInfo',
> @@ -222,7 +238,8 @@
     'data': {'*status': 'MigrationStatus', '*ram': 'MigrationStats',
              '*disk': 'MigrationStats',
              '*xbzrle-cache': 'XBZRLECacheStats',
              '*total-time': 'int',
              '*expected-downtime': 'int',
              '*downtime': 'int',
              '*setup-time': 'int',
              '*cpu-throttle-percentage': 'int',
              '*error-desc': 'str',
>             '*postcopy-blocktime' : 'uint32',
>             '*postcopy-vcpu-blocktime': ['uint32'],
>             '*compression': 'CompressionStats',
> -           '*socket-address': ['SocketAddress'] } }
> +           '*socket-address': ['SocketAddress'],
> +           '*vfio': 'VfioStats' } }
>  
>  ##
>  # @query-migrate:

If @vfio and @ram are related, add @vfio next to @ram and @disk, please.