[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 06/10] migration: Make sure that we pass the rig
From: |
Juan Quintela |
Subject: |
Re: [Qemu-devel] [PATCH 06/10] migration: Make sure that we pass the right cache size |
Date: |
Wed, 18 Oct 2017 10:45:22 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) |
Peter Xu <address@hidden> 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 <address@hidden>
>> ---
>> 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
>>
- Re: [Qemu-devel] [PATCH 02/10] migratiom: Remove max_item_age parameter, (continued)
- [Qemu-devel] [PATCH 03/10] migration: Make cache size elements use the right types, Juan Quintela, 2017/10/10
- [Qemu-devel] [PATCH 04/10] migration: Move xbzrle cache resize error handling to xbzrle_cache_resize, Juan Quintela, 2017/10/10
- [Qemu-devel] [PATCH 05/10] migration: Make cache_init() take an error parameter, Juan Quintela, 2017/10/10
- [Qemu-devel] [PATCH 07/10] migration: Don't play games with the requested cache size, Juan Quintela, 2017/10/10
- [Qemu-devel] [PATCH 06/10] migration: Make sure that we pass the right cache size, Juan Quintela, 2017/10/10
- [Qemu-devel] [PATCH 08/10] migration: No need to return the size of the cache, Juan Quintela, 2017/10/10
- [Qemu-devel] [PATCH 09/10] migration: Make xbzrle_cache_size a migration parameter, Juan Quintela, 2017/10/10
- [Qemu-devel] [PATCH 10/10] migration: [RFC] Use proper types in json, Juan Quintela, 2017/10/10