Re: [Qemu-devel] [PATCH v12 12/13] Add set_cachesize command

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2867 bytes --]

On 06/19/2012 09:43 AM, Orit Wasserman wrote:
> Change XBZRLE cache size in bytes (the size should be a power of 2).
> If XBZRLE cache size is too small there will be many cache miss.
> 
> Signed-off-by: Benoit Hudzia <benoit.hudzia@sap.com>
> Signed-off-by: Petter Svard <petters@cs.umu.se>
> Signed-off-by: Aidan Shribman <aidan.shribman@sap.com>
> Signed-off-by: Orit Wasserman <owasserm@redhat.com>
> ---

> +++ b/arch_init.c
> @@ -24,6 +24,7 @@
>  #include <stdint.h>
>  #include <stdarg.h>
>  #include <stdlib.h>
> +#include <math.h>

What for?


> +
> +    {
> +        .name       = "migrate_set_cachesize",
> +        .args_type  = "value:o",
> +        .params     = "value",
> +        .help       = "set cache size (in bytes) for XBZRLE migrations,"
> +		      "the cache size will be round down to the nearest power of 2.\n"

s/round/rounded/

> +		      "The cache size effects the number of cache misses."

s/effects/affects/


> +++ b/migration.c
> @@ -22,6 +22,7 @@
>  #include "qemu_socket.h"
>  #include "block-migration.h"
>  #include "qmp-commands.h"
> +#include <math.h>

What for?

>  
>  //#define DEBUG_MIGRATION
>  
> @@ -43,7 +44,7 @@ enum {
>  
>  #define MAX_THROTTLE  (32 << 20)      /* Migration speed throttling */
>  
> -/* Migration XBZRLE cache size */
> +/* Migration XBZRLE default cache size */

Squash this change into the earlier patch that introduced the #define.

>  #define DEFAULT_MIGRATE_CACHE_SIZE (64 * 1024 * 1024)
>  
>  static NotifierList migration_state_notifiers =
> @@ -498,6 +499,26 @@ void qmp_migrate_cancel(Error **errp)
>      migrate_fd_cancel(migrate_get_current());
>  }
>  
> +void qmp_migrate_set_cachesize(int64_t value, Error **errp)
> +{
> +    MigrationState *s = migrate_get_current();
> +
> +    /* Check for truncation */
> +    if (value != (size_t)value) {

This fails to filter out a value of -1, at least on platforms where
size_t and int64_t are the same size.  It might be better to use
explicit comparison:

if (value < 0 || value > SIZE_MAX)


>  ##
> +# @migrate_set_cachesize

migrate-set-cachesize - QMP prefers '-' over '_'

> +#
> +# Set XBZRLE cache size
> +#
> +# @value: cache size in bytes
> +#
> +# The size will be round down to the nearest power of 2.

s/round/rounded/

> +
> +Set cache size to be used by XBZRLE migration, the cache size will be round down

s/round/rounded/

> +to the nearset power of 2

s/nearset/nearest/

> +
> +Arguments:
> +
> +- "value": cache size in bytes (json-int)
> +
> +Example:
> +
> +-> { "execute": "migrate_set_cachesize", "arguments": { "value": 512M } }

512M is not a valid json-int.  Spell it out as 536870912.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]