Re: [PATCH 5/5] i386: provide simple 'hyperv=on' option to x86 machine types

[Eduardo Habkost]

On Wed, Jan 06, 2021 at 02:38:56PM +0100, Vitaly Kuznetsov wrote:
> Igor Mammedov <imammedo@redhat.com> writes:
> 
> > On Tue, 05 Jan 2021 17:31:43 +0100
> > Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
> >
> >> Igor Mammedov <imammedo@redhat.com> writes:
> >> 
> >> > On Tue, 05 Jan 2021 12:50:05 +0100
> >> >
> >> > I think there is a misunderstanding, idea was:
> >> >
> >> > cpu_initfn() {
> >> >     //current set
> >> >     cpu->default_hyperv_cpu_features = ACD
> >> > }
> >> >
> >> > compat_props_5.1 {
> >> >    cpu.default_hyperv_cpu_features = AB
> >> > }
> >> >
> >> > compat_props_5.2 {
> >> >    cpu.default_hyperv_cpu_features = ABC
> >> > }
> >> >  
> >> 
> >> ...
> >> 
> >> > I was talking about CPU features/properties only, it doesn't apply to 
> >> > other devices.
> >> > It makes sense for machine to have a knob to create onboard hyperv 
> >> > specific
> >> > devices if there is any (do we have any?).
> >> >
> >> > If there aren't any currently, I wouldn't bother with machine knob
> >> > and just use -cpu foo,hv_default=on or -device cpu,hv_default=on
> >> > like any other cpu feature.
> >> >  
> >> 
> >> We don't currently have any devices which are not 'CPU features' (in
> >> QEMU terminology), however, we already have Vmbus and I can easily
> >> imagine us implementing e.g. hartbeat/kvp/vss/... devices on top. We
> >> *may* want to enable these 'automatically' and that's what make
> >> '-machine' option preferable. It is, however, not a *must* right now and
> >> we can indeed wait until these devices appear and be happy with
> >> 'hv_default' -cpu option for now. We will, however, need to teach upper
> >> layers about the change when/if it happens.
> >
> > which makes me think we are trying to bite something that we shouldn't.
> > Do we really need this patch (QEMU knob) to magically enable subset of
> > features and/or devices for a specific OS flavor?

I think we really want this, yes.  It's not for a specific OS
flavor, it is just a machine feature.

> >
> > It's job of upper layers to abstract low level QEMU details in to coarse
> > grained knobs (libvirt/virt-install/virt-manager/...).
> > For example virt-install may know that it installing a specific Windows
> > version, and can build a tailored for that OS configuration including
> > needed hyperv CPU features and hyperv specific devices.
> > (if I'm not mistaken libosinfo is used to get metadata for preferred
> > configuration, so perhaps this should become a patch for that library
> > and its direct users).

virt-install/libosinfo/etc can be used to enable a feature
automatically, but the coarse grained knob may be provided by
QEMU.

> >
> > What we actually lack is a documentation for preferred configuration
> > in docs/hyperv.txt, currently it just enumerates possible features.
> > We can just document a recommended 'best practices' there without
> > putting it in QEMU code and let upper layers to do their job in
> > the stack.
> 
> The problem we're facing here is that when a new enlightenment is
> implemented it takes forever to propagate to the whole stack. We don't
> have any different recommendations for different Windows versions,
> neither does genuine Hyper-V. The 'fine grained' mechanis we have just
> contributes to the creation of various Frankenstein configurations
> (which look nothing like real Hyper-V), people just google for 'Windows
> KVM slow', add something to their scripts and this keeps propagating.

Exactly.  Requiring new code to be added to all other components
in the stack every time we add a low level feature to KVM or QEMU
is not working.  It's even worse when we require users to
manually update their configurations with low level bits.

> 
> Every time I see a configuration with only a few 'hv_*' options I ask
> 'why don't you enable the rest?' and I'm yet to receive an answer
> different from 'hm, I don't know, I copied it from somewhere and it
> worked'.
> 
> Setting 'hv_*' options individually should be considered debug only.

They can also be useful in production to work around
unexpected issues (not just debugging).

I don't think we should prevent other layers from controlling low
level knobs.  We just shouldn't make the low level knobs
necessary for making the feature work.

-- 
Eduardo