[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v13 12/13] Add set_cachesize command
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v13 12/13] Add set_cachesize command |
Date: |
Wed, 27 Jun 2012 14:04:36 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120615 Thunderbird/13.0.1 |
On 06/27/2012 04:34 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 <address@hidden>
> Signed-off-by: Petter Svard <address@hidden>
> Signed-off-by: Aidan Shribman <address@hidden>
> Signed-off-by: Orit Wasserman <address@hidden>
> ---
> arch_init.c | 9 +++++++++
> hmp-commands.hx | 18 ++++++++++++++++++
> hmp.c | 13 +++++++++++++
> hmp.h | 1 +
> migration.c | 23 ++++++++++++++++++++++-
> migration.h | 2 ++
> qapi-schema.json | 16 ++++++++++++++++
> qmp-commands.hx | 23 +++++++++++++++++++++++
> 8 files changed, 104 insertions(+), 1 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index bfc9f27..ae7c8b1 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -24,6 +24,7 @@
> #include <stdint.h>
> #include <stdarg.h>
> #include <stdlib.h>
> +#include <math.h>
Why?
Furthermore, I think I asked the same question about v12; it's rather
depressing to review a patch when not all the review comments from the
previous revision were addressed.
> + .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/
> +void qmp_migrate_set_cachesize(int64_t value, Error **errp)
> +{
> + MigrationState *s = migrate_get_current();
> +
> + /* Check for truncation */
> + if (value != (size_t)value) {
> + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "cache size",
> + "exceeding address space");
> + return;
> + }
> +
> + /* no change */
> + if (value == s->xbzrle_cache_size) {
> + return;
Another place where factoring the rounding-down will make it more
efficient. On the other hand, why do you even worry about it, since...
> + }
> +
> + s->xbzrle_cache_size = value;
> + xbzrle_cache_resize(value);
...xbzrle_cache_resize should be a relatively fast no-op if the
rounded-down size is equal? Also, aren't you better off storing the
rounded value and not the user's value (in which case, should
xbzrle_cache_resize return a value)? For that matter, what if the user
requests a resize to a larger value that exceeds available host memory?
Normally, out-of-memory makes qemu exit, but in this particular case,
we are better off leaving qemu running but just failing the monitor command.
> +# @migrate_set_cachesize
> +#
> +# Set XBZRLE cache size
> +#
> +# @value: cache size in bytes
> +#
> +# The size will be round down to the nearest power of 2.
s/round/rounded/
> +Example:
> +
> +-> { "execute": "migrate_set_cachesize", "arguments": { "value": 512M } }
512M is not a json-int. I really don't think you even saw my v12
comments on this patch.
--
Eric Blake address@hidden +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v13 08/13] Change ram_save_block to return -1 if there are no more changes, (continued)
- [Qemu-devel] [PATCH v13 08/13] Change ram_save_block to return -1 if there are no more changes, Orit Wasserman, 2012/06/27
- [Qemu-devel] [PATCH v13 11/13] Add XBZRLE to ram_save_block and ram_save_live, Orit Wasserman, 2012/06/27
- [Qemu-devel] [PATCH v13 10/13] Add xbzrle_encode_buffer and xbzrle_decode_buffer functions, Orit Wasserman, 2012/06/27
- [Qemu-devel] [PATCH v13 12/13] Add set_cachesize command, Orit Wasserman, 2012/06/27
- [Qemu-devel] [PATCH v13 13/13] Add XBZRLE statistics, Orit Wasserman, 2012/06/27
- [Qemu-devel] [PATCH v13 09/13] Add migration_end function, Orit Wasserman, 2012/06/27