Re: [Qemu-devel] [PATCH] migration: Improve QMP documentation

[Markus Armbruster]

Juan Quintela <address@hidden> writes:

> Signed-off-by: Juan Quintela <address@hidden>
> ---
>  qmp-commands.hx | 50 ++++++++++++++++++++++++++++++--------------------
>  1 file changed, 30 insertions(+), 20 deletions(-)
>
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 799adea..6d037bd 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -644,7 +644,7 @@ EQMP
>
>  SQMP
>  migrate-set-cache-size
> ----------------------
> +----------------------
>
>  Set cache size to be used by XBZRLE migration, the cache size will be rounded
>  down to the nearest power of 2
> @@ -667,7 +667,7 @@ EQMP
>
>  SQMP
>  query-migrate-cache-size
> ----------------------
> +------------------------
>
>  Show cache size to be used by XBZRLE migration
>
> @@ -2419,6 +2419,7 @@ SQMP
>  query-migrate
>  -------------
>
> +
>  Migration status.
>
>  Return a json-object. If migration is active there will be another 
> json-object

Extra blank line; please drop this hunk.

> @@ -2438,25 +2439,34 @@ The main json-object contains the following:
>                  total amount in ms for downtime that was calculated on
>               the last bitmap round (json-int)
>  - "ram": only present if "status" is "active", it is a json-object with the
> -  following RAM information (in bytes):
> -         - "transferred": amount transferred (json-int)
> -         - "remaining": amount remaining (json-int)
> -         - "total": total (json-int)
> -         - "duplicate": number of duplicated pages (json-int)
> -         - "normal" : number of normal pages transferred (json-int)
> -         - "normal-bytes" : number of normal bytes transferred (json-int)
> +  following RAM information:
> +         - "transferred": amount transferred in bytes (json-int)
> +         - "remaining": amount remaining to transfer in bytes (json-int)
> +         - "total": total amount of memory in bytes (json-int)
> +         - "duplicate": number of pages containing the same byte. They
> +            are sent over the wire as a single byte (json-int)

I'm confused.  Do you mean pages that are entirely filled with the same
byte?  Or pages with contents identical to some other, non-duplicate
page?

Sure we want to promise these are sent as a single byte?

> +         - "normal" : number of whole pages transfered.  I.e. they
> +            were not sent as duplicate or xbzrle pages (json-int)
> +         - "normal-bytes" : number of bytes transferred in whole
> +            pages. This is just normal pages times size of one page,
> +            but this way upper levels don't need to care about page
> +            size (json-int)

Begs the question who wants "normal" then.

Why don't "upper levels" want duplicate-bytes, too?

A page is either sent normally (and counted in "normal"), or as
duplicate (and counted in "duplicate"), or XBZRLE compressed.  Correct?

>  - "disk": only present if "status" is "active" and it is a block migration,
> -  it is a json-object with the following disk information (in bytes):
> -         - "transferred": amount transferred (json-int)
> -         - "remaining": amount remaining (json-int)
> -         - "total": total (json-int)
> +  it is a json-object with the following disk information:
> +         - "transferred": amount transferred in bytes (json-int)
> +         - "remaining": amount remaining to transfer in bytes json-int)
> +         - "total": total disk amount in bytes (json-int)

"disk amount" sounds weird.  "disk size"?  "size of disk"?

>  - "xbzrle-cache": only present if XBZRLE is active.
>    It is a json-object with the following XBZRLE information:
> -         - "cache-size": XBZRLE cache size
> -         - "bytes": total XBZRLE bytes transferred
> +         - "cache-size": XBZRLE cache size in bytes
> +         - "bytes": total XBZRLE bytes transferred as xbzrle pages

Is this the number of bytes before or after compression?

>           - "pages": number of XBZRLE compressed pages

If "bytes" is after compression: why don't "upper levels" want #bytes
before compression, like they want "normal-bytes" rather than "normal"?

> -         - "cache-miss": number of cache misses
> -         - "overflow": number of XBZRLE overflows
> +         - "cache-miss": number of XBRZRLE page cache misses
> +         - "overflow": number of times XBZRLE overflows.  This means
> +           that the XBZRLE encoding was bigger than just sent the
> +           whole page, and then we sent the whole page instead (as as
> +           normal page).
> +
>  Examples:
>
>  1. Before the first migration
> @@ -2567,11 +2577,11 @@ EQMP
>
>  SQMP
>  migrate-set-capabilities
> --------
> +------------------------
>
>  Enable/Disable migration capabilities
>
> -- "xbzrle": xbzrle support
> +- "xbzrle": XBZRLE support
>
>  Arguments:
>

Extra points for attention to detail :)

> @@ -2590,7 +2600,7 @@ EQMP
>      },
>  SQMP
>  query-migrate-capabilities
> --------
> +--------------------------
>
>  Query current migration capabilities