Message ID | 5ce704edab8951c4e244d1e33e85d384dabb368e.1375194476.git.minovotn@redhat.com |
---|---|
State | New |
Headers | show |
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
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 --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) {
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(+)