| Message ID | 90998dac0113b5e791189eb3240d60c13446c709.1391607703.git.mrezanin@redhat.com |
|---|---|
| State | New |
| Headers | show |
Hi, is there any issue with this patch? Mirek ----- Original Message ----- > From: mrezanin@redhat.com > To: qemu-devel@nongnu.org > Sent: Wednesday, February 5, 2014 2:44:23 PM > Subject: [Qemu-devel] [PATCH v6] vl.c: Output error on invalid machine type > > From: Miroslav Rezanina <mrezanin@redhat.com> > > Output error message using qemu's error_report() function 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: Miroslav Rezanina <mrezanin@redhat.com> > --- > v6: > - print help instead of list supported machines on error > vl.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/vl.c b/vl.c > index 383be1b..3297c0a 100644 > --- a/vl.c > +++ b/vl.c > @@ -2600,13 +2600,19 @@ static QEMUMachine *machine_parse(const char *name) > if (machine) { > return machine; > } > - printf("Supported machines are:\n"); > - for (m = first_machine; m != NULL; m = m->next) { > - if (m->alias) { > - printf("%-20s %s (alias of %s)\n", m->alias, m->desc, m->name); > + > + if (name && !is_help_option(name)) { > + error_report("Unsupported machine type"); > + printf("\nUse '-M help' to list supported machines!\n"); > + } else { > + printf("Supported machines are:\n"); > + for (m = first_machine; m != NULL; m = m->next) { > + if (m->alias) { > + printf("%-20s %s (alias of %s)\n", m->alias, m->desc, > m->name); > + } > + printf("%-20s %s%s\n", m->name, m->desc, > + m->is_default ? " (default)" : ""); > } > - printf("%-20s %s%s\n", m->name, m->desc, > - m->is_default ? " (default)" : ""); > } > exit(!name || !is_help_option(name)); > } > -- > 1.8.5.3 > > >
mrezanin@redhat.com writes: > From: Miroslav Rezanina <mrezanin@redhat.com> > > Output error message using qemu's error_report() function 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: Miroslav Rezanina <mrezanin@redhat.com> > --- > v6: > - print help instead of list supported machines on error > vl.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/vl.c b/vl.c > index 383be1b..3297c0a 100644 > --- a/vl.c > +++ b/vl.c > @@ -2600,13 +2600,19 @@ static QEMUMachine *machine_parse(const char *name) static QEMUMachine *machine_parse(const char *name) { QEMUMachine *m, *machine = NULL; if (name) { machine = find_machine(name); } > if (machine) { > return machine; > } > - printf("Supported machines are:\n"); > - for (m = first_machine; m != NULL; m = m->next) { > - if (m->alias) { > - printf("%-20s %s (alias of %s)\n", m->alias, m->desc, m->name); > + > + if (name && !is_help_option(name)) { I don't think !name can happen, but you're sticking to what the existing code does. Okay. > + error_report("Unsupported machine type"); > + printf("\nUse '-M help' to list supported machines!\n"); I like this. It gives a concise error message followed by a hint, instead of dumping lengthy help on me without a clear indication why I need it. -M TYPE is sugar for -machine type=TYPE. It was deprecated in commit 80f52a6, and purged from documentation and help texts, except for two occurences in qemu-doc.texi. Unless we un-deprecate -M, this patch should point to the documented option -machine type=help, and we should update qemu-doc.texi to use -machine instead of -M. > + } else { > + printf("Supported machines are:\n"); > + for (m = first_machine; m != NULL; m = m->next) { > + if (m->alias) { > + printf("%-20s %s (alias of %s)\n", m->alias, m->desc, m->name); > + } > + printf("%-20s %s%s\n", m->name, m->desc, > + m->is_default ? " (default)" : ""); > } > - printf("%-20s %s%s\n", m->name, m->desc, > - m->is_default ? " (default)" : ""); > } > exit(!name || !is_help_option(name)); > }
Hi, Am 10.03.2014 09:35, schrieb Miroslav Rezanina: > Hi, > is there any issue with this patch? It conflicts with Marcel's machine rework that I have queued. I wonder if we can avoid the reindent to minimize the collision by adding an exit(EXIT_FAILURE) in the new if? Regards, Andreas > Mirek > ----- Original Message ----- >> From: mrezanin@redhat.com >> To: qemu-devel@nongnu.org >> Sent: Wednesday, February 5, 2014 2:44:23 PM >> Subject: [Qemu-devel] [PATCH v6] vl.c: Output error on invalid machine type >> >> From: Miroslav Rezanina <mrezanin@redhat.com> >> >> Output error message using qemu's error_report() function 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: Miroslav Rezanina <mrezanin@redhat.com> >> --- >> v6: >> - print help instead of list supported machines on error >> vl.c | 18 ++++++++++++------ >> 1 file changed, 12 insertions(+), 6 deletions(-) >> >> diff --git a/vl.c b/vl.c >> index 383be1b..3297c0a 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -2600,13 +2600,19 @@ static QEMUMachine *machine_parse(const char *name) >> if (machine) { >> return machine; >> } >> - printf("Supported machines are:\n"); >> - for (m = first_machine; m != NULL; m = m->next) { >> - if (m->alias) { >> - printf("%-20s %s (alias of %s)\n", m->alias, m->desc, m->name); >> + >> + if (name && !is_help_option(name)) { >> + error_report("Unsupported machine type"); >> + printf("\nUse '-M help' to list supported machines!\n"); >> + } else { >> + printf("Supported machines are:\n"); >> + for (m = first_machine; m != NULL; m = m->next) { >> + if (m->alias) { >> + printf("%-20s %s (alias of %s)\n", m->alias, m->desc, >> m->name); >> + } >> + printf("%-20s %s%s\n", m->name, m->desc, >> + m->is_default ? " (default)" : ""); >> } >> - printf("%-20s %s%s\n", m->name, m->desc, >> - m->is_default ? " (default)" : ""); >> } >> exit(!name || !is_help_option(name)); >> } >> -- >> 1.8.5.3
diff --git a/vl.c b/vl.c index 383be1b..3297c0a 100644 --- a/vl.c +++ b/vl.c @@ -2600,13 +2600,19 @@ static QEMUMachine *machine_parse(const char *name) if (machine) { return machine; } - printf("Supported machines are:\n"); - for (m = first_machine; m != NULL; m = m->next) { - if (m->alias) { - printf("%-20s %s (alias of %s)\n", m->alias, m->desc, m->name); + + if (name && !is_help_option(name)) { + error_report("Unsupported machine type"); + printf("\nUse '-M help' to list supported machines!\n"); + } else { + printf("Supported machines are:\n"); + for (m = first_machine; m != NULL; m = m->next) { + if (m->alias) { + printf("%-20s %s (alias of %s)\n", m->alias, m->desc, m->name); + } + printf("%-20s %s%s\n", m->name, m->desc, + m->is_default ? " (default)" : ""); } - printf("%-20s %s%s\n", m->name, m->desc, - m->is_default ? " (default)" : ""); } exit(!name || !is_help_option(name)); }