* [Qemu-devel] [PATCH 1/2] vl: Fix the confused logic for '-m' option @ 2014-09-12 5:58 zhanghailiang 2014-09-12 6:46 ` Li Liu 2014-09-12 13:52 ` Igor Mammedov 0 siblings, 2 replies; 4+ messages in thread From: zhanghailiang @ 2014-09-12 5:58 UTC (permalink / raw) To: qemu-devel; +Cc: imammedo, luonengjun, peter.huangpeng, aliguori, zhanghailiang It should be valid for the follow configure: -m 256,slots=0 -m 256,maxmem=256M -m 256,slots=0,maxmem=256M -m 256,slots=x,maxmem=y where x > 0 and y > 256M Fix the confused code logic and use error_report instead of fprintf. Printing the maxmem in hex, same with ram_size. Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> --- vl.c | 46 +++++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/vl.c b/vl.c index 9c9acf5..f547405 100644 --- a/vl.c +++ b/vl.c @@ -3306,6 +3306,7 @@ int main(int argc, char **argv, char **envp) break; case QEMU_OPTION_m: { uint64_t sz; + uint64_t slots; const char *mem_str; const char *maxmem_str, *slots_str; @@ -3353,40 +3354,47 @@ int main(int argc, char **argv, char **envp) maxmem_str = qemu_opt_get(opts, "maxmem"); slots_str = qemu_opt_get(opts, "slots"); - if (maxmem_str && slots_str) { - uint64_t slots; - + if (maxmem_str) { sz = qemu_opt_get_size(opts, "maxmem", 0); + } + if (slots_str) { + slots = qemu_opt_get_number(opts, "slots", 0); + } + if (maxmem_str && slots_str) { if (sz < ram_size) { - fprintf(stderr, "qemu: invalid -m option value: maxmem " - "(%" PRIu64 ") <= initial memory (" + error_report("qemu: invalid -m option value: maxmem " + "(%" PRIx64 ") < initial memory (" RAM_ADDR_FMT ")\n", sz, ram_size); exit(EXIT_FAILURE); } - - slots = qemu_opt_get_number(opts, "slots", 0); - if ((sz > ram_size) && !slots) { - fprintf(stderr, "qemu: invalid -m option value: maxmem " - "(%" PRIu64 ") more than initial memory (" + if (!slots && (sz != ram_size)) { + error_report("qemu: invalid -m option value: maxmem " + "(%" PRIx64 ") more than initial memory (" RAM_ADDR_FMT ") but no hotplug slots where " "specified\n", sz, ram_size); exit(EXIT_FAILURE); } - - if ((sz <= ram_size) && slots) { - fprintf(stderr, "qemu: invalid -m option value: %" + if (slots && (sz == ram_size)) { + error_report("qemu: invalid -m option value: %" PRIu64 " hotplug slots where specified but " - "maxmem (%" PRIu64 ") <= initial memory (" + "maxmem (%" PRIx64 ") = initial memory (" RAM_ADDR_FMT ")\n", slots, sz, ram_size); exit(EXIT_FAILURE); } maxram_size = sz; ram_slots = slots; - } else if ((!maxmem_str && slots_str) || - (maxmem_str && !slots_str)) { - fprintf(stderr, "qemu: invalid -m option value: missing " - "'%s' option\n", slots_str ? "maxmem" : "slots"); - exit(EXIT_FAILURE); + } else if (!maxmem_str && slots_str) { + if (slots > 0) { + error_report("qemu: invalid -m option value: missing " + "'maxmem' option\n"); + exit(EXIT_FAILURE); + } + } else if (maxmem_str && !slots_str) { + if (sz != ram_size) { + error_report("qemu: invalid -m option value: missing " + "'slot' option\n"); + exit(EXIT_FAILURE); + } } break; } -- 1.7.12.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] vl: Fix the confused logic for '-m' option 2014-09-12 5:58 [Qemu-devel] [PATCH 1/2] vl: Fix the confused logic for '-m' option zhanghailiang @ 2014-09-12 6:46 ` Li Liu 2014-09-12 13:52 ` Igor Mammedov 1 sibling, 0 replies; 4+ messages in thread From: Li Liu @ 2014-09-12 6:46 UTC (permalink / raw) To: zhanghailiang, qemu-devel; +Cc: imammedo, luonengjun, peter.huangpeng, aliguori On 2014/9/12 13:58, zhanghailiang wrote: > It should be valid for the follow configure: > -m 256,slots=0 > -m 256,maxmem=256M > -m 256,slots=0,maxmem=256M > -m 256,slots=x,maxmem=y where x > 0 and y > 256M > > Fix the confused code logic and use error_report instead of fprintf. > > Printing the maxmem in hex, same with ram_size. > > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > --- > vl.c | 46 +++++++++++++++++++++++++++------------------- > 1 file changed, 27 insertions(+), 19 deletions(-) > > diff --git a/vl.c b/vl.c > index 9c9acf5..f547405 100644 > --- a/vl.c > +++ b/vl.c > @@ -3306,6 +3306,7 @@ int main(int argc, char **argv, char **envp) > break; > case QEMU_OPTION_m: { > uint64_t sz; > + uint64_t slots; > const char *mem_str; > const char *maxmem_str, *slots_str; > > @@ -3353,40 +3354,47 @@ int main(int argc, char **argv, char **envp) > > maxmem_str = qemu_opt_get(opts, "maxmem"); > slots_str = qemu_opt_get(opts, "slots"); > - if (maxmem_str && slots_str) { > - uint64_t slots; > - > + if (maxmem_str) { > sz = qemu_opt_get_size(opts, "maxmem", 0); > + } > + if (slots_str) { > + slots = qemu_opt_get_number(opts, "slots", 0); > + } > + if (maxmem_str && slots_str) { > if (sz < ram_size) { > - fprintf(stderr, "qemu: invalid -m option value: maxmem " > - "(%" PRIu64 ") <= initial memory (" > + error_report("qemu: invalid -m option value: maxmem " > + "(%" PRIx64 ") < initial memory (" > RAM_ADDR_FMT ")\n", sz, ram_size); error_report will add a '\n' automatically. Below lines have the same issue. > exit(EXIT_FAILURE); > } > - > - slots = qemu_opt_get_number(opts, "slots", 0); > - if ((sz > ram_size) && !slots) { > - fprintf(stderr, "qemu: invalid -m option value: maxmem " > - "(%" PRIu64 ") more than initial memory (" > + if (!slots && (sz != ram_size)) { > + error_report("qemu: invalid -m option value: maxmem " > + "(%" PRIx64 ") more than initial memory (" > RAM_ADDR_FMT ") but no hotplug slots where " > "specified\n", sz, ram_size); > exit(EXIT_FAILURE); > } > - > - if ((sz <= ram_size) && slots) { > - fprintf(stderr, "qemu: invalid -m option value: %" > + if (slots && (sz == ram_size)) { > + error_report("qemu: invalid -m option value: %" > PRIu64 " hotplug slots where specified but " > - "maxmem (%" PRIu64 ") <= initial memory (" > + "maxmem (%" PRIx64 ") = initial memory (" > RAM_ADDR_FMT ")\n", slots, sz, ram_size); > exit(EXIT_FAILURE); > } > maxram_size = sz; > ram_slots = slots; > - } else if ((!maxmem_str && slots_str) || > - (maxmem_str && !slots_str)) { > - fprintf(stderr, "qemu: invalid -m option value: missing " > - "'%s' option\n", slots_str ? "maxmem" : "slots"); > - exit(EXIT_FAILURE); > + } else if (!maxmem_str && slots_str) { > + if (slots > 0) { > + error_report("qemu: invalid -m option value: missing " > + "'maxmem' option\n"); > + exit(EXIT_FAILURE); > + } > + } else if (maxmem_str && !slots_str) { > + if (sz != ram_size) { > + error_report("qemu: invalid -m option value: missing " > + "'slot' option\n"); > + exit(EXIT_FAILURE); > + } > } > break; > } > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] vl: Fix the confused logic for '-m' option 2014-09-12 5:58 [Qemu-devel] [PATCH 1/2] vl: Fix the confused logic for '-m' option zhanghailiang 2014-09-12 6:46 ` Li Liu @ 2014-09-12 13:52 ` Igor Mammedov 2014-09-15 7:15 ` zhanghailiang 1 sibling, 1 reply; 4+ messages in thread From: Igor Mammedov @ 2014-09-12 13:52 UTC (permalink / raw) To: zhanghailiang; +Cc: luonengjun, qemu-devel, aliguori, peter.huangpeng On Fri, 12 Sep 2014 13:58:55 +0800 zhanghailiang <zhang.zhanghailiang@huawei.com> wrote: > It should be valid for the follow configure: > -m 256,slots=0 > -m 256,maxmem=256M Doc comment/help says that slots & maxmem must be in pair so above is not valid CLI " -m[emory] [size=]megs[,slots=n,maxmem=size] configure guest RAM size: initial amount of guest memory (default: 128MiB) slots: number of hotplug slots (default: none) maxmem: maximum amount of guest memory (default: none) " > -m 256,slots=0,maxmem=256M > -m 256,slots=x,maxmem=y where x > 0 and y > 256M s/-m 256,slots=x,maxmem=y where x > 0 and y > 256M/-m z,slots=x,maxmem=y where x > 0 and y > z(M) > > Fix the confused code logic and use error_report instead of fprintf. > > Printing the maxmem in hex, same with ram_size. > > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > --- > vl.c | 46 +++++++++++++++++++++++++++------------------- > 1 file changed, 27 insertions(+), 19 deletions(-) > > diff --git a/vl.c b/vl.c > index 9c9acf5..f547405 100644 > --- a/vl.c > +++ b/vl.c > @@ -3306,6 +3306,7 @@ int main(int argc, char **argv, char **envp) > break; > case QEMU_OPTION_m: { > uint64_t sz; > + uint64_t slots; > const char *mem_str; > const char *maxmem_str, *slots_str; > > @@ -3353,40 +3354,47 @@ int main(int argc, char **argv, char **envp) > > maxmem_str = qemu_opt_get(opts, "maxmem"); > slots_str = qemu_opt_get(opts, "slots"); > - if (maxmem_str && slots_str) { > - uint64_t slots; > - > + if (maxmem_str) { > sz = qemu_opt_get_size(opts, "maxmem", 0); > + } > + if (slots_str) { > + slots = qemu_opt_get_number(opts, "slots", 0); > + } > + if (maxmem_str && slots_str) { > if (sz < ram_size) { > - fprintf(stderr, "qemu: invalid -m option value: maxmem " > - "(%" PRIu64 ") <= initial memory (" > + error_report("qemu: invalid -m option value: maxmem " > + "(%" PRIx64 ") < initial memory (" it may be worth to add 0x before hex number, maybe printing maxmem_str instead of number here would be better, since user would able to match easily maxmem value and what is printed in error message. > RAM_ADDR_FMT ")\n", sz, ram_size); > exit(EXIT_FAILURE); > } > - > - slots = qemu_opt_get_number(opts, "slots", 0); > - if ((sz > ram_size) && !slots) { > - fprintf(stderr, "qemu: invalid -m option value: maxmem " > - "(%" PRIu64 ") more than initial memory (" > + if (!slots && (sz != ram_size)) { it would report wrong message in case of "-m 256,slots=0,maxmem=128M" > + error_report("qemu: invalid -m option value: maxmem " > + "(%" PRIx64 ") more than initial memory (" > RAM_ADDR_FMT ") but no hotplug slots where " > "specified\n", sz, ram_size); > exit(EXIT_FAILURE); > } > - > - if ((sz <= ram_size) && slots) { > - fprintf(stderr, "qemu: invalid -m option value: %" > + if (slots && (sz == ram_size)) { What takes care about the case "-m 256,slots=1,maxmem=128M"? > + error_report("qemu: invalid -m option value: %" > PRIu64 " hotplug slots where specified but " > - "maxmem (%" PRIu64 ") <= initial memory (" > + "maxmem (%" PRIx64 ") = initial memory (" > RAM_ADDR_FMT ")\n", slots, sz, ram_size); > exit(EXIT_FAILURE); > } > maxram_size = sz; > ram_slots = slots; > - } else if ((!maxmem_str && slots_str) || > - (maxmem_str && !slots_str)) { > - fprintf(stderr, "qemu: invalid -m option value: missing " > - "'%s' option\n", slots_str ? "maxmem" : "slots"); > - exit(EXIT_FAILURE); > + } else if (!maxmem_str && slots_str) { > + if (slots > 0) { > + error_report("qemu: invalid -m option value: missing " > + "'maxmem' option\n"); > + exit(EXIT_FAILURE); > + } > + } else if (maxmem_str && !slots_str) { > + if (sz != ram_size) { > + error_report("qemu: invalid -m option value: missing " > + "'slot' option\n"); > + exit(EXIT_FAILURE); > + } > } > break; > } ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] vl: Fix the confused logic for '-m' option 2014-09-12 13:52 ` Igor Mammedov @ 2014-09-15 7:15 ` zhanghailiang 0 siblings, 0 replies; 4+ messages in thread From: zhanghailiang @ 2014-09-15 7:15 UTC (permalink / raw) To: Igor Mammedov; +Cc: luonengjun, qemu-devel, aliguori, peter.huangpeng On 2014/9/12 21:52, Igor Mammedov wrote: > On Fri, 12 Sep 2014 13:58:55 +0800 > zhanghailiang<zhang.zhanghailiang@huawei.com> wrote: > >> It should be valid for the follow configure: >> -m 256,slots=0 >> -m 256,maxmem=256M > Doc comment/help says that slots& maxmem must be in pair so above is not valid CLI > " Hmm, yes, you are right;), Sorry for the noise. > -m[emory] [size=]megs[,slots=n,maxmem=size] > configure guest RAM > size: initial amount of guest memory (default: 128MiB) > slots: number of hotplug slots (default: none) > maxmem: maximum amount of guest memory (default: none) > " > >> -m 256,slots=0,maxmem=256M >> -m 256,slots=x,maxmem=y where x> 0 and y> 256M > s/-m 256,slots=x,maxmem=y where x> 0 and y> 256M/-m z,slots=x,maxmem=y where x> 0 and y> z(M) > >> >> Fix the confused code logic and use error_report instead of fprintf. >> >> Printing the maxmem in hex, same with ram_size. >> >> Signed-off-by: zhanghailiang<zhang.zhanghailiang@huawei.com> >> --- >> vl.c | 46 +++++++++++++++++++++++++++------------------- >> 1 file changed, 27 insertions(+), 19 deletions(-) >> >> diff --git a/vl.c b/vl.c >> index 9c9acf5..f547405 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -3306,6 +3306,7 @@ int main(int argc, char **argv, char **envp) >> break; >> case QEMU_OPTION_m: { >> uint64_t sz; >> + uint64_t slots; >> const char *mem_str; >> const char *maxmem_str, *slots_str; >> >> @@ -3353,40 +3354,47 @@ int main(int argc, char **argv, char **envp) >> >> maxmem_str = qemu_opt_get(opts, "maxmem"); >> slots_str = qemu_opt_get(opts, "slots"); >> - if (maxmem_str&& slots_str) { >> - uint64_t slots; >> - >> + if (maxmem_str) { >> sz = qemu_opt_get_size(opts, "maxmem", 0); >> + } >> + if (slots_str) { >> + slots = qemu_opt_get_number(opts, "slots", 0); >> + } >> + if (maxmem_str&& slots_str) { >> if (sz< ram_size) { >> - fprintf(stderr, "qemu: invalid -m option value: maxmem " >> - "(%" PRIu64 ")<= initial memory (" >> + error_report("qemu: invalid -m option value: maxmem " >> + "(%" PRIx64 ")< initial memory (" > it may be worth to add 0x before hex number, > maybe printing maxmem_str instead of number here would be better, since > user would able to match easily maxmem value and what is printed in error message. > Good idea, will fix it. > >> RAM_ADDR_FMT ")\n", sz, ram_size); >> exit(EXIT_FAILURE); >> } >> - >> - slots = qemu_opt_get_number(opts, "slots", 0); >> - if ((sz> ram_size)&& !slots) { >> - fprintf(stderr, "qemu: invalid -m option value: maxmem " >> - "(%" PRIu64 ") more than initial memory (" >> + if (!slots&& (sz != ram_size)) { > it would report wrong message in case of "-m 256,slots=0,maxmem=128M" > No, in this case, actually it will go into the above *if (sz< ram_size) {* branch statements and reports "qemu: invalid -m option value: maxmem (8000000) < initial memory (10000000)":) >> + error_report("qemu: invalid -m option value: maxmem " >> + "(%" PRIx64 ") more than initial memory (" >> RAM_ADDR_FMT ") but no hotplug slots where " >> "specified\n", sz, ram_size); >> exit(EXIT_FAILURE); >> } >> - >> - if ((sz<= ram_size)&& slots) { >> - fprintf(stderr, "qemu: invalid -m option value: %" >> + if (slots&& (sz == ram_size)) { > What takes care about the case "-m 256,slots=1,maxmem=128M"? > Same with the above explain. >> + error_report("qemu: invalid -m option value: %" >> PRIu64 " hotplug slots where specified but " >> - "maxmem (%" PRIu64 ")<= initial memory (" >> + "maxmem (%" PRIx64 ") = initial memory (" >> RAM_ADDR_FMT ")\n", slots, sz, ram_size); >> exit(EXIT_FAILURE); >> } >> maxram_size = sz; >> ram_slots = slots; >> - } else if ((!maxmem_str&& slots_str) || >> - (maxmem_str&& !slots_str)) { >> - fprintf(stderr, "qemu: invalid -m option value: missing " >> - "'%s' option\n", slots_str ? "maxmem" : "slots"); >> - exit(EXIT_FAILURE); >> + } else if (!maxmem_str&& slots_str) { >> + if (slots> 0) { >> + error_report("qemu: invalid -m option value: missing " >> + "'maxmem' option\n"); >> + exit(EXIT_FAILURE); >> + } >> + } else if (maxmem_str&& !slots_str) { >> + if (sz != ram_size) { >> + error_report("qemu: invalid -m option value: missing " >> + "'slot' option\n"); >> + exit(EXIT_FAILURE); >> + } >> } >> break; >> } > > > . > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-09-15 7:21 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-09-12 5:58 [Qemu-devel] [PATCH 1/2] vl: Fix the confused logic for '-m' option zhanghailiang 2014-09-12 6:46 ` Li Liu 2014-09-12 13:52 ` Igor Mammedov 2014-09-15 7:15 ` zhanghailiang
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).