diff mbox

vl.c: Output error on invalid machine type provided

Message ID 5ce704edab8951c4e244d1e33e85d384dabb368e.1375194476.git.minovotn@redhat.com
State New
Headers show

Commit Message

Michal Novotny July 30, 2013, 2:28 p.m. UTC
Output error message when user provides the invalid machine type
on the command line. This also saves time to find what issue is
when you upgrade from one version of qemu to another version that
doesn't support required machine type any longer.

Michal

Signed-off-by: Michal Novotny <minovotn@redhat.com>
---
 vl.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Peter Maydell July 30, 2013, 2:46 p.m. UTC | #1
On 30 July 2013 15:28, Michal Novotny <minovotn@redhat.com> wrote:
> Output error message when user provides the invalid machine type
> on the command line. This also saves time to find what issue is
> when you upgrade from one version of qemu to another version that
> doesn't support required machine type any longer.
>
> Michal
>
> Signed-off-by: Michal Novotny <minovotn@redhat.com>
> ---
>  vl.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/vl.c b/vl.c
> index 25b8f2f..4455b26 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2671,6 +2671,10 @@ static QEMUMachine *machine_parse(const char *name)
>      if (machine) {
>          return machine;
>      }
> +
> +    if (*name != '0')
> +        printf("Error: Unsupported machine type '%s'\n", name);
> +

Hi; thanks for the patch. I think the idea is a good
one but there are some minor issues with the implementation:

This now causes "-M help" to print a spurious line
"Error: Unsupported machine type 'help'".

You need braces around if statements, even one-liners.
(scripts/checkpatch.pl may help with this kind of thing.)

This function can be called with name being NULL but
you try to dereference it.

My suggestion is that your condition should be
    if (name && !is_help_option(name)) {
        ...
    }

>      printf("Supported machines are:\n");
>      for (m = first_machine; m != NULL; m = m->next) {
>          if (m->alias) {
> --
> 1.7.11.7
>

thanks
-- PMM
Michal Novotny July 30, 2013, 2:56 p.m. UTC | #2
On 07/30/2013 04:46 PM, Peter Maydell wrote:
> On 30 July 2013 15:28, Michal Novotny <minovotn@redhat.com> wrote:
>> Output error message when user provides the invalid machine type
>> on the command line. This also saves time to find what issue is
>> when you upgrade from one version of qemu to another version that
>> doesn't support required machine type any longer.
>>
>> Michal
>>
>> Signed-off-by: Michal Novotny <minovotn@redhat.com>
>> ---
>>  vl.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/vl.c b/vl.c
>> index 25b8f2f..4455b26 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2671,6 +2671,10 @@ static QEMUMachine *machine_parse(const char *name)
>>      if (machine) {
>>          return machine;
>>      }
>> +
>> +    if (*name != '0')
>> +        printf("Error: Unsupported machine type '%s'\n", name);
>> +
> Hi; thanks for the patch. I think the idea is a good
> one but there are some minor issues with the implementation:
>
> This now causes "-M help" to print a spurious line
> "Error: Unsupported machine type 'help'".
>
> You need braces around if statements, even one-liners.
> (scripts/checkpatch.pl may help with this kind of thing.)
>
> This function can be called with name being NULL but
> you try to dereference it.
>
> My suggestion is that your condition should be
>     if (name && !is_help_option(name)) {
>         ...
>     }

Hi Peter,
thanks a lot for your reply. I rewrote the patch and checked it using
scripts/checkpatch.pl. It's already sent as v2 to the list.

Thanks a lot for your feedback,
Michal
diff mbox

Patch

diff --git a/vl.c b/vl.c
index 25b8f2f..4455b26 100644
--- a/vl.c
+++ b/vl.c
@@ -2671,6 +2671,10 @@  static QEMUMachine *machine_parse(const char *name)
     if (machine) {
         return machine;
     }
+
+    if (*name != '0')
+        printf("Error: Unsupported machine type '%s'\n", name);
+
     printf("Supported machines are:\n");
     for (m = first_machine; m != NULL; m = m->next) {
         if (m->alias) {