* [Qemu-devel] [PATCH] vl: exit if a bad property value is passed to -global
@ 2016-07-13 18:05 Greg Kurz
2016-07-14 1:42 ` David Gibson
0 siblings, 1 reply; 5+ messages in thread
From: Greg Kurz @ 2016-07-13 18:05 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: David Gibson, qemu-devel, Bharata B Rao
When passing '-global driver=host-powerpc64-cpu,property=compat,value=foo'
on the command line, without this patch, we get the following warning per
device (which means many lines if the guests has many cpus):
qemu-system-ppc64: Warning: can't apply global host-powerpc64-cpu.compat=foo:
Invalid compatibility mode "foo"
... and QEMU continues execution, ignoring the property.
With this patch, we get a single line:
qemu-system-ppc64: can't apply global host-powerpc64-cpu.compat=foo:
Invalid compatibility mode "foo"
... and QEMU exits.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
vl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/vl.c b/vl.c
index 356713ea075c..a7cc92781687 100644
--- a/vl.c
+++ b/vl.c
@@ -2922,6 +2922,7 @@ static int global_init_func(void *opaque, QemuOpts *opts, Error **errp)
g->property = qemu_opt_get(opts, "property");
g->value = qemu_opt_get(opts, "value");
g->user_provided = true;
+ g->errp = errp;
qdev_prop_register_global(g);
return 0;
}
@@ -4451,7 +4452,7 @@ int main(int argc, char **argv, char **envp)
machine_register_compat_props(current_machine);
qemu_opts_foreach(qemu_find_opts("global"),
- global_init_func, NULL, NULL);
+ global_init_func, NULL, &error_fatal);
/* This checkpoint is required by replay to separate prior clock
reading from the other reads, because timer polling functions query
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] vl: exit if a bad property value is passed to -global
2016-07-13 18:05 [Qemu-devel] [PATCH] vl: exit if a bad property value is passed to -global Greg Kurz
@ 2016-07-14 1:42 ` David Gibson
2016-07-14 8:29 ` Igor Mammedov
0 siblings, 1 reply; 5+ messages in thread
From: David Gibson @ 2016-07-14 1:42 UTC (permalink / raw)
To: Greg Kurz; +Cc: Paolo Bonzini, qemu-devel, Bharata B Rao
[-- Attachment #1: Type: text/plain, Size: 2062 bytes --]
On Wed, Jul 13, 2016 at 08:05:03PM +0200, Greg Kurz wrote:
> When passing '-global driver=host-powerpc64-cpu,property=compat,value=foo'
> on the command line, without this patch, we get the following warning per
> device (which means many lines if the guests has many cpus):
>
> qemu-system-ppc64: Warning: can't apply global host-powerpc64-cpu.compat=foo:
> Invalid compatibility mode "foo"
>
> ... and QEMU continues execution, ignoring the property.
>
> With this patch, we get a single line:
>
> qemu-system-ppc64: can't apply global host-powerpc64-cpu.compat=foo:
> Invalid compatibility mode "foo"
>
> ... and QEMU exits.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Not really up to me to decide if this is the right approach for global
option handling, but it seems like the way to go to me.
> ---
> vl.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/vl.c b/vl.c
> index 356713ea075c..a7cc92781687 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2922,6 +2922,7 @@ static int global_init_func(void *opaque, QemuOpts *opts, Error **errp)
> g->property = qemu_opt_get(opts, "property");
> g->value = qemu_opt_get(opts, "value");
> g->user_provided = true;
> + g->errp = errp;
> qdev_prop_register_global(g);
> return 0;
> }
> @@ -4451,7 +4452,7 @@ int main(int argc, char **argv, char **envp)
> machine_register_compat_props(current_machine);
>
> qemu_opts_foreach(qemu_find_opts("global"),
> - global_init_func, NULL, NULL);
> + global_init_func, NULL, &error_fatal);
>
> /* This checkpoint is required by replay to separate prior clock
> reading from the other reads, because timer polling functions query
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] vl: exit if a bad property value is passed to -global
2016-07-14 1:42 ` David Gibson
@ 2016-07-14 8:29 ` Igor Mammedov
2016-07-20 20:18 ` Eduardo Habkost
0 siblings, 1 reply; 5+ messages in thread
From: Igor Mammedov @ 2016-07-14 8:29 UTC (permalink / raw)
To: David Gibson
Cc: Greg Kurz, Paolo Bonzini, qemu-devel, Bharata B Rao, ehabkost
On Thu, 14 Jul 2016 11:42:40 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:
> On Wed, Jul 13, 2016 at 08:05:03PM +0200, Greg Kurz wrote:
> > When passing '-global driver=host-powerpc64-cpu,property=compat,value=foo'
> > on the command line, without this patch, we get the following warning per
> > device (which means many lines if the guests has many cpus):
> >
> > qemu-system-ppc64: Warning: can't apply global host-powerpc64-cpu.compat=foo:
> > Invalid compatibility mode "foo"
> >
> > ... and QEMU continues execution, ignoring the property.
> >
> > With this patch, we get a single line:
> >
> > qemu-system-ppc64: can't apply global host-powerpc64-cpu.compat=foo:
> > Invalid compatibility mode "foo"
> >
> > ... and QEMU exits.
> >
> > Signed-off-by: Greg Kurz <groug@kaod.org>
>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>
> Not really up to me to decide if this is the right approach for global
> option handling, but it seems like the way to go to me.
CCing Eduardo
as he basically rewrote that path several times and might know why it's this way now.
>
> > ---
> > vl.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/vl.c b/vl.c
> > index 356713ea075c..a7cc92781687 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -2922,6 +2922,7 @@ static int global_init_func(void *opaque, QemuOpts *opts, Error **errp)
> > g->property = qemu_opt_get(opts, "property");
> > g->value = qemu_opt_get(opts, "value");
> > g->user_provided = true;
> > + g->errp = errp;
> > qdev_prop_register_global(g);
> > return 0;
> > }
> > @@ -4451,7 +4452,7 @@ int main(int argc, char **argv, char **envp)
> > machine_register_compat_props(current_machine);
> >
> > qemu_opts_foreach(qemu_find_opts("global"),
> > - global_init_func, NULL, NULL);
> > + global_init_func, NULL, &error_fatal);
> >
> > /* This checkpoint is required by replay to separate prior clock
> > reading from the other reads, because timer polling functions query
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] vl: exit if a bad property value is passed to -global
2016-07-14 8:29 ` Igor Mammedov
@ 2016-07-20 20:18 ` Eduardo Habkost
2016-07-21 10:16 ` Greg Kurz
0 siblings, 1 reply; 5+ messages in thread
From: Eduardo Habkost @ 2016-07-20 20:18 UTC (permalink / raw)
To: Igor Mammedov
Cc: David Gibson, Greg Kurz, Paolo Bonzini, qemu-devel, Bharata B Rao
Sorry for the long delay:
On Thu, Jul 14, 2016 at 10:29:57AM +0200, Igor Mammedov wrote:
> On Thu, 14 Jul 2016 11:42:40 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
>
> > On Wed, Jul 13, 2016 at 08:05:03PM +0200, Greg Kurz wrote:
> > > When passing '-global driver=host-powerpc64-cpu,property=compat,value=foo'
> > > on the command line, without this patch, we get the following warning per
> > > device (which means many lines if the guests has many cpus):
> > >
> > > qemu-system-ppc64: Warning: can't apply global host-powerpc64-cpu.compat=foo:
> > > Invalid compatibility mode "foo"
> > >
> > > ... and QEMU continues execution, ignoring the property.
> > >
> > > With this patch, we get a single line:
> > >
> > > qemu-system-ppc64: can't apply global host-powerpc64-cpu.compat=foo:
> > > Invalid compatibility mode "foo"
> > >
> > > ... and QEMU exits.
> > >
> > > Signed-off-by: Greg Kurz <groug@kaod.org>
> >
> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> >
> > Not really up to me to decide if this is the right approach for global
> > option handling, but it seems like the way to go to me.
> CCing Eduardo
> as he basically rewrote that path several times and might know why it's this way now.
>
> >
> > > ---
> > > vl.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/vl.c b/vl.c
> > > index 356713ea075c..a7cc92781687 100644
> > > --- a/vl.c
> > > +++ b/vl.c
> > > @@ -2922,6 +2922,7 @@ static int global_init_func(void *opaque, QemuOpts *opts, Error **errp)
> > > g->property = qemu_opt_get(opts, "property");
> > > g->value = qemu_opt_get(opts, "value");
> > > g->user_provided = true;
> > > + g->errp = errp;
Exiting QEMU like you suggest is valid, but we can't set errp to
error_fatal here, because it could make QEMU abort when using
device_add.
Probably the simplest solution is to ignore GlobalProperty.errp
in qdev_prop_set_globals_for_type() if dev->hotplugged is set.
Then we can safely set errp here.
--
Eduardo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] vl: exit if a bad property value is passed to -global
2016-07-20 20:18 ` Eduardo Habkost
@ 2016-07-21 10:16 ` Greg Kurz
0 siblings, 0 replies; 5+ messages in thread
From: Greg Kurz @ 2016-07-21 10:16 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Igor Mammedov, David Gibson, Paolo Bonzini, qemu-devel, Bharata B Rao
On Wed, 20 Jul 2016 17:18:26 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> Sorry for the long delay:
>
> On Thu, Jul 14, 2016 at 10:29:57AM +0200, Igor Mammedov wrote:
> > On Thu, 14 Jul 2016 11:42:40 +1000
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >
> > > On Wed, Jul 13, 2016 at 08:05:03PM +0200, Greg Kurz wrote:
> > > > When passing '-global driver=host-powerpc64-cpu,property=compat,value=foo'
> > > > on the command line, without this patch, we get the following warning per
> > > > device (which means many lines if the guests has many cpus):
> > > >
> > > > qemu-system-ppc64: Warning: can't apply global host-powerpc64-cpu.compat=foo:
> > > > Invalid compatibility mode "foo"
> > > >
> > > > ... and QEMU continues execution, ignoring the property.
> > > >
> > > > With this patch, we get a single line:
> > > >
> > > > qemu-system-ppc64: can't apply global host-powerpc64-cpu.compat=foo:
> > > > Invalid compatibility mode "foo"
> > > >
> > > > ... and QEMU exits.
> > > >
> > > > Signed-off-by: Greg Kurz <groug@kaod.org>
> > >
> > > Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> > >
> > > Not really up to me to decide if this is the right approach for global
> > > option handling, but it seems like the way to go to me.
> > CCing Eduardo
> > as he basically rewrote that path several times and might know why it's this way now.
> >
> > >
> > > > ---
> > > > vl.c | 3 ++-
> > > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/vl.c b/vl.c
> > > > index 356713ea075c..a7cc92781687 100644
> > > > --- a/vl.c
> > > > +++ b/vl.c
> > > > @@ -2922,6 +2922,7 @@ static int global_init_func(void *opaque, QemuOpts *opts, Error **errp)
> > > > g->property = qemu_opt_get(opts, "property");
> > > > g->value = qemu_opt_get(opts, "value");
> > > > g->user_provided = true;
> > > > + g->errp = errp;
>
> Exiting QEMU like you suggest is valid, but we can't set errp to
> error_fatal here, because it could make QEMU abort when using
> device_add.
>
Indeed QEMU exits :\
> Probably the simplest solution is to ignore GlobalProperty.errp
> in qdev_prop_set_globals_for_type() if dev->hotplugged is set.
> Then we can safely set errp here.
>
That should do the trick. I'll send a v2.
Thanks !
--
Greg
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-07-21 10:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-13 18:05 [Qemu-devel] [PATCH] vl: exit if a bad property value is passed to -global Greg Kurz
2016-07-14 1:42 ` David Gibson
2016-07-14 8:29 ` Igor Mammedov
2016-07-20 20:18 ` Eduardo Habkost
2016-07-21 10:16 ` Greg Kurz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.