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?
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
