Re: [Qemu-devel] [PATCH v8 09/10] Add set_cachesize command

[Avi Kivity <avi@redhat.com>]

On 04/05/2012 01:47 PM, Orit Wasserman wrote:
> Change XBZRLE cache size in MB (the size should be a power of 2).

In bytes

>  
> +void xbzrle_cache_resize(int64_t new_size)
> +{
> +    if (page_cache) {
> +        cache_fini();
> +        cache_init(new_size);
> +    }
> +}

A little sad to drop the cache, especially if we're enlarging it.  But
this can be improved later.

> +
> +ETEXI
> +
> +    {
> +        .name       = "migrate_set_cachesize",
> +        .args_type  = "value:o",
> +        .params     = "value",
> +        .help       = "set cache size (in MB) for XBZRLE migrations",

In bytes.

> +        .mhandler.cmd = hmp_migrate_set_cachesize,
> +    },
> +
> +STEXI
> +@item migrate_set_cachesize @var{value}
> +@findex migrate_set_cache
> +Set cache size to @var{value} (in MB) for xbzrle migrations.

Need to document the constraints, and say something about how a larger
cache size can reduce the needed bandwidth.

We need to either document the default or (better) add a command to get
the current cache size (perhaps with some statistics about hit rate and
average data reduction).

>  
> +void qmp_migrate_set_cachesize(int64_t value, Error **errp)
> +{
> +    MigrationState *s;
> +
> +    /* On 32-bit hosts, QEMU is limited by virtual address space */
> +    if (value > (2047 << 20) && HOST_LONG_BITS == 32) {

Could be made clearer by using

   /* Check for truncation */
   if (value != (size_t)value)) {
      ...

(assumes value is in bytes)

>  ##
> +# @migrate_set_cachesize
> +#
> +# Set XBZRLE cache size
> +#
> +# @value: cache size in bytes

Here it's in bytes, good.

For the human monitor we can use MB as the unit, or allow suffixes as we
do for -m <memory>.

-- 
error compiling committee.c: too many arguments to function