Peter Xu <pet...@redhat.com> wrote:
> On Tue, Oct 10, 2017 at 08:15:38PM +0200, Juan Quintela wrote:
>> Instead of passing silently round down the number of pages, make it an
>> error that the cache size is not a multiple of 2.
>
> s/multiple/power/?
Thanks.
> Would this patch break existing users?
I have a problem here. Current code:
- "silently" truncate the value that we pass
- "silently" uses a different value that the one that it shows as used
- "shows" a value that is different of what has been set to
So, I have to break one of them:
- only allow valid values (and then we output the same value that user
input)
- store "setup" value, and use something different "internaly"
No clear winner. This way we are at least consistent with everything
else. Documentation already required a power of 2 value. And we give a
good error message.
Later, Juan.
>
>>
>> Signed-off-by: Juan Quintela <quint...@redhat.com>
>> ---
>> migration/page_cache.c | 12 +++++++-----
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/migration/page_cache.c b/migration/page_cache.c
>> index 9a9d13d6a2..96268c3aea 100644
>> --- a/migration/page_cache.c
>> +++ b/migration/page_cache.c
>> @@ -58,6 +58,13 @@ PageCache *cache_init(int64_t new_size, size_t page_size,
>> Error **errp)
>> return NULL;
>> }
>>
>> + /* round down to the nearest power of 2 */
>> + if (!is_power_of_2(num_pages)) {
>> + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "cache size",
>> + "is not a power of two number of pages");
>> + return NULL;
>> + }
>> +
>> /* We prefer not to abort if there is no memory */
>> cache = g_try_malloc(sizeof(*cache));
>> if (!cache) {
>> @@ -65,11 +72,6 @@ PageCache *cache_init(int64_t new_size, size_t page_size,
>> Error **errp)
>> "Failed to allocate cache");
>> return NULL;
>> }
>> - /* round down to the nearest power of 2 */
>> - if (!is_power_of_2(num_pages)) {
>> - num_pages = pow2floor(num_pages);
>> - DPRINTF("rounding down to %" PRId64 "\n", num_pages);
>> - }
>> cache->page_size = page_size;
>> cache->num_items = 0;
>> cache->max_num_items = num_pages;
>> --
>> 2.13.6
>>
