[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] Handling errors caused by -global (was Re: [Qemu-devel] [
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-arm] Handling errors caused by -global (was Re: [Qemu-devel] [PATCH v2 4/6] cpu: use CPUClass->parse_features() as convertor to global properties) |
Date: |
Tue, 14 Jun 2016 18:48:27 -0300 |
User-agent: |
Mutt/1.6.1 (2016-04-27) |
On Tue, Jun 14, 2016 at 05:41:03PM -0400, Paolo Bonzini wrote:
>
>
> ----- Original Message -----
> > From: "Eduardo Habkost" <address@hidden>
> > To: "Igor Mammedov" <address@hidden>
> > Cc: address@hidden, "peter maydell" <address@hidden>, "mark cave-ayland"
> > <address@hidden>, address@hidden, address@hidden, address@hidden,
> > address@hidden,
> > "Markus Armbruster" <address@hidden>
> > Sent: Tuesday, June 14, 2016 9:47:18 PM
> > Subject: Handling errors caused by -global (was Re: [Qemu-devel] [PATCH v2
> > 4/6] cpu: use CPUClass->parse_features()
> > as convertor to global properties)
> >
> > On Thu, Jun 09, 2016 at 07:11:01PM +0200, Igor Mammedov wrote:
> > [...]
> > > -static void cpu_common_parse_features(CPUState *cpu, char *features,
> > > +static void cpu_common_parse_features(const char *typename, char
> > > *features,
> > > Error **errp)
> > > {
> > > char *featurestr; /* Single "key=value" string being parsed */
> > > char *val;
> > > - Error *err = NULL;
> > > + static bool cpu_globals_initialized;
> > > +
> > > + /* TODO: all callers of ->parse_features() need to be changed to
> > > + * call it only once, so we can remove this check (or change it
> > > + * to assert(!cpu_globals_initialized).
> > > + * Current callers of ->parse_features() are:
> > > + * - machvirt_init()
> > > + * - cpu_generic_init()
> > > + * - cpu_x86_create()
> > > + */
> > > + if (cpu_globals_initialized) {
> > > + return;
> > > + }
> > > + cpu_globals_initialized = true;
> > >
> > > featurestr = features ? strtok(features, ",") : NULL;
> > >
> > > while (featurestr) {
> > > val = strchr(featurestr, '=');
> > > if (val) {
> > > + GlobalProperty *prop = g_new0(typeof(*prop), 1);
> > > *val = 0;
> > > val++;
> > > - object_property_parse(OBJECT(cpu), val, featurestr, &err);
> > > - if (err) {
> > > - error_propagate(errp, err);
> > > - return;
> > > - }
> > > + prop->driver = typename;
> > > + prop->property = g_strdup(featurestr);
> > > + prop->value = g_strdup(val);
> > > + qdev_prop_register_global(prop);
> >
> > This allows the user to trigger an assert:
> >
> > $ ./x86_64-softmmu/qemu-system-x86_64 -cpu qemu64,INVALID=on
> > qemu-system-x86_64: hw/core/qdev-properties.c:1087:
> > qdev_prop_set_globals_for_type: Assertion `prop->user_provided' failed.
> > Aborted (core dumped)
> >
> > but even if we fix the assert by setting
> > prop->user_provided=true, we have a problem. Previous behavior
> > was:
> >
> > $ ./x86_64-softmmu/qemu-system-x86_64 -cpu qemu64,INVALID=on
> > qemu-system-x86_64: Property '.INVALID' not found
> > $
> >
> > after this patch, and setting prop->user_provided=true, we have:
> >
> > $ ./x86_64-softmmu/qemu-system-x86_64 -cpu qemu64,INVALID=on
> > qemu-system-x86_64: Warning: global qemu64-x86_64-cpu.INVALID=on ignored:
> > Property '.INVALID' not found
> > [QEMU keeps running]
> >
> > QEMU needs to refuse to run if an invalid property is specified
> > on -cpu. It is an important mechanism to prevent VMs from running
> > if the user is requesting for a unsupported feature that requires
> > newer QEMU.
> >
> > Any suggestions on how to fix that?
>
> Replace user_provided with a Error* argument to qdev_prop_register_global.
> It can be &error_abort and NULL for the current users of the function,
> while -cpu can use &error_fatal.
Brilliant. This should work.
--
Eduardo
[Qemu-arm] [PATCH v2 5/6] arm: virt: parse cpu_model only once, Igor Mammedov, 2016/06/09
[Qemu-arm] [PATCH v2 2/6] target-i386: print obsolete warnings if +-features are used, Igor Mammedov, 2016/06/09
[Qemu-arm] [PATCH v2 4/6] cpu: use CPUClass->parse_features() as convertor to global properties, Igor Mammedov, 2016/06/09
[Qemu-arm] [PATCH v2 4/6] fixup! cpu: use CPUClass->parse_features() as convertor to global properties, Igor Mammedov, 2016/06/21
[Qemu-arm] [PATCH v2 6/6] pc: parse cpu features only once, Igor Mammedov, 2016/06/09