Re: [Qemu-devel] [PATCH v4] s390: diagnose 318 info reset and migration support

[David Hildenbrand <david@redhat.com>]

On 13.05.19 11:51, Christian Borntraeger wrote:
> 
> 
> On 13.05.19 11:40, David Hildenbrand wrote:
>> On 13.05.19 11:34, Christian Borntraeger wrote:
>>>
>>>
>>> On 13.05.19 10:03, David Hildenbrand wrote:
>>>>>> +    if ((SCCB_SIZE - sizeof(ReadInfo)) / sizeof(CPUEntry) < S390_MAX_CPUS)
>>>>>> +        mc->max_cpus = S390_MAX_CPUS - 8;
>>>>>
>>>>> This is too complicated, just set it always to 240.
>>>>>
>>>>> However, I am still not sure how to best handle this scenario. One
>>>>> solution is
>>>>>
>>>>> 1. Set it statically to 240 for machine > 4.1
>>>>> 2. Keep the old machines unmodifed
>>>>> 3. Don't indicate the CPU feature for machines <= 4.0
>>>>>
>>>>> #3 is the problematic part, as it mixes host CPU features and machines.
>>>>> Bad. The host CPU model should always look the same on all machines. I
>>>>> don't like this.
>>>>>
>>>>
>>>> FWIW, #3 is only an issue when modeling it via the CPU model, like
>>>> Christian suggested.
>>>>
>>>> I suggest the following
>>>>
>>>> 1. Set the max #cpus for 4.1 to 240 (already done)
>>>> 2. Keep it for the other machines unmodified (as suggested by Thomas)
>>>> 3. Create the layout of the SCCB depending on the machine type (to be done)
>>>>
>>>> If we want to model diag318 via a CPU feature (which makes sense for
>>>> migration):
>>>>
>>>> 4. Disable diag318 with a warning if used with a machine < 4.1
>>>>
>>>
>>> I think there is a simpler solution. It is perfectly fine to fail the startup
>>> if we cannot fulfil the cpu model. So lets just allow 248 and allow this feature 
>>> also for older machines. And if somebody chooses both at the same time,
>>> lets fails the startup.
>>
>> To which knob do you want to glue the layout of the SCLP response? Like
>> I described?  Do you mean instead of warning and masking the feature off
>> as I suggested, simply failing?
> 
> The sclp response will depend on the dia318 cpu model flag. If its on, the sclp
> response will have it, otherwise not.
> - host-passthrough: not migration safe anyway
> - host-model: if the target has diag318 good, otherwise we reject migration 
>>
>> In that case, -machine ..-4.0 -cpu host will not work on new HW with new
>> KVM. Just noting.
> 
> Only if you have 248 CPUs (which is unlikely). My point was to do that for all
> machine levels.
> 

The issue with this approach is that e.g. libvirt is not aware of this
restriction. It could query "max_cpus" and expand the host-cpu model,
but starting a guest with > 240 cpus would fail. Maybe this is acceptable.

The approach you describe is actually pretty nice. We would have to

1. Modify SCLP code to change the layout based on the feature availability
2. Check against the cpus when trying to enable the cpu model.

-- 

Thanks,

David / dhildenb