qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v9 09/10] Add set_cachesize command


From: Orit Wasserman
Subject: Re: [Qemu-devel] [PATCH v9 09/10] Add set_cachesize command
Date: Thu, 19 Apr 2012 09:20:02 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120329 Thunderbird/11.0.1

On 04/18/2012 08:07 PM, Juan Quintela wrote:
> Orit Wasserman <address@hidden> 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.
> 
>> +        .name       = "migrate_set_cachesize",
>> +        .args_type  = "value:o",
> 
> This mean that we can assign values like 256M, right?  i.e. no way to
> pass negative values?
>> index 7578163..071a1b9 100644
>> --- a/migration.c
>> +++ b/migration.c
>> @@ -167,7 +167,7 @@ MigrationCapList *qmp_query_migration_caps(Error **errp)
>>      MigrationCapList *caps_list = g_malloc0(sizeof(*caps_list));
>>  
>>      caps_list->value = g_malloc(sizeof(*caps_list->value));
>> -    caps_list->value->name = g_strdup("uleb");
>> +    caps_list->value->name = g_strdup("xbzrle");
>>      caps_list->next = NULL;
>>  
> 
> Shouldn't this go in the previous patche?sure
> 
>> +void qmp_migrate_set_cachesize(int64_t value, Error **errp)
>> +{
>> +    MigrationState *s;
>> +
>> +    /* Check for truncation */
>> +    if (value != (size_t)value) {
>> +        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "cache size",
>> +                  "exceeding address space");
>> +        return;
>> +    }
>> +
>> +    /* power of 2 */
>> +    if (value != 1 && (value & (value - 1))) {
>> +        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "cache size",
>> +                  "needs to be power of 2");
>> +        return;
>> +    }
> 
> Don't we have a function to calculate if a value is a power of 2 in
> qemu?  I guess we need one _now_.
I will look into it. maybe there is a function like that ...
.
> 
>> +
>> +    value = MIN(UINT64_MAX, value);
>> +    if (value ==  migrate_cache_size) {
>> +        return;
>> +    }
>> +
>> +    migrate_cache_size =  value;
> 
> Can't we just remove the migrate_cache_size as a new field of
> migration_state.  bandwidth_limit is the example on how to do it.
Ok

Thanks,
Orit

> 
> Thanks, Juan.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]