On 02/07/16 17:24, Marcel Apfelbaum wrote: > 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 saving the error location and also extracted the code > dealing with machine options into a separate function. > > Reported-by: Michael S. Tsirkin <[email protected]> > Signed-off-by: Marcel Apfelbaum <[email protected]> > --- > > Hi, > > I hope I used "location" correctly, if anyone is aware of a simpler > approach please advice. > > Thanks, > Marcel > > vl.c | 34 +++++++++++++++++++++++----------- > 1 file changed, 23 insertions(+), 11 deletions(-) > > diff --git a/vl.c b/vl.c > index f043009..f8a9213 100644 > --- a/vl.c > +++ b/vl.c > @@ -2751,6 +2751,26 @@ static const QEMUOption *lookup_opt(int argc, char > **argv, > return popt; > } > > +static void machine_set_options(Location *machine_opt_loc, > + MachineClass **machine_class) > +{ > + const char *optarg; > + Location *curr = loc_push_restore(machine_opt_loc); > + > + optarg = qemu_opt_get(qemu_get_machine_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(curr); > +} > + > static int machine_set_property(void *opaque, > const char *name, const char *value, > Error **errp) > @@ -2975,6 +2995,7 @@ int main(int argc, char **argv, char **envp) > const char *optarg; > const char *loadvm = NULL; > MachineClass *machine_class; > + Location machine_opt_loc = {0}; > const char *cpu_model; > const char *vga_model = NULL; > const char *qtest_chrdev = NULL; > @@ -3671,6 +3692,7 @@ int main(int argc, char **argv, char **envp) > break; > case QEMU_OPTION_M: > case QEMU_OPTION_machine: > + loc_save(&machine_opt_loc); > olist = qemu_find_opts("machine"); > opts = qemu_opts_parse_noisily(olist, optarg, true); > if (!opts) { > @@ -4019,17 +4041,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); > - } > + machine_set_options(&machine_opt_loc, &machine_class); > > set_memory_options(&ram_slots, &maxram_size, machine_class); > >
Sorry, I sent my email before noticing yours (in the original thread, "Re: [Qemu-devel] command line error handling broken?"). Looks good (extracting machine_set_options()), but I think you don't need to save the location for the option separately; the location gets saved in the option automatically. It can be restored with qemu_opts_loc_restore(), however it doesn't combine push & restore, so a none location should be pushed first. Also, "set_machine_options" would match "set_memory_options" better, just below. Thanks Laszlo
