Patchwork migration: Improve QMP documentation

login
register
mail settings
Submitter Juan Quintela
Date Feb. 22, 2013, 10:38 a.m.
Message ID <1361529519-15839-1-git-send-email-quintela@redhat.com>
Download mbox | patch
Permalink /patch/222511/
State New
Headers show

Comments

Juan Quintela - Feb. 22, 2013, 10:38 a.m.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 qmp-commands.hx | 50 ++++++++++++++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 20 deletions(-)
Markus Armbruster - Feb. 22, 2013, 1:03 p.m.
Juan Quintela <quintela@redhat.com> writes:

> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  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
Juan Quintela - Feb. 22, 2013, 1:34 p.m.
Markus Armbruster <armbru@redhat.com> wrote:
> Juan Quintela <quintela@redhat.com> 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.

>
>> +         - "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.

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

duplicate-bytes == dupplicate, by definition.  for each duplicate page,
we just sent one byte.

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

Yeap.

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

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

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

>
>> -         - "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 :)

Emacs regexp was invented to do this kind of things, no? O:-)

Thanks, Juan.
>
>> @@ -2590,7 +2600,7 @@ EQMP
>>      },
>>  SQMP
>>  query-migrate-capabilities
>> --------
>> +--------------------------
>>
>>  Query current migration capabilities
Markus Armbruster - Feb. 22, 2013, 4:18 p.m.
Juan Quintela <quintela@redhat.com> writes:

> Markus Armbruster <armbru@redhat.com> wrote:
>> Juan Quintela <quintela@redhat.com> 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
[...]
Juan Quintela - Feb. 22, 2013, 4:40 p.m.
Markus Armbruster <armbru@redhat.com> wrote:
> Juan Quintela <quintela@redhat.com> writes:
>
>>> 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.

name is older than my involvement with migration.  And it is externally
visible, so we can't change them.

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

ok.  Changing to it.

>>>> +         - "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")?

we can't remove values.  And we have to say the amount on bytes because
we don't export anywhere what is the guest page_size.  Otherwise libvirt
would need to make tricks.  If I would be designing this today, I would
just add a field:

page_size: in bytes

And just sent everything else in pages.  Middleware can do any
calculation that they want then.

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

As said, historical reasons.

>>> 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?

As said, upper levels don't know the page size.
And for the case of duplicate'd pages, we sent "one byte" for each page,
so "duplicate" and "duplicate-bytes" would be exactly the same number.

I know that you like this "optimization" O:-)

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

Thanks for the review.

Patch

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
@@ -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)
+         - "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)
 - "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)
 - "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
          - "pages": number of XBZRLE compressed pages
-         - "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:

@@ -2590,7 +2600,7 @@  EQMP
     },
 SQMP
 query-migrate-capabilities
--------
+--------------------------

 Query current migration capabilities