Eduardo Habkost <ehabk...@redhat.com> writes: > On Tue, Jan 05, 2021 at 12:36:50AM +0100, Igor Mammedov wrote: >> >> documenting is good, but if it adds new semantics to how CPU features are >> handled >> users up the stack will need code it up as well and juggle with >> -machine + -cpu + -device cpu-foo >> not to mention poor developers who will have to figure out why we do >> set CPU properties in multiple different ways. >> >> however if we add it as CPU properties that behave the same way as other >> properties, all mgmt has to do is expose new property to user for usage. > > I think we need to be careful here. Sometimes just exposing the > QOM properties used to implemented a feature is not the best user > interface. e.g.: even if using compat_props for implementing the > hyperv features preset, that doesn't automatically mean we want > hyperv=on to be a -cpu option. > > I would even argue we shouldn't be focusing on implementation > details (like we are doing right now) until the desired external > interface is described clearly.
I agree, the interface is definitely more important than the implementation here. AFAIU we have two options suggested: 1) 'hyperv=on' option for x86 machine types. Pros: we can use it later to create non-CPU Hyper-V devices (e.g. Vmbus). Cons: two different places for the currently existing Hyper-V features enablement (-cpu and -machine), non-standard way of doing things (code-wise). 2) 'hv_default=on' -cpu option Pros: Single place to enable all Hyper-V enlightenments, we can make it mutually exclusive with other hv_* options including hv_passthrough (clear semantics). Cons: This can't be reused to create non-CPU objects in the future and so upper layers will (again) need to be modified. There's probably more, please feel free to add. >> however in this case we are talking about a set of cpu features, >> if there is no way to implement it as cpu properties + compat properties >> and requires opencodding it within machine code it might be fine >> but I fail to see a very good reason for doing that at this momment. > > The reason would be just simplicity of implementation. > > I understand there are reasons to suggest using compat_props if > it makes things simpler, but I don't see why we would reject a > patch because the implementation is not based purely on > compat_props. > > I will let Vitaly to decide how to proceed, based on our > feedback. I encourage him to use compat_props like you suggest, > but I don't plan to make this a requirement. > Like I replied to Igor in a parallel thread, I hardly see how using compat_props can simplify things in case we decide to keep 'hyperv=on' a machine type option. It doesn't seem to fit our use-case when we need a mechanism to alter CPU properties for the current machine type as well as subtract some features for the old ones. If we, however, decide that '-cpu' option is better, then we can try to make it work (but the implementation won't be straitforward either). -- Vitaly
