Patchwork [v4] vl.c: Output error on invalid machine type provided

login
register
mail settings
Submitter Michal Novotny
Date July 31, 2013, 6:30 a.m.
Message ID <0e2ed0a9f48b8e6bbfbfa127e2e92c3d01b591fd.1375251985.git.minovotn@redhat.com>
Download mbox | patch
Permalink /patch/263590/
State New
Headers show

Comments

Michal Novotny - July 31, 2013, 6:30 a.m.
Output error message to stderr when user 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 to another that doesn't
support required machine type yet (the version user downgraded to have
to have this patch applied too, of course).

Signed-off-by: Michal Novotny <minovotn@redhat.com>
---
 vl.c | 5 +++++
 1 file changed, 5 insertions(+)
Markus Armbruster - July 31, 2013, 6:45 a.m.
Michal Novotny <minovotn@redhat.com> writes:

> Output error message to stderr when user 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 to another that doesn't
> support required machine type yet (the version user downgraded to have
> to have this patch applied too, of course).
>
> Signed-off-by: Michal Novotny <minovotn@redhat.com>
> ---
>  vl.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/vl.c b/vl.c
> index f422a1c..6ee1a03 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)) {
> +        fprintf(stderr, "Error: Unsupported machine type '%s'\n", name);
> +    }
> +
>      printf("Supported machines are:\n");
>      for (m = first_machine; m != NULL; m = m->next) {
>          if (m->alias) {

Sorry, should've looked more closely.  Please use error_report() like
this:

        error_report("Unsupported machine type");

Improves the message from

    Error: Unsupported machine type 'HAL-9000'

to the standard form

    qemu-system-x86_64: -M HAL-9000: Unsupported machine type
Michal Novotny - July 31, 2013, 6:54 a.m.
On 07/31/2013 08:45 AM, Markus Armbruster wrote:
> Michal Novotny <minovotn@redhat.com> writes:
>
>> Output error message to stderr when user 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 to another that doesn't
>> support required machine type yet (the version user downgraded to have
>> to have this patch applied too, of course).
>>
>> Signed-off-by: Michal Novotny <minovotn@redhat.com>
>> ---
>>  vl.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/vl.c b/vl.c
>> index f422a1c..6ee1a03 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)) {
>> +        fprintf(stderr, "Error: Unsupported machine type '%s'\n", name);
>> +    }
>> +
>>      printf("Supported machines are:\n");
>>      for (m = first_machine; m != NULL; m = m->next) {
>>          if (m->alias) {
> Sorry, should've looked more closely.  Please use error_report() like
> this:
>
>         error_report("Unsupported machine type");
>
> Improves the message from
>
>     Error: Unsupported machine type 'HAL-9000'
>
> to the standard form
>
>     qemu-system-x86_64: -M HAL-9000: Unsupported machine type

That's nice qemu has this function, I didn't know. However, there are
still some code parts that are not using error_report() as well, e.g.

fprintf(stderr, "Error: standard VGA not available\n");

in the select_vgahw() function (line 2168) or:

            fprintf(stderr, "Failed to start VNC server on `%s': %s\n",
                    vnc_display, error_get_pretty(local_err));

on line 4385 or:

fprintf(stderr, "-incoming %s: %s\n", incoming,
error_get_pretty(local_err));

on line 4428 (I've been looking just to vl.c file so far).

Shouldn't all this be using error_report() as well? If so, a follow-up
patch would be nice.

Thanks,
Michal
Michal Novotny - July 31, 2013, 7:05 a.m.
On 07/31/2013 08:45 AM, Markus Armbruster wrote:
> Michal Novotny <minovotn@redhat.com> writes:
>
>> Output error message to stderr when user 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 to another that doesn't
>> support required machine type yet (the version user downgraded to have
>> to have this patch applied too, of course).
>>
>> Signed-off-by: Michal Novotny <minovotn@redhat.com>
>> ---
>>  vl.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/vl.c b/vl.c
>> index f422a1c..6ee1a03 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)) {
>> +        fprintf(stderr, "Error: Unsupported machine type '%s'\n", name);
>> +    }
>> +
>>      printf("Supported machines are:\n");
>>      for (m = first_machine; m != NULL; m = m->next) {
>>          if (m->alias) {
> Sorry, should've looked more closely.  Please use error_report() like
> this:
>
>         error_report("Unsupported machine type");
>
> Improves the message from
>
>     Error: Unsupported machine type 'HAL-9000'
>
> to the standard form
>
>     qemu-system-x86_64: -M HAL-9000: Unsupported machine type

FYI: v5 has been sent using the error_report()

Michal
>
Markus Armbruster - July 31, 2013, 7:56 a.m.
Michal Novotny <minovotn@redhat.com> writes:

> On 07/31/2013 08:45 AM, Markus Armbruster wrote:
>> Michal Novotny <minovotn@redhat.com> writes:
>>
>>> Output error message to stderr when user 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 to another that doesn't
>>> support required machine type yet (the version user downgraded to have
>>> to have this patch applied too, of course).
>>>
>>> Signed-off-by: Michal Novotny <minovotn@redhat.com>
>>> ---
>>>  vl.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/vl.c b/vl.c
>>> index f422a1c..6ee1a03 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)) {
>>> +        fprintf(stderr, "Error: Unsupported machine type '%s'\n", name);
>>> +    }
>>> +
>>>      printf("Supported machines are:\n");
>>>      for (m = first_machine; m != NULL; m = m->next) {
>>>          if (m->alias) {
>> Sorry, should've looked more closely.  Please use error_report() like
>> this:
>>
>>         error_report("Unsupported machine type");
>>
>> Improves the message from
>>
>>     Error: Unsupported machine type 'HAL-9000'
>>
>> to the standard form
>>
>>     qemu-system-x86_64: -M HAL-9000: Unsupported machine type
>
> That's nice qemu has this function, I didn't know. However, there are
> still some code parts that are not using error_report() as well, e.g.
>
> fprintf(stderr, "Error: standard VGA not available\n");
>
> in the select_vgahw() function (line 2168) or:
>
>             fprintf(stderr, "Failed to start VNC server on `%s': %s\n",
>                     vnc_display, error_get_pretty(local_err));
>
> on line 4385 or:
>
> fprintf(stderr, "-incoming %s: %s\n", incoming,
> error_get_pretty(local_err));
>
> on line 4428 (I've been looking just to vl.c file so far).

Lots of them, all over the place.

> Shouldn't all this be using error_report() as well? If so, a follow-up
> patch would be nice.

As usual, patches welcome :)
Andreas Färber - July 31, 2013, 8:03 a.m.
Am 31.07.2013 08:54, schrieb Michal Novotny:
> there are
> still some code parts that are not using error_report() as well, e.g.
> 
> fprintf(stderr, "Error: standard VGA not available\n");
> 
> in the select_vgahw() function (line 2168) or:
> 
>             fprintf(stderr, "Failed to start VNC server on `%s': %s\n",
>                     vnc_display, error_get_pretty(local_err));
> 
> on line 4385 or:
> 
> fprintf(stderr, "-incoming %s: %s\n", incoming,
> error_get_pretty(local_err));
> 
> on line 4428 (I've been looking just to vl.c file so far).
> 
> Shouldn't all this be using error_report() as well? If so, a follow-up
> patch would be nice.

See http://patchwork.ozlabs.org/patch/260801/ for a start. :)

Regards,
Andreas

Patch

diff --git a/vl.c b/vl.c
index f422a1c..6ee1a03 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)) {
+        fprintf(stderr, "Error: Unsupported machine type '%s'\n", name);
+    }
+
     printf("Supported machines are:\n");
     for (m = first_machine; m != NULL; m = m->next) {
         if (m->alias) {