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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
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,_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2010-May-21 19:25 UTC
Re: [Xen-devel] [PATCH]xl: Some small fixes to xl
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, > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-May-24 09:16 UTC
Re: [Xen-devel] [PATCH]xl: Some small fixes to xl
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. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-May-24 10:31 UTC
Re: [Xen-devel] [PATCH]xl: Some small fixes to xl
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2010-May-24 17:32 UTC
Re: [Xen-devel] [PATCH]xl: Some small fixes to xl
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel