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 <[email protected]> > Signed-off-by: Petter Svard <[email protected]> > Signed-off-by: Aidan Shribman <[email protected]> > Signed-off-by: Orit Wasserman <[email protected]> > --- > 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 [email protected] +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
