----- Original Message -----
> From: "Eduardo Habkost" <ehabk...@redhat.com>
> To: "Igor Mammedov" <imamm...@redhat.com>
> Cc: qemu-devel@nongnu.org, "peter maydell" <peter.mayd...@linaro.org>, "mark
> cave-ayland"
> <mark.cave-ayl...@ilande.co.uk>, blauwir...@gmail.com, qemu-...@nongnu.org,
> pbonz...@redhat.com, r...@twiddle.net,
> "Markus Armbruster" <arm...@redhat.com>
> 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.
Paolo
> Maybe qdev_prop_set_globals() can collect errors in a list in
> DeviceState, and we can check for them in code that creates
> device objects (like cpu_generic_init(), qdev_device_add()), or
> in the beginning of device_set_realized().
>
> --
> Eduardo
>
