Luiz Capitulino <lcapitul...@redhat.com> writes: > On Fri, 08 Feb 2013 19:58:42 +0100 > Markus Armbruster <arm...@redhat.com> wrote: > >> Luiz Capitulino <lcapitul...@redhat.com> writes: >> >> > On Fri, 8 Feb 2013 17:17:10 +0100 >> > Markus Armbruster <arm...@redhat.com> wrote: >> > >> >> commit 8be7e7e4 and commit ec7b2ccb messed up the ordering of error >> >> message and the helpful explanation that should follow it, like this: >> >> >> >> $ qemu-system-x86_64 --nodefaults -S --vnc :0 --chardev null,id=, >> >> Identifiers consist of letters, digits, '-', '.', '_', starting >> >> with a letter. >> >> qemu-system-x86_64: -chardev null,id=,: Parameter 'id' expects >> >> an identifier >> >> >> >> $ qemu-system-x86_64 --nodefaults -S --vnc :0 --machine >> >> kvm_shadow_mem=dunno >> >> You may use k, M, G or T suffixes for kilobytes, megabytes, >> >> gigabytes and terabytes. >> >> qemu-system-x86_64: -machine kvm_shadow_mem=dunno: Parameter >> >> kvm_shadow_mem' expects a size >> >> >> >> Pity. Disable them for now. >> > >> > Oops, sorry. I think I'd prefer to drop them, but as this fixes the >> > problem: >> > >> > Reviewed-by: Luiz Capitulino <lcapitul...@redhat.com> >> > >> > Also, the real problem here is that general functions like >> > parse_option_size() >> > should never assume/try to guess in which context they're running. So, the >> > best solution here is either to have a general enough error message that's >> > not tied to any context, or let up layers (say vl.c) concatenate the >> > context-dependent part of the error message. >> >> I'm open to suggestions on how to better code the pattern "report an >> error (a short string without newlines) together with some explanation >> or hint (possibly longer string, newlines okay). > > The real problem here is that the k, M, G suffixes, for example, are not > good to be reported by QMP. So maybe we should refactor the code in a way > that we separate what's done in QMP from what is done in HMP/command-line.
Isn't it separated already? parse_option_size() is used when parsing key=value,... Such strings should not exist in QMP. If they do, it's a design bug.
