[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7] vl.c: Output error on invalid machine type
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v7] vl.c: Output error on invalid machine type |
Date: |
Mon, 17 Mar 2014 09:04:52 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) |
address@hidden writes:
> From: Miroslav Rezanina <address@hidden>
>
> 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 <address@hidden>
> ---
> v7:
> - use -machine instead of -M in error help message
> - rebase to commit 0056ae2
>
> v6:
> - print help instead of list supported machines on error
> ---
> vl.c | 21 +++++++++++++--------
> 1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 862cf20..cbd1381 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2649,15 +2649,20 @@ static MachineClass *machine_parse(const char *name)
> if (mc) {
> return mc;
> }
> - printf("Supported machines are:\n");
> - for (el = machines; el; el = el->next) {
> - MachineClass *mc = el->data;
> - QEMUMachine *m = mc->qemu_machine;
> - 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 -machine help to list supported machines!\n");
Why the newline before 'Use'?
Recommend to write the hint to the same fd as the error message. An
obvious way to do that is error_printf(), and it's what we do elsewhere.
Both missed this in my review of v1, sorry.
> + } else {
> + printf("Supported machines are:\n");
> + for (el = machines; el; el = el->next) {
> + MachineClass *mc = el->data;
> + QEMUMachine *m = mc->qemu_machine;
> + 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)" : "");
> }
>
> g_slist_free(machines);
The functions logic is a bit tortured (not your fault). Here's how I'd
clean it up:
static MachineClass *machine_parse(const char *name)
{
MachineClass *mc;
GSList *el, *machines;
if (is_help_option(name)) {
machines = object_class_get_list(TYPE_MACHINE, false);
printf("Supported machines are:\n");
for (el = machines; el; el = el->next) {
MachineClass *mc = el->data;
QEMUMachine *m = mc->qemu_machine;
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)" : "");
}
g_slist_free(machines);
exit(0);
}
mc = find_machine(name);
if (!mc) {
error_report("Unsupported machine type");
error_printf("Use '-machine help' to list supported machines\n");
exit(1);
}
return mc;
}