Re: [Qemu-devel] [QEMU-1.6 PATCH] vl.c: Output error on invalid machine type provided

[Markus Armbruster]

Andreas Färber <address@hidden> writes:

> Am 23.08.2013 17:52, schrieb Michal Novotny:
>> Ping? There are reviews already? Anybody to apply it?
>
> There is no submaintainer for vl.c, so it must go through Anthony.
> Anthony uses the patches tool for such patches and there is an
> unresolved review comment from Eric, so please respin.
>
> Following Eric's remarks it should be [PATCH v6] then (this one
> should've been [PATCH for-1.6 v5]).
>
> Additionally...
>
>> On 08/12/2013 06:34 PM, Michal Novotny wrote:
>>> Output error message using qemu's error_report() function when user
>
> "QEMU's" (or shorter: "using error_report() when")
>
>>> provides the invalid machine type on the command line. This also saves
>>> time to find what issue is when you downgrade from one version of qemu
>
> "what the issue is", "QEMU"
>
>>> to another that doesn't support required machine type yet (the version
>>> user downgraded to have to have this patch applied too, of course).
>
> If you want to have this patch backported to 1.6 and (with
> error_report() replaced) earlier versions then you need to add a line
> "Cc: address@hidden" to the commit message.
>
>>>
>>> Signed-off-by: Michal Novotny <address@hidden>
>>> ---
>>>  vl.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/vl.c b/vl.c
>>> index f422a1c..9b4a3f9 100644
>>> --- a/vl.c
>>> +++ b/vl.c
>>> @@ -2671,6 +2671,11 @@ static QEMUMachine *machine_parse(const char *name)
>>>      if (machine) {
>>>          return machine;
>>>      }
>>> +
>>> +    if (name && !is_help_option(name)) {
>>> +        error_report("Unsupported machine type");
>
> Not seeing a change log below --- nor remembering it, was name
> intentionally not incorporated into the error message via '%s'? I'd
> consider that handy when the person getting the error is not the one
> typing the command, such as libvirt. Either way this is an improvement,

Message looks like this:

qemu-system-x86_64: -M HAL-9000: Unsupported machine type

> Reviewed-by: Andreas Färber <address@hidden>
>
> Regards,
> Andreas
>
>>> +    }
>>> +
>>>      printf("Supported machines are:\n");
>>>      for (m = first_machine; m != NULL; m = m->next) {
>>>          if (m->alias) {

The list of supported machines can scroll the error message right off
the screen.  I'd print just "Use '-M help' to list supported machines"
after the error.  Can be done on top, of course.