On Tue, 05 Jan 2021 16:10:36 +0100 Vitaly Kuznetsov <vkuzn...@redhat.com> wrote:
> 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. #1 can be implemented on top of #2, when it becomes necessary. > >> 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). lets discuss it in that thread.
