Re: [PATCH]xl: Some small fixes to xl

[Yang Hongyang <yanghy@cn.fujitsu.com>]

On 05/24/2010 05:16 PM, Stefano Stabellini wrote:
> On Mon, 24 May 2010, Yang Hongyang wrote:
>> Hi jeremy,
>>
>> Thank you for your review.
>>
>> On 05/22/2010 03:25 AM, Jeremy Fitzhardinge wrote:
>>> On 05/21/2010 12:01 AM, Yang Hongyang wrote:
>>>> The patch fixes following problems:
>>>> -When use mem-set, I got suspicious error output:
>>>>  # xl mem-set 1 256g
>>>>  setting domid 1 memory to : 268435456
>>>>  [0] libxl.c:2535:libxl_set_memory_target: memory_dynamic_max must be less than or equal to memory_static_max
>>>>  : Success
>>>> -parse_mem_size_kb() returns type int64_t
>>>> -String generated by strdup() should be freed
>>>> -When using 'xl help', mem-max and mem-set's output is not as intend, and it's also
>>>>  breaks bash completion, fix it.
>>>>
>>>> diff -r 840f269d95fb tools/libxl/libxl.c
>>>> --- a/tools/libxl/libxl.c	Wed May 19 22:59:52 2010 +0100
>>>> +++ b/tools/libxl/libxl.c	Fri May 21 22:56:02 2010 +0800
>> ...snip...
>>>>      }
>>>>  
>>>> -    printf("setting domid %d memory to : %lld\n", domid, memorykb);
>>>> +    printf("setting domid %d memory to : %ld\n", domid, memorykb);
>>>>   
>>>
>>> This is wrong; "int64_t" isn't necessarily long.  What was wrong with
>>> the previous version?
>>
>> There's a problem. libxl_set_memory_target() takes memorykb as uint32_t,
>> so, if you want set as 'long long int', you should also modify this lib
>> function I think.
>> For this reason, when I set the memory to 100t, libxl_set_memory_target()
>> will take the memorykb as 0...
>>
> 
> Yes, we need to use uint64_t in libxenlight too, also I prefer uint64_t
> to unsigned long long.
> Finally in the printf call the best way to print a uint64_t in decimal
> format is using PRId64.
> 
> 

By the way, xc_domain_setmaxmem() take max_memkb as 'unsigned int'
It we use uint64_t, we should change that also.

> 
> 


-- 
Regards
Yang Hongyang