On Wed, Jan 16, 2013 at 10:25:31AM -0700, Eric Blake wrote:
> On 01/16/2013 08:24 AM, Eduardo Habkost wrote:
> > This should catch many kinds of errors that the current code wasn't
> > checking for:
> >
> > - Values that can't be parsed as a number
> > - Negative values
> > - Overflow
> > - Empty string
> >
> > Signed-off-by: Eduardo Habkost <ehabk...@redhat.com>
> > ---
> > Cc: Eric Blake <ebl...@redhat.com>
> > ---
> > vl.c | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
>
> > +++ b/vl.c
> > @@ -1264,11 +1264,14 @@ static void numa_add(const char *optarg)
> > if (get_param_value(option, 128, "nodeid", optarg) == 0) {
> > nodenr = nb_numa_nodes;
> > } else {
> > - nodenr = strtoull(option, NULL, 10);
> > + if (parse_uint_full(option, &nodenr) < 0) {
>
> This allows a user to pass octal or hex numbers, where previously it was
> forced to be decimal. That means 'nodeid=010' is now '8' instead of
> '10'; is that intentional?
It wasn't intentional, thanks for catching it!
However, the existing option parsing infra-structure
(parse_option_number() at qemu-option.c and opts_type_uint64() at
opts-visitor.c) use 0 as the 'base' parameter. So when we convert this
code to use that infra-structure, it will end up accepting hex and octal
numbers as well.
So I believe I will fix it this way:
- Add 'base' parameter to parse_uint*()
- Use base=10 when parsing "-numa" (keeping "nodeid=010" working, just
in case)
- Use the QemuOpts number parser when introducing the "numa-node"
config section, and manually parse/convert the (then obsolete)
"-numa node,..." option to "numa-node"
Or, we could simply change the meaning of "nodeid=010" and document it
in the commit message and QEMU ChageLog. Which option do the QEMU
maintainers prefer?
>
> >
> > if (nodenr >= MAX_NODES) {
> > - fprintf(stderr, "qemu: invalid NUMA nodeid: %d\n", nodenr);
> > + fprintf(stderr, "qemu: invalid NUMA nodeid: %llu\n", nodenr);
>
> Already mentions that this part belongs in 5/8.
I will fix it. Thanks.
--
Eduardo
