Cc: correct libvirt list address
Peter Xu <peterx@redhat.com> writes:
> This stats is only about RAM, so make it accurate. This paves way for
> statistics for all devices.
>
> Note that even if this is part of qapi/, this should not be part of ABI of
docs/devel/qapi-code-gen.rst section "Compatibility considerations":
Since type names are not visible in the Client JSON Protocol, types
may be freely renamed. Even certain refactorings are invisible, such
as splitting members from one type into a common base type.
So, s/should not be/is not/.
> at least query-migrate, because the structure is not changed, and this
> stats is always reported only under the "ram" section.
Why "at least query-migrate"? Do you have other interfaces in mind, or
are you just hedging?
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Libvirt Mailing List <libvir-list@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> docs/about/removed-features.rst | 2 +-
> qapi/migration.json | 8 ++++----
> migration/migration-stats.h | 2 +-
> 3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
> index 6f4447993c..7c4f4325f7 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -699,7 +699,7 @@ was superseded by ``sections``.
> ``query-migrate`` return value member ``skipped`` (removed in 9.1)
> ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
>
> -Member ``skipped`` of the ``MigrationStats`` struct hasn't been used
> +Member ``skipped`` of the ``MigrationRAMStats`` struct hasn't been used
> for more than 10 years. Removed with no replacement.
Could simply use "of the return value". But this is fine, too.
>
> ``migrate`` command option ``inc`` (removed in 9.1)
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 7134d4ce47..cfc6ccee26 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -12,7 +12,7 @@
> { 'include': 'sockets.json' }
>
> ##
> -# @MigrationStats:
> +# @MigrationRAMStats:
> #
> # Detailed migration status.
> #
> @@ -64,7 +64,7 @@
> #
> # Since: 0.14
> ##
> -{ 'struct': 'MigrationStats',
> +{ 'struct': 'MigrationRAMStats',
> 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
> 'duplicate': 'int',
> 'normal': 'int',
> @@ -209,7 +209,7 @@
> # If this field is not returned, no migration process has been
> # initiated
> #
> -# @ram: `MigrationStats` containing detailed migration status, only
> +# @ram: `MigrationRAMStats` containing detailed migration status, only
> # returned if status is 'active' or 'completed'(since 1.2)
This gets rendered like
* ram (MigrationStats, *optional*) -- MigrationStats
containing detailed migration status, only returned if status
is 'active' or 'completed'(since 1.2)
Recommend to rephrase to avoid the repetition. Maybe
# @ram: Detailed migration RAM status, only returned if status is
# 'active' or 'completed' (since 1.2)
Also consider replacing "status" by "statistics".
> #
> # @xbzrle-cache: `XBZRLECacheStats` containing detailed XBZRLE
> @@ -309,7 +309,7 @@
> # Since: 0.14
> ##
> { 'struct': 'MigrationInfo',
> - 'data': {'*status': 'MigrationStatus', '*ram': 'MigrationStats',
> + 'data': {'*status': 'MigrationStatus', '*ram': 'MigrationRAMStats',
> '*vfio': 'VfioStats',
> '*xbzrle-cache': 'XBZRLECacheStats',
> '*total-time': 'int',
> diff --git a/migration/migration-stats.h b/migration/migration-stats.h
> index c0f50144c9..1153520f7a 100644
> --- a/migration/migration-stats.h
> +++ b/migration/migration-stats.h
> @@ -27,7 +27,7 @@
>
> /*
> * These are the ram migration statistic counters. It is loosely
> - * based on MigrationStats.
> + * based on MigrationRAMStats.
> */
> typedef struct {
> /*
Only minor issues, and none added by this patch, so
Acked-by: Markus Armbruster <armbru@redhat.com>