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.