On Fri, 12 Sep 2014 13:58:55 +0800 zhanghailiang <zhang.zhanghaili...@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.zhanghaili...@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; > }