Re: [Qemu-devel] [PATCH 2/3] s390x/kvm: Handle bpb feature

[David Hildenbrand]

On 17.01.2018 17:07, Halil Pasic wrote:
> 
> 
> On 01/17/2018 04:10 PM, David Hildenbrand wrote:
>>>> As soon as we enable bits for CPU models, we guarantee that migration
>>>> works. While introducing this change we already had one example where
>>>> this was not the case. Not good. (and remember having another such
>>>> exception)
>>> The point is migration continues to work. In fact I had a different version
>>> of this patch set that did it the other way around. Keep 82 a transparent
>>> and add a new cpu misc facility that takes care of the migration state.
>>>> It is easier to patch a feature in than silently enabling *anything*
>>>> somebody thinks is transparent (but its not). Especially not for the
>>>> host model. The expanded host model is migration safe.
>>> I really do not care about -cpu host (host-passthrough) for migration 
>>> safety, 
>>> because its not. And as you said: host-model (expanded) will work.
>>>
>> It will if the world would be perfect.
>>
>> expand "-cpu host" -> -cpu z14-base,stfle_82=on
>>
>> stfle_82 would now not be properly migrated. Yes, it might work somehow
>> right now. But it is not clean.
>>
> 

Please don't only focus on this case. I had different reasons why this
is a bad idea (especially IBC and feature names). And this is still my
position.

> Based on the code (did not test) I would expect to see this in the
> scenario I think you are talking about David:
> 
> 0. Libvirt uses cpu mode host (not host-passtrough).
> 1. Source: -"cpu host" expands to "-cpu z14-base,stfle_82=on" as you said
> the whole environment (host cpu, kvm, qemu) is supporting 82 inclusive 
> migration
> 2. Target: libvirt requires "-cpu z14-base,stfle_82=on" when trying to migrate
> 2.1. If the target environment as a whole does not fully support 82 the model
> checking refuses the migration. It does not matter if the reason is QEMU 
> missing
> patch #2 or lack of HW support or whatever.

Let's assume stfle_999, because this is obviously not a problem for 82.

If source and target blindly forward a feature they think is migration
safe, but is not (again, I am being conservative here as I was given a
counter example in the very same patch set), migration does not fail but
the guest might see a difference, because some state is not properly
migrated.

> 
> Assumed my reasoning above is correct, I really don't get what is not clean.
> Except 'blindly' trusting the hardware that it works as advertised (and
> fixing the mess only if it turns out that there is a mess).
> 
> On a meta level I think I understand your concerns David to some extent.
> But for my taste here you are too concerned about the user shooting herself
> into the foot (because equipment malfunction or user error).

Using the host model in QEMU shoots you in the foot. And it shouldn't.
Better safe than sorry.

> 
> Let me also note, that while we might very well end up propagating bugs
> to the user, we also give the user the means to mitigate these (e.g.
> by turning the buggy features explicitly off like -cpu host,stfle_82=off).

Shoot first and then ask questions? :D

I don't see any valid reason for this risk. Forwarding "transparent"
features from KVM to QEMU - perfect idea. Forwarding "transparent"
features from QEMU to the guest - not a good idea.

New HW -> new features -> new QEMU CPU model.

Patching in CPU features is really "out of order" ( ;) )

> 
> Regards,
> Halil
> 


-- 

Thanks,

David / dhildenb