On Mon, Dec 17, 2012 at 05:01:23PM +0100, Igor Mammedov wrote: > commit d480e1af which introduced vendor property was setting > env->cpuid_vendor_override = 1, which prevents using vendor property > on its own without triggering vendor override. > Fix it by removing setting cpuid_vendor_override in x86_cpuid_set_vendor() > to allow to use vendor property in other places that doesn't require > cpuid_vendor_override to be set to 1.
By making "vendor" not force override, you are making "-cpu vendor=xxx" behave differently from setting "vendor" using all other interfaces (e.g. -device, -global, QMP commands). What about taking the opposite approach? Setting "vendor" could always force vendor override, but the code that initialize the defaults would take care of not overriding the vendor ID if unsafe. e.g.: it could just do this: if (!kvm_enabled() || def->vendor_override) { object_property_set_str(OBJECT(cpu), def->vendor, "vendor", errp); } /* else, leave the "vendor" property untouched" */ (something equivalent could be done inside class_init() when we introduce subclasses) On all I cases I can think of somebody setting the "vendor" property (e.g. using -cpu, QMP, -device, or -global), it means they want vendor override (otherwise, what's the point of setting the property?). Setting vendor in no-override mode is the special case, not the other way around. > > Signed-off-by: Igor Mammedov <imamm...@redhat.com> > --- > target-i386/cpu.c | 1 - > 1 files changed, 0 insertions(+), 1 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index a74d74b..c6c074f 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -1163,7 +1163,6 @@ static void x86_cpuid_set_vendor(Object *obj, const > char *value, > env->cpuid_vendor2 |= ((uint8_t)value[i + 4]) << (8 * i); > env->cpuid_vendor3 |= ((uint8_t)value[i + 8]) << (8 * i); > } > - env->cpuid_vendor_override = 1; > } > > static char *x86_cpuid_get_model_id(Object *obj, Error **errp) > -- > 1.7.1 > > -- Eduardo