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 <benoit.hud...@sap.com> > Signed-off-by: Petter Svard <pett...@cs.umu.se> > Signed-off-by: Aidan Shribman <aidan.shrib...@sap.com> > Signed-off-by: Orit Wasserman <owass...@redhat.com> > --- > 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 ebl...@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature