From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:57525)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <avi@redhat.com>) id 1SFlbp-0003Ds-Dr
	for qemu-devel@nongnu.org; Thu, 05 Apr 2012 08:16:22 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <avi@redhat.com>) id 1SFlbd-0002Gf-Kg
	for qemu-devel@nongnu.org; Thu, 05 Apr 2012 08:16:11 -0400
Received: from mx1.redhat.com ([209.132.183.28]:62925)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <avi@redhat.com>) id 1SFlbd-0002GB-CW
	for qemu-devel@nongnu.org; Thu, 05 Apr 2012 08:16:01 -0400
Message-ID: <4F7D8CF8.7040108@redhat.com>
Date: Thu, 05 Apr 2012 15:15:52 +0300
From: Avi Kivity <avi@redhat.com>
MIME-Version: 1.0
References: <1333622879-12055-1-git-send-email-owasserm@redhat.com>
	<1333622879-12055-10-git-send-email-owasserm@redhat.com>
In-Reply-To: <1333622879-12055-10-git-send-email-owasserm@redhat.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v8 09/10] Add set_cachesize command
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Orit Wasserman <owasserm@redhat.com>
Cc: blauwirbel@gmail.com, stefanha@gmail.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, quintela@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