* Markus Armbruster (arm...@redhat.com) wrote:
> "Dr. David Alan Gilbert (git)" <dgilb...@redhat.com> writes:
>
> > From: "Dr. David Alan Gilbert" <dgilb...@redhat.com>
> >
> > The 'name' option silently failed when used in config files
> > ( http://lists.gnu.org/archive/html/qemu-devel/2014-04/msg00378.html )
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> > Reported-by: William Dauchy <wdau...@gmail.com>
> > ---
> > vl.c | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/vl.c b/vl.c
> > index 8411a4a..47c199a 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -965,7 +965,7 @@ static int parse_sandbox(QemuOpts *opts, void *opaque)
> > return 0;
> > }
> >
> > -static void parse_name(QemuOpts *opts)
> > +static int parse_name(QemuOpts *opts, void *opaque)
> > {
> > const char *proc_name;
> >
> > @@ -978,6 +978,8 @@ static void parse_name(QemuOpts *opts)
> > if (proc_name) {
> > os_set_proc_name(proc_name);
> > }
> > +
> > + return 0;
> > }
> >
> > bool usb_enabled(bool default_usb)
> > @@ -3780,7 +3782,6 @@ int main(int argc, char **argv, char **envp)
> > if (!opts) {
> > exit(1);
> > }
> > - parse_name(opts);
> > break;
> > case QEMU_OPTION_prom_env:
> > if (nb_prom_envs >= MAX_PROM_ENVS) {
> > @@ -3955,6 +3956,10 @@ int main(int argc, char **argv, char **envp)
> > exit(1);
> > }
> >
> > + if (qemu_opts_foreach(qemu_find_opts("name"), parse_name, NULL, 1)) {
> > + exit(1);
> > + }
> > +
>
> This will never exit, but that's okay.
Ah, because my parse_name currently never fails?
> > #ifndef _WIN32
> > if (qemu_opts_foreach(qemu_find_opts("add-fd"), parse_add_fd, NULL,
> > 1)) {
> > exit(1);
>
> -readconfig stores the configuration read in QemuOpts. Command line
> option parsing should do the same, and no more. In particular it should
> not act upon the option. That needs to be done separately, where both
> command line and -readconfig settings are visible in QemuOpts.
>
> Your patch does exactly that. I think amending the commit message with
> the previous paragraph would improve it.
>
> Have you checked command line option parsing (the big switch) for
> similar problems?
I hadn't, other than fixing up -name; tbh It's taken me a while to understand
how QemuOpts is supposed to work (and hence why I didn't get this in the 1st
patch); I'd seen the qemu_opts_foreach uses at the bottom of the switch, but
since they looked like a loop, I'd assumed they were only for options with
multiple sets of values and not looked any further.
The big switch seems to be a mix of a lot of different ways of doing things;
A quick scan shows rtc, option_rom, usb_device, and others all use
qemu_opts_parse
in the big switch but also taking an action in the switch.
> Reviewed-by: Markus Armbruster <arm...@redhat.com>
Thanks.
Dave
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
