[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH-for-6.2 v3] qdev-monitor: Only allow full --global <drive
From: |
Daniel P . Berrangé |
Subject: |
Re: [RFC PATCH-for-6.2 v3] qdev-monitor: Only allow full --global <driver>.<property>=<val> option |
Date: |
Mon, 22 Nov 2021 13:21:21 +0000 |
User-agent: |
Mutt/2.1.3 (2021-09-10) |
On Sat, Nov 20, 2021 at 07:53:20AM +0100, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>
> > When not all fields of the --global option are provided,
> > QEMU might crash:
> >
> > $ qemu-system-x86_64 -global driver=isa-fdc
> > qemu-system-x86_64: ../../devel/qemu/qapi/string-input-visitor.c:394:
> > string_input_visitor_new: Assertion `str' failed.
> > Aborted (core dumped)
> >
> > Fix by only allowing --global with all 3 fields:
> >
> > $ qemu-system-x86_64 -global driver=isa-fdc
> > Invalid 'global' option format. It must be provided as:
> > --global <driver>.<property>=<value>
> >
> > Reported-by: Thomas Huth <thuth@redhat.com>
> > Suggested-by: Markus Armbruster <armbru@redhat.com>
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/604
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > ---
> > v3: Change qemu_global_option (Markus)
> >
> > Supersedes: <20211119122911.365036-1-philmd@redhat.com>
> > ---
> > softmmu/qdev-monitor.c | 9 +++------
> > 1 file changed, 3 insertions(+), 6 deletions(-)
> >
> > diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
> > index 01f3834db57..558272b147c 100644
> > --- a/softmmu/qdev-monitor.c
> > +++ b/softmmu/qdev-monitor.c
> > @@ -1029,13 +1029,10 @@ int qemu_global_option(const char *str)
> > qemu_opt_set(opts, "value", str + offset + 1, &error_abort);
> > return 0;
> > }
> > + printf("Invalid 'global' option format. It must be provided as:\n");
> > + printf(" --global <driver>.<property>=<value>\n");
> >
> > - opts = qemu_opts_parse_noisily(&qemu_global_opts, str, false);
> > - if (!opts) {
> > - return -1;
> > - }
> > -
> > - return 0;
> > + return -1;
> > }
> >
> > bool qmp_command_available(const QmpCommand *cmd, Error **errp)
>
> This drops a documented part of the external interface:
>
> $ qemu-system-x86_64 -help | grep -C 1 global
> i.e. -set drive.$id.file=/path/to/image
> -global driver.property=value
> --> -global driver=driver,property=property,value=value
> set a global default for a driver property
This doc makes it look like the two syntaxes are functionally
equivalent, but it seems that's not quite the case.
libvirt uses the driver.propert=value syntax for everything
except one case
-global driver=cfi.pflash01,property=secure,value=on
for that one if we try to use
-global cfi.pflash01.secure=on
it complains
qemu-system-x86_64: warning: global cfi.pflash01.secure has invalid class name
what's going on here ?
> -boot [order=drives][,once=drives][,menu=on|off]
>
> It goes back to commit 3751d7c43f "vl: allow full-blown QemuOpts syntax
> for -global", v2.4.0.
>
> The appropriate fix is to check @opts for presence of all three
> parameters.
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|