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

[Markus Armbruster]

Juan Quintela <address@hidden> writes:

> Markus Armbruster <address@hidden> wrote:
>> Juan Quintela <address@hidden> writes:
>>
>>>  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.
>
> thanks.
>
>>
>>> @@ -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?
>
> It is a page full of zeros.  Yes, we can have pages full of any other
> char, they just haven't happened on my testing ever.
>
>> Sure we want to promise these are sent as a single byte?
>
> It is a binary format, we need to be incompatible to do it.

"duplicate" is a badly misleading name.  I'm not asking you to change it
in this patch.

What about:

         - "duplicate": number of pages filled entirely with the same
           byte (json-int)
           These are sent over the wire much more efficiently.

>>> +         - "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.
>
> Normal: we sent the whole page. 4096 bytes load on x86 (and almost
> everything else).  "full" would be a better name.

I'm afraid I was too terse, let me retry.  If "upper levels" want the
amount of memory sent as whole pages as number of bytes
("normal-bytes"), who wants the same information as number of pages
("normal")?

I guess the answer is "no idea, I'm just trying to make the docs suck
less."  And that's fair.

>> Why don't "upper levels" want duplicate-bytes, too?
>
> duplicate-bytes == dupplicate, by definition.  for each duplicate page,
> we just sent one byte.

Retry: if "upper levels" want the amount of memory sent as whole pages
as number of bytes rather than as number of pages, why are they happy to
get the amount of memory sent more efficiently as "duplicates" in number
of pages rather than number of bytes?

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

Thanks!

>>>  - "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"?
>
> you are right.  Changing this.
>
>>
>>>  - "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?
>
> After compression.

What about:

         - "bytes": number of bytes transferred for XBZRLE compressed pages

>>>           - "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"?
>
> Dunno.  This are the values that existed already.  I just tried to
> improve the docs.

Fair enough, and I'm not asking you to widen the scope of this patch
beyond that.

> You know the page size with:
>
> normal-bytes/normal = page_size
>
> now you can calculate: pages * page_size = size needed to send pages
> without xbzrle
>            xbzrle-cache.bytes: real size transferred

Yes.  Assumes a single page size.

>>> -         - "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
[...]