On 1/28/21 6:29 AM, Philippe Mathieu-Daudé wrote: > On 1/28/21 5:08 PM, Alex Bennée wrote: >> >> Claudio Fontana <cfont...@suse.de> writes: >> >>> On 1/28/21 2:03 PM, Philippe Mathieu-Daudé wrote: >>>> On 1/28/21 10:28 AM, Claudio Fontana wrote: >> <snip> >>>>> + >>>>> +#define TYPE_ACCEL_CPU "accel-" CPU_RESOLVING_TYPE >>>>> +#define ACCEL_CPU_NAME(name) (name "-" TYPE_ACCEL_CPU) >>>>> +typedef struct AccelCPUClass AccelCPUClass; >>>>> +DECLARE_CLASS_CHECKERS(AccelCPUClass, ACCEL_CPU, TYPE_ACCEL_CPU) >>>>> + >>>>> +typedef struct AccelCPUClass { >>>>> + /*< private >*/ >>>>> + ObjectClass parent_class; >>>>> + /*< public >*/ >>>>> + >>>>> + void (*cpu_class_init)(CPUClass *cc); >>>>> + void (*cpu_instance_init)(CPUState *cpu); >>>>> + void (*cpu_realizefn)(CPUState *cpu, Error **errp); >>>> >>>> If we want callers to check errp, better have the prototype return >>>> a boolean. >>> >>> Good point, the whole errp thing is worth revisiting in the series, >>> there are many cases (which are basically reproduced in the refactoring >>> from existing code), >>> where errp is passed but is really unused. >>> >>> I am constantly internally debating whether to remove the parameter >>> altogether, or to keep it in there. >>> >>> What would you suggest? >> >> I think it really depends on if we can expect the realizefn to usefully >> return an error message that can be read and understood by the user. I >> guess this comes down to how much user config is going to be checked at >> the point we realize the CPU? > > cpu_realize() is were various feature checks are, isn't it? > > -cpu mycpu,feat1=on,feat2=off > CPU 'mycpu' can not disable feature 'feat2' because of REASON.
Yes. And while changing the return type of realize is probably a good idea, it should be a separate patch. r~