From 34f405ae6d5c4170b192a12b2e654a2aea0c3b50 Mon Sep 17 00:00:00 2001 From: Marcel Apfelbaum Date: Fri, 12 Feb 2016 17:02:24 -0200 Subject: [PATCH 1/5] vl.c: Fix regression in machine error message Commit e1ce0c3cb (vl.c: fix regression when reading machine type from config file) fixed the error message when the machine type was supplied inside the config file. However now the option name is not displayed correctly if the error happens when the machine is specified at command line. Running ./x86_64-softmmu/qemu-system-x86_64 -M q35-1.5 -redir tcp:8022::22 will result in the error message: qemu-system-x86_64: -redir tcp:8022::22: unsupported machine type Use -machine help to list supported machines Fixed it by restoring the error location and also extracted the code dealing with machine options into a separate function. Reported-by: Michael S. Tsirkin Reviewed-by: Laszlo Ersek Signed-off-by: Marcel Apfelbaum Reviewed-by: Eduardo Habkost Signed-off-by: Eduardo Habkost Message-Id: <1455303747-19776-2-git-send-email-ehabkost@redhat.com> Signed-off-by: Markus Armbruster --- vl.c | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/vl.c b/vl.c index 18e6086ffe..226bdeb9af 100644 --- a/vl.c +++ b/vl.c @@ -2762,6 +2762,31 @@ static const QEMUOption *lookup_opt(int argc, char **argv, return popt; } +static void set_machine_options(MachineClass **machine_class) +{ + const char *optarg; + QemuOpts *opts; + Location loc; + + loc_push_none(&loc); + + opts = qemu_get_machine_opts(); + qemu_opts_loc_restore(opts); + + optarg = qemu_opt_get(opts, "type"); + if (optarg) { + *machine_class = machine_parse(optarg); + } + + if (*machine_class == NULL) { + error_report("No machine specified, and there is no default"); + error_printf("Use -machine help to list supported machines\n"); + exit(1); + } + + loc_pop(&loc); +} + static int machine_set_property(void *opaque, const char *name, const char *value, Error **errp) @@ -3986,17 +4011,7 @@ int main(int argc, char **argv, char **envp) replay_configure(icount_opts); - opts = qemu_get_machine_opts(); - optarg = qemu_opt_get(opts, "type"); - if (optarg) { - machine_class = machine_parse(optarg); - } - - if (machine_class == NULL) { - error_report("No machine specified, and there is no default"); - error_printf("Use -machine help to list supported machines\n"); - exit(1); - } + set_machine_options(&machine_class); set_memory_options(&ram_slots, &maxram_size, machine_class); From 43fa1e0bd98887fb5ead745de13dc9961799e97e Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 12 Feb 2016 17:02:25 -0200 Subject: [PATCH 2/5] vl: Reset location after handling command-line arguments After looping through all command-line arguments, error location info becomes obsolete, and any function calling error_report() will print misleading information. This breaks error reporting for some option handling, like: $ qemu-system-x86_64 -icount rr=x -vnc :0 qemu-system-x86_64: -vnc :0: Invalid icount rr option: x $ qemu-system-x86_64 -m size= -vnc :0 qemu-system-x86_64: -vnc :0: missing 'size' option value Fix this by resetting location info as soon as we exit the command-line handling loop. With this, replay_configure() and set_memory_options() won't print any location info yet, but at least they won't print incorrect information. Signed-off-by: Eduardo Habkost Message-Id: <1455303747-19776-3-git-send-email-ehabkost@redhat.com> Reviewed-by: Laszlo Ersek ["Do not insert code here" comment added to prevent regressions] Signed-off-by: Markus Armbruster --- vl.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/vl.c b/vl.c index 226bdeb9af..bf0ef90cf9 100644 --- a/vl.c +++ b/vl.c @@ -4008,6 +4008,11 @@ int main(int argc, char **argv, char **envp) } } } + /* + * Clear error location left behind by the loop. + * Best done right after the loop. Do not insert code here! + */ + loc_set_none(); replay_configure(icount_opts); @@ -4015,8 +4020,6 @@ int main(int argc, char **argv, char **envp) set_memory_options(&ram_slots, &maxram_size, machine_class); - loc_set_none(); - os_daemonize(); if (qemu_init_main_loop(&main_loop_err)) { From 890ad5508e0e450272faa4b38611b9df5bab871a Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 12 Feb 2016 17:02:26 -0200 Subject: [PATCH 3/5] replay: Set error location properly when parsing options Set error location so the error_report() calls will show appropriate command-line argument or config file info. Signed-off-by: Eduardo Habkost Message-Id: <1455303747-19776-4-git-send-email-ehabkost@redhat.com> Reviewed-by: Laszlo Ersek Signed-off-by: Markus Armbruster --- replay/replay.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/replay/replay.c b/replay/replay.c index 9cac178697..f8739c26c8 100644 --- a/replay/replay.c +++ b/replay/replay.c @@ -262,6 +262,14 @@ void replay_configure(QemuOpts *opts) const char *fname; const char *rr; ReplayMode mode = REPLAY_MODE_NONE; + Location loc; + + if (!opts) { + return; + } + + loc_push_none(&loc); + qemu_opts_loc_restore(opts); rr = qemu_opt_get(opts, "rr"); if (!rr) { @@ -283,6 +291,8 @@ void replay_configure(QemuOpts *opts) } replay_enable(fname, mode); + + loc_pop(&loc); } void replay_start(void) From bbe2d25c8f89db4f0075a1321b1911b42d97e436 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 12 Feb 2016 17:02:27 -0200 Subject: [PATCH 4/5] vl: Set error location when parsing memory options Set error location so the error_report() calls will show appropriate command-line argument or config file info. Signed-off-by: Eduardo Habkost Message-Id: <1455303747-19776-5-git-send-email-ehabkost@redhat.com> Reviewed-by: Laszlo Ersek Signed-off-by: Markus Armbruster --- vl.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/vl.c b/vl.c index bf0ef90cf9..8c1a1ffbc5 100644 --- a/vl.c +++ b/vl.c @@ -2863,6 +2863,10 @@ static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size, const char *maxmem_str, *slots_str; const ram_addr_t default_ram_size = mc->default_ram_size; QemuOpts *opts = qemu_find_opts_singleton("memory"); + Location loc; + + loc_push_none(&loc); + qemu_opts_loc_restore(opts); sz = 0; mem_str = qemu_opt_get(opts, "size"); @@ -2937,6 +2941,8 @@ static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size, "'%s' option", slots_str ? "maxmem" : "slots"); exit(EXIT_FAILURE); } + + loc_pop(&loc); } int main(int argc, char **argv, char **envp) From 7580f231cf3f1710951416ec85df7b3f79dbaf86 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 16 Feb 2016 15:51:53 +0100 Subject: [PATCH 5/5] vl: Clean up machine selection in main(). We set machine_class to the default first, and update it to the real one later. Any use of machine_class in between is almost certainly wrong (there are no such uses right now). Set it once and for all instead. Signed-off-by: Markus Armbruster Reviewed-by: Laszlo Ersek Reviewed-by: Marcel Apfelbaum --- vl.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/vl.c b/vl.c index 8c1a1ffbc5..b87e29268c 100644 --- a/vl.c +++ b/vl.c @@ -2762,8 +2762,9 @@ static const QEMUOption *lookup_opt(int argc, char **argv, return popt; } -static void set_machine_options(MachineClass **machine_class) +static MachineClass *select_machine(void) { + MachineClass *machine_class = find_default_machine(); const char *optarg; QemuOpts *opts; Location loc; @@ -2775,16 +2776,17 @@ static void set_machine_options(MachineClass **machine_class) optarg = qemu_opt_get(opts, "type"); if (optarg) { - *machine_class = machine_parse(optarg); + machine_class = machine_parse(optarg); } - if (*machine_class == NULL) { + if (!machine_class) { error_report("No machine specified, and there is no default"); error_printf("Use -machine help to list supported machines\n"); exit(1); } loc_pop(&loc); + return machine_class; } static int machine_set_property(void *opaque, @@ -3031,7 +3033,6 @@ int main(int argc, char **argv, char **envp) os_setup_early_signal_handling(); module_call_init(MODULE_INIT_MACHINE); - machine_class = find_default_machine(); cpu_model = NULL; snapshot = 0; cyls = heads = secs = 0; @@ -4022,7 +4023,7 @@ int main(int argc, char **argv, char **envp) replay_configure(icount_opts); - set_machine_options(&machine_class); + machine_class = select_machine(); set_memory_options(&ram_slots, &maxram_size, machine_class);