Re: [Qemu-devel] [PATCH for-1.4 v2 4/6] qemu-option: Disable two helpful messages that got broken recently

[Luiz Capitulino]

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.

> 
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  util/qemu-option.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/util/qemu-option.c b/util/qemu-option.c
> index c12e724..5a1d03c 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -231,8 +231,10 @@ static void parse_option_size(const char *name, const 
> char *value,
>              break;
>          default:
>              error_set(errp, QERR_INVALID_PARAMETER_VALUE, name, "a size");
> +#if 0 /* conversion from qerror_report() to error_set() broke this: */
>              error_printf_unless_qmp("You may use k, M, G or T suffixes for "
>                      "kilobytes, megabytes, gigabytes and terabytes.\n");
> +#endif
>              return;
>          }
>      } else {
> @@ -771,7 +773,9 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char 
> *id,
>      if (id) {
>          if (!id_wellformed(id)) {
>              error_set(errp,QERR_INVALID_PARAMETER_VALUE, "id", "an 
> identifier");
> +#if 0 /* conversion from qerror_report() to error_set() broke this: */
>              error_printf_unless_qmp("Identifiers consist of letters, digits, 
> '-', '.', '_', starting with a letter.\n");
> +#endif
>              return NULL;
>          }
>          opts = qemu_opts_find(list, id);