* [PATCH]xl: Some small fixes to xl
@ 2010-05-21 7:01 Yang Hongyang
2010-05-21 14:26 ` Keir Fraser
2010-05-21 19:25 ` Jeremy Fitzhardinge
0 siblings, 2 replies; 10+ messages in thread
From: Yang Hongyang @ 2010-05-21 7:01 UTC (permalink / raw)
To: xen-devel@lists.xensource.com
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
@@ -2531,7 +2531,7 @@
}
if (target_memkb > memorykb) {
- XL_LOG_ERRNO(ctx, XL_LOG_ERROR,
+ XL_LOG(ctx, XL_LOG_ERROR,
"memory_dynamic_max must be less than or equal to memory_static_max\n");
return 1;
}
diff -r 840f269d95fb tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Wed May 19 22:59:52 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c Fri May 21 22:56:02 2010 +0800
@@ -1300,7 +1300,7 @@
void set_memory_target(char *p, char *mem)
{
- long long int memorykb;
+ int64_t memorykb;
find_domain(p);
@@ -1310,7 +1310,7 @@
exit(3);
}
- printf("setting domid %d memory to : %lld\n", domid, memorykb);
+ printf("setting domid %d memory to : %ld\n", domid, memorykb);
libxl_set_memory_target(&ctx, domid, memorykb, /* enforce */ 1);
}
@@ -3644,7 +3644,8 @@
int fd;
char buf[512];
uint32_t uptime = 0;
- char *uptime_str = 0;
+ char *uptime_str = NULL;
+ char *now_str = NULL;
fd = open("/proc/uptime", O_RDONLY);
if (fd == -1)
@@ -3661,9 +3662,10 @@
if (short_mode)
{
+ now_str = current_time_to_string(now);
uptime_str = uptime_to_string(uptime, 1);
- printf(" %s up %s, %s (%d)\n", current_time_to_string(now),
- uptime_str, libxl_domid_to_name(&ctx, 0), 0);
+ printf(" %s up %s, %s (%d)\n", now_str, uptime_str,
+ libxl_domid_to_name(&ctx, 0), 0);
}
else
{
@@ -3672,6 +3674,8 @@
0, uptime_str);
}
+ if (now_str)
+ free(now_str);
if (uptime_str)
free(uptime_str);
return;
@@ -3684,7 +3688,8 @@
{
uint32_t s_time = 0;
uint32_t uptime = 0;
- char *uptime_str = 0;
+ char *uptime_str = NULL;
+ char *now_str = NULL;
s_time = libxl_vm_get_start_time(&ctx, domuid);
if (s_time == -1)
@@ -3692,9 +3697,10 @@
uptime = now - s_time;
if (short_mode)
{
+ now_str = current_time_to_string(now);
uptime_str = uptime_to_string(uptime, 1);
- printf(" %s up %s, %s (%d)\n", current_time_to_string(now),
- uptime_str, libxl_domid_to_name(&ctx, domuid), domuid);
+ printf(" %s up %s, %s (%d)\n", now_str, uptime_str,
+ libxl_domid_to_name(&ctx, domuid), domuid);
}
else
{
@@ -3703,6 +3709,8 @@
domuid, uptime_str);
}
+ if (now_str)
+ free(now_str);
if (uptime_str)
free(uptime_str);
return;
diff -r 840f269d95fb tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c Wed May 19 22:59:52 2010 +0100
+++ b/tools/libxl/xl_cmdtable.c Fri May 21 22:56:02 2010 +0800
@@ -110,17 +110,13 @@
},
{ "mem-max",
&main_memmax,
- "Set the maximum amount reservation for a domain.\n"
- "Units default to kilobytes, but can be suffixed with\n"
- "'b' (bytes), 'k' (KB), 'm' (MB), 'g' (GB) or 't' (TB)",
- "<Domain> <MemKB>",
+ "Set the maximum amount reservation for a domain",
+ "<Domain> <MemKB['b'[bytes]|'k'[KB]|'m'[MB]|'g'[GB]|'t'[TB]]>",
},
{ "mem-set",
&main_memset,
- "Set the current memory usage for a domain.\n"
- "Units default to kilobytes, but can be suffixed with\n"
- "'b' (bytes), 'k' (KB), 'm' (MB), 'g' (GB) or 't' (TB)",
- "<Domain> <MemKB>",
+ "Set the current memory usage for a domain",
+ "<Domain> <MemKB['b'[bytes]|'k'[KB]|'m'[MB]|'g'[GB]|'t'[TB]]>",
},
{ "button-press",
&main_button_press,
--
Regards
Yang Hongyang
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH]xl: Some small fixes to xl
2010-05-21 7:01 [PATCH]xl: Some small fixes to xl Yang Hongyang
@ 2010-05-21 14:26 ` Keir Fraser
2010-05-26 0:55 ` Yang Hongyang
2010-05-21 19:25 ` Jeremy Fitzhardinge
1 sibling, 1 reply; 10+ messages in thread
From: Keir Fraser @ 2010-05-21 14:26 UTC (permalink / raw)
To: Yang Hongyang, xen-devel@lists.xensource.com
Please provide a signed-off-by line.
K.
On 21/05/2010 08:01, "Yang Hongyang" <yanghy@cn.fujitsu.com> 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
> @@ -2531,7 +2531,7 @@
> }
>
> if (target_memkb > memorykb) {
> - XL_LOG_ERRNO(ctx, XL_LOG_ERROR,
> + XL_LOG(ctx, XL_LOG_ERROR,
> "memory_dynamic_max must be less than or equal to
> memory_static_max\n");
> return 1;
> }
> diff -r 840f269d95fb tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c Wed May 19 22:59:52 2010 +0100
> +++ b/tools/libxl/xl_cmdimpl.c Fri May 21 22:56:02 2010 +0800
> @@ -1300,7 +1300,7 @@
>
> void set_memory_target(char *p, char *mem)
> {
> - long long int memorykb;
> + int64_t memorykb;
>
> find_domain(p);
>
> @@ -1310,7 +1310,7 @@
> exit(3);
> }
>
> - printf("setting domid %d memory to : %lld\n", domid, memorykb);
> + printf("setting domid %d memory to : %ld\n", domid, memorykb);
> libxl_set_memory_target(&ctx, domid, memorykb, /* enforce */ 1);
> }
>
> @@ -3644,7 +3644,8 @@
> int fd;
> char buf[512];
> uint32_t uptime = 0;
> - char *uptime_str = 0;
> + char *uptime_str = NULL;
> + char *now_str = NULL;
>
> fd = open("/proc/uptime", O_RDONLY);
> if (fd == -1)
> @@ -3661,9 +3662,10 @@
>
> if (short_mode)
> {
> + now_str = current_time_to_string(now);
> uptime_str = uptime_to_string(uptime, 1);
> - printf(" %s up %s, %s (%d)\n", current_time_to_string(now),
> - uptime_str, libxl_domid_to_name(&ctx, 0), 0);
> + printf(" %s up %s, %s (%d)\n", now_str, uptime_str,
> + libxl_domid_to_name(&ctx, 0), 0);
> }
> else
> {
> @@ -3672,6 +3674,8 @@
> 0, uptime_str);
> }
>
> + if (now_str)
> + free(now_str);
> if (uptime_str)
> free(uptime_str);
> return;
> @@ -3684,7 +3688,8 @@
> {
> uint32_t s_time = 0;
> uint32_t uptime = 0;
> - char *uptime_str = 0;
> + char *uptime_str = NULL;
> + char *now_str = NULL;
>
> s_time = libxl_vm_get_start_time(&ctx, domuid);
> if (s_time == -1)
> @@ -3692,9 +3697,10 @@
> uptime = now - s_time;
> if (short_mode)
> {
> + now_str = current_time_to_string(now);
> uptime_str = uptime_to_string(uptime, 1);
> - printf(" %s up %s, %s (%d)\n", current_time_to_string(now),
> - uptime_str, libxl_domid_to_name(&ctx, domuid), domuid);
> + printf(" %s up %s, %s (%d)\n", now_str, uptime_str,
> + libxl_domid_to_name(&ctx, domuid), domuid);
> }
> else
> {
> @@ -3703,6 +3709,8 @@
> domuid, uptime_str);
> }
>
> + if (now_str)
> + free(now_str);
> if (uptime_str)
> free(uptime_str);
> return;
> diff -r 840f269d95fb tools/libxl/xl_cmdtable.c
> --- a/tools/libxl/xl_cmdtable.c Wed May 19 22:59:52 2010 +0100
> +++ b/tools/libxl/xl_cmdtable.c Fri May 21 22:56:02 2010 +0800
> @@ -110,17 +110,13 @@
> },
> { "mem-max",
> &main_memmax,
> - "Set the maximum amount reservation for a domain.\n"
> - "Units default to kilobytes, but can be suffixed with\n"
> - "'b' (bytes), 'k' (KB), 'm' (MB), 'g' (GB) or 't' (TB)",
> - "<Domain> <MemKB>",
> + "Set the maximum amount reservation for a domain",
> + "<Domain> <MemKB['b'[bytes]|'k'[KB]|'m'[MB]|'g'[GB]|'t'[TB]]>",
> },
> { "mem-set",
> &main_memset,
> - "Set the current memory usage for a domain.\n"
> - "Units default to kilobytes, but can be suffixed with\n"
> - "'b' (bytes), 'k' (KB), 'm' (MB), 'g' (GB) or 't' (TB)",
> - "<Domain> <MemKB>",
> + "Set the current memory usage for a domain",
> + "<Domain> <MemKB['b'[bytes]|'k'[KB]|'m'[MB]|'g'[GB]|'t'[TB]]>",
> },
> { "button-press",
> &main_button_press,
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH]xl: Some small fixes to xl
2010-05-21 7:01 [PATCH]xl: Some small fixes to xl Yang Hongyang
2010-05-21 14:26 ` Keir Fraser
@ 2010-05-21 19:25 ` Jeremy Fitzhardinge
2010-05-24 0:48 ` Yang Hongyang
1 sibling, 1 reply; 10+ messages in thread
From: Jeremy Fitzhardinge @ 2010-05-21 19:25 UTC (permalink / raw)
To: Yang Hongyang; +Cc: xen-devel@lists.xensource.com
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
> @@ -2531,7 +2531,7 @@
> }
>
> if (target_memkb > memorykb) {
> - XL_LOG_ERRNO(ctx, XL_LOG_ERROR,
> + XL_LOG(ctx, XL_LOG_ERROR,
> "memory_dynamic_max must be less than or equal to memory_static_max\n");
> return 1;
> }
> diff -r 840f269d95fb tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c Wed May 19 22:59:52 2010 +0100
> +++ b/tools/libxl/xl_cmdimpl.c Fri May 21 22:56:02 2010 +0800
> @@ -1300,7 +1300,7 @@
>
> void set_memory_target(char *p, char *mem)
> {
> - long long int memorykb;
> + int64_t memorykb;
>
> find_domain(p);
>
> @@ -1310,7 +1310,7 @@
> exit(3);
> }
>
> - 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?
J
> libxl_set_memory_target(&ctx, domid, memorykb, /* enforce */ 1);
> }
>
> @@ -3644,7 +3644,8 @@
> int fd;
> char buf[512];
> uint32_t uptime = 0;
> - char *uptime_str = 0;
> + char *uptime_str = NULL;
> + char *now_str = NULL;
>
> fd = open("/proc/uptime", O_RDONLY);
> if (fd == -1)
> @@ -3661,9 +3662,10 @@
>
> if (short_mode)
> {
> + now_str = current_time_to_string(now);
> uptime_str = uptime_to_string(uptime, 1);
> - printf(" %s up %s, %s (%d)\n", current_time_to_string(now),
> - uptime_str, libxl_domid_to_name(&ctx, 0), 0);
> + printf(" %s up %s, %s (%d)\n", now_str, uptime_str,
> + libxl_domid_to_name(&ctx, 0), 0);
> }
> else
> {
> @@ -3672,6 +3674,8 @@
> 0, uptime_str);
> }
>
> + if (now_str)
> + free(now_str);
> if (uptime_str)
> free(uptime_str);
> return;
> @@ -3684,7 +3688,8 @@
> {
> uint32_t s_time = 0;
> uint32_t uptime = 0;
> - char *uptime_str = 0;
> + char *uptime_str = NULL;
> + char *now_str = NULL;
>
> s_time = libxl_vm_get_start_time(&ctx, domuid);
> if (s_time == -1)
> @@ -3692,9 +3697,10 @@
> uptime = now - s_time;
> if (short_mode)
> {
> + now_str = current_time_to_string(now);
> uptime_str = uptime_to_string(uptime, 1);
> - printf(" %s up %s, %s (%d)\n", current_time_to_string(now),
> - uptime_str, libxl_domid_to_name(&ctx, domuid), domuid);
> + printf(" %s up %s, %s (%d)\n", now_str, uptime_str,
> + libxl_domid_to_name(&ctx, domuid), domuid);
> }
> else
> {
> @@ -3703,6 +3709,8 @@
> domuid, uptime_str);
> }
>
> + if (now_str)
> + free(now_str);
> if (uptime_str)
> free(uptime_str);
> return;
> diff -r 840f269d95fb tools/libxl/xl_cmdtable.c
> --- a/tools/libxl/xl_cmdtable.c Wed May 19 22:59:52 2010 +0100
> +++ b/tools/libxl/xl_cmdtable.c Fri May 21 22:56:02 2010 +0800
> @@ -110,17 +110,13 @@
> },
> { "mem-max",
> &main_memmax,
> - "Set the maximum amount reservation for a domain.\n"
> - "Units default to kilobytes, but can be suffixed with\n"
> - "'b' (bytes), 'k' (KB), 'm' (MB), 'g' (GB) or 't' (TB)",
> - "<Domain> <MemKB>",
> + "Set the maximum amount reservation for a domain",
> + "<Domain> <MemKB['b'[bytes]|'k'[KB]|'m'[MB]|'g'[GB]|'t'[TB]]>",
> },
> { "mem-set",
> &main_memset,
> - "Set the current memory usage for a domain.\n"
> - "Units default to kilobytes, but can be suffixed with\n"
> - "'b' (bytes), 'k' (KB), 'm' (MB), 'g' (GB) or 't' (TB)",
> - "<Domain> <MemKB>",
> + "Set the current memory usage for a domain",
> + "<Domain> <MemKB['b'[bytes]|'k'[KB]|'m'[MB]|'g'[GB]|'t'[TB]]>",
> },
> { "button-press",
> &main_button_press,
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH]xl: Some small fixes to xl
2010-05-21 19:25 ` Jeremy Fitzhardinge
@ 2010-05-24 0:48 ` Yang Hongyang
2010-05-24 9:16 ` Stefano Stabellini
0 siblings, 1 reply; 10+ messages in thread
From: Yang Hongyang @ 2010-05-24 0:48 UTC (permalink / raw)
To: Jeremy Fitzhardinge; +Cc: xen-devel@lists.xensource.com
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...
>
> J
>
>> libxl_set_memory_target(&ctx, domid, memorykb, /* enforce */ 1);
>> }
>>
>> @@ -3644,7 +3644,8 @@
>> int fd;
>> char buf[512];
>> uint32_t uptime = 0;
>> - char *uptime_str = 0;
>> + char *uptime_str = NULL;
>> + char *now_str = NULL;
>>
>> fd = open("/proc/uptime", O_RDONLY);
>> if (fd == -1)
>> @@ -3661,9 +3662,10 @@
>>
>> if (short_mode)
>> {
>> + now_str = current_time_to_string(now);
>> uptime_str = uptime_to_string(uptime, 1);
>> - printf(" %s up %s, %s (%d)\n", current_time_to_string(now),
>> - uptime_str, libxl_domid_to_name(&ctx, 0), 0);
>> + printf(" %s up %s, %s (%d)\n", now_str, uptime_str,
>> + libxl_domid_to_name(&ctx, 0), 0);
>> }
>> else
>> {
>> @@ -3672,6 +3674,8 @@
>> 0, uptime_str);
>> }
>>
>> + if (now_str)
>> + free(now_str);
>> if (uptime_str)
>> free(uptime_str);
>> return;
>> @@ -3684,7 +3688,8 @@
>> {
>> uint32_t s_time = 0;
>> uint32_t uptime = 0;
>> - char *uptime_str = 0;
>> + char *uptime_str = NULL;
>> + char *now_str = NULL;
>>
>> s_time = libxl_vm_get_start_time(&ctx, domuid);
>> if (s_time == -1)
>> @@ -3692,9 +3697,10 @@
>> uptime = now - s_time;
>> if (short_mode)
>> {
>> + now_str = current_time_to_string(now);
>> uptime_str = uptime_to_string(uptime, 1);
>> - printf(" %s up %s, %s (%d)\n", current_time_to_string(now),
>> - uptime_str, libxl_domid_to_name(&ctx, domuid), domuid);
>> + printf(" %s up %s, %s (%d)\n", now_str, uptime_str,
>> + libxl_domid_to_name(&ctx, domuid), domuid);
>> }
>> else
>> {
>> @@ -3703,6 +3709,8 @@
>> domuid, uptime_str);
>> }
>>
>> + if (now_str)
>> + free(now_str);
>> if (uptime_str)
>> free(uptime_str);
>> return;
>> diff -r 840f269d95fb tools/libxl/xl_cmdtable.c
>> --- a/tools/libxl/xl_cmdtable.c Wed May 19 22:59:52 2010 +0100
>> +++ b/tools/libxl/xl_cmdtable.c Fri May 21 22:56:02 2010 +0800
>> @@ -110,17 +110,13 @@
>> },
>> { "mem-max",
>> &main_memmax,
>> - "Set the maximum amount reservation for a domain.\n"
>> - "Units default to kilobytes, but can be suffixed with\n"
>> - "'b' (bytes), 'k' (KB), 'm' (MB), 'g' (GB) or 't' (TB)",
>> - "<Domain> <MemKB>",
>> + "Set the maximum amount reservation for a domain",
>> + "<Domain> <MemKB['b'[bytes]|'k'[KB]|'m'[MB]|'g'[GB]|'t'[TB]]>",
>> },
>> { "mem-set",
>> &main_memset,
>> - "Set the current memory usage for a domain.\n"
>> - "Units default to kilobytes, but can be suffixed with\n"
>> - "'b' (bytes), 'k' (KB), 'm' (MB), 'g' (GB) or 't' (TB)",
>> - "<Domain> <MemKB>",
>> + "Set the current memory usage for a domain",
>> + "<Domain> <MemKB['b'[bytes]|'k'[KB]|'m'[MB]|'g'[GB]|'t'[TB]]>",
>> },
>> { "button-press",
>> &main_button_press,
>>
>>
>
>
>
--
Regards
Yang Hongyang
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH]xl: Some small fixes to xl
2010-05-24 0:48 ` Yang Hongyang
@ 2010-05-24 9:16 ` Stefano Stabellini
2010-05-24 10:24 ` Ian Jackson
2010-05-26 1:02 ` Yang Hongyang
0 siblings, 2 replies; 10+ messages in thread
From: Stefano Stabellini @ 2010-05-24 9:16 UTC (permalink / raw)
To: Yang Hongyang; +Cc: Jeremy Fitzhardinge, xen-devel@lists.xensource.com
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH]xl: Some small fixes to xl
2010-05-24 9:16 ` Stefano Stabellini
@ 2010-05-24 10:24 ` Ian Jackson
2010-05-24 10:31 ` Stefano Stabellini
2010-05-26 1:02 ` Yang Hongyang
1 sibling, 1 reply; 10+ messages in thread
From: Ian Jackson @ 2010-05-24 10:24 UTC (permalink / raw)
To: Stefano Stabellini
Cc: Jeremy Fitzhardinge, Yang Hongyang, xen-devel@lists.xensource.com
Stefano Stabellini writes ("Re: [Xen-devel] [PATCH]xl: Some small fixes to xl"):
> Finally in the printf call the best way to print a uint64_t in decimal
> format is using PRId64.
PRIu64 I think you mean. PRId64 is for int64_t.
Ian.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH]xl: Some small fixes to xl
2010-05-24 10:24 ` Ian Jackson
@ 2010-05-24 10:31 ` Stefano Stabellini
2010-05-24 17:32 ` Jeremy Fitzhardinge
0 siblings, 1 reply; 10+ messages in thread
From: Stefano Stabellini @ 2010-05-24 10:31 UTC (permalink / raw)
To: Ian Jackson
Cc: Jeremy Fitzhardinge, Yang Hongyang, xen-devel@lists.xensource.com,
Stefano Stabellini
On Mon, 24 May 2010, Ian Jackson wrote:
> Stefano Stabellini writes ("Re: [Xen-devel] [PATCH]xl: Some small fixes to xl"):
> > Finally in the printf call the best way to print a uint64_t in decimal
> > format is using PRId64.
>
> PRIu64 I think you mean. PRId64 is for int64_t.
Yes, thank you
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH]xl: Some small fixes to xl
2010-05-24 10:31 ` Stefano Stabellini
@ 2010-05-24 17:32 ` Jeremy Fitzhardinge
0 siblings, 0 replies; 10+ messages in thread
From: Jeremy Fitzhardinge @ 2010-05-24 17:32 UTC (permalink / raw)
To: Stefano Stabellini
Cc: Yang Hongyang, Ian Jackson, xen-devel@lists.xensource.com
On 05/24/2010 03:31 AM, Stefano Stabellini wrote:
> On Mon, 24 May 2010, Ian Jackson wrote:
>
>> Stefano Stabellini writes ("Re: [Xen-devel] [PATCH]xl: Some small fixes to xl"):
>>
>>> Finally in the printf call the best way to print a uint64_t in decimal
>>> format is using PRId64.
>>>
>> PRIu64 I think you mean. PRId64 is for int64_t.
>>
>
> Yes, thank you
>
I personally find those format macros pretty odious and avoid them
unless there's no other option, but I'll leave that up to you.
J
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH]xl: Some small fixes to xl
2010-05-21 14:26 ` Keir Fraser
@ 2010-05-26 0:55 ` Yang Hongyang
0 siblings, 0 replies; 10+ messages in thread
From: Yang Hongyang @ 2010-05-26 0:55 UTC (permalink / raw)
To: Keir Fraser; +Cc: xen-devel@lists.xensource.com
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
-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.
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
diff -r 93410e5e4ad8 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c Sat May 22 06:36:41 2010 +0100
+++ b/tools/libxl/libxl.c Wed May 26 08:54:39 2010 +0800
@@ -2531,7 +2531,7 @@
}
if (target_memkb > memorykb) {
- XL_LOG_ERRNO(ctx, XL_LOG_ERROR,
+ XL_LOG(ctx, XL_LOG_ERROR,
"memory_dynamic_max must be less than or equal to memory_static_max\n");
return 1;
}
diff -r 93410e5e4ad8 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Sat May 22 06:36:41 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c Wed May 26 08:54:39 2010 +0800
@@ -3644,7 +3644,8 @@
int fd;
char buf[512];
uint32_t uptime = 0;
- char *uptime_str = 0;
+ char *uptime_str = NULL;
+ char *now_str = NULL;
fd = open("/proc/uptime", O_RDONLY);
if (fd == -1)
@@ -3661,9 +3662,10 @@
if (short_mode)
{
+ now_str = current_time_to_string(now);
uptime_str = uptime_to_string(uptime, 1);
- printf(" %s up %s, %s (%d)\n", current_time_to_string(now),
- uptime_str, libxl_domid_to_name(&ctx, 0), 0);
+ printf(" %s up %s, %s (%d)\n", now_str, uptime_str,
+ libxl_domid_to_name(&ctx, 0), 0);
}
else
{
@@ -3672,6 +3674,8 @@
0, uptime_str);
}
+ if (now_str)
+ free(now_str);
if (uptime_str)
free(uptime_str);
return;
@@ -3684,7 +3688,8 @@
{
uint32_t s_time = 0;
uint32_t uptime = 0;
- char *uptime_str = 0;
+ char *uptime_str = NULL;
+ char *now_str = NULL;
s_time = libxl_vm_get_start_time(&ctx, domuid);
if (s_time == -1)
@@ -3692,9 +3697,10 @@
uptime = now - s_time;
if (short_mode)
{
+ now_str = current_time_to_string(now);
uptime_str = uptime_to_string(uptime, 1);
- printf(" %s up %s, %s (%d)\n", current_time_to_string(now),
- uptime_str, libxl_domid_to_name(&ctx, domuid), domuid);
+ printf(" %s up %s, %s (%d)\n", now_str, uptime_str,
+ libxl_domid_to_name(&ctx, domuid), domuid);
}
else
{
@@ -3703,6 +3709,8 @@
domuid, uptime_str);
}
+ if (now_str)
+ free(now_str);
if (uptime_str)
free(uptime_str);
return;
diff -r 93410e5e4ad8 tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c Sat May 22 06:36:41 2010 +0100
+++ b/tools/libxl/xl_cmdtable.c Wed May 26 08:54:39 2010 +0800
@@ -110,17 +110,13 @@
},
{ "mem-max",
&main_memmax,
- "Set the maximum amount reservation for a domain.\n"
- "Units default to kilobytes, but can be suffixed with\n"
- "'b' (bytes), 'k' (KB), 'm' (MB), 'g' (GB) or 't' (TB)",
- "<Domain> <MemKB>",
+ "Set the maximum amount reservation for a domain",
+ "<Domain> <MemKB['b'[bytes]|'k'[KB]|'m'[MB]|'g'[GB]|'t'[TB]]>",
},
{ "mem-set",
&main_memset,
- "Set the current memory usage for a domain.\n"
- "Units default to kilobytes, but can be suffixed with\n"
- "'b' (bytes), 'k' (KB), 'm' (MB), 'g' (GB) or 't' (TB)",
- "<Domain> <MemKB>",
+ "Set the current memory usage for a domain",
+ "<Domain> <MemKB['b'[bytes]|'k'[KB]|'m'[MB]|'g'[GB]|'t'[TB]]>",
},
{ "button-press",
&main_button_press,
--
Regards
Yang Hongyang
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH]xl: Some small fixes to xl
2010-05-24 9:16 ` Stefano Stabellini
2010-05-24 10:24 ` Ian Jackson
@ 2010-05-26 1:02 ` Yang Hongyang
1 sibling, 0 replies; 10+ messages in thread
From: Yang Hongyang @ 2010-05-26 1:02 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: Jeremy Fitzhardinge, xen-devel@lists.xensource.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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-05-26 1:02 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-21 7:01 [PATCH]xl: Some small fixes to xl Yang Hongyang
2010-05-21 14:26 ` Keir Fraser
2010-05-26 0:55 ` Yang Hongyang
2010-05-21 19:25 ` Jeremy Fitzhardinge
2010-05-24 0:48 ` Yang Hongyang
2010-05-24 9:16 ` Stefano Stabellini
2010-05-24 10:24 ` Ian Jackson
2010-05-24 10:31 ` Stefano Stabellini
2010-05-24 17:32 ` Jeremy Fitzhardinge
2010-05-26 1:02 ` Yang Hongyang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).