qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [PATCH for-1.4 v2 4/6] qemu-option: Disable two helpf


From: Markus Armbruster
Subject: Re: [Qemu-trivial] [PATCH for-1.4 v2 4/6] qemu-option: Disable two helpful messages that got broken recently
Date: Fri, 08 Feb 2013 20:34:20 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux)

Luiz Capitulino <address@hidden> writes:

> On Fri, 08 Feb 2013 19:58:42 +0100
> Markus Armbruster <address@hidden> wrote:
>
>> Luiz Capitulino <address@hidden> writes:
>> 
>> > On Fri,  8 Feb 2013 17:17:10 +0100
>> > Markus Armbruster <address@hidden> 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 <address@hidden>
>> >
>> > 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.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]