On 05/16/2012 05:59 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>
> > + > +void xbzrle_cache_resize(int64_t order) > +{ > + cache_resize(XBZRLE.cache, pow(2, order)); '1 << order' is much more efficient than a call to pow(). > +void qmp_migrate_set_cachesize(int64_t value, Error **errp) > + > + /* power of 2 */ > + if (value != 1 && !is_power_of_2(value)) { > + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "cache size", > + "needs to be power of 2"); We already have QERR_PROPERTY_VALUE_NOT_POWER_OF_2, why aren't you using that here? > + return; > + } > + > + s->xbzrle_cache_size = value; > + xbzrle_cache_resize(log2(value)); log2() is rather expensive, ffs() from <strings.h> is more efficient at converting a single bit into the appropriate order. > ## > +# @migrate_set_cachesize > +# > +# Set XBZRLE cache size > +# > +# @value: cache size in bytes > +# > +# Returns: nothing on success Document the error for a non-power-of-2 or for overflow. Document whether this command is safe for an ongoing migration, or whether it must be called in advance of a migration. > +# > +# Since: 1.1 1.2. > +static inline bool is_power_of_2(int64_t value) > +{ > + return !(value & (value - 1)); > +} This says '0' is a power of 2, which is not true. Either fix the logic to exclude 0, or fix the function name to state that you are really checking that at most one bit is set. Also, if value is 0x8000000000000000, you are triggering unspecified behavior per C99. Is it worth using uint64_t for defined behavior, or do you need to take precautions regarding negative values? > +SQMP > +migrate_set_cachesize > +--------------------- > + > +Set cache size to be used by XBZRLE migration > + > +Arguments: > + > +- "value": cache size in bytes (json-int) Would it be any easier to take 'order' (log2 of the size) instead of the actual cache size? That is, instead of calling "value":1048576, I would rather type "value":20. > + > +Example: > + > +-> { "execute": "migrate_set_cachesize", "arguments": { "value": 512 } } Isn't 512 bytes rather small? And given my argument about taking order rather than bytes as being easier to use, don't you really mean 512 megabytes (order 29) rather than 512 bytes (order 9)? -- Eric Blake ebl...@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature